All structs have a default constructor, so we can't prevent people from writing code like this:
C#
var x = new ImmutableArray<int>();
x.Add(42); // Null reference exception here
The reason this doesn't work is ImmutableArray<T> is just wrapping an array. By calling the default constructor the underlying field storing the array is initialized to null. For performance reason it's not viable to have null-checks in all methods that access this field.
Unfortunately we can't obsolete the default struct constructor because C# doesn't let us define one.
All expressions of the form new ImmutableArray<T>() should be flagged. A code fixer should offer to replace those with ImmutableArray<T>.Empty.
Wouldn't this analyzer be rendered superfluous if dotnet/csharplang#99 gets implemented, allowing the declaration of a no-arg constructor in ImmutableArray<T> that initializes its internal array?
Tho you'd still want a warning on the use of default(ImmutableArray<T>).
Maybe we should just turn on the rule SA1129 ("Do not use default value type constructor"). Using default or default(T) makes it at least clear you don't run any constructor and just rely on all zeros to be a good initial state.
But as you pointed out: we should probably still warn people who do that on ImmutableArray<T>.
Maybe we should just turn on the rule SA1129 ("Do not use default value type constructor"). Using default or default(T) makes it at least clear you don't run any constructor and just rely on all zeros to be a good initial state.
FWIW, that's already enabled in dotnet/runtime.
If you read further, there's a related proposal to add some flow analysis similar to NRTs that'll report for "Non-defaultable Value Types", because it'd be great if something like that can be applied generally instead of making the compiler aware of a single type like a special snowflake.
Most helpful comment
FWIW, that's already enabled in dotnet/runtime.