Fix S6966 FPs/FNs: FPs/FNs after peach validation (#9222)

analyzers/src/SonarAnalyzer.CSharp/Rules/UseAwaitableMethod.cs

@@ -48,7 +48,7 @@ public sealed class UseAwaitableMethod : SonarDiagnosticAnalyzer
                     {
                         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<ISymbol> FindAwaitableAlternatives(WellKnownExtensionMethodContainer wellKnownExtensionMethodContainer, SyntaxNode codeBlock,
+    private static ImmutableArray<ISymbol> FindAwaitableAlternatives(WellKnownExtensionMethodContainer wellKnownExtensionMethodContainer,
         InvocationExpressionSyntax invocationExpression, SemanticModel semanticModel, ISymbol containingSymbol, CancellationToken cancel)
     {
         var awaitableRoot = GetAwaitableRootOfInvocation(invocationExpression);
@@ -101,7 +101,10 @@ private static WellKnownExtensionMethodContainer BuildWellKnownExtensionMethodCo
                 : 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<ISymbol>.Empty;
@@ -120,23 +123,31 @@ private static WellKnownExtensionMethodContainer BuildWellKnownExtensionMethodCo
             ? extensionMethodContainer
             : [];
 
-    private static IEnumerable<ISymbol> SpeculativeBindCandidates(SemanticModel semanticModel, SyntaxNode codeBlock, SyntaxNode awaitableRoot,
+    private static IEnumerable<ISymbol> SpeculativeBindCandidates(SemanticModel semanticModel, SyntaxNode awaitableRoot,
         InvocationExpressionSyntax invocationExpression, IEnumerable<IMethodSymbol> 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 WellKnownExtensionMethodContainer BuildWellKnownExtensionMethodCo
             }
             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) =>

analyzers/tests/SonarAnalyzer.Test/Rules/UseAwaitableMethodTest.cs

@@ -93,5 +93,13 @@ async Task MethodInvocations()
         .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
 }

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<T>() { }
+
+    public async Task GenericAsync<T>()
+    {
+        Generic<T>(); // Compliant. The fix would cause an endless loop
+    }
+}

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<Person> personCollection)
+    {
+        var sort = Builders<Person>.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);
+    }
+}

analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs

@@ -120,6 +120,10 @@ public static class NuGetMetadataReference
     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);