xUnit1051 code fix chooses incorrect overload for CancellationToken #3068
Comments
|
I'm tempted, given the |
|
That sounds reasonable to me. Although, I wonder what CA2016 - Forward the CancellationToken parameter to methods that take one does in this case? I might be able to work on this by the way, I have a little bit of experience with analyzers. |
|
That'd be awesome! Thanks! |
|
The code fix for CA2016 in this instance will create a new array for you when it switches to the other overload: public class C
{
public async Task M(CancellationToken cancellationToken = default)
{
- await M2(1, 2, 3);
+ await M2(new int[] { 1, 2, 3 }, cancellationToken: cancellationToken);
}
public async Task M2(params int[] values)
{
}
public async Task M2(int[] values, CancellationToken cancellationToken = default)
{
}
}So it's perhaps not unreasonable to attempt making xUnit1051 do the same. However, the complexity of this may have increased a bit with the new support in C# 13 for non-array |
If the analyzer has fired on a `params` method, it means that the new overload is actually array + `CancellationToken`, since C# forbids placing any argument after a params argument. To avoid generating invalid code, we need to replace the params expression with an array creation expression. Does not currently handle the expanded range of allowed `params` types implemented in C# 13. Considering you can use _any_ type that implements `IEnumerable<T>`, I think it would be quite difficult to handle all the different cases there. Being a new feature, it's also less likely to be used, though this will of course change over time... Closes [#3068](xunit/xunit#3068)
|
I've gone with copying CA2016 and attempting to create an array, since it wasn't that hard to do in the end (assuming my solution works). Let me know if you'd prefer to go the route of not firing this diagnostic in the first place, though. |
|
This sounds good, I'll take a look at the PR. |
|
Available in |
I ran into this while upgrading to v3 and with EF Core's FindAsync method inside my tests, which exposes a
params object[] keyValuesoverload and anobject[] keyValues, CancellationToken cancellationTokenoverload.Given the following code:
I get a diagnostic for xUnit1051 on the invocation of
Min UnitTest1. But if I apply the code fix, I getwhich is a compile error. The correct code is:
There's an argument here that the EF Core API is not the best (ReSharper does warn me about the second method being hidden). In fact, it's a bit worse in that case since
params object[]accepts the CancellationToken but just leads to a runtime failure by trying to treat it as a primary key.That being said, I didn't see any existing issues with this so I thought I'd raise it all the same - maybe it's an easy fix.
The text was updated successfully, but these errors were encountered: