[BUG] NUnit1018 doesn't recognize collection expression

[adaskos-signal]

In the following pseudo-code:

[TestCaseSource(typeof(MyClass), nameof(MyMethod), [1, 2, 3])]

NUnit analyzer raises an error:

The TestCaseSource provides '0' parameter(s), but the target method expects '3' parameter(s)

It seems it doesn't understand [1,2,3] as an object array. So one needs to avoid/supress IDE0300 suggestion for those cases.

[manfred-brands]

Thanks for reporting.

Object arrays didn't exist when this rule was written. My main concern is if I can add this without requiring NET 8.0 SDK or if we need different binaries depending on SDK.

[adaskos-signal]

It seems CollectionExpressionSyntax was introduced in Roslyn 4.7.0. Theoretically it's still .NET Standard compatibile.

[manfred-brands]

Roslyn versions are not related to SDK versions. It is part of C#12.

[adaskos-signal]

All I meant is that for you to handle the new collection expression syntax, you'll have to update Microsoft.CodeAnalysis, but it doesn't require targeting a higher SDK.

[manfred-brands]

Unfortunately it is not as simple. Using NET 8 SDK to build the analyzer is not the problem. Increasing the version of that packet means that anybody using the analyzer needs to use an SDK with that version as the nuget package depends on a dll which only comes with the compiler.

When we moved to version 3 we maintained two different versions of the analyzer.

There are ways to support multiple SDKs one which I have done for another analyzer is to add an extra dll to the package for analyzers requiring later versions. Another way is to use private versions of the new types like StyleCop does.

[adaskos-signal]

I read a bit about it. It's a pity it works like this. Makes life hard for those publishing analyzers if they want to keep up with the new syntax.

[Rabadash8820]

Just ran into this issue myself. @manfred-brands Any progress on any of the solutions that you suggested above?

[mikkelbu]

@Rabadash8820 So far no one has started working on this

[PiotrKlecha]

It seems that other analyzers actually support the collection initializer syntax, e.g.:

[Test]
public void ValuesTest([Values(args: [ 1, 2.0, 3, ])] int i)

is correctly reported as NUnit1031: The value of the argument at position '1' of type double cannot be assigned to the parameter 'i' of type int.

So maybe the current issue is no longer as problematic as described previously and can finally be resolved?

[mikkelbu]

I guess the reason for why NUnit1031 works is that it cares less about the concrete syntax, but rather uses the semantic representation - i.e.

        internal static ImmutableArray<TypedConstant> AdjustArguments(this ImmutableArray<TypedConstant> attributePositionalArguments)
#pragma warning restore SA1202 // Elements should be ordered by access
        {
            if (attributePositionalArguments.Length == 1
                && attributePositionalArguments[0] is { Kind: TypedConstantKind.Array } arrayArgument
                && arrayArgument.Type is IArrayTypeSymbol arrayType
                && arrayType.ElementType.SpecialType == SpecialType.System_Object)
....
                    return arrayArgument.Values;
....

I think it will be a larger rewrite and I have the feeling it would be better to pursuit one of the approaches mentioned in https://github.com/nunit/nunit.analyzers/issues/726#issuecomment-2049294149, but I'm unsure about the scope of each of the alternatives

[manfred-brands]

NUnit1018 is a Syntax analyzer which looks at tokens, like [ etc. NUnit1031 is a Symbol analyzer which kicks in later after the compiler converted the parameter to an array.

The number 1018 vs 1031 means the first is a much older analyzer (7 years vs 2 years old).

Yes, if not caring about the syntax (new[] { xx } vs [xx]), a symbol analyzer is a better option, but would be a rewrite.