xunit/xunit#2628: Add "Don't use ConfigureAwait" analyzer and fixer (…

…xUnit1030)

src/xunit.analyzers.fixes/X1000/DoNotUseConfigureAwaitFixer.cs

@@ -0,0 +1,50 @@
+using System.Composition;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.Editing;
+
+namespace Xunit.Analyzers.Fixes;
+
+[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
+public class DoNotUseConfigureAwaitFixer : BatchedCodeFixProvider
+{
+	public const string Key_RemoveConfigureAwait = "xUnit1030_RemoveConfigureAwait";
+
+	public DoNotUseConfigureAwaitFixer() :
+		base(Descriptors.X1030_DoNotUseConfigureAwait.Id)
+	{ }
+
+	public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+	{
+		var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+		if (root is null)
+			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);
+
+		context.RegisterCodeFix(
+			CodeAction.Create(
+				"Remove ConfigureAwait call",
+				async ct =>
+				{
+					var editor = await DocumentEditor.CreateAsync(context.Document, ct).ConfigureAwait(false);
+					editor.ReplaceNode(syntaxNode, newSyntaxNode);
+					return editor.GetChangedDocument();
+				},
+				Key_RemoveConfigureAwait
+			),
+			context.Diagnostics
+		);
+	}
+}

src/xunit.analyzers.tests/Analyzers/X1000/DoNotUseConfigureAwaitTests.cs

@@ -0,0 +1,78 @@
+using Xunit;
+using Verify = CSharpVerifier<Xunit.Analyzers.DoNotUseConfigureAwait>;
+
+public class DoNotUseConfigureAwaitTests
+{
+	[Fact]
+	public async void SuccessCase()
+	{
+		var source = @"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {
+    [Fact]
+    public async Task TestMethod() {
+        await Task.Delay(1);
+    }
+}";
+
+		await Verify.VerifyAnalyzer(source);
+	}
+
+	[Theory]
+	[InlineData("true")]
+	[InlineData("false")]
+	[InlineData("1 == 2")]
+	public async void FailureCase_Async(string argumentValue)
+	{
+		var source = @$"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {{
+    [Fact]
+    public async Task TestMethod() {{
+        await Task.Delay(1).[|ConfigureAwait({argumentValue})|];
+    }}
+}}";
+
+		await Verify.VerifyAnalyzer(source);
+	}
+
+	[Theory]
+	[InlineData("true")]
+	[InlineData("false")]
+	[InlineData("1 == 2")]
+	public async void FailureCase_NonAsync(string argumentValue)
+	{
+		var source = @$"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {{
+    [Fact]
+    public void TestMethod() {{
+        Task.Delay(1).[|ConfigureAwait({argumentValue})|].GetAwaiter().GetResult();
+    }}
+}}";
+
+		await Verify.VerifyAnalyzer(source);
+	}
+
+	[Fact]
+	public async void IgnoredCase()
+	{
+		var source = @"
+using System.Threading.Tasks;
+using Xunit;
+
+public class NonTestClass {
+    public async Task NonTestMethod() {
+        await Task.Delay(1).ConfigureAwait(false);
+    }
+}";
+
+		await Verify.VerifyAnalyzer(source);
+	}
+}

src/xunit.analyzers.tests/Fixes/X1000/DoNotUseConfigureAwaitFixerTests.cs

@@ -0,0 +1,68 @@
+using Xunit;
+using Xunit.Analyzers.Fixes;
+using Verify = CSharpVerifier<Xunit.Analyzers.DoNotUseConfigureAwait>;
+
+public class DoNotUseConfigureAwaitFixerTests
+{
+	[Theory]
+	[InlineData("true")]
+	[InlineData("false")]
+	[InlineData("1 == 2")]
+	public async void RemovesConfigureAwait_Async(string argumentValue)
+	{
+		var before = @$"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {{
+    [Fact]
+    public async Task TestMethod() {{
+        await Task.Delay(1).[|ConfigureAwait({argumentValue})|];
+    }}
+}}";
+
+		var after = @"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {
+    [Fact]
+    public async Task TestMethod() {
+        await Task.Delay(1);
+    }
+}";
+
+		await Verify.VerifyCodeFix(before, after, DoNotUseConfigureAwaitFixer.Key_RemoveConfigureAwait);
+	}
+
+	[Theory]
+	[InlineData("true")]
+	[InlineData("false")]
+	[InlineData("1 == 2")]
+	public async void RemovesConfigureAwait_NonAsync(string argumentValue)
+	{
+		var before = @$"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {{
+    [Fact]
+    public void TestMethod() {{
+        Task.Delay(1).[|ConfigureAwait({argumentValue})|].GetAwaiter().GetResult();
+    }}
+}}";
+
+		var after = @"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {
+    [Fact]
+    public void TestMethod() {
+        Task.Delay(1).GetAwaiter().GetResult();
+    }
+}";
+
+		await Verify.VerifyCodeFix(before, after, DoNotUseConfigureAwaitFixer.Key_RemoveConfigureAwait);
+	}
+}

