dotnet / csharplang

The official repo for the design of the C# programming language
11.55k stars 1.03k forks source link

Nullable Reference Types analysis can be thrown off by condition #2820

Closed danwalmsley closed 5 years ago

danwalmsley commented 5 years ago

See screenshot here: Iv enabled NRT im on .net core 3.0 release and latest visual studio.

image

then if I add in a call with a condition on a value being not null...

image

It seems to suggest to the compiler that after this line, x might be null, because the programmer checked for it being null.

Is this intended behavior?

My helper methods are defined as:

[MethodImpl(MethodImplOptions.AggressiveInlining)]
        [ContractAnnotation("condition:false=>stop")]        
        public static void Requires<TException>(bool condition) where TException : Exception, new()
        {
            if (!condition)
            {
                throw new TException();
            }
        }
333fred commented 5 years ago

Contract.Requires<TException> needs a [DoesNotReturnIf(false)] on the condition. See how Debug.Assert does it. If that still doesn't fix it, I'd suggest opening an issue on dotnet/roslyn, as this isn't the right venue.

333fred commented 5 years ago

Also, yes, the warning is intended behavior. != null is a pure null test: https://github.com/dotnet/csharplang/blob/c229cae634/meetings/2019/LDM-2019-02-20.md#deliberate-tests-for-nullability

danwalmsley commented 5 years ago

@333fred many thanks for feedback, I will test your suggestions.