Fix to improve code fixer heuristic employed by CA1835 (dotnet#4269)

* Shared Fixer: detect arguments 0 and buffer.Length.

* CSharp fixer: Implement method that detects 0 and buffer.Length syntax nodes.

* VB fixer: Implement method that detects 0 and buffer.Length syntax nodes.

* Test data:

Add new test cases to handle partial buffer read/write.

Adjust existing test cases that use the full buffer to avoid adding the AsMemory invocation.

* ReadAsync tests: Consume the new test data.

* WriteAsync tests: Consume the new test data.

* Suggestions by Evangelink.

* Pass the semantic model to the syntax-specific method.

* Adapt the C# syntax-specific method to take the semantic model instance.

* Adapt the VB syntax-specific method to take the semantic model instance.

* Fix some failing new test cases.

* Update the unit tests.

* Simplify check for 0 by directly using the constant pattern check on the Value without having to cast to integer first.
Use existing WellKnownMemberName instead "Length".

Co-authored-by: carlossanlop <carlossanlop@users.noreply.github.com>

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

@@ -51,5 +51,21 @@ protected override bool IsSystemNamespaceImported(IReadOnlyList<SyntaxNode> impo
             }
             return false;
         }
+
+        protected override bool IsPassingZeroAndBufferLength(SemanticModel model, SyntaxNode bufferValueNode, SyntaxNode offsetValueNode, SyntaxNode countValueNode)
+        {
+            return
+                // First argument should be an identifier name node
+                bufferValueNode is IdentifierNameSyntax firstArgumentIdentifierName &&
+                // Second argument should be a literal expression node with a constant value of zero
+                model.GetConstantValue(offsetValueNode) is Optional<object> optionalValue && optionalValue.HasValue && optionalValue.Value is 0 &&
+                // Third argument should be a member access node...
+                countValueNode is MemberAccessExpressionSyntax thirdArgumentMemberAccessExpression &&
+                thirdArgumentMemberAccessExpression.Expression is IdentifierNameSyntax thirdArgumentIdentifierName &&
+                // whose identifier is that of the first argument...
+                firstArgumentIdentifierName.Identifier.ValueText == thirdArgumentIdentifierName.Identifier.ValueText &&
+                // and the member name is `Length`
+                thirdArgumentMemberAccessExpression.Name.Identifier.ValueText == WellKnownMemberNames.LengthPropertyName;
+        }
     }
 }

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

@@ -37,6 +37,10 @@ public abstract class PreferStreamAsyncMemoryOverloadsFixer : CodeFixProvider
         // Verifies if a namespace has already been added to the usings/imports list.
         protected abstract bool IsSystemNamespaceImported(IReadOnlyList<SyntaxNode> importList);
 
+        // Verifies if the user passed `0` as the 1st argument (`offset`) and `buffer.Length` as the 2nd argument (`count`),
+        // where `buffer` is the name of the variable passed as the 0th argument.
+        protected abstract bool IsPassingZeroAndBufferLength(SemanticModel model, SyntaxNode bufferValueNode, SyntaxNode offsetValueNode, SyntaxNode countValueNode);
+
         public sealed override ImmutableArray<string> FixableDiagnosticIds =>
             ImmutableArray.Create(PreferStreamAsyncMemoryOverloads.RuleId);
 
@@ -90,7 +94,8 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
             string title = MicrosoftNetCoreAnalyzersResources.PreferStreamAsyncMemoryOverloadsTitle;
 