src/xunit.analyzers/Utility/Descriptors.cs

@@ -305,7 +305,14 @@ static DiagnosticDescriptor Rule(
 			"Local functions cannot be test functions. Remove '{0}'."
 		);
 
-	// Placeholder for rule X1030
+	public static DiagnosticDescriptor X1030_DoNotUseConfigureAwait { get; } =
+		Rule(
+			"xUnit1030",
+			"Do not call ConfigureAwait in test method",
+			Usage,
+			Warning,
+			"Test methods should not call ConfigureAwait(), as it may bypass parallelization limits."
+		);
 
 	// Placeholder for rule X1031
 

src/xunit.analyzers/X1000/DoNotUseConfigureAwait.cs

@@ -0,0 +1,81 @@
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+using Microsoft.CodeAnalysis.Text;
+
+namespace Xunit.Analyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class DoNotUseConfigureAwait : XunitDiagnosticAnalyzer
+{
+	public DoNotUseConfigureAwait() :
+		base(Descriptors.X1030_DoNotUseConfigureAwait)
+	{ }
+
+	public override void AnalyzeCompilation(
+		CompilationStartAnalysisContext context,
+		XunitContext xunitContext)
+	{
+		var taskType = TypeSymbolFactory.Task(context.Compilation);
+
+		if (xunitContext.Core.FactAttributeType is null || xunitContext.Core.TheoryAttributeType is null)
+			return;
+
+		context.RegisterOperationAction(context =>
+		{
+			if (context.Operation is not IInvocationOperation invocation)
+				return;
+
+			var methodSymbol = invocation.TargetMethod;
+			if (methodSymbol.MethodKind != MethodKind.Ordinary || !SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingType, taskType) || methodSymbol.Name != nameof(Task.ConfigureAwait))
+				return;
+
+			var semanticModel = invocation.SemanticModel;
+			if (semanticModel is null)
+				return;
+
+			// Only trigger when you're inside a test method
+			for (var parent = invocation.Parent; parent != null; parent = parent.Parent)
+			{
+				if (parent is not IMethodBodyOperation methodBodyOperation)
+					continue;
+				if (methodBodyOperation.Syntax is not MethodDeclarationSyntax methodSyntax)
+					continue;
+
+				var inTestMethod = methodSyntax.AttributeLists.SelectMany(list => list.Attributes).Any(attr =>
+				{
+					var typeInfo = semanticModel.GetTypeInfo(attr);
+					if (typeInfo.Type is null)
+						return false;
+
+					return
+						SymbolEqualityComparer.Default.Equals(typeInfo.Type, xunitContext.Core.FactAttributeType) ||
+						SymbolEqualityComparer.Default.Equals(typeInfo.Type, xunitContext.Core.TheoryAttributeType);
+				});
+
+				if (!inTestMethod)
+					return;
+
+				// invocation should be two nodes: "(some other code).ConfigureAwait" and the arguments (like "(false)")
+				var invocationChildren = invocation.Syntax.ChildNodes().ToList();
+				if (invocationChildren.Count != 2)
+					return;
+
+				// First child node should be split into three pieces: "(some other code)", ".", and "ConfigureAwait"
+				var methodCallChildren = invocationChildren[0].ChildNodesAndTokens().ToList();
+				if (methodCallChildren.Count != 3)
+					return;
+
+				// Construct a location that covers "ConfigureAwait(arguments)"
+				var length = methodCallChildren[2].Span.Length + invocationChildren[1].Span.Length;
+				var textSpan = new TextSpan(methodCallChildren[2].SpanStart, length);
+				var location = Location.Create(invocation.Syntax.SyntaxTree, textSpan);
+
+				context.ReportDiagnostic(Diagnostic.Create(Descriptors.X1030_DoNotUseConfigureAwait, location));
+			}
+		}, OperationKind.Invocation);
+	}
+}