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

Analyzer + Fixer: Forward cancellation token to method invocations #3641

Merged
merged 20 commits into from
Jun 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9d71274
Analyzer + Fixer: Forward cancellation token to async methods
carlossanlop May 18, 2020
4cc0fba
Address PR suggestions:
carlossanlop May 20, 2020
084dc36
Address case where CancellationToken is an aliased using
carlossanlop May 20, 2020
5bfb7d8
Address PR suggestions
carlossanlop May 22, 2020
efa1838
Fix breaking change introduced by most recent rebased changes.
carlossanlop May 22, 2020
4c5776d
Address PR suggestions
carlossanlop May 27, 2020
981095f
Address named argument problem in C#, add two cases with diagnostics …
carlossanlop May 27, 2020
9b86720
Offer diagnostic only on the invocation expression name.
carlossanlop May 28, 2020
8b32db5
Bail out early in fixer before registering code fix
carlossanlop Jun 1, 2020
f776fbb
address analyzer attributes in wrong locations
carlossanlop Jun 4, 2020
e809c9a
Fix newest edge cases except lambda that does not take token
carlossanlop Jun 5, 2020
d08aada
Address last false positives, add trivia support
carlossanlop Jun 6, 2020
6cc2cc7
Fix comment formatting warning
carlossanlop Jun 6, 2020
f4b4bdd
Additional case for ct params
carlossanlop Jun 6, 2020
d5ddc40
Added more edge cases, ensured to do the most basic check at the begi…
carlossanlop Jun 6, 2020
b5a3fd6
Add method extension unit test
carlossanlop Jun 6, 2020
4061840
Address suggestions and more edge cases
carlossanlop Jun 11, 2020
8b2f0dc
Address latest suggestions, fixed one VB case where extension method …
carlossanlop Jun 17, 2020
9167229
Address latest suggestions
carlossanlop Jun 19, 2020
d5af47e
Address last suggestions
carlossanlop Jun 22, 2020
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
17 changes: 0 additions & 17 deletions RoslynAnalyzers.sln
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ Global
src\Utilities\Workspaces\Workspaces.Utilities.projitems*{1c62a2f6-7a44-4030-9d1a-7e7a966736e5}*SharedItemsImports = 5
src\Utilities\Compiler\Analyzer.Utilities.projitems*{2a4af3a4-307b-4252-888e-e6546244585a}*SharedItemsImports = 5
src\Utilities\Workspaces\Workspaces.Utilities.projitems*{2a4af3a4-307b-4252-888e-e6546244585a}*SharedItemsImports = 5
src\Utilities\Compiler\Analyzer.Utilities.projitems*{2a520f3e-8c5d-43fe-aa03-fe5e3c5f23d1}*SharedItemsImports = 5
src\Utilities\Compiler\Analyzer.Utilities.projitems*{730c2d1c-0276-4132-85ea-675ce60068bb}*SharedItemsImports = 5
src\Utilities\Workspaces\Workspaces.Utilities.projitems*{730c2d1c-0276-4132-85ea-675ce60068bb}*SharedItemsImports = 5
src\Utilities\Workspaces\Workspaces.Utilities.projitems*{99f594b1-3916-471d-a761-a6731fc50e9a}*SharedItemsImports = 13
Expand Down Expand Up @@ -265,22 +264,6 @@ Global
{0A7C9A18-888A-4028-B88F-989DA6A0140B}.Debug|Any CPU.Build.0 = Debug|Any CPU
{0A7C9A18-888A-4028-B88F-989DA6A0140B}.Release|Any CPU.ActiveCfg = Release|Any CPU
{0A7C9A18-888A-4028-B88F-989DA6A0140B}.Release|Any CPU.Build.0 = Release|Any CPU
{2A520F3E-8C5D-43FE-AA03-FE5E3C5F23D1}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{2A520F3E-8C5D-43FE-AA03-FE5E3C5F23D1}.Debug|Any CPU.Build.0 = Debug|Any CPU
{2A520F3E-8C5D-43FE-AA03-FE5E3C5F23D1}.Release|Any CPU.ActiveCfg = Release|Any CPU
{2A520F3E-8C5D-43FE-AA03-FE5E3C5F23D1}.Release|Any CPU.Build.0 = Release|Any CPU
{D70D8CBB-1AD0-49DE-83A8-07990915A843}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{D70D8CBB-1AD0-49DE-83A8-07990915A843}.Debug|Any CPU.Build.0 = Debug|Any CPU
{D70D8CBB-1AD0-49DE-83A8-07990915A843}.Release|Any CPU.ActiveCfg = Release|Any CPU
{D70D8CBB-1AD0-49DE-83A8-07990915A843}.Release|Any CPU.Build.0 = Release|Any CPU
{390DBE36-C8AF-4882-BB5D-87EFBF06B8A8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{390DBE36-C8AF-4882-BB5D-87EFBF06B8A8}.Debug|Any CPU.Build.0 = Debug|Any CPU
{390DBE36-C8AF-4882-BB5D-87EFBF06B8A8}.Release|Any CPU.ActiveCfg = Release|Any CPU
{390DBE36-C8AF-4882-BB5D-87EFBF06B8A8}.Release|Any CPU.Build.0 = Release|Any CPU
{722141A2-91FE-411A-A64F-A2A2A7150122}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{722141A2-91FE-411A-A64F-A2A2A7150122}.Debug|Any CPU.Build.0 = Debug|Any CPU
{722141A2-91FE-411A-A64F-A2A2A7150122}.Release|Any CPU.ActiveCfg = Release|Any CPU
{722141A2-91FE-411A-A64F-A2A2A7150122}.Release|Any CPU.Build.0 = Release|Any CPU
{774B5DF6-D14F-4839-95F3-D3632B754765}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{774B5DF6-D14F-4839-95F3-D3632B754765}.Debug|Any CPU.Build.0 = Debug|Any CPU
{774B5DF6-D14F-4839-95F3-D3632B754765}.Release|Any CPU.ActiveCfg = Release|Any CPU
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}

}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading