Why can't the C# compiler detect that this can't be null?-CodePudding

The Safe extension method can never return null, but the compiler seemingly isn't able to detect that. I think I read something about this being an intentional design decision for strings, but don't recall when or where. Can anyone explain how to fix this (i.e., help the compiler along) or explain why it's intentionally this way?

Foo? foo = null;
Bar  bar = new();

// CS8601: Possible null reference assignment.
bar.NoNullsHere = foo?.Text().Safe();

// No error with the trailing exclamation point
bar.NoNullsHere = foo?.Text().Safe()!;

public static class StringExtensionMethods
{
    public static string Safe(this string? obj) => obj is null ? string.Empty : obj!;
}

public class Foo
{
    public string? Text() => "has a value, but could be null or empty";
}

public class Bar
{
    public string NoNullsHere { get; set; } = string.Empty;
}

Edit: the "right associative" answer provided by @sweeper is correct - the Safe method will not get invoked. Given that, is it possible to do what I set out to accomplish? I really prefer the syntax and intellisense support. Also, my actual implementation accepts other arguments to the Safe method to optionally do things like surround the resulting string with some token (e.g., double quote).

bar.NoNullsHere = foo?.Text().Safe();

instead of either of these

bar.NoNullsHere = StringExtensionMethods.Safe(foo?.Text());
bar.NoNullsHere = foo?.Text() ?? string.Empty;

Edit 2: forgot to mention that @sweeper does provide a work around. The required parentheses are a little annoying, but better than the alternatives. Thanks all!

bar.NoNullsHere = (foo?.Text()).Safe();

CodePudding user response：

You'r using the null propagation operator.

    bar.NoNullsHere = foo?.Text().Safe();

    // consider it will work like below
    if (foo != null)
    {
        bar.NoNullsHere = foo.Text().Safe();
    }else
    {
        bar.NoNullsHere = null;
    }

So the compiler is right bar.NoNullsHere can be null

CodePudding user response：

The ?. kind of makes everything go "right associative" (sorry I can't find a better word).

foo?.Text().Safe();

means "run Text().Safe() if foo is not null". So if foo is null, it will do nothing and evaluate to null. If it helps, think of it as:

// not valid syntax, but IMO a good mental image
foo?.(Text().Safe());

This behaviour is also why you are able to just use one ? in a chain:

someNullableThing?.Property1.Property2.Property3

If ?. were "left associative", someNullableThing?.Property1 would have been nullable, and you would have to put ? everywhere on the chain, even though PropertyNs are not nullable. Note that some languages like Kotlin actually implement it this way.

That said, I think what you meant was:

bar.NoNullsHere = (foo?.Text()).Safe();

This will run foo.Safe() even when foo is null.

CodePudding user response：

Sure, okay, Safe() can't return null. But that's not really your problem. Here's how you use it:

foo?.Text().Safe();

What if foo is null? What then? What should be the result of that expression? It's going to be null, of course, because that's how the ?. operator behaves. And that's why you get that error.

You will have to check that foo isn't null for it to work as you want.

Alternatively, you don't really need Safe(), you can just do that instead:

bar.NoNullsHere = foo?.Text() ?? string.Empty;

That handles both foo being null and Text() returning null, and coalesces that to an empty string.

CodePudding user response：

Interesting case... This question was asked to dotnet team :

Warn about use of conditional null operator with an extension method that accepts null reference

At the core the nullable reference type feature is imperfect, it's more a heuristic for detecting null than a guarantee. That means our analysis can be wrong in cases. The expectation is that code will hit situations where the compiler is incorrectly inferring a value as maybe-null when it's definitely not and vice versa. Hence as a general rule we do not provide warnings to customers when they make null checks that appear redundant to the nullable type system.

In summary, the analyzer isn't perfect and don't manage this case.

To fix this (remove the warning), you can :

bar.NoNullsHere = foo?.Text().Safe()!
//or
bar.NoNullsHere = StringExtensionMethods.Safe(foo?.Text());

This is useless extra work... but it's analyzer limitation.

Several answers propose to use the null propagation operator. It's inaccurate, because this don't call the method Safe that is responsible to generate the default value :

string? text = null;
string noNullsHere;

noNullsHere = text?.Safe() ?? "Default from Main";
Console.WriteLine(noNullsHere);
// Output : Default from Main

noNullsHere = StringExtensionMethods.Safe(text);
Console.WriteLine(noNullsHere);
// Ouput : Default from Safe

public static class StringExtensionMethods
{
    public static string Safe(this string? obj) => obj is null ? "Default from Safe" : obj!;
}

public class Bar
{
    public string NoNullsHere { get; set; } = string.Empty;
}