Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to improve code fixer heuristic employed by CA1835 #4269

Merged
merged 13 commits into from
Oct 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
// and the member name is `Length`
thirdArgumentMemberAccessExpression.Name.Identifier.ValueText == WellKnownMemberNames.LengthPropertyName;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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);

Expand Down
Loading