xunit/xunit#2346: Add xUnit2021 to warn when async assertions are not…

… being awaited/stored

src/xunit.analyzers.fixes/X2000/AsyncAssertsShouldBeAwaitedFixer.cs

@@ -0,0 +1,73 @@
+using System.Composition;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Editing;
+using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
+
+namespace Xunit.Analyzers.Fixes;
+
+[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
+public class AsyncAssertsShouldBeAwaitedFixer : BatchedCodeFixProvider
+{
+	public const string Key_AddAwait = "xUnit2021_AddAwait";
+
+	public AsyncAssertsShouldBeAwaitedFixer() :
+		base(Descriptors.X2021_AsyncAssertionsShouldBeAwaited.Id)
+	{ }
+
+	public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
+	{
+		var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+		if (root is null)
+			return;
+
+		var invocation = root.FindNode(context.Span).FirstAncestorOrSelf<InvocationExpressionSyntax>();
+		if (invocation is null)
+			return;
+
+		var method = invocation.FirstAncestorOrSelf<MethodDeclarationSyntax>();
+		if (method is null)
+			return;
+
+		var diagnostic = context.Diagnostics.FirstOrDefault();
+		if (diagnostic is null)
+			return;
+
+		context.RegisterCodeFix(
+			CodeAction.Create(
+				"Add await",
+				ct => UseAsyncAwait(context.Document, invocation, method, ct),
+				Key_AddAwait
+			),
+			context.Diagnostics
+		);
+	}
+
+	static async Task<Document> UseAsyncAwait(
+		Document document,
+		InvocationExpressionSyntax invocation,
+		MethodDeclarationSyntax method,
+		CancellationToken cancellationToken)
+	{
+		var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
+		var modifiers = AsyncHelper.GetModifiersWithAsyncKeywordAdded(method);
+		var returnType = await AsyncHelper.GetReturnType(method, invocation, document, editor, cancellationToken);
+		var asyncThrowsInvocation = AwaitExpression(invocation.WithoutLeadingTrivia()).WithLeadingTrivia(invocation.GetLeadingTrivia());
+
+		if (returnType is not null)
+			editor.ReplaceNode(
+				method,
+				method
+					.ReplaceNode(invocation, asyncThrowsInvocation)
+					.WithModifiers(modifiers)
+					.WithReturnType(returnType)
+			);
+
+		return editor.GetChangedDocument();
+	}
+}

src/xunit.analyzers.tests/Analyzers/X2000/AsyncAssertsShouldBeAwaitedTests.cs

@@ -0,0 +1,138 @@
+using System.Globalization;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Xunit;
+using Verify = CSharpVerifier<Xunit.Analyzers.AsyncAssertsShouldBeAwaited>;
+
+public class AsyncAssertsShouldBeAwaitedTests
+{
+	[Fact]
+	public async void UnawaitedNonAssertionDoesNotTrigger()
+	{
+		var code = @"
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass {
+    [Fact]
+    public void TestMethod() {
+        Task.Delay(1);
+    }
+}";
+
+		await Verify.VerifyAnalyzer(code);
+	}
+
+	string codeTemplate = @"
+using System;
+using System.ComponentModel;
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass : INotifyPropertyChanged {{
+    public int Property {{ get; set; }}
+
+    public event PropertyChangedEventHandler? PropertyChanged;
+    public event EventHandler? SimpleEvent;
+    public event EventHandler<int>? SimpleIntEvent;
+
+    [Fact]
+    public async void TestMethod() {{
+        {0}
+    }}
+}}
+
+public static class MyTaskExtensions {{
+    public static void ConsumeTask(this Task t) {{ }}
+}}";
+
+	public static TheoryData<string, string> AsyncAssertions = new()
+	{
+		{ "PropertyChangedAsync", "Assert.PropertyChangedAsync(this, nameof(Property), async () => throw new DivideByZeroException())" },
+		{ "RaisesAnyAsync", "Assert.RaisesAnyAsync(eh => SimpleEvent += eh, eh => SimpleEvent -= eh, async () => throw new DivideByZeroException())" },
+		{ "RaisesAnyAsync", "Assert.RaisesAnyAsync<int>(eh => SimpleIntEvent += eh, eh => SimpleIntEvent -= eh, async () => throw new DivideByZeroException())" },
+		{ "RaisesAsync", "Assert.RaisesAsync<int>(eh => SimpleIntEvent += eh, eh => SimpleIntEvent -= eh, async () => throw new DivideByZeroException())" },
+		{ "ThrowsAnyAsync", "Assert.ThrowsAnyAsync<Exception>(async () => throw new DivideByZeroException())" },
+		{ "ThrowsAsync", "Assert.ThrowsAsync(typeof(DivideByZeroException), async () => throw new DivideByZeroException())" },
+		{ "ThrowsAsync", "Assert.ThrowsAsync<DivideByZeroException>(async () => throw new DivideByZeroException())" },
+		{ "ThrowsAsync", "Assert.ThrowsAsync<ArgumentException>(\"argName\", async () => throw new DivideByZeroException())" },
+	};
+
+	[Theory]
+	[MemberData(nameof(AsyncAssertions))]
+	public async void AwaitedAssertDoesNotTrigger(
+		string _,
+		string assertion)
+	{
+		var code = string.Format(CultureInfo.InvariantCulture, codeTemplate, $"await {assertion};");
+
+		await Verify.VerifyAnalyzer(LanguageVersion.CSharp8, code);
+	}
+
+	[Theory]
+	[MemberData(nameof(AsyncAssertions))]
+	public async void AssertionWithConsumptionNotTrigger(
+		string _,
+		string assertion)
+	{
+		var code = string.Format(CultureInfo.InvariantCulture, codeTemplate, $"MyTaskExtensions.ConsumeTask({assertion});");
+
+		await Verify.VerifyAnalyzer(LanguageVersion.CSharp8, code);
+	}
+
+	[Theory]
+	[MemberData(nameof(AsyncAssertions))]
+	public async void AssertionWithConsumptionViaExtensionNotTrigger(
+		string _,
+		string assertion)
+	{
+		var code = string.Format(CultureInfo.InvariantCulture, codeTemplate, $"{assertion}.ConsumeTask();");
+
+		await Verify.VerifyAnalyzer(LanguageVersion.CSharp8, code);
+	}
+
+	[Theory]
+	[MemberData(nameof(AsyncAssertions))]
+	public async void AssertionWithStoredTaskDoesNotTrigger(
+		string _,
+		string assertion)
+	{
+		var code = string.Format(CultureInfo.InvariantCulture, codeTemplate, $"var task = {assertion};");
+
+		await Verify.VerifyAnalyzer(LanguageVersion.CSharp8, code);
+	}
+
+	[Theory]
+	[MemberData(nameof(AsyncAssertions))]
+	public async void AssertionWithoutAwaitTriggers(
+		string assertionName,
+		string assertion)
+	{
+		var code = string.Format(CultureInfo.InvariantCulture, codeTemplate, $"{assertion};");
+		var expected =
+			Verify
+				.Diagnostic()
+				.WithSpan(16, 9, 16, 9 + assertion.Length)
+				.WithSeverity(DiagnosticSeverity.Error)
+				.WithArguments(assertionName);
+
+		await Verify.VerifyAnalyzer(LanguageVersion.CSharp8, code, expected);
+	}
+
+	[Theory]
+	[MemberData(nameof(AsyncAssertions))]
+	public async void AssertionWithUnawaitedContinuationTriggers(
+		string assertionName,
+		string assertion)
+	{
+		var code = string.Format(CultureInfo.InvariantCulture, codeTemplate, $"{assertion}.ContinueWith(t => {{ }});");
+		var expected =
+			Verify
+				.Diagnostic()
+				.WithSpan(16, 9, 16, 9 + assertion.Length)
+				.WithSeverity(DiagnosticSeverity.Error)
+				.WithArguments(assertionName);
+
+		await Verify.VerifyAnalyzer(LanguageVersion.CSharp8, code, expected);
+	}
+}

src/xunit.analyzers.tests/Fixes/X2000/AsyncAssertsShouldBeAwaitedFixerTests.cs

@@ -0,0 +1,52 @@
+using Microsoft.CodeAnalysis.CSharp;
+using Xunit;
+using Xunit.Analyzers.Fixes;
+using Verify = CSharpVerifier<Xunit.Analyzers.AsyncAssertsShouldBeAwaited>;
+
+public class AsyncAssertsShouldBeAwaitedFixerTests
+{
+	string codeTemplate = @"
+using System;
+using System.ComponentModel;
+using System.Threading.Tasks;
+using Xunit;
+
+public class TestClass : INotifyPropertyChanged {{
+    public int Property {{ get; set; }}
+
+    public event PropertyChangedEventHandler? PropertyChanged;
+    public event EventHandler? SimpleEvent;
+    public event EventHandler<int>? SimpleIntEvent;
+
+    [Fact]
+    public void TestMethod() {{
+        {0}
+    }}
+}}
+
+public static class MyTaskExtensions {{
+    public static void ConsumeTask(this Task t) {{ }}
+}}";
+
+	public static TheoryData<string> AsyncAssertions = new()
+	{
+		"Assert.PropertyChangedAsync(this, nameof(Property), async () => throw new DivideByZeroException())",
+		"Assert.RaisesAnyAsync(eh => SimpleEvent += eh, eh => SimpleEvent -= eh, async () => throw new DivideByZeroException())",
+		"Assert.RaisesAnyAsync<int>(eh => SimpleIntEvent += eh, eh => SimpleIntEvent -= eh, async () => throw new DivideByZeroException())",
+		"Assert.RaisesAsync<int>(eh => SimpleIntEvent += eh, eh => SimpleIntEvent -= eh, async () => throw new DivideByZeroException())",
+		"Assert.ThrowsAnyAsync<Exception>(async () => throw new DivideByZeroException())",
+		"Assert.ThrowsAsync(typeof(DivideByZeroException), async () => throw new DivideByZeroException())",
+		"Assert.ThrowsAsync<DivideByZeroException>(async () => throw new DivideByZeroException())",
+		"Assert.ThrowsAsync<ArgumentException>(\"argName\", async () => throw new DivideByZeroException())",
+	};
+
+	[Theory]
+	[MemberData(nameof(AsyncAssertions))]
+	public async void AddsAsyncAndAwait(string assertion)
+	{
+		var before = string.Format(codeTemplate, $"[|{assertion}|];");
+		var after = string.Format(codeTemplate, $"await {assertion};").Replace("public void TestMethod", "public async Task TestMethod");
+
+		await Verify.VerifyCodeFix(LanguageVersion.CSharp8, before, after, AsyncAssertsShouldBeAwaitedFixer.Key_AddAwait);
+	}
+}

src/xunit.analyzers/Utility/Constants.cs

@@ -37,6 +37,9 @@ public static class Asserts
 		public const string NotSame = "NotSame";
 		public const string NotStrictEqual = "NotStrictEqual";
 		public const string Null = "Null";
+		public const string PropertyChangedAsync = "PropertyChangedAsync";
+		public const string RaisesAnyAsync = "RaisesAnyAsync";
+		public const string RaisesAsync = "RaisesAsync";
 		public const string Same = "Same";
 		public const string Single = "Single";
 		public const string StartsWith = "StartsWith";

src/xunit.analyzers/Utility/Descriptors.cs

@@ -606,7 +606,14 @@ public static DiagnosticDescriptor X2019_AssertThrowsShouldNotBeUsedForAsyncThro
 			"Do not use Assert.{0}({1}, message) to fail a test. Use Assert.Fail(message) instead."
 		);
 
