diff --git a/README.md b/README.md index d9ce452b9..8d0b618a8 100644 --- a/README.md +++ b/README.md @@ -149,6 +149,7 @@ If you are already using other analyzers, you can check [which rules are duplica |[MA0131](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0131.md)|Usage|ArgumentNullException.ThrowIfNull should not be used with non-nullable types|⚠️|✔️|❌| |[MA0132](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0132.md)|Design|Do not convert implicitly to DateTimeOffset|⚠️|✔️|❌| |[MA0133](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0133.md)|Design|Use DateTimeOffset instead of relying on the implicit conversion|ℹ️|✔️|❌| +|[MA0134](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0134.md)|Usage|Observe result of async calls|⚠️|✔️|❌| diff --git a/docs/README.md b/docs/README.md index 1e8d22b33..981be4653 100644 --- a/docs/README.md +++ b/docs/README.md @@ -133,6 +133,7 @@ |[MA0131](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0131.md)|Usage|ArgumentNullException.ThrowIfNull should not be used with non-nullable types|⚠️|✔️|❌| |[MA0132](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0132.md)|Design|Do not convert implicitly to DateTimeOffset|⚠️|✔️|❌| |[MA0133](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0133.md)|Design|Use DateTimeOffset instead of relying on the implicit conversion|ℹ️|✔️|❌| +|[MA0134](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0134.md)|Usage|Observe result of async calls|⚠️|✔️|❌| |Id|Suppressed rule|Justification| |--|---------------|-------------| @@ -538,6 +539,9 @@ dotnet_diagnostic.MA0132.severity = warning # MA0133: Use DateTimeOffset instead of relying on the implicit conversion dotnet_diagnostic.MA0133.severity = suggestion + +# MA0134: Observe result of async calls +dotnet_diagnostic.MA0134.severity = warning ``` # .editorconfig - all rules disabled @@ -938,4 +942,7 @@ dotnet_diagnostic.MA0132.severity = none # MA0133: Use DateTimeOffset instead of relying on the implicit conversion dotnet_diagnostic.MA0133.severity = none + +# MA0134: Observe result of async calls +dotnet_diagnostic.MA0134.severity = none ``` diff --git a/docs/Rules/MA0134.md b/docs/Rules/MA0134.md new file mode 100644 index 000000000..a65745928 --- /dev/null +++ b/docs/Rules/MA0134.md @@ -0,0 +1,15 @@ +# MA0134 - Observe result of async calls + +The result of awaitable method should be observed by using `await`, `Result`, `Wait`, or other methods. + +Note: [CS4014](https://learn.microsoft.com/en-US/dotnet/csharp/language-reference/compiler-messages/cs4014?WT.mc_id=DT-MVP-5003978) is similar but only operate in `async` methods. MA0134 operates in non-async methods. + +````c# +void Sample() +{ + Task.Delay(1); // non-compliant + + Task.Delay(1).Wait(); // ok + _ = Task.Delay(1); // ok +} +```` diff --git a/src/Meziantou.Analyzer/RuleIdentifiers.cs b/src/Meziantou.Analyzer/RuleIdentifiers.cs index 5c88f88ea..6607bbf85 100644 --- a/src/Meziantou.Analyzer/RuleIdentifiers.cs +++ b/src/Meziantou.Analyzer/RuleIdentifiers.cs @@ -136,6 +136,7 @@ internal static class RuleIdentifiers public const string ThrowIfNullWithNonNullableInstance = "MA0131"; public const string DoNotImplicitlyConvertDateTimeToDateTimeOffset = "MA0132"; public const string UseDateTimeOffsetInsteadOfDateTime = "MA0133"; + public const string AwaitAwaitableMethodInSyncMethod = "MA0134"; public static string GetHelpUri(string identifier) { diff --git a/src/Meziantou.Analyzer/Rules/AwaitAwaitableMethodInSyncMethodAnalyzer.cs b/src/Meziantou.Analyzer/Rules/AwaitAwaitableMethodInSyncMethodAnalyzer.cs new file mode 100644 index 000000000..aa7256bd1 --- /dev/null +++ b/src/Meziantou.Analyzer/Rules/AwaitAwaitableMethodInSyncMethodAnalyzer.cs @@ -0,0 +1,62 @@ +using System.Collections.Immutable; +using Meziantou.Analyzer.Internals; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Meziantou.Analyzer.Rules; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class AwaitAwaitableMethodInSyncMethodAnalyzer : DiagnosticAnalyzer +{ + private static readonly DiagnosticDescriptor s_rule = new( + RuleIdentifiers.AwaitAwaitableMethodInSyncMethod, + title: "Observe result of async calls", + messageFormat: "Observe result of async calls", + RuleCategories.Usage, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.AwaitAwaitableMethodInSyncMethod)); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(s_rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterCompilationStartAction(context => + { + var awaitableTypes = new AwaitableTypes(context.Compilation); + context.RegisterSymbolStartAction(context => + { + if (context.Symbol is IMethodSymbol method && (method.IsAsync || method.IsTopLevelStatementsEntryPointMethod())) + return; // Already handled by CS4014 + + context.RegisterOperationAction(context => AnalyzeOperation(context, awaitableTypes), OperationKind.Invocation); + }, SymbolKind.Method); + }); + } + + private static void AnalyzeOperation(OperationAnalysisContext context, AwaitableTypes awaitableTypes) + { + var operation = (IInvocationOperation)context.Operation; + + var parent = operation.Parent; + if (parent is null or IBlockOperation or IExpressionStatementOperation or IConditionalAccessOperation) + { + var semanticModel = operation.SemanticModel!; + var position = operation.Syntax.GetLocation().SourceSpan.End; + + // While there is a check in RegisterSymbolStartAction, this is needed to handle lambda and delegates + var enclosingSymbol = semanticModel.GetEnclosingSymbol(position, context.CancellationToken); + if (enclosingSymbol is IMethodSymbol method && (method.IsAsync || method.IsTopLevelStatementsEntryPointMethod())) + return; + + if (!awaitableTypes.IsAwaitable(operation.Type, semanticModel, position)) + return; + + context.ReportDiagnostic(s_rule, operation); + } + } +} diff --git a/src/Meziantou.Analyzer/Rules/NamedParameterAnalyzer.cs b/src/Meziantou.Analyzer/Rules/NamedParameterAnalyzer.cs index 23aa4e76e..6e7e0b200 100644 --- a/src/Meziantou.Analyzer/Rules/NamedParameterAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/NamedParameterAnalyzer.cs @@ -125,7 +125,7 @@ public override void Initialize(AnalysisContext context) if (argumentList is null) return; - var invokedMethodSymbol = syntaxContext.SemanticModel.GetSymbolInfo(invocationExpression).Symbol; + var invokedMethodSymbol = syntaxContext.SemanticModel.GetSymbolInfo(invocationExpression, syntaxContext.CancellationToken).Symbol; if (invokedMethodSymbol == null && invocationExpression.IsKind(SyntaxKind.ElementAccessExpression)) return; // Skip Array[index] @@ -160,7 +160,7 @@ bool IsParams(SyntaxNode node) if (expression.IsKind(SyntaxKind.NullLiteralExpression)) return false; - var type = syntaxContext.SemanticModel.GetTypeInfo(node).ConvertedType; + var type = syntaxContext.SemanticModel.GetTypeInfo(node, syntaxContext.CancellationToken).ConvertedType; return !type.IsEqualTo(lastParameter.Type); } diff --git a/src/Meziantou.Analyzer/Rules/PreferReturningCollectionAbstractionInsteadOfImplementationAnalyzer.cs b/src/Meziantou.Analyzer/Rules/PreferReturningCollectionAbstractionInsteadOfImplementationAnalyzer.cs index 79b35b521..bc3c59484 100644 --- a/src/Meziantou.Analyzer/Rules/PreferReturningCollectionAbstractionInsteadOfImplementationAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/PreferReturningCollectionAbstractionInsteadOfImplementationAnalyzer.cs @@ -1,6 +1,5 @@ using System.Collections.Generic; using System.Collections.Immutable; -using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -197,12 +196,12 @@ private bool IsValidType(ITypeSymbol? symbol) return true; var originalDefinition = symbol.OriginalDefinition; - if (ConcreteCollectionSymbols.Any(t => t.IsEqualTo(originalDefinition))) + if (ConcreteCollectionSymbols.Exists(t => t.IsEqualTo(originalDefinition))) return false; if (symbol is INamedTypeSymbol namedTypeSymbol) { - if (TaskSymbols.Any(t => t.IsEqualTo(symbol.OriginalDefinition))) + if (TaskSymbols.Exists(t => t.IsEqualTo(symbol.OriginalDefinition))) { return IsValidType(namedTypeSymbol.TypeArguments[0]); } diff --git a/tests/Meziantou.Analyzer.Test/Rules/AwaitAwaitableMethodInSyncMethodAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/AwaitAwaitableMethodInSyncMethodAnalyzerTests.cs new file mode 100644 index 000000000..cbefd02bd --- /dev/null +++ b/tests/Meziantou.Analyzer.Test/Rules/AwaitAwaitableMethodInSyncMethodAnalyzerTests.cs @@ -0,0 +1,350 @@ +using System.Threading.Tasks; +using Meziantou.Analyzer.Rules; +using Microsoft.CodeAnalysis; +using TestHelper; +using Xunit; + +namespace Meziantou.Analyzer.Test.Rules; +public sealed class AwaitAwaitableMethodInSyncMethodAnalyzerTests +{ + private static ProjectBuilder CreateProjectBuilder() + { + return new ProjectBuilder() + .WithAnalyzer(); + } + + [Fact] + public async Task NoReport_NonAwaitedTaskInAsyncMethod() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + async Task A() + { + Task.Delay(0); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task NoReport_AwaitedTaskInAsyncMethod() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + async Task A() + { + await Task.Delay(0); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task NoReport_TaskInAsyncLocalFunction() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + B(); + async void B() + { + Task.Delay(0); + } + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task NoReport_TaskInAsyncLambdaFunction() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + _ = new System.Action(async () => + { + Task.Delay(0); + }); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task NoReport_Discard() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + _ = Task.Delay(0); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task NoReport_TopLevelStatement() + { + await CreateProjectBuilder() + .WithOutputKind(OutputKind.ConsoleApplication) + .WithSourceCode(""" + using System.Threading.Tasks; + + Task.Delay(0); + """) + .ValidateAsync(); + } + + [Fact] + public async Task NoReport_FireAndForget() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + Task.Delay(0).FireAndForget(); + } + } + + static class Extensions + { + public static void FireAndForget(this Task task) => throw null; + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_TaskInSyncMethodReturningTask() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + Task A() + { + [||]Task.Delay(0); + return Task.CompletedTask; + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_TaskInSyncVoidMethod() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + [||]Task.Delay(0); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_CustomAwaitableInSyncMethod() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + [||]Task.Yield(); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_TaskInLambda() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + _ = new System.Action(() => + { + [||]Task.Delay(0); + }); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_TaskInLambda_Arrow() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + _ = new System.Action(() => [||]Task.Delay(0)); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_TaskInDelegate() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + _ = new System.Action(delegate + { + [||]Task.Delay(0); + }); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_TaskInDelegate_Parentheses() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + _ = new System.Action(delegate() + { + [||]Task.Delay(0); + }); + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_TaskInGetter() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + int A + { + get + { + [||]Task.Delay(0); + return 0; + } + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_TaskInLocalFunction() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + B(); + void B() + { + [||]Task.Delay(0); + } + } + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task Report_TaskConfigureAwait() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + void A() + { + [||]Task.Delay(0).ConfigureAwait(false); + } + } + """) + .ValidateAsync(); + } + + + [Fact] + public async Task Report_ConditionalInvoke() + { + await CreateProjectBuilder() + .WithSourceCode(""" + using System.Threading.Tasks; + class Test + { + Task ReturnTask() => throw null; + + void A(Test instance) + { + instance?[|.ReturnTask()|]; + } + } + """) + .ValidateAsync(); + } +}