Analyzer + Fixer: Forward cancellation token to method invocations

[carlossanlop]

CA2016: Forward CancellationToken to methods.

Fixes dotnet/runtime#33774

Summary

The rule should try to identify places where a CancellationToken (ct) should have been passed but wasn't. For example, in a method that takes a ct, a method is called that has an overload that takes a ct but a shorter overload that doesn't take a ct was used instead.

Conditions for positive cases

• The node to analyze is an awaited invocation, or is the origin invocation of a ConfigureAwait.
• The parent method signature has a ct parameter.
• The invocation is not receiving a ct argument.
• The invocation method either:
  • Only has one method version, but the signature has one optional ct.
  • Has a method overload with the exact same arguments in the same order, plus one ct parameter at the end.
• A normal method, a lambda or a nested method receives a ct, and there's an awaited invocation inside it.

Conditions for negative cases

• The parent method signature does not have a ct parameter.
• The node to analyze is not an awaited invocation.
• The awaited invocation is already receiving a ct argument.
• The invocation method does not have an overload with the exact same arguments that also receives a ct.
• The user is using the overload that takes the ct, but is already passing a value. Can be default or default(CancellationToken) in C#, Nothing in VB, or CancellationToken.None, or the token from a CancellationTokenSource, or something else.
• If the overload that receives a ct receives more than one ct, do not suggest it.

Pending to address

• Case where CancellationToken is renamed among the usings directives (is it called an alias?).
• Trivia
• Ensure we have unit tests verifying false negatives with the cases not handled below (potential future improvements).
• Appropriate strings to message, title and description.

Potential future improvements

• Finding an overload with one ct parameter, but it is not located in the last position. For example, some ContinueWith overloads have the ct in the middle.
• Passing a named ct in a different position than the last.
• Detecting extension methods as suggested in this comment.

[AArnott]

Why the focus on whether awaits exist? This rule should apply to synchronous methods just as much as async ones. If you search dotnet/runtime#33774 for the term "async" you'll find a few discussion items on this question where @stephentoub agreed with this assertion.

[carlossanlop]

I would like to read what people think about the "Potential future improvements" section in the main description - is it ok to get them addressed later, or do you think I should address them now?

[mavasani]

Overall, looks great to me! Thanks for the extensive test suite! I have some suggestions around certain conditions that seem always false in the analyzer and other minor nit comments.

I am really looking forward to enabling this analyzer in all our repos!! Thanks.

[mavasani]

Thanks!