diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/UseAwaitableMethod.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/UseAwaitableMethod.cs index 79211463390..8897c6257cb 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/UseAwaitableMethod.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/UseAwaitableMethod.cs @@ -48,7 +48,7 @@ protected override void Initialize(SonarAnalysisContext context) => { var invocationExpression = (InvocationExpressionSyntax)nodeContext.Node; - var awaitableAlternatives = FindAwaitableAlternatives(wellKnownExtensionMethodContainer, codeBlockStart.CodeBlock, invocationExpression, + var awaitableAlternatives = FindAwaitableAlternatives(wellKnownExtensionMethodContainer, invocationExpression, nodeContext.SemanticModel, nodeContext.ContainingSymbol, nodeContext.Cancel); if (awaitableAlternatives.FirstOrDefault() is { Name: { } alternative }) { @@ -85,7 +85,7 @@ private static WellKnownExtensionMethodContainer BuildWellKnownExtensionMethodCo return wellKnownExtensionMethodContainer; } - private static ImmutableArray FindAwaitableAlternatives(WellKnownExtensionMethodContainer wellKnownExtensionMethodContainer, SyntaxNode codeBlock, + private static ImmutableArray FindAwaitableAlternatives(WellKnownExtensionMethodContainer wellKnownExtensionMethodContainer, InvocationExpressionSyntax invocationExpression, SemanticModel semanticModel, ISymbol containingSymbol, CancellationToken cancel) { var awaitableRoot = GetAwaitableRootOfInvocation(invocationExpression); @@ -101,7 +101,10 @@ private static ImmutableArray FindAwaitableAlternatives(WellKnownExtens : containingSymbol.ContainingType; // If not dotted, than the scope is the current type. Local function support is missing here. var members = GetMethodSymbolsInScope($"{methodSymbol.Name}Async", wellKnownExtensionMethodContainer, invokedType, methodSymbol.ContainingType); var awaitableCandidates = members.Where(x => x.IsAwaitableNonDynamic()); - var awaitableAlternatives = SpeculativeBindCandidates(semanticModel, codeBlock, awaitableRoot, invocationExpression, awaitableCandidates).ToImmutableArray(); + // Get the method alternatives and exclude candidates that would resolve to the containing method (endless loop) + var awaitableAlternatives = SpeculativeBindCandidates(semanticModel, awaitableRoot, invocationExpression, awaitableCandidates) + .Where(x => !containingSymbol.Equals(x)) + .ToImmutableArray(); return awaitableAlternatives; } return ImmutableArray.Empty; @@ -120,23 +123,31 @@ private static IEnumerable WellKnownExtensionMethodContainer(W ? extensionMethodContainer : []; - private static IEnumerable SpeculativeBindCandidates(SemanticModel semanticModel, SyntaxNode codeBlock, SyntaxNode awaitableRoot, + private static IEnumerable SpeculativeBindCandidates(SemanticModel semanticModel, SyntaxNode awaitableRoot, InvocationExpressionSyntax invocationExpression, IEnumerable awaitableCandidates) => awaitableCandidates .Select(x => x.Name) .Distinct() - .Select(x => SpeculativeBindCandidate(semanticModel, x, codeBlock, awaitableRoot, invocationExpression)) + .Select(x => SpeculativeBindCandidate(semanticModel, x, awaitableRoot, invocationExpression)) .WhereNotNull(); - private static IMethodSymbol SpeculativeBindCandidate(SemanticModel semanticModel, string candidateName, SyntaxNode codeBlock, SyntaxNode awaitableRoot, + private static IMethodSymbol SpeculativeBindCandidate(SemanticModel semanticModel, string candidateName, SyntaxNode awaitableRoot, InvocationExpressionSyntax invocationExpression) { - var root = codeBlock.SyntaxTree.GetRoot(); var invocationIdentifierName = invocationExpression.GetMethodCallIdentifier()?.Parent; if (invocationIdentifierName is null) { return null; } + var invocationReplaced = ReplaceInvocation(awaitableRoot, invocationExpression, invocationIdentifierName, candidateName); + var speculativeSymbolInfo = semanticModel.GetSpeculativeSymbolInfo(invocationReplaced.SpanStart, invocationReplaced, SpeculativeBindingOption.BindAsExpression); + var speculativeSymbol = speculativeSymbolInfo.Symbol as IMethodSymbol; + return speculativeSymbol; + } + + private static SyntaxNode ReplaceInvocation(SyntaxNode awaitableRoot, InvocationExpressionSyntax invocationExpression, SyntaxNode invocationIdentifierName, string candidateName) + { + var root = invocationExpression.SyntaxTree.GetRoot(); var invocationAnnotation = new SyntaxAnnotation(); var replace = root.ReplaceNodes([awaitableRoot, invocationIdentifierName, invocationExpression], (original, newNode) => { @@ -158,14 +169,12 @@ private static IMethodSymbol SpeculativeBindCandidate(SemanticModel semanticMode } if (original == awaitableRoot && result is ExpressionSyntax resultExpression) { - result = SyntaxFactory.AwaitExpression(resultExpression); + result = SyntaxFactory.ParenthesizedExpression( + SyntaxFactory.AwaitExpression(resultExpression.WithoutTrivia().WithLeadingTrivia(SyntaxFactory.ElasticSpace))).WithTriviaFrom(resultExpression); } return result; }); - var invocationReplaced = replace.GetAnnotatedNodes(invocationAnnotation).First(); - var speculativeSymbolInfo = semanticModel.GetSpeculativeSymbolInfo(invocationReplaced.SpanStart, invocationReplaced, SpeculativeBindingOption.BindAsExpression); - var speculativeSymbol = speculativeSymbolInfo.Symbol as IMethodSymbol; - return speculativeSymbol; + return replace.GetAnnotatedNodes(invocationAnnotation).First(); } private static ExpressionSyntax GetAwaitableRootOfInvocation(ExpressionSyntax expression) => diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/UseAwaitableMethodTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/UseAwaitableMethodTest.cs index 6d1511af95a..ead8ee2c595 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/UseAwaitableMethodTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/UseAwaitableMethodTest.cs @@ -93,5 +93,13 @@ public void UseAwaitableMethod_EF() => .AddReferences(NuGetMetadataReference.MicrosoftEntityFrameworkCoreSqlServer(EntityFrameworkVersion)) .AddPaths("UseAwaitableMethod_EF.cs") .Verify(); + + [TestMethod] + public void UseAwaitableMethod_MongoDb() => + builder + .WithOptions(ParseOptionsHelper.FromCSharp11) + .AddReferences(NuGetMetadataReference.MongoDBDriver()) + .AddPaths("UseAwaitableMethod_MongoDBDriver.cs") + .Verify(); #endif } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod.cs index cdac7df5126..cf3193fb06b 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod.cs @@ -244,3 +244,20 @@ async Task Test() VoidMethod(1); // FN. CancellationToken.None could be provided by the code fix } } + +class ResolvesToSelf +{ + public void Synchronous() { } + + public async Task SynchronousAsync() + { + Synchronous(); // Compliant. The fix would cause an endless loop + } + + public void Generic() { } + + public async Task GenericAsync() + { + Generic(); // Compliant. The fix would cause an endless loop + } +} diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_MongoDBDriver.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_MongoDBDriver.cs new file mode 100644 index 00000000000..f29db6e30c2 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_MongoDBDriver.cs @@ -0,0 +1,23 @@ +using System; +using System.Linq; +using System.Threading.Tasks; +using MongoDB.Driver; + +public record Person(int Id, string Name); + +public class MongoDBDriver +{ + public async Task Query(IMongoCollection personCollection) + { + var sort = Builders.Sort.Descending(nameof(Person.Name)); + // FP for + // * Find https://mongodb.github.io/mongo-csharp-driver/2.8/apidocs/html/M_MongoDB_Driver_IMongoCollectionExtensions_Find__1_3.htm + // * FindAsync https://mongodb.github.io/mongo-csharp-driver/2.8/apidocs/html/M_MongoDB_Driver_IMongoCollectionExtensions_FindAsync__1_3.htm + // Speculative binding finds "FindAsync" but the return type IAsyncCursor<> of FindAsync is not compatible with return type IFindFluent<,> of "Find" + // Speculative binding does overload resolution according to the C# rules, which ignore return types. + // It seems to ignore the compiler binding error for the following "Sort" which is only defined on IFindFluent, but not in IAsyncCursor. + var snapshot = await personCollection.Find(s => s.Id > 10) // Noncompliant FP + .Sort(sort) // Not defined on IAsyncCursor (return type of FindAsync) + .FirstOrDefaultAsync().ConfigureAwait(false); + } +} diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs b/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs index 4d396c48f30..5561c3ff775 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs @@ -119,6 +119,10 @@ public static References MicrosoftNetSdkFunctions(string packageVersion = Consta public static References MicrosoftNetWebApiCore(string packageVersion) => Create("Microsoft.AspNet.WebApi.Core", packageVersion); public static References MicrosoftSqlServerCompact(string packageVersion = "4.0.8876.1") => Create("Microsoft.SqlServer.Compact", packageVersion); public static References MicrosoftWebXdt(string packageVersion = "3.0.0") => Create("Microsoft.Web.Xdt", packageVersion); + public static References MongoDBDriver(string packageVersion = Constants.NuGetLatestVersion) => + Create("MongoDB.Driver", packageVersion) + .Concat(MongoDBDriverCore(packageVersion)); + public static References MongoDBDriverCore(string packageVersion = Constants.NuGetLatestVersion) => Create("MongoDB.Driver.Core", packageVersion); public static References MonoPosixNetStandard(string packageVersion = "1.0.0") => Create("Mono.Posix.NETStandard", packageVersion, "linux-x64"); public static References MonoDataSqlite(string packageVersion = Constants.NuGetLatestVersion) => Create("Mono.Data.Sqlite", packageVersion); public static References Moq(string packageVersion) => Create("Moq", packageVersion);