dotnet / roslyn-analyzers

MIT License
1.56k stars 462 forks source link

Make IDE0028 configurable to only apply to "true" collections. #7257

Open bzd3y opened 3 months ago

bzd3y commented 3 months ago

Analyzer

Diagnostic ID: IDE0028

This also could possibly apply to IDE0300, but the when_types_exactly_match option might mitigate the need some.

I have considered that maybe this doesn't belong here because the IDE rules seem to work differently from the CA rules. I'm not entirely clear on what the difference is. So if this should go somewhere else, let me know.

Describe the improvement

Allow the types to which this rule applies to be configured, similar to how some other rules can be made less strict.

Describe suggestions on how to achieve the rule

The configuration would look something like one or more of the following:

dotnet_style_collection_initializer = only_for_true_collections
dotnet_style_collection_initializer = exclude_derived_collections
dotnet_style.IDE0028.exclude_derived_collections = true

As for applying the rule, I'm not familiar with exactly what is going on with the analyzer, but I'm guessing the analyzer probably looks to see if the type implements one or more of ICollection or IEnumerable. But it doesn't seem to trigger on Stack<T> so maybe something else is going on.

A few, possibly non-mutually exclusive, ways to implement this option are:

  1. look at the type and its ancestors and check if it has any concrete ancestors that do not implement those interfaces or derived interfaces.
  2. just look at the least derived concrete ancestor (or the type itself) and check if it implements those interfaces or derived interfaces
  3. just look at the type itself (or maybe its most derived concrete ancestor? This would make it work for something like ObservableCollection<T>) and check if it implements those interfaces or derived interfaces
  4. check if the type implements any interfaces that do not derive from those interfaces, i.e. it has some other non-collection semantics
  5. only apply the rule when the type is a built-in collection, e.g. arrays, List<T>, Dictionary<K, V>, etc.

I'm not sure which of the methods 1-4 would be the simplest/most effective.

It might be useful to also allow the rule to be configurable between using something like methods 1-4 and 5. So something like:

#uses method 5 from above
dotnet_style_collection_initializer = only_for_built_in_collections

Additional context

An example of why I think this would be helpful are the Maui controls that implement IList<T> (so any Layout, for a more specific example). This rule suggests to use a collection initializer for them, which I don't think is very readable or makes perfect sense. Obviously it isn't entirely wrong, since they certainly are conceptually a collection of Views, but they seem like a proper object first and foremost. Object initialization seems to fit better than collection initialization. And for somebody or a situation where a collection initializer would make sense that would obviously still be correct syntax and could still be used and available from the "Quick Actions and Refactorings" menu if configured.