Code fix for IDE0028 produces invalid collection expression

[meziantou]

Version Used: '4.9.0-2.23578.7 (984df16)'. Language version: 12.0

Steps to Reproduce:

The actual use case is using Xunit.TheoryData<T>, but here's a small repro

public class UnitTest1
{
    void A()
    {
        Sample<string> a = new Sample<string> // IDE0028: Collection expression can be simplified
        {
            "test",
        };

        // After applying the fix
        Sample<string> a =
        [
            "test", // CS0029 Cannot implicitly convert type 'string' to 'object[]' 
        ];
    }
}

class Sample<T> : IEnumerable<object[]>
{
    public void Add(T p) => throw null;
    IEnumerator IEnumerable.GetEnumerator() => throw null;
    IEnumerator<object[]> IEnumerable<object[]>.GetEnumerator() => throw null;
}

Diagnostic Id: IDE0028

Expected Behavior: The code is valid after applying the code fix

Actual Behavior: The build fails

[CyrusNajmabadi]

@cston @RikkiGibson this seems strange. This is a legal collection initializer type. And we're not using the collection builder attribute. I would expect the to be a legal target, with the expressions validated against the add metgod.

Do you know why this fails?

[cston]

The element expressions must be convertible to the enumerable element type, in this case to object[].

From Conversions:

• To a struct or class type that implements System.Collections.Generic.IEnumerable<T> where:

  • For each element Ei there is an implicit conversion to T.

[CyrusNajmabadi]

That applies to All collections chuck? Not just Collection-Builder collections? I don't think i realized that :(

I suppose it works for everything else, but does put us in this awkward spot with these sort of collection-initializers.

Hrmm... Will have to noodle on this a bit.

[sharwell]

It's weird that collection expressions don't behave the same way as collection initializers here. For the above type, both of the following fail:

Sample<string> a =
[
  "test", // CS0029 Cannot implicitly convert type 'string' to 'object[]' 
];

Sample<string> a =
[
  ["test"], // CS1950 The best overloaded Add method 'Sample<string>.Add(string)' for the collection initializer has some invalid arguments
];

📝 Note that Xunit's TheoryData<T> type falls victim to this issue

[jaredpar]

Resolving as a dupe of 72098

[cston]

Reopening since this is not a duplicate of #72098.

[cston]

Closing as by design since element expressions must be convertible to the enumerable element type as mentioned above.

[spanglerco]

By design for collection expressions makes sense, but what about IDE0028? A code analysis rule shouldn't say to switch from valid code to invalid code. How it is now, I need to add a suppression every time I want to use TheoryData with a collection initializer.

Edit: At least for TheoryData specifically, the xunit maintainer added a workaround in the form of a constructor that takes in T[], enabling new TheoryData<string>(["value"]).

[CyrusNajmabadi]

A code analysis rule shouldn't say to switch from valid code to invalid code.

The invalid code is a bug in the compiler that is being fixed currently.

[cston]

The invalid code is a bug in the compiler that is being fixed currently.

The compiler bug that is being fixed will no longer require the Add method to be callable with an argument of the iteration type (see dotnet/csharplang#8022), but each element must still be implicitly convertible to the iteration type. In short, the repro above will still result in a conversion error, as with 17.8.

[CyrusNajmabadi]

@cston

but each element must still be implicitly convertible to the iteration type.

Wait, why is that the case? That would break all sorts of collection-initializable types from being used with collection exprs. I thought that that's exactly what we were addressing just by looking for an "Add" method and no longer caring about convertibility.

[CyrusNajmabadi]

Reopening, the rule that "element expressions must be convertible to the enumerable element type as #71240 (comment)" is both undesirable and surprising. It goes against hte spirit of the collection-exprs design, and introduces arbitrary limitations that make normal collection-initializable types not work with collection exprs.

[CyrusNajmabadi]

I do not repro this.