AddNullCheck should use new parameter! syntax

[jcouv]

Instead of inserting if (parameter is null) throw new ArgumentNullException(nameof(parameter));, we should just mark the parameter declaration with a !.

We should also not offer this fix if the type of the parameter is annotated with ?.

FYI @fayrose @agocke

[CyrusNajmabadi]

Has the compiler side of this been done? If so, i'm happy to do the IDE side. Is this currently in a branch, or in master? What langversion is it gated on?

Thanks!

[jasonmalinowski]

@jcouv Is the compiler support in for this?

[jcouv]

The compiler feature is in feature branch https://github.com/dotnet/roslyn/tree/features/param-nullchecking
LDM actually decided yesterday to keep this feature out of 8.0, so no rush.

[jasonmalinowski]

@jcouv I forget, what langver is this going under? 9.0 or something sooner?

[jcouv]

The feature is now targeting 9.0.

[Youssef1313]

@jcouv

We should also not offer this fix if the type of the parameter is annotated with ?.

For public method, there is no guarantee that consumers will have NRT feature enabled. So, isn't it better to still offer this fix if the paramater is annotated with ?, BUT for public methods only?

[jasonmalinowski]

@Youssef1313: this is the fix for adding null checks at the library side of things, not the consumer side of things; the check for ? would only be if your method is directly annotated as allowing a null in that case.

[Youssef1313]

@jasonmalinowski Yes I see my comment is wrong. I don't know what I was thinking about when I wrote that 😄

[jasonmalinowski]

@Youssef1313, eh, it's Monday. 😄

[CyrusNajmabadi]

Closing out. I believe @RikkiGibson has done this.