ulrichb/ImplicitNullability

Assume NotNull for Inherited (Implemented) Members

Mike-E-angelo opened this issue · 6 comments

Mike-E-angelo commented

I installed your extension and got some warnings right away. In this case it is from implementing an IList<> where it cannot determine the nullability of the members. Is there a way to mark these all as implicitly null?

Thanks for the cool project here!

ulrichb commented

Many thanks!

I assume you mean the substitutability warnings of IN (e.g. "Implicit NotNull conflicts with nullability in base type" or "Implicit NotNull overrides unknown nullability of base member, nullability should be explicit").

In the case of IList<T>-implementations it depends if you target .NET Framework, which has external annotations for IList<T> or .NET Standard/Core, which doesn't have these atm. BTW: feel free to issue a PR to https://github.com/JetBrains/ExternalAnnotations :)

In both cases the warnings for the "item" parameters are correct because e.g. IList<T>.Add(item) or IList<T>.Contains(item) allows null values, and so should your implementation, otherwise it violates substitutability with the IList<T> contract. => Add [CanBeNull] to the item parameters and the warning there disappears.

Of course you can also ignore/suppress the individual warnings, or disable the warnings at all (in R# severity options).

  • 👍1
ulrichb commented

There is one exemption: The array parameter of the CopyTo() method which has a [NotNull] contract (but only in the .NET FW external contracts).

Here you have two options.

  1. Just put a [NotNull] in your implementation, or
  2. (Better) issue a PR with the missing external annotations for .NET Core.
  • 👍1
Mike-E-angelo commented

Great... thank you for the information. In my case I am indeed using .NET Standard, so that makes sense now. :) This is very cool stuff! I have been playing hard and loose with my nulls hehe. In fact I don't do any checking at all (i know i know) and have been wanting to get back into PostSharp to weave my guards into place. I actually did do this a while ago but used the [Required] and [Optional] attributes to generate them. Maybe I'll revisit this with the new fangled toys. 😄

I'll close this issue now, but I am curious... what are your thoughts on the new proposed .NET null reference types? If the comments are any measure, it looks like it has an uphill climb hehe.

  • 👍1
ulrichb commented

Regarding rewriting / injecting null checks: Maybe you want to take a look at Fody NullGuard.

Regarding C# Nullable reference types: I just/finally wrote a few sentences about it into the README.

  • 🎉1
Mike-E-angelo commented

Coooool... just one quick sanity check:

string? Bar([string? a, string b)

You mean string? Bar(string? a, string b) correct? Or maybe new magic I do not know about. 😄

  • 👍1
ulrichb commented

Yes of course. Fixed that. Thanks!

  • 🎉1