Skip to content

Pass CancellationToken at the Analyzers layer#52279

Merged
jmarolf merged 2 commits intodotnet:mainfrom
jmarolf:always-pass-cancellation-tokens-analyzer
Mar 31, 2021
Merged

Pass CancellationToken at the Analyzers layer#52279
jmarolf merged 2 commits intodotnet:mainfrom
jmarolf:always-pass-cancellation-tokens-analyzer

Conversation

@jmarolf
Copy link
Contributor

@jmarolf jmarolf commented Mar 31, 2021

Redo of #52247

This cleans up passing cancellation tokens in the analyzers layer

@ghost ghost added the Area-Analyzers label Mar 31, 2021
@jmarolf jmarolf mentioned this pull request Mar 31, 2021
Merged
7 tasks
@jmarolf jmarolf requested a review from sharwell March 31, 2021 01:27
Youssef1313
Youssef1313 reviewed Mar 31, 2021
View reviewed changes
Copy link
Member

@Youssef1313 Youssef1313 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why

roslyn/src/Analyzers/CSharp/Analyzers/UsePatternMatching/CSharpAsAndNullCheckDiagnosticAnalyzer.cs

Line 95 in 50fad85

if (semanticModel.GetSymbolInfo(comparison).GetAnySymbol().IsUserDefinedOperator())
wasn't flagged?

GetSymbolInfo can be passed a cancellation token. A cancellation token is defined in scope in

roslyn/src/Analyzers/CSharp/Analyzers/UsePatternMatching/CSharpAsAndNullCheckDiagnosticAnalyzer.cs

Line 62 in 50fad85

var cancellationToken = syntaxContext.CancellationToken;

Youssef1313
Youssef1313 reviewed Mar 31, 2021
View reviewed changes
Copy link
Member

@Youssef1313 Youssef1313 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the following can be passed a cancellation token (it's less straightforward to do since there is no cancellation token in scope - the private method will need to have a cancellation token parameter)

roslyn/src/Analyzers/CSharp/CodeFixes/InlineDeclaration/CSharpInlineDeclarationCodeFixProvider.cs

Line 357 in bd010fb

var previousSymbol = semanticModel.GetSymbolInfo(nodeToReplace).Symbol;

roslyn/src/Analyzers/CSharp/CodeFixes/ConvertTypeOfToNameOf/CSharpConvertTypeOfToNameOfCodeFixProvider.cs

Line 31 in bd010fb

var typeSymbol = model.GetSymbolInfo(typeOfExpression.Type).Symbol.GetSymbolType();

There should be a few more similar cases.

@jmarolf
Copy link
Contributor Author

jmarolf commented Mar 31, 2021

@Youssef1313 I believe the analyzer is just looking for cancelation tokens passed in as parameters. We should investigate improving this over at dotnet/roslyn-analyzers

@jmarolf
Copy link
Contributor Author

jmarolf commented Mar 31, 2021

Also the following can be passed a cancellation token (it's less straightforward to do since there is no cancellation token in scope - the private method will need to have a cancellation token parameter)

It is ok to not pass a cancellation token if the user cannot cancel the underlying operation (no dialog to click on etc). In these cases (codefixes) they can be so we should try and pass these down.

@jmarolf jmarolf requested a review from a team March 31, 2021 21:18
@jmarolf jmarolf merged commit 3ff77fb into dotnet:main Mar 31, 2021
@ghost ghost added this to the Next milestone Mar 31, 2021
@dibarbet dibarbet modified the milestones: Next, 16.10.P3 Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@genlu genlu genlu approved these changes

@sharwell sharwell Awaiting requested review from sharwell

+2 more reviewers

@Youssef1313 Youssef1313 Youssef1313 left review comments

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-Analyzers

Projects

None yet

Milestone

16.10.P3

Development

Successfully merging this pull request may close these issues.

5 participants

@jmarolf @genlu @CyrusNajmabadi @Youssef1313 @dibarbet