-            Task<Document> createChangedDocument(CancellationToken _) => FixInvocation(doc, root, invocation, invocation.TargetMethod.Name,
+            Task<Document> createChangedDocument(CancellationToken _) => FixInvocation(model, doc, root,
+                                                         invocation, invocation.TargetMethod.Name,
                                                          bufferOperation.Value.Syntax, isBufferNamed,
                                                          offsetOperation.Value.Syntax, isOffsetNamed,
                                                          countOperation.Value.Syntax, isCountNamed,
@@ -104,34 +109,48 @@ Task<Document> createChangedDocument(CancellationToken _) => FixInvocation(doc,
                 context.Diagnostics);
         }
 
-        private Task<Document> FixInvocation(Document doc, SyntaxNode root, IInvocationOperation invocation, string methodName,
+        private Task<Document> FixInvocation(SemanticModel model, Document doc, SyntaxNode root,
+            IInvocationOperation invocation, string methodName,
             SyntaxNode bufferValueNode, bool isBufferNamed,
             SyntaxNode offsetValueNode, bool isOffsetNamed,
             SyntaxNode countValueNode, bool isCountNamed,
             SyntaxNode? cancellationTokenValueNode, bool isCancellationTokenNamed)
         {
             SyntaxGenerator generator = SyntaxGenerator.GetGenerator(doc);
 
-            // The stream object
+            // The stream-derived instance
             SyntaxNode streamInstanceNode = invocation.Instance.Syntax;
 
-            // buffer.AsMemory(int start, int length)
-            // offset should become start
-            // count should become length
-            SyntaxNode namedStartNode = isOffsetNamed ? generator.Argument(name: "start", RefKind.None, offsetValueNode) : offsetValueNode;
-            SyntaxNode namedLengthNode = isCountNamed ? generator.Argument(name: "length", RefKind.None, countValueNode) : countValueNode;
-
-            // Generate an invocation of the AsMemory() method from the byte array object, using the correct named arguments
-            SyntaxNode asMemoryExpressionNode = generator.MemberAccessExpression(bufferValueNode.WithoutTrivia(), memberName: "AsMemory");
-            SyntaxNode asMemoryInvocationNode = generator.InvocationExpression(
-                asMemoryExpressionNode,
-                namedStartNode.WithTriviaFrom(offsetValueNode),
-                namedLengthNode.WithTriviaFrom(countValueNode));
-
-            // Generate the new buffer argument, ensuring we include the argument name if the user originally indicated one
-            SyntaxNode namedBufferWithAsMemoryInvocationNode =
+            // Depending on the arguments being passed to Read/WriteAsync, it's the substitution we will make
+            SyntaxNode replacedInvocationNode;
+
+            if (IsPassingZeroAndBufferLength(model, bufferValueNode, offsetValueNode, countValueNode))
+            {
+                // Remove 0 and buffer.length
+                replacedInvocationNode =
+                    (isBufferNamed ? generator.Argument(name: "buffer", RefKind.None, bufferValueNode) : bufferValueNode)
+                    .WithTriviaFrom(bufferValueNode);
+            }
+            else
+            {
+                // buffer.AsMemory(int start, int length)
+                // offset should become start
+                // count should become length
+                SyntaxNode namedStartNode = isOffsetNamed ? generator.Argument(name: "start", RefKind.None, offsetValueNode) : offsetValueNode;
+                SyntaxNode namedLengthNode = isCountNamed ? generator.Argument(name: "length", RefKind.None, countValueNode) : countValueNode;
+
+                // Generate an invocation of the AsMemory() method from the byte array object, using the correct named arguments
+                SyntaxNode asMemoryExpressionNode = generator.MemberAccessExpression(bufferValueNode.WithoutTrivia(), memberName: "AsMemory");
+                SyntaxNode asMemoryInvocationNode = generator.InvocationExpression(
+                    asMemoryExpressionNode,
+                    namedStartNode.WithTriviaFrom(offsetValueNode),
+                    namedLengthNode.WithTriviaFrom(countValueNode));
+
+                // Generate the new buffer argument, ensuring we include the buffer argument name if the user originally indicated one
+                replacedInvocationNode =
                     (isBufferNamed ? generator.Argument(name: "buffer", RefKind.None, asMemoryInvocationNode) : asMemoryInvocationNode)
-                .WithTriviaFrom(bufferValueNode);
+                    .WithTriviaFrom(bufferValueNode);
+            }
 
             // Create an async method call for the stream object with no arguments
             SyntaxNode asyncMethodNode = generator.MemberAccessExpression(streamInstanceNode, methodName);
@@ -141,11 +160,11 @@ private Task<Document> FixInvocation(Document doc, SyntaxNode root, IInvocationO
             if (cancellationTokenValueNode != null)
             {
                 SyntaxNode namedCancellationTokenNode = isCancellationTokenNamed ? generator.Argument(name: "cancellationToken", RefKind.None, cancellationTokenValueNode) : cancellationTokenValueNode;
-                nodeArguments = new SyntaxNode[] { namedBufferWithAsMemoryInvocationNode, namedCancellationTokenNode.WithTriviaFrom(cancellationTokenValueNode) };
+                nodeArguments = new SyntaxNode[] { replacedInvocationNode, namedCancellationTokenNode.WithTriviaFrom(cancellationTokenValueNode) };
             }
             else
             {
-                nodeArguments = new SyntaxNode[] { namedBufferWithAsMemoryInvocationNode };
+                nodeArguments = new SyntaxNode[] { replacedInvocationNode };
             }
             SyntaxNode newInvocationExpression = generator.InvocationExpression(asyncMethodNode, nodeArguments).WithTriviaFrom(streamInstanceNode);