Normalize API surface to use abstractions vs concrete implementations

[egil]

The generated code should follow the general API design pattern related to MA0016.

There many places in the generated code which could use abstractions instead of the concrete types. This will make it easier for us to change implementation details in the future without introducing a breaking change.

This issue should serve as a parent feature for updating all the parts of the code that can generate a collection type.

• All "result" types should take an IEnumerable<T> as input instead of List<T>. #99 All "result" types should take an IEnumerable<T> as input instead of List<T>.
  Non-breaking change.
• All "parameter" types should use IReadOnlyList<T> for array properties instead of List<T>. The default value assigned to these should be Array.Empty<T>. E.g.: public IReadOnlyList<string> Items { get; set; } = Array.Empty<string>()
  Breaking change.
• All "model" types should use IReadOnlyList<T> for array properties instead of List<T>. The default value assigned to these should be Array.Empty<T>. E.g.: public IReadOnlyList<string> Items { get; set; } = Array.Empty<string>()
  Breaking change.

Related: https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0016.md

[egil]

@LarsSkovslund / @rickykaare / @davidkallesen / @perkops / @cjakobsen / et. al. please share your input on the suggested changes above.

Here are a few questions/thoughts:

1. Is this all the places where we are using concrete types?
2. I like to keep parameters as IReadOnlyList<T> since there are very few scenarios where the endpoint handler needs to modify the received list, and in those cases, a LINQ statement or similar will usually result in a new array/list being created anyway.
3. Should models also default to IReadOnlyList<T> or perhaps be ICollection<T>/IList<T>? I do not see a point in making the returned collections mutable, since they are just getting serialized to JSON, and having the type as an IReadOnlyList<T> means pretty much any collection type in the framework can be used to initialize the property. But maybe I am missing something here?

[cjakobsen]

I support the suggested changes, it will improve usability of the generated code and add more flexibility in the generator.

Why change the default value to Array.Empty()?

I noticed that in Atc.Rest.Results.ResultFactory we use string[], changing this to IEnumerable could be another improvement.

For the non-breaking part I suggest to just move ahead. For the breaking part we need to define the release process for this, including how this is communicated and versioned.

[LarsSkovslund]

@cjakobsen because Array.Empty<T> is more performant than new string[].

[cjakobsen]

That is correct but if we want to optimize for that as well then we should be even more crisp and use Enumerable.Empty<T>()

[egil]

That is correct but if we want to optimize for that as well then we should be even more crisp and use Enumerable.Empty<T>()

Interesting, wasn't aware of that. Looking at the code involved, I am not sure it is more "crisp" though:

Array.Empty() is just an static new T[0] under the hood.

Enumerable.Empty() points to a EmptyPartition() that does seem to be a bit more complex.

What are your reasons to prefer Enumerable.Empty<T>() Claus?