-	// Placeholder for rule X2021
+	public static DiagnosticDescriptor X2021_AsyncAssertionsShouldBeAwaited { get; } =
+		Rule(
+			"xUnit2021",
+			"Async assertions should be awaited",
+			Assertions,
+			Error,
+			"Assert.{0} is async. The resulting task should be awaited (or stored for later awaiting)."
+		);
 
 	// Placeholder for rule X2022
 

src/xunit.analyzers/X2000/AsyncAssertsShouldBeAwaited.cs

@@ -0,0 +1,64 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Xunit.Analyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class AsyncAssertsShouldBeAwaited : AssertUsageAnalyzerBase
+{
+	static readonly string[] targetMethods =
+	{
+		Constants.Asserts.PropertyChangedAsync,
+		Constants.Asserts.RaisesAnyAsync,
+		Constants.Asserts.RaisesAsync,
+		Constants.Asserts.ThrowsAnyAsync,
+		Constants.Asserts.ThrowsAsync,
+	};
+
+	public AsyncAssertsShouldBeAwaited() :
+		base(Descriptors.X2021_AsyncAssertionsShouldBeAwaited, targetMethods)
+	{ }
+
+	protected override void AnalyzeInvocation(
+		OperationAnalysisContext context,
+		XunitContext xunitContext,
+		IInvocationOperation invocationOperation,
+		IMethodSymbol method)
+	{
+		var taskType = TypeSymbolFactory.Task(context.Compilation);
+		var taskOfTType = TypeSymbolFactory.TaskOfT(context.Compilation)?.ConstructUnboundGenericType();
+
+		for (IOperation? current = invocationOperation; current is not null; current = current?.Parent)
+		{
+			// Stop looking when we've hit the enclosing block
+			if (current is IBlockOperation)
+				return;
+
+			// Only interested in something that results in an expression to a named type
+			if (current is not IExpressionStatementOperation expression || expression.Operation.Type is not INamedTypeSymbol namedReturnType)
+				continue;
+
+			if (namedReturnType.IsGenericType)
+			{
+				// Does it return Task<T>?
+				if (!SymbolEqualityComparer.Default.Equals(namedReturnType.ConstructUnboundGenericType(), taskOfTType))
+					continue;
+			}
+			else
+			{
+				// Does it return Task?
+				if (!SymbolEqualityComparer.Default.Equals(namedReturnType, taskType))
+					continue;
+			}
+
+			context.ReportDiagnostic(
+				Diagnostic.Create(
+					Descriptors.X2021_AsyncAssertionsShouldBeAwaited,
+					invocationOperation.Syntax.GetLocation(),
+					method.Name
+				)
+			);
+		}
+	}
+}