IDE0028 raised when initializing a collection from another collection.

[ericstj]

Version Used:
Dotnet 9.0.

Steps to Reproduce:

1. Compile:

using System.Collections.Generic;

string[] strings = ["hello", "world"];

HashSet<string> myset = new(strings);

A minimal repro, with source-code provided, is ideal. Most compiler/language issues can be distilled into a snippet that can be pasted into sharplab.

Diagnostic Id:

IDE0028: Collection initialization can be simplified

Expected Behavior:

No diagnostic.

Actual Behavior:

IDE0028 is raised and suggests changing syntax to [.. strings]. This syntax is effectively:

HashSet<string> hashSet = new HashSet<string>();
for (int i = 0; i < strings.Length; i++)
{
    hashSet.Add(strings[i]);
}

This is less efficient since it will initialize the collection with default capacity and then grow / copy as Adds are made. Collection constructors should be preferred in most cases since they can more-efficiently initialize the internal collection than calling Add N times.

[CyrusNajmabadi]

Closing as by design. The IDE has no knowledge (or desire to have knowledge) of how the compiler chooses to initialize aa value. In a similar vein, the compiler is tasked with and recommended to produce the most efficient code possible when using collection expressions. The idea is that users should not have to know or care. they should just use collection expressions and get good results. The analyzer/fixer is written with that design in mind.

Tagging @RikkiGibson if he wants the compiler to do something here. It certainly does seem reasonable to recompile [.. whatever] into new X(whatever). At least for the MS collections where we have confidence they will do a good job.

[RikkiGibson]

I'd like to use this issue to track adding the optimization listed in #68785:

Use EnsureCapacity(int) for collection initializer types with no spreads; potentially also when length is known and collection satisfies certain heuristics.

---

It looks like a HashSet created from IEnumerable will:

• if the input collection is also an ICollection, initialize the internal storage to its Count
• foreach on the collection and add the elements one by one.

In other words if we simply ensure the capacity here then we should get the perf we want, and save the array alloc from the original code when the IDE suggestion is applied.

(It's not obvious to me whether calling EnsureCapacity simplifies things more than calling the constructor which takes a capacity, but it is likely not a significant distinction.)