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 proposal: Async methods should take a CancellationToken #394

Open
AArnott opened this issue Sep 23, 2018 · 2 comments
Open

Analyzer proposal: Async methods should take a CancellationToken #394

AArnott opened this issue Sep 23, 2018 · 2 comments

Comments

@AArnott
Copy link
Member

AArnott commented Sep 23, 2018

Is your feature request related to a problem?

Based on the idea in DotNetAnalyzers/AsyncUsageAnalyzers#40 and discussed in #382 (comment), let's use this issue to track the proposal for an analyzer that creates a diagnostic for async methods that do not accept a CancellationToken.

Describe the solution you'd like

This could be a Hidden diagnostic with a code fix that makes adding a CancellationToken parameter easy.
It might also be a more visible Info or Warning diagnostic that activates when no CancellationToken is taken, but only after the async method calls into other code that can take a CancellationToken.

Another idea is to have an analyzer that highlights any calls to async methods where a CancellationToken that is available is not provided, but could be.

@AArnott
Copy link
Member Author

AArnott commented Sep 23, 2018

Some good ideas from @sharwell when he implemented this in AsyncUsageAnalyzers: DotNetAnalyzers/AsyncUsageAnalyzers#32 (comment)

I'm liking the idea of splitting this into two analyzers:

  1. Take a CancellationToken as a parameter in any public, async method, or any other method that internally calls methods that can accept a CancellationToken.
  2. Propagate CancellationToken: when a method accepts a CancellationToken emit a diagnostic when it isn't propagated in invocations that may accept one (by omitting an optional parameter, or by calling a different overload). But do not emit a diagnostic for invocations that occur after any code comment that mentions "CancellationToken", supposing it explains that the method has past the point of no return. Should we produce a diagnostic even if CancellationToken.None is explicitly provided as the argument? (maybe not, but then it could be a carry-over from before the calling method accepted a CancellationToken. This would have Info or Warning severity.

@AArnott
Copy link
Member Author

AArnott commented Oct 8, 2019

Revising no. 2 above consistent with an offline conversation that IIRC @sharwell and I had a while back:

Passing CancellationToken.None still gets flagged. If a method passes "the point of no cancellation" it should assign a default token to the cancellationToken local variable and continue to pass that to all async methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant