0

I am trying to use Nullable Reference Types in C# 8.0. I have set up a sample code to test it, but something has confused me.

As I understand it, when you do not add the ? symbol to a type, it is treated as a non-nullable reference type. However, in the Main method of my code, the compiler does not complain when I check if myClass is null. Shouldn't the compiler know by this point that myClass is non-nullable?

Update:

enter image description here

To help clarify this, I have added a stub method to the MyClass class. As you can see, at this point the compiler is aware that myClass is not null. But why does it allow the next null check? It does not make sense to me."

#nullable enable
using System;

namespace ConsoleApp4
{
    class Program
    {
        static void Main()
        {
            var service = new Service();
            MyClass myClass = service.GetMyClass();

            myClass.Write();

            // I am curious why this line compiles!
            if (myClass == null)
            {
                throw new ArgumentNullException();
            }
        }
    }

    public sealed class Service
    {
        private MyClass? _obj;

        public MyClass GetMyClass()
        {
            if (_obj == null)
            {
                _obj = new MyClass();
            }

            return _obj;
        }
    }

    public sealed class MyClass
    {
        public void Write()
        {

        }
    }
}
Vahid
  • 5,144
  • 13
  • 70
  • 146
  • `MyClass` is a `class` and therefore a reference type. Whether it is declared with the `?` operator or not, it's still nullable. – Barry O'Kane Jun 17 '20 at 10:58
  • 4
    @BarryO'Kane C# 8 introduced nullable reference types, things were changed a little bit. OP is explicitly tagged the question with specific version – Pavel Anikhouski Jun 17 '20 at 10:59
  • 1
    One can't *assign* a `null` value, but I'm struggling to find in the spec where it says one can't *compare* with a `null` value. – David Jun 17 '20 at 11:02
  • 1
    @BarryO'Kane See: [Nullable reference types](https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references) – Rufus L Jun 17 '20 at 11:02
  • @Vahid Why did expect that line with check for `null` won't compile? You can write `MyClass myClass = null!;` without any warning and your check is perfectly valid – Pavel Anikhouski Jun 17 '20 at 11:02
  • @PavelAnikhouski Because GetMyClass method returns MyService not something like MyService!, I assumed that when it already returns a non-nullable type the calling code knows that it already has a non-null value, thus the compiler should complain about it, when we check it against null. – Vahid Jun 17 '20 at 11:06
  • @David Wouldn't that make sense? Please see my last reply to Pavel. – Vahid Jun 17 '20 at 11:08
  • The new feature has to be enabled explicitly, it isn't enabled by default for any new project. You have to add `enable` into csproj files in the solution. – cassandrad Jun 17 '20 at 11:09
  • 3
    @cassandrad: See the first directive line at the top of the OP's code. – David Jun 17 '20 at 11:09
  • @cassandrad I have enabled it already. – Vahid Jun 17 '20 at 11:09
  • @Vahid You can use nullable flow attributes, like `[AllowNull]`/`[MaybeNull]`, which allows to set or return a `null` value even it isn't allowed by `MyClass` type. Therefore compiler wouldn't complain about `null` check – Pavel Anikhouski Jun 17 '20 at 11:11
  • What behavior are you expecting? If anything, I would expect it to tell you *"Expression is always false"*, but would still compile. Similar to comparing a `const` to some other value. – Rufus L Jun 17 '20 at 11:15
  • `if (1 == null)` compiles. Why is this any different? The fact that code always evaluates to `false` isn't the same as it not compiling... – mjwills Jun 17 '20 at 11:25
  • @mjwills Wouldn't compile if you turned on the warnings! – Vahid Jun 17 '20 at 11:27
  • @Vahid Have you read https://stackoverflow.com/questions/61000518/in-c-sharp-8-how-do-i-detect-impossible-null-checks ? – mjwills Jun 17 '20 at 11:36
  • @mjwills Thank you. I will check it now. – Vahid Jun 17 '20 at 11:40
  • @mjwills The link was very helpful. I was able to force the compile to complain about the null check though. This is helpful, when we own the caller and callee. Please check my answer. – Vahid Jun 17 '20 at 13:09

2 Answers2

3

Shouldn't it be obvious by this point to the compiler that myClass is non-nullable?

It does, kind of. Hover over myClass on the null check, and the tool tip from static analysis it will tell you that myClass is not null there.

So, why can you check it against null? Because you can. It is still a reference type. The fact that it will not be null is not annotated in the type.

Furthermore, notice that it is not telling you that the exception is not reachable code. Why is that?

Well, C# 8.0 nullable references static analysis is not foolproof. It is good enough for most cases. However, from time to time there are patterns that they do not recognize. Needing the help of annotations and the null-forgiving operator (!). Which you can use to tell the compiler when you know better. However, these let room for telling lies to the compiler:

public MyClass GetMyClass()
{
    return _obj!;
}

Similarly, you can check for null in something that according to the static compiler cannot be null. Because you know better:

public sealed class Service
{
    private MyClass? _obj;

    public MyClass GetMyClass()
    {
        return _obj!;
    }
}

internal static class Program
{
    private static void Main()
    {
        var service = new Service();
        var myClass = service.GetMyClass();

        // I am curious why this line compiles!
        if (myClass == null)
        {
            throw new ArgumentNullException();
        }
    }
}

Of course, I would not recommend to actually go ahead and lie to the compiler. Yet, if you are receiving parameters from a third party, it is advised to still do null checks. Similarly when consuming a public API, when you cannot trust the implementation.


I will also remind you that C# 8.0 nullable references is a feature that was intended to be rolled out steadily across big legacy codebases. It was not intended to stop them from compiling when you had it enabled in some part and not in others. In fact, as far as I can tell, it will issue warnings at worst. Making null check not compile would go against that.


Why not a warning on the null check?

Well, Roslyn is deferring to the developer in this case. If you try to use myClass inside body of the conditional, and hover over it, it will tell you that it can be null there. Which, of course, would have been necessary to enter the conditional in the first place.

Furthermore, remove the throw and use myClass after the conditional. The static analysis will also say that myClass can be null after the conditional. Thus, Roslyn is using the null check as a hint that the variable is actually nullable.

Why did they opt to use it as a hint instead of issuing a warning? That I don't know. I could not find a discussion about it on dotnet/csharplang or dotnet/roslyn on github.

Theraot
  • 31,890
  • 5
  • 57
  • 86
  • Thanks Theraot, that is my point. I would expect the compiler to issue a minor warning here at least to remind me that the null-checking I am doing here is not needed! It already has the information it needs to do that. – Vahid Jun 17 '20 at 11:21
0

For future reference, here is how I was able to force the compiler to actually NOT compile when null-checking is redundant.

For anyone interested, please also check this:

In C# 8, how do I detect impossible null checks?

For those unfamiliar with DDD, ValueObject is the base class for our immutable objects.

#nullable enable
using System;

namespace ConsoleApp4
{
    internal static class Program
    {
        private static void Main()
        {
            var service = new Service();
            MyClass myClass = service.GetMyClass();

            myClass.Write();

            // Compiler now will not compile! mcClass can never be null!
            if (myClass == null)
            {
                throw new ArgumentNullException();
            }
        }
    }

    public sealed class Service
    {
        private MyClass? _obj;

        public MyClass GetMyClass()
        {
            return _obj ?? (_obj = new MyClass());
        }
    }

    public sealed class MyClass : ValueObject<MyClass>
    {
        public void Write()
        {
        }

        protected override bool EqualsCore(MyClass other)
        {
            throw new NotImplementedException();
        }

        protected override int GetHashCodeCore()
        {
            throw new NotImplementedException();
        }
    }
}


#nullable enable

namespace ConsoleApp4
{
    public abstract class ValueObject<T> where T : ValueObject<T>
    {
        public sealed override bool Equals(object obj)
        {
            return obj is T valueObject && EqualsCore(valueObject);
        }

        public sealed override int GetHashCode()
        {
            return GetHashCodeCore();
        }

        protected abstract bool EqualsCore(T other);

        protected abstract int GetHashCodeCore();

        public static bool operator ==(ValueObject<T> a, ValueObject<T> b)
        {
            return a.Equals(b);
        }

        public static bool operator !=(ValueObject<T> a, ValueObject<T> b)
        {
            return !(a == b);
        }
    }
}
Vahid
  • 5,144
  • 13
  • 70
  • 146