Analyzer + Fixer: Forward cancellation token to method invocations (#…

…3641)

* Analyzer + Fixer: Forward cancellation token to async methods

* Address PR suggestions:
- Do not limit to only await. Detect all invocations.
- Fix the resource messages.
- Fix some warnings.

* Address case where CancellationToken is an aliased using

* Address PR suggestions

* Fix breaking change introduced by most recent rebased changes.

* Address PR suggestions

* Address named argument problem in C#, add two cases with diagnostics but no fix.

* Offer diagnostic only on the invocation expression name.

* Bail out early in fixer before registering code fix

* address analyzer attributes in wrong locations

* Fix newest edge cases except lambda that does not take token

* Address last false positives, add trivia support

* Fix comment formatting warning

* Additional case for ct params

* Added more edge cases, ensured to do the most basic check at the beginning

* Add method extension unit test

* Address suggestions and more edge cases

* Address latest suggestions, fixed one VB case where extension method parameters were not detected correctly

* Address latest suggestions

* Address last suggestions

src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpForwardCancellationTokenToInvocations.Analyzer.cs

@@ -0,0 +1,39 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using Microsoft.CodeAnalysis;
+using Microsoft.NetCore.Analyzers.Runtime;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis.Operations;
+using System.Linq;
+
+namespace Microsoft.NetCore.CSharp.Analyzers.Runtime
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class CSharpForwardCancellationTokenToInvocationsAnalyzer : ForwardCancellationTokenToInvocationsAnalyzer
+    {
+        protected override SyntaxNode? GetInvocationMethodNameNode(SyntaxNode invocationNode)
+        {
+            if (invocationNode is InvocationExpressionSyntax invocationExpression)
+            {
+                if (invocationExpression.Expression is MemberBindingExpressionSyntax memberBindingExpression)
+                {
+                    // When using nullability features, specifically attempting to dereference possible null references,
+                    // the dot becomes part of the member invocation expression, so we need to return just the name,
+                    // so that the diagnostic gets properly returned in the method name only.
+                    return memberBindingExpression.Name;
+                }
+                return invocationExpression.Expression;
+            }
+            return null;
+        }
+        protected override bool ArgumentsImplicitOrNamed(INamedTypeSymbol cancellationTokenType, ImmutableArray<IArgumentOperation> arguments)
+        {
+            return arguments.Any(a =>
+                (a.IsImplicit && !a.Parameter.Type.Equals(cancellationTokenType)) ||
+                (a.Syntax is ArgumentSyntax argumentNode && argumentNode.NameColon != null));
+        }
+
+    }
+}

src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpForwardCancellationTokenToInvocations.Fixer.cs

@@ -0,0 +1,64 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.Linq;
+using System.Threading;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Operations;
+using Microsoft.NetCore.Analyzers.Runtime;
+
+namespace Microsoft.NetCore.CSharp.Analyzers.Runtime
+{
+    [ExportCodeFixProvider(LanguageNames.CSharp)]
+    public sealed class CSharpForwardCancellationTokenToInvocationsFixer : ForwardCancellationTokenToInvocationsFixer
+    {
+        protected override bool TryGetInvocation(
+            SemanticModel model,
+            SyntaxNode node,
+            CancellationToken ct,
+            [NotNullWhen(true)] out IInvocationOperation? invocation)
+        {
+            // If the method was invoked using nullability for the case of attempting to dereference a possibly null reference,
+            // then the node.Parent.Parent is the actual invocation (and it will contain the dot as well)
+
+            var operation = node.Parent.IsKind(SyntaxKind.MemberBindingExpression)
+                ? model.GetOperation(node.Parent.Parent, ct)
+                : model.GetOperation(node.Parent, ct);
+
+            invocation = operation as IInvocationOperation;
+
+            return invocation != null;
+        }
+
+        protected override bool IsArgumentNamed(IArgumentOperation argumentOperation)
+        {
+            return argumentOperation.Syntax is ArgumentSyntax argumentNode && argumentNode.NameColon != null;
+        }
+
+        protected override SyntaxNode GetConditionalOperationInvocationExpression(SyntaxNode invocationNode)
+        {
+            return ((InvocationExpressionSyntax)invocationNode).Expression;
+        }
+
+        protected override bool TryGetExpressionAndArguments(
+            SyntaxNode invocationNode,
+            [NotNullWhen(returnValue: true)] out SyntaxNode? expression,
+            [NotNullWhen(returnValue: true)] out List<SyntaxNode>? arguments)
+        {
+            if (invocationNode is InvocationExpressionSyntax invocationExpression)
+            {
+                expression = invocationExpression.Expression;
+                arguments = invocationExpression.ArgumentList.Arguments.Cast<SyntaxNode>().ToList();
+                return true;
+            }
+
+            expression = null;
+            arguments = null;
+            return false;
+        }
+    }
+}

src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md

@@ -24,6 +24,7 @@ CA2012 | Reliability | Hidden | UseValueTasksCorrectlyAnalyzer, [Documentation](
 CA2013 | Reliability | Warning | DoNotUseReferenceEqualsWithValueTypesAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2013)
 CA2014 | Reliability | Warning | DoNotUseStackallocInLoopsAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2014)
 CA2015 | Reliability | Warning | DoNotDefineFinalizersForTypesDerivedFromMemoryManager, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2015)
+CA2016 | Reliability | Info | ForwardCancellationTokenToInvocationsAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2016)
 CA2247 | Usage | Warning | DoNotCreateTaskCompletionSourceWithWrongArguments, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2247)
 CA2248 | Usage | Info | DoNotCheckFlagFromDifferentEnum, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2248)
 CA2249 | Usage | Info | PreferStringContainsOverIndexOfAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2249)

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx

@@ -1279,14 +1279,23 @@
     <value>Potential stack overflow. Move the stackalloc out of the loop.</value>
   </data>
   <data name="PreferStreamAsyncMemoryOverloadsTitle" xml:space="preserve">
-    <value>Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync'.</value>
+    <value>Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync'</value>
   </data>
   <data name="PreferStreamAsyncMemoryOverloadsDescription" xml:space="preserve">
     <value>'Stream' has a 'ReadAsync' overload that takes a 'Memory&lt;Byte&gt;' as the first argument, and a 'WriteAsync' overload that takes a 'ReadOnlyMemory&lt;Byte&gt;' as the first argument. Prefer calling the memory based overloads, which are more efficient.</value>
   </data>
   <data name="PreferStreamAsyncMemoryOverloadsMessage" xml:space="preserve">
     <value>Change the '{0}' method call to use the '{1}' overload</value>
   </data>
+  <data name="ForwardCancellationTokenToInvocationsDescription" xml:space="preserve">
+    <value>Forward the 'CancellationToken' parameter to methods that take one to ensure the operation cancellation notifications gets properly propagated, or pass in 'CancellationToken.None' explicitly to indicate intentionally not propagating the token.</value>
+  </data>
+  <data name="ForwardCancellationTokenToInvocationsMessage" xml:space="preserve">
+    <value>Forward the '{0}' parameter to the '{1}' method or pass in 'CancellationToken.None' explicitly to indicate intentionally not propagating the token</value>
+  </data>
+  <data name="ForwardCancellationTokenToInvocationsTitle" xml:space="preserve">
+    <value>Forward the 'CancellationToken' parameter to methods that take one</value>
+  </data>
   <data name="InstantiateArgumentExceptionsCorrectlyChangeToTwoArgumentCodeFixTitle" xml:space="preserve">
     <value>Change to call the two argument constructor, pass null for the message.</value>
   </data>