Analyzer + Fixer: Forward cancellation token to method invocations

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/ForwardCancellationTokenToInvocations.Analyzer.cs

@@ -123,7 +123,7 @@ private bool ShouldDiagnose(
 
             IMethodSymbol method = invocation.TargetMethod;
 
-            // Verify that the current invocation is not passing an explicitly token already
+            // Verify that the current invocation is not passing an explicit token already
             if (AnyArgument(invocation.Arguments,
                             a => a.Parameter.Type.Equals(cancellationTokenType) && !a.IsImplicit))
             {
@@ -267,7 +267,7 @@ private static bool InvocationIgnoresOptionalCancellationToken(
                 // Need to check among all arguments in case the user is passing them named and unordered (despite the ct being defined as the last parameter)
                 return AnyArgument(
                     arguments,
-                    a => a.Parameter.Equals(cancellationTokenType) && a.ArgumentKind == ArgumentKind.DefaultValue);
+                    a => a.Parameter.Type.Equals(cancellationTokenType) && a.ArgumentKind == ArgumentKind.DefaultValue);
             }
             return false;
         }
@@ -299,9 +299,10 @@ private static bool MethodHasCancellationTokenOverload(
             ITypeSymbol cancellationTokenType,
             [NotNullWhen(returnValue: true)] out IMethodSymbol? overload)
         {
-            ImmutableArray<ISymbol> overloads = method.ContainingType.GetMembers(method.Name);
-            System.Collections.Generic.List<IMethodSymbol> ofType = overloads.OfType<IMethodSymbol>().ToList();
-            overload = ofType.FirstOrDefault(methodToCompare =>
+            overload = method.ContainingType
+                                .GetMembers(method.Name)
+                                .OfType<IMethodSymbol>()
+                                .FirstOrDefault(methodToCompare =>
                 HasSameParametersPlusCancellationToken(cancellationTokenType, method, methodToCompare));
 
             return overload != null;
@@ -310,22 +311,23 @@ private static bool MethodHasCancellationTokenOverload(
             static bool HasSameParametersPlusCancellationToken(ITypeSymbol cancellationTokenType, IMethodSymbol originalMethod, IMethodSymbol methodToCompare)
             {
                 // Avoid comparing to itself, or when there are no parameters, or when the last parameter is not a ct
-                if (originalMethod.Equals(methodToCompare.Name) ||
+                if (originalMethod.Equals(methodToCompare) ||
                     methodToCompare.Parameters.Count(p => p.Type.Equals(cancellationTokenType)) != 1 ||
                     !methodToCompare.Parameters[^1].Type.Equals(cancellationTokenType))
                 {
                     return false;
                 }
 
-                IMethodSymbol originalMethodWithAllParameters = originalMethod.ReducedFrom ?? originalMethod.ConstructedFrom ?? originalMethod;
-                IMethodSymbol methodToCompareWithAllParameters = methodToCompare.ReducedFrom ?? methodToCompare.ConstructedFrom ?? methodToCompare;
+                IMethodSymbol originalMethodWithAllParameters = (originalMethod.ReducedFrom ?? originalMethod).OriginalDefinition;
+                IMethodSymbol methodToCompareWithAllParameters = (methodToCompare.ReducedFrom ?? methodToCompare).OriginalDefinition;
 
                 // Now compare the types of all parameters before the ct
+                // The largest i is the number of parameters in the method that has fewer parameters
                 for (int i = 0; i < originalMethodWithAllParameters.Parameters.Length; i++)
                 {
-                    IParameterSymbol? originalParameter = originalMethodWithAllParameters.Parameters[i];
-                    IParameterSymbol? comparedParameter = methodToCompareWithAllParameters.Parameters[i];
-                    if (originalParameter == null || comparedParameter == null || !originalParameter.Type.Equals(comparedParameter.Type))
+                    IParameterSymbol originalParameter = originalMethodWithAllParameters.Parameters[i];
+                    IParameterSymbol comparedParameter = methodToCompareWithAllParameters.Parameters[i];
+                    if (!originalParameter.Type.Equals(comparedParameter.Type))
                     {
                         return false;
                     }

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/ForwardCancellationTokenToInvocations.Fixer.cs

@@ -77,39 +77,37 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
             string title = MicrosoftNetCoreAnalyzersResources.ForwardCancellationTokenToInvocationsTitle;
 
-            if (!TryGenerateNewDocumentRoot(doc, root, invocation, argumentName, parameterName, out SyntaxNode? newRoot))
+            if (!TryGetExpressionAndArguments(invocation.Syntax, out SyntaxNode? expression, out List<SyntaxNode>? newArguments))
             {
                 return;
             }
 
-            Task<Document> createChangedDocument(CancellationToken _) =>
-                Task.FromResult(doc.WithSyntaxRoot(newRoot));
+            Task<Document> CreateChangedDocument(CancellationToken _)
+            {
+                SyntaxNode newRoot = TryGenerateNewDocumentRoot(doc, root, invocation, argumentName, parameterName, expression, newArguments);
+                Document newDocument = doc.WithSyntaxRoot(newRoot);
+                return Task.FromResult(newDocument);
+            }
 
             context.RegisterCodeFix(
                 new MyCodeAction(
                     title: title,
-                    createChangedDocument,
+                    CreateChangedDocument,
                     equivalenceKey: title),
                 context.Diagnostics);
         }
 
-        private bool TryGenerateNewDocumentRoot(
+        private SyntaxNode TryGenerateNewDocumentRoot(
             Document doc,
             SyntaxNode root,
             IInvocationOperation invocation,
             string invocationTokenArgumentName,
             string ancestorTokenParameterName,
-            [NotNullWhen(returnValue: true)] out SyntaxNode? newRoot)
+            SyntaxNode expression,
+            List<SyntaxNode> newArguments)
         {
-            newRoot = null;
-
             SyntaxGenerator generator = SyntaxGenerator.GetGenerator(doc);
 
-            if (!TryGetExpressionAndArguments(invocation.Syntax, out SyntaxNode? expression, out List<SyntaxNode>? newArguments))
-            {
-                return false;
-            }
-
             SyntaxNode identifier = generator.IdentifierName(invocationTokenArgumentName);
             SyntaxNode cancellationTokenArgument;
             if (!string.IsNullOrEmpty(ancestorTokenParameterName))
@@ -123,34 +121,32 @@ private bool TryGenerateNewDocumentRoot(
 
             newArguments.Add(cancellationTokenArgument);
 
-            SyntaxNode newInvocation;
+            SyntaxNode newInstance;
             // The instance is null when calling a static method from another type
             switch (invocation.Instance)
             {
                 case null:
-                    newInvocation = expression;
+                    newInstance = expression;
                     break;
                 case IConditionalAccessInstanceOperation _:
-                    newInvocation = GetConditionalOperationInvocationExpression(invocation.Syntax);
+                    newInstance = GetConditionalOperationInvocationExpression(invocation.Syntax);
                     break;
                 default:
                     if (invocation.Instance.IsImplicit)
                     {
-                        newInvocation = invocation.GetInstanceSyntax()!;
+                        newInstance = invocation.GetInstanceSyntax()!;
                     }
                     // Calling a method from an object, we must include the instance variable name
                     else
                     {
-                        newInvocation = generator.MemberAccessExpression(invocation.GetInstanceSyntax(), invocation.TargetMethod.Name);
+                        newInstance = generator.MemberAccessExpression(invocation.GetInstanceSyntax(), invocation.TargetMethod.Name);
                     }
                     break;
             }
             // Insert the new arguments to the new invocation
-            SyntaxNode newInvocationWithArguments = generator.InvocationExpression(newInvocation, newArguments).WithTriviaFrom(invocation.Syntax);
-
-            newRoot = generator.ReplaceNode(root, invocation.Syntax, newInvocationWithArguments);
+            SyntaxNode newInvocationWithArguments = generator.InvocationExpression(newInstance, newArguments).WithTriviaFrom(invocation.Syntax);
 
-            return true;
+            return generator.ReplaceNode(root, invocation.Syntax, newInvocationWithArguments);
         }
 
         // Needed for Telemetry (https://github.com/dotnet/roslyn-analyzers/issues/192) 

src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/ForwardCancellationTokenToInvocationsTests.cs

@@ -299,7 +299,7 @@ async void M(CancellationToken ct)
         }
 
         [Fact]
-        public Task CS_NoDiagnostic_CancellationTokenSource_ParamsUsed_Order1()
+        public Task CS_NoDiagnostic_CancellationTokenSource_ParamsUsed_Order()
         {
             /*
             CancellationTokenSource has 3 different overloads that take CancellationToken arguments.
@@ -330,122 +330,6 @@ public static void Method(params CancellationToken[] tokens){}
             ");
         }
 
-        [Fact]
-        public Task CS_NoDiagnostic_CancellationTokenSource_ParamsUsed_Order2()
-        {
-            /*
-            CancellationTokenSource has 3 different overloads that take CancellationToken arguments.
-            We should detect if a ct is passed and not offer a diagnostic, because it's considered one of the `params`.
-
-            public class CancellationTokenSource : IDisposable
-            {
-                public static CancellationTokenSource CreateLinkedTokenSource(CancellationToken token);
-                public static CancellationTokenSource CreateLinkedTokenSource(CancellationToken token1, CancellationToken token2);
-                public static CancellationTokenSource CreateLinkedTokenSource(params CancellationToken[] tokens);
-            }
-            */
-            return CS8VerifyAnalyzerAsync(@"
-using System.Threading;
-class C
-{
-    void M(CancellationToken ct)
-    {
-        CTS.Method(ct); // Don't diagnose
-    }
-}
-class CTS
-{
-    public static void Method(CancellationToken token){}
-    public static void Method(params CancellationToken[] tokens){}
-    public static void Method(CancellationToken token1, CancellationToken token2){}
-}
-            ");
-        }
-
-        [Fact]
-        public Task CS_NoDiagnostic_CancellationTokenSource_ParamsUsed_Order3()
-        {
-            return CS8VerifyAnalyzerAsync(@"
-using System.Threading;
-class C
-{
-    void M(CancellationToken ct)
-    {
-        CTS.Method(ct); // Don't diagnose
-    }
-}
-class CTS
-{
-    public static void Method(CancellationToken token1, CancellationToken token2){}
-    public static void Method(params CancellationToken[] tokens){}
-    public static void Method(CancellationToken token){}
-}
-            ");
-        }
-
-        [Fact]
-        public Task CS_NoDiagnostic_CancellationTokenSource_ParamsUsed_Order4()
-        {
-            return CS8VerifyAnalyzerAsync(@"
-using System.Threading;
-class C
-{
-    void M(CancellationToken ct)
-    {
-        CTS.Method(ct); // Don't diagnose
-    }
-}
-class CTS
-{
-    public static void Method(CancellationToken token1, CancellationToken token2){}
-    public static void Method(CancellationToken token){}
-    public static void Method(params CancellationToken[] tokens){}
-}
-            ");
-        }
-
-        [Fact]
-        public Task CS_NoDiagnostic_CancellationTokenSource_ParamsUsed_Order5()
-        {
-            return CS8VerifyAnalyzerAsync(@"
-using System.Threading;
-class C
-{
-    void M(CancellationToken ct)
-    {
-        CTS.Method(ct); // Don't diagnose
-    }
-}
-class CTS
-{
-    public static void Method(params CancellationToken[] tokens){}
-    public static void Method(CancellationToken token){}
-    public static void Method(CancellationToken token1, CancellationToken token2){}
-}
-            ");
-        }
-
-        [Fact]
-        public Task CS_NoDiagnostic_CancellationTokenSource_ParamsUsed_Order6()
-        {
-            return CS8VerifyAnalyzerAsync(@"
-using System.Threading;
-class C
-{
-    void M(CancellationToken ct)
-    {
-        CTS.Method(ct); // Don't diagnose
-    }
-}
-class CTS
-{
-    public static void Method(params CancellationToken[] tokens){}
-    public static void Method(CancellationToken token1, CancellationToken token2){}
-    public static void Method(CancellationToken token){}
-}
-            ");
-        }
-
         [Fact]
         public Task CS_NoDiagnostic_ExtensionMethodTakesToken()
         {