From a041234dfb6080a0906787acca4de81fb815c53c Mon Sep 17 00:00:00 2001 From: Brad Wilson Date: Tue, 12 Dec 2023 11:41:04 -0800 Subject: [PATCH] Restructure DoNotUseConfigureAwaitTests --- .../X1000/DoNotUseConfigureAwaitTests.cs | 168 +++++++++++------- src/xunit.analyzers/Utility/Descriptors.cs | 2 +- .../X1000/DoNotUseConfigureAwait.cs | 24 ++- 3 files changed, 121 insertions(+), 73 deletions(-) diff --git a/src/xunit.analyzers.tests/Analyzers/X1000/DoNotUseConfigureAwaitTests.cs b/src/xunit.analyzers.tests/Analyzers/X1000/DoNotUseConfigureAwaitTests.cs index 3ac5b05..092d59f 100644 --- a/src/xunit.analyzers.tests/Analyzers/X1000/DoNotUseConfigureAwaitTests.cs +++ b/src/xunit.analyzers.tests/Analyzers/X1000/DoNotUseConfigureAwaitTests.cs @@ -4,7 +4,7 @@ using Verify = CSharpVerifier; public class DoNotUseConfigureAwaitTests { [Fact] - public async void SuccessCase_NoCall() + public async void NoCall_DoesNotTrigger() { var source = @" using System.Threading.Tasks; @@ -20,10 +20,28 @@ public class TestClass { await Verify.VerifyAnalyzer(source); } - [Fact] - public async void SuccessCase_ConfigureAwaitTrue() + public class ConfigureAwait_Boolean { - var source = @" + [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(false); + } +}"; + + await Verify.VerifyAnalyzer(source); + } + + [Fact] + public async void True_DoesNotTrigger() + { + var source = @" using System.Threading.Tasks; using Xunit; @@ -34,120 +52,138 @@ public class TestClass { } }"; - await Verify.VerifyAnalyzer(source); - } + await Verify.VerifyAnalyzer(source); + } - [Theory] - [InlineData("false")] - [InlineData("1 == 2")] - public async void FailureCase_Task_Async(string argumentValue) - { - var source = @$" + public static TheoryData InvalidValues = new() + { + "false", // Literal false + "1 == 2", // Logical false (we don't compute) + "1 == 1", // Logical true (we don't compute) + "booleanVar", // Reference value (we don't do lookup) + }; + + [Theory] + [MemberData(nameof(InvalidValues))] + public async void InvalidValue_TaskWithAwait_Triggers(string argumentValue) + { + var source = @$" using System.Threading.Tasks; using Xunit; public class TestClass {{ [Fact] public async Task TestMethod() {{ - await Task.Delay(1).[|ConfigureAwait({argumentValue})|]; + var booleanVar = true; + await Task.Delay(1).ConfigureAwait({argumentValue}); }} }}"; + var expected = + Verify + .Diagnostic() + .WithSpan(9, 29, 9, 45 + argumentValue.Length) + .WithArguments(argumentValue, "Omit ConfigureAwait, or use ConfigureAwait(true) to avoid CA2007."); - await Verify.VerifyAnalyzer(source); - } + await Verify.VerifyAnalyzer(source, expected); + } - [Theory] - [InlineData("false")] - [InlineData("1 == 2")] - public async void FailureCase_Task_NonAsync(string argumentValue) - { - var source = @$" + [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() {{ - Task.Delay(1).[|ConfigureAwait({argumentValue})|].GetAwaiter().GetResult(); + var booleanVar = true; + Task.Delay(1).ConfigureAwait({argumentValue}).GetAwaiter().GetResult(); }} }}"; + var expected = + Verify + .Diagnostic() + .WithSpan(9, 23, 9, 39 + argumentValue.Length) + .WithArguments(argumentValue, "Omit ConfigureAwait, or use ConfigureAwait(true) to avoid CA2007."); - await Verify.VerifyAnalyzer(source); - } + await Verify.VerifyAnalyzer(source, expected); + } - [Theory] - [InlineData("false")] - [InlineData("1 == 2")] - public async void FailureCase_TaskOfT(string argumentValue) - { - var source = @$" + [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 booleanVar = true; var task = Task.FromResult(42); - await task.[|ConfigureAwait({argumentValue})|]; + await task.ConfigureAwait({argumentValue}); }} }}"; + var expected = + Verify + .Diagnostic() + .WithSpan(10, 20, 10, 36 + argumentValue.Length) + .WithArguments(argumentValue, "Omit ConfigureAwait, or use ConfigureAwait(true) to avoid CA2007."); - await Verify.VerifyAnalyzer(source); - } + await Verify.VerifyAnalyzer(source, expected); + } - [Theory] - [InlineData("false")] - [InlineData("1 == 2")] - public async void FailureCase_ValueTask(string argumentValue) - { - var source = @$" + [Theory] + [MemberData(nameof(InvalidValues))] + public async void InvalidValue_ValueTask_Triggers(string argumentValue) + { + var source = @$" using System.Threading.Tasks; using Xunit; public class TestClass {{ [Fact] public async Task TestMethod() {{ + var booleanVar = true; var valueTask = default(ValueTask); - await valueTask.[|ConfigureAwait({argumentValue})|]; + await valueTask.ConfigureAwait({argumentValue}); }} }}"; + var expected = + Verify + .Diagnostic() + .WithSpan(10, 25, 10, 41 + argumentValue.Length) + .WithArguments(argumentValue, "Omit ConfigureAwait, or use ConfigureAwait(true) to avoid CA2007."); - await Verify.VerifyAnalyzer(source); - } + await Verify.VerifyAnalyzer(source, expected); + } - [Theory] - [InlineData("false")] - [InlineData("1 == 2")] - public async void FailureCase_ValueTaskOfT(string argumentValue) - { - var source = @$" + [Theory] + [MemberData(nameof(InvalidValues))] + public async void InvalidValue_ValueTaskOfT_Triggers(string argumentValue) + { + var source = @$" using System.Threading.Tasks; using Xunit; public class TestClass {{ [Fact] public async Task TestMethod() {{ + var booleanVar = true; var valueTask = default(ValueTask); - await valueTask.[|ConfigureAwait({argumentValue})|]; + await valueTask.ConfigureAwait({argumentValue}); }} }}"; + var expected = + Verify + .Diagnostic() + .WithSpan(10, 25, 10, 41 + argumentValue.Length) + .WithArguments(argumentValue, "Omit ConfigureAwait, or use ConfigureAwait(true) to avoid CA2007."); - 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); + await Verify.VerifyAnalyzer(source, expected); + } } } diff --git a/src/xunit.analyzers/Utility/Descriptors.cs b/src/xunit.analyzers/Utility/Descriptors.cs index f561b09..5377c0a 100644 --- a/src/xunit.analyzers/Utility/Descriptors.cs +++ b/src/xunit.analyzers/Utility/Descriptors.cs @@ -310,7 +310,7 @@ public static class Descriptors "Do not call ConfigureAwait(false) in test method", Usage, Warning, - "Test methods should not call ConfigureAwait(false), as it may bypass parallelization limits. Omit ConfigureAwait, or use ConfigureAwait(true) to avoid CA2007." + "Test methods should not call ConfigureAwait({0}), as it may bypass parallelization limits. {1}" ); public static DiagnosticDescriptor X1031_DoNotUseBlockingTaskOperations { get; } = diff --git a/src/xunit.analyzers/X1000/DoNotUseConfigureAwait.cs b/src/xunit.analyzers/X1000/DoNotUseConfigureAwait.cs index 6fc87f0..c3b4657 100644 --- a/src/xunit.analyzers/X1000/DoNotUseConfigureAwait.cs +++ b/src/xunit.analyzers/X1000/DoNotUseConfigureAwait.cs @@ -66,13 +66,25 @@ public class DoNotUseConfigureAwait : XunitDiagnosticAnalyzer if (invocationChildren.Count != 2) return; - // we want to exempt calls with "(true)" because of CA2007 + // We only care about invocations with a single parameter var arguments = invocationChildren[1]; var argumentChildren = arguments.ChildNodes().ToList(); - if (argumentChildren.Count == 1 && - argumentChildren[0] is ArgumentSyntax argumentSyntax && - argumentSyntax.ChildNodes().FirstOrDefault() is LiteralExpressionSyntax literalExpression && - literalExpression.IsKind(SyntaxKind.TrueLiteralExpression)) + if (argumentChildren.Count != 1 || argumentChildren[0] is not ArgumentSyntax argumentSyntax) + return; + + // Determine the invocation type and resolution + var parameterType = invocation.TargetMethod.Parameters[0].Type; + string resolution; + + // We want to exempt calls with "(true)" because of CA2007 + if (SymbolEqualityComparer.Default.Equals(parameterType, context.Compilation.GetSpecialType(SpecialType.System_Boolean))) + { + if (argumentSyntax.Expression is LiteralExpressionSyntax literalExpression && literalExpression.IsKind(SyntaxKind.TrueLiteralExpression)) + return; + + resolution = "Omit ConfigureAwait, or use ConfigureAwait(true) to avoid CA2007."; + } + else return; // First child node should be split into three pieces: "(some other code)", ".", and "ConfigureAwait" @@ -85,7 +97,7 @@ public class DoNotUseConfigureAwait : XunitDiagnosticAnalyzer var textSpan = new TextSpan(methodCallChildren[2].SpanStart, length); var location = Location.Create(invocation.Syntax.SyntaxTree, textSpan); - context.ReportDiagnostic(Diagnostic.Create(Descriptors.X1030_DoNotUseConfigureAwait, location)); + context.ReportDiagnostic(Diagnostic.Create(Descriptors.X1030_DoNotUseConfigureAwait, location, argumentSyntax.ToFullString(), resolution)); }, OperationKind.Invocation); } }