xunit/xunit#2849: Update xUnit1030 to handle ConfigureAwaitOptions

xunit · Dec 13, 2023 · 8aa39e4 · 8aa39e4
1 parent 49afdc9
commit 8aa39e4
Show file tree

Hide file tree

Showing 6 changed files with 495 additions and 6 deletions.
diff --git a/src/xunit.analyzers.fixes/X1000/DoNotUseConfigureAwaitFixer.cs b/src/xunit.analyzers.fixes/X1000/DoNotUseConfigureAwaitFixer.cs
@@ -1,4 +1,6 @@
 using System.Composition;
+using System.Globalization;
+using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeActions;
@@ -12,6 +14,7 @@ namespace Xunit.Analyzers.Fixes;
 public class DoNotUseConfigureAwaitFixer : BatchedCodeFixProvider
 {
 	public const string Key_RemoveConfigureAwait = "xUnit1030_RemoveConfigureAwait";
+	public const string Key_ReplaceArgumentValue = "xUnit1030_ReplaceArgumentValue";
 
 	public DoNotUseConfigureAwaitFixer() :
 		base(Descriptors.X1030_DoNotUseConfigureAwait.Id)
@@ -23,26 +26,57 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
 		if (root is null)
 			return;
 
+		var diagnostic = context.Diagnostics.FirstOrDefault();
+		if (diagnostic is null)
+			return;
+
+		// Get the original and replacement values
+		if (!diagnostic.Properties.TryGetValue(Constants.Properties.ArgumentValue, out var original))
+			return;
+		if (!diagnostic.Properties.TryGetValue(Constants.Properties.Replacement, out var replacement))
+			return;
+
 		// The syntax node (the invocation) will include "(any preceding trivia)(any preceding code).ConfigureAwait(args)" despite
 		// the context.Span only covering "ConfigureAwait(args)". So we need to replace the whole invocation
 		// with an invocation that does not include the ConfigureAwait call.
 		var syntaxNode = root.FindNode(context.Span);
 		var syntaxText = syntaxNode.ToFullString();
 
 		// Remove the context span (plus the preceding .)
-		var newSyntaxText = syntaxText.Substring(0, context.Span.Start - syntaxNode.FullSpan.Start - 1);
-		var newSyntaxNode = SyntaxFactory.ParseExpression(newSyntaxText);
+		var removeConfigureAwaitText = syntaxText.Substring(0, context.Span.Start - syntaxNode.FullSpan.Start - 1);
+		var removeConfigureAwaitNode = SyntaxFactory.ParseExpression(removeConfigureAwaitText);
+
+		// Only offer the removal fix if the replacement value is 'true', because anybody using ConfigureAwaitOptions
+		// will want to just add the extra value, not remove the call entirely.
+		if (replacement == "true")
+			context.RegisterCodeFix(
+				CodeAction.Create(
+					"Remove ConfigureAwait call",
+					async ct =>
+					{
+						var editor = await DocumentEditor.CreateAsync(context.Document, ct).ConfigureAwait(false);
+						editor.ReplaceNode(syntaxNode, removeConfigureAwaitNode);
+						return editor.GetChangedDocument();
+					},
+					Key_RemoveConfigureAwait
+				),
+				context.Diagnostics
+			);
+
+		// Offer the replacement fix
+		var replaceConfigureAwaitText = removeConfigureAwaitText + ".ConfigureAwait(" + replacement + ")";
+		var replaceConfigureAwaitNode = SyntaxFactory.ParseExpression(replaceConfigureAwaitText);
 
 		context.RegisterCodeFix(
 			CodeAction.Create(
-				"Remove ConfigureAwait call",
+				string.Format(CultureInfo.CurrentCulture, "Replace ConfigureAwait({0}) with ConfigureAwait({1})", original, replacement),
 				async ct =>
 				{
 					var editor = await DocumentEditor.CreateAsync(context.Document, ct).ConfigureAwait(false);
-					editor.ReplaceNode(syntaxNode, newSyntaxNode);
+					editor.ReplaceNode(syntaxNode, replaceConfigureAwaitNode);
 					return editor.GetChangedDocument();
 				},
-				Key_RemoveConfigureAwait
+				Key_ReplaceArgumentValue
 			),
 			context.Diagnostics
 		);

diff --git a/src/xunit.analyzers.tests/Analyzers/X1000/DoNotUseConfigureAwaitTests.cs b/src/xunit.analyzers.tests/Analyzers/X1000/DoNotUseConfigureAwaitTests.cs
@@ -186,4 +186,130 @@ public async Task TestMethod() {{
 			await Verify.VerifyAnalyzer(source, expected);
 		}
 	}
+
+#if NETCOREAPP
+
+	public class ConfigureAwait_ConfigureAwaitOptions
+	{
+		[Fact]
+		public async void NonTestMethod_DoesNotTrigger()
+		{
+			var source = @"
+using System.Threading.Tasks;
+using Xunit;
+
+public class NonTestClass {
+    public async Task NonTestMethod() {
+        await Task.Delay(1).ConfigureAwait(ConfigureAwaitOptions.None);
+    }
+}";
+
+			await Verify.VerifyAnalyzer(source);
+		}
+
+		[Theory]
+		[InlineData("ConfigureAwaitOptions.ContinueOnCapturedContext")]
+		[InlineData("ConfigureAwaitOptions.SuppressThrowing | ConfigureAwaitOptions.ContinueOnCapturedContext")]
+		[InlineData("ConfigureAwaitOptions.ForceYielding | ConfigureAwaitOptions.SuppressThrowing | ConfigureAwaitOptions.ContinueOnCapturedContext")]
+		public async void ValidValue_DoesNotTrigger(string enumValue)
+		{
+			var source = $@"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {{
+    [Fact]
+    public async Task TestMethod() {{
+        await Task.Delay(1).ConfigureAwait({enumValue});
+    }}
+}}";
+
+			await Verify.VerifyAnalyzer(source);
+		}
+
+		public static TheoryData<string> InvalidValues = new()
+		{
+			// Literal values
+			"ConfigureAwaitOptions.None",
+			"ConfigureAwaitOptions.SuppressThrowing",
+			"ConfigureAwaitOptions.ForceYielding | ConfigureAwaitOptions.SuppressThrowing",
+			// Reference values (we don't do lookup)
+			"enumVar",
+		};
+
+		[Theory]
+		[MemberData(nameof(InvalidValues))]
+		public async void InvalidValue_TaskWithAwait_DoesNotTrigger(string enumValue)
+		{
+			var source = $@"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {{
+    [Fact]
+    public async Task TestMethod() {{
+        var enumVar = ConfigureAwaitOptions.ContinueOnCapturedContext;
+        await Task.Delay(1).ConfigureAwait({enumValue});
+    }}
+}}";
+			var expected =
+				Verify
+					.Diagnostic()
+					.WithSpan(9, 29, 9, 45 + enumValue.Length)
+					.WithArguments(enumValue, "Ensure ConfigureAwaitOptions.ContinueOnCapturedContext in the flags.");
+
+			await Verify.VerifyAnalyzer(source, expected);
+		}
+
+		[Theory]
+		[MemberData(nameof(InvalidValues))]
+		public async void InvalidValue_TaskWithoutAwait_Triggers(string argumentValue)
+		{
+			var source = @$"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {{
+    [Fact]
+    public void TestMethod() {{
+        var enumVar = ConfigureAwaitOptions.ContinueOnCapturedContext;
+        Task.Delay(1).ConfigureAwait({argumentValue}).GetAwaiter().GetResult();
+    }}
+}}";
+			var expected =
+				Verify
+					.Diagnostic()
+					.WithSpan(9, 23, 9, 39 + argumentValue.Length)
+					.WithArguments(argumentValue, "Ensure ConfigureAwaitOptions.ContinueOnCapturedContext in the flags.");
+
+			await Verify.VerifyAnalyzer(source, expected);
+		}
+
+		[Theory]
+		[MemberData(nameof(InvalidValues))]
+		public async void InvalidValue_TaskOfT_Triggers(string argumentValue)
+		{
+			var source = @$"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {{
+    [Fact]
+    public async Task TestMethod() {{
+        var enumVar = ConfigureAwaitOptions.ContinueOnCapturedContext;
+        var task = Task.FromResult(42);
+        await task.ConfigureAwait({argumentValue});
+    }}
+}}";
+			var expected =
+				Verify
+					.Diagnostic()
+					.WithSpan(10, 20, 10, 36 + argumentValue.Length)
+					.WithArguments(argumentValue, "Ensure ConfigureAwaitOptions.ContinueOnCapturedContext in the flags.");
+
+			await Verify.VerifyAnalyzer(source, expected);
+		}
+	}
+
+#endif
 }