CA2016 Not aware of functions available for cancellation

[AlmightyLks]

Analyzer

Diagnostic ID: CA2016: Forward the CancellationToken parameter to methods that take one

Describe the improvement

Parameters with [CallerMemberName], [CallerFilePath] or [CallerLineNumber] attributes will be ignored, and the CancellationToken parameter that comes before it counts as last parameter.

Describe suggestions on how to achieve the rule

Count the CancellationToken parameter as last parameter, if the following parameters have [CallerMemberName], [CallerFilePath] or [CallerLineNumber] attributes.

Additional context

The docs explicitly mention that the analyzer does not fire if the CancellationToken is not stricly the very last parameter.
Is there a reason for it?
In our case, I am asking to create an exception for Caller-Attributed parameters, as they are never provided by the caller, but by the compiler.

We've got a custom extension method, to add source tagging to our EFC queries

    public static Task<bool> AnyAsync<TSource>(this IQueryable<TSource> source, 
        Expression<Func<TSource, bool>> predicate, 
        CancellationToken cancellationToken = default, 
        [System.Runtime.CompilerServices.CallerMemberName] string memberName = "",
        [System.Runtime.CompilerServices.CallerFilePath] string sourceFilePath = "",
        [System.Runtime.CompilerServices.CallerLineNumber] int sourceLineNumber = 0)
    {
        source = AddTag(source, memberName, sourceFilePath, sourceLineNumber);
        return EntityFrameworkQueryableExtensions.AnyAsync(source, predicate, cancellationToken);
    }

Of course we want to fill out the predicate, and many times the cancellationToken, but the Caller-Attributed parameters will never be filled out by the caller explicitly, thus they are at very last, and never mentioned.
Making the CancellationToken the last parameter would require a rewrite to always explicitly pass the argument in a named manner.

This begs the question: Why do we ignore every possible function that has a CancellationToken parameter, but it simply isnt the last argument? It causes great oversights, as for example with this, where SaveChangesAsync is obvious, but various SELECT queries of ours are not recognized:

And I wouldn't want to imagine third party dependencies which happen to also not have their CancellationToken at last, and I as the consumer have to suffer from the analyzer refusing to pick it up

[buyaa-n]

The docs explicitly mention that the analyzer does not fire if the CancellationToken is not stricly the very last parameter.
Is there a reason for it?

As it is documented there must be a reason, which I don't know, tag @carlossanlop @stephentoub for answer

[AlmightyLks]

bump

[stephentoub]

As it is documented there must be a reason, which I don't know, tag @carlossanlop @stephentoub for answer

The analyzer has to make an assumption about the purpose of the argument; it's not a given (though very likely) that a CancellationToken passed to the function is the one that should be used to cancel the function's invocation. As such, the analyzer is trying to reduce the chances of false positives by only handling the case where the token is at the end, as by design guidelines / convention that's where the token should be to represent canceling the function's invocation.

[carlossanlop]

To support Stephen's comment above:

The PR introducing the CA2016 analyzer+fixer indicates in the description that finding cancellationToken arguments that are not in the last position would be a future improvement. I am really not convinced we would want such improvement to be made, because if there's a case where multiple CancellationToken arguments are passed, we do not know which one to choose to pass to the various invocations that take one. Because of that, we decided to limit this to just analyzing and fixing for the very last argument (convention).

[AlmightyLks]

I cannot think of a purpose/use-case or specific API that would

• be async
• return task/valuetask
• accept a single cancellationtoken in any parameter position

where the cancellationtoken is not for the purpose of cancelling the action/process behind the function call

Regarding multiple cancellationtoken parameters: Yeah, naturally, the analyzer should not fire for such cases where it is just not possible to determine the expected usage of the function

But, I would still like to hear your opinion on ignoring the compiler-served parameters 😄

[IanKemp]

As such, the analyzer is trying to reduce the chances of false positives by only handling the case where the token is at the end, as by design guidelines / convention that's where the token should be to represent cancelling the function's invocation.

This statement is true for all cases except the one presented in the OP. You are never going to manually populate the Caller*Attribute argument(s) because the compiler does it for you (that is after all the whole point of these attributes), but you are going to pass through the cancellation token manually, for example if we consider a likely invocation of the provided method:

public static async Task<bool> CheckAndAnyAsync(
    IEnumerable<SomePoco> enumerable,
    CancellationToken cancellationToken = default)
{
    if (await DoSomeCheckAsync(enumerable, cancellationToken)) return false;

    // calling the method from the OP
    // compiler fills in values as per Caller*Attributes
    return await enumerable.AnyAsync(cancellationToken);
}

So I submit that the way the OP has ordered their method arguments is the most logical and therefore correct, and the guideline/convention and this analyser both need to be updated accordingly to state/comply with the following:

If an argument of type CancellationToken is present, it should be the last argument.
If an argument annotated with Caller*Attribute is present, it should be the last argument.
If both types of arguments are present, the argument annotated with Caller*Attribute should appear last, and the CancellationToken argument should appear directly before the former.