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 6 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,23 @@ protected override bool IsSystemNamespaceImported(IReadOnlyList<SyntaxNode> impo
}
return false;
}

protected override bool IsPassingZeroAndBufferLength(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...
offsetValueNode is LiteralExpressionSyntax secondArgumentLiteralExpression &&
// and its value should be zero
secondArgumentLiteralExpression.Token.ValueText == "0" &&
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
// 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 == "Length";
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
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(SyntaxNode bufferValueNode, SyntaxNode offsetValueNode, SyntaxNode countValueNode);

public sealed override ImmutableArray<string> FixableDiagnosticIds =>
ImmutableArray.Create(PreferStreamAsyncMemoryOverloads.RuleId);

Expand Down Expand Up @@ -112,26 +116,39 @@ private Task<Document> FixInvocation(Document doc, SyntaxNode root, IInvocationO
{
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));
// Depending on the arguments being passed to Read/WriteAsync, it's the substitution we will make
SyntaxNode replacedInvocationNode;

// Generate the new buffer argument, ensuring we include the argument name if the user originally indicated one
SyntaxNode namedBufferWithAsMemoryInvocationNode =
(isBufferNamed ? generator.Argument(name: "buffer", RefKind.None, asMemoryInvocationNode) : asMemoryInvocationNode)
.WithTriviaFrom(bufferValueNode);
if(IsPassingZeroAndBufferLength(bufferValueNode, offsetValueNode, countValueNode))
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
{
// Remove 0 and buffer.length
replacedInvocationNode =
(isBufferNamed ? generator.Argument(name: "buffer", RefKind.None, bufferValueNode) : bufferValueNode)
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
.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)
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
.WithTriviaFrom(bufferValueNode);
}

// Create an async method call for the stream object with no arguments
SyntaxNode asyncMethodNode = generator.MemberAccessExpression(streamInstanceNode, methodName);
Expand All @@ -141,11 +158,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