Apply CA2016 - Forward CancellationToken to methods that can take it

[carlossanlop]

This PR applies Roslyn analyzer CA2016: It identified methods that had a CancellationToken parameter, and then looked inside its method body for any method invocations that could accept a token as an argument, but weren't taking it yet.

The Roslyn analyzer/fixer is up for review here: #3641
Original issue here: #33774

@mavasani the rule behaved strange in all the model.GetDeclaredSymbol(classDecl) invocations - The method overload that takes a CancellationToken is located in an extension class, which I wasn't expecting to diagnose, but it was diagnosed and an incorrect fix was offered, because the object was being substituted by the class name.

[carlossanlop]

FYI @Youssef1313

[Evangelink]

This isn't related to this change but is it safe to use a direct cast here? Can't we have a ITypeSymbol which won't be a INamedTypeSymbol?

[Youssef1313]

The analyzer associated with this fixer report diagnostics only for INamedTypeSymbol, so I expect the cast is valid:

Here is a snippet from the analyzer:

                foreach (INamedTypeSymbol fixer in _codeFixProviders)
                {
                    if (OverridesGetFixAllProvider(fixer))
                    {
                        AnalyzeFixerWithFixAll(fixer, context);
                    }
                    else if (fixer.BaseType != null && fixer.BaseType.Equals(_codeFixProviderSymbol))
                    {
                        Diagnostic diagnostic = Diagnostic.Create(OverrideGetFixAllProviderRule, fixer.Locations.First(), fixer.Name);
                        context.ReportDiagnostic(diagnostic);
                    }
                }

[Evangelink]

I would still do safe-cast as the code might have changed between the diagnostic and the codefix registration being called but I might be wrong.

[Youssef1313]

@carlossanlop The analyzer doesn't cover the case where a cancellation token doesn't exist in the method but the caller can forward it right?

In that case, I might give a fast look throughout the code base and try to identify as much as I can of such occurrences.

Another case is if the method takes something that has CancellationToken property that can be used.
For example, if a method takes CodeFixContext which has a CancellationToken property that isn't used where it can be used.

Any chance that you can cover any of these cases? (especially the second, I think it might be easier, instead of just looking at the argument, inspect the properties on them too if any contains a CancellationToken)

[carlossanlop]

The analyzer doesn't cover the case where a cancellation token doesn't exist in the method but the caller can forward it right?

There is nothing to forward if:

• The containing method does not have a CancellationToken parameter in the signature.
• The method invocation does not take a token, or does not have an overload that takes a token.

In that case, I might give a fast look throughout the code base and try to identify as much as I can of such occurrences.

Can you clarify? Are you talking about cases where you would explicitly pass default or CancellationToken.None because there was no token parameter in the parent method signature? Those cases are outside the scope of this analyzer and you can manually look for them if you'd like.

Another case is if the method takes something that has CancellationToken property that can be used.
For example, if a method takes CodeFixContext which has a CancellationToken property that isn't used where it can be used.
Any chance that you can cover any of these cases? (especially the second, I think it might be easier, instead of just looking at the argument, inspect the properties on them too if any contains a CancellationToken)

Looking for properties is outside the scope of this rule. This analyzer will only try to forward a token that is a parameter in the method signature.

[Youssef1313]

Can you clarify? Are you talking about cases where you would explicitly pass default or CancellationToken.None because there was no token parameter in the parent method signature?

I meant to try to manually spot and fix the cases that aren't coveres by the analyzer.

[carlossanlop]

@mavasani is there something I should do to trigger the licence/cla CI leg to run or finish running?

[mavasani]

@carlossanlop I am not sure what happened there. I can force merge if you are good to go with the PR.

[carlossanlop]

I am not sure what happened there. I can force merge if you are good to go with the PR.

@mavasani I am good with the PR. Can you force merge it, please?