Roslyn: Nullable reference type checking ignores [Optional][DefaultParameterValue(null)]

[voroninp]

With enabled nullable types:

#nullable enable
void Foo(object param = null) {}

I get the warning from the compiler: > Cannot convert null literal to non-nullable reference type.
However, if I change signature to this:

#nullable enable
void Foo([Optional, DefaultParameterValue(null)] object param ) {}

Compiler is totally fine with this.

P.S. I've discovered this issue for the case of caller member name which requires default value:

[CallerMemberName, DisallowNull, Optional, DefaultParameterValue(null)] string memberName

[Joe4evr]

You really shouldn't be using the attributes [Optional] and [DefaultParameterValue] manually in the first place.

[voroninp]

There are many things which I shouldn't do, but which are fine for the compiler.

[jnm2]

[CallerMemberName] string memberName = null! is preferable. It emits different IL because DisallowNull is not present (it would be redundant).

[voroninp]

@jnm2 , I agree, but [DisallowNull] would indicate to the caller that null is not ok. Anyway, this parameter should not be used explicitly, but it is still possible.

[alrz]

Anyway, this parameter should not be used explicitly, but it is still possible.

R# does warn on such usages. I think this is a good candidate for warning waves.

[jnm2]

@voroninp DisallowNull is redundant because the parameter type string is already non-nullable. DisallowNull was designed to be applied to generic types or to nullable reference types where the expression must not be null before the call but may be null when the call returns. Because this is not a ref parameter or a generic type, DisallowNull is the wrong thing to use.

[AartBluestoke]

Because this is not a ref parameter or a generic type, DisallowNull is the wrong thing to use.

Sounds like this should be a warning then :)

[jnm2]

I agree, and I wish analyzers were considered in every API pass. Lots of untapped timesavers hiding around the BCL IMO.

[CyrusNajmabadi]

Moving to roslyn as this sounds like more of an anlayzer/compiler choice rather than lang decision.

[RikkiGibson]

The above scenarios work now, so I'll close this out.

Because this is not a ref parameter or a generic type, DisallowNull is the wrong thing to use.

Sounds like this should be a warning then :)

This is tracked by #36073. I would be interested to see a proposal for precisely what usages of nullability attributes should produce warnings. I think it would probably help steer people onto the right path.

[CallerMemberName] string memberName = null! is preferable. It emits different IL because DisallowNull is not present (it would be redundant).

Note that there isn't a guarantee that the CallerMemberName will be substituted by the compiler in all scenarios. In some scenarios involving implicit code, the explicit default value (null in this case) will be used instead.