! has shocking behavior

[kjpgit]

EDIT: Found this awesome project https://github.com/tom-englert/Nullable.Extended
It warns on usage of ! unless you have a special comment to suppress the warning. Check it out! (And hopefully it becomes a standard warning you can request from the compiler!)

I wrote this line of code today in .net 6

var db = new CPStorage(Environment.GetEnvironmentVariable("CPS_TABLE_NAME")!);

And then started up my app to verify the error that would be thrown since this env var doesn't exist yet, and CPStorage() requires a non-null string. (And I was as expected getting a nullability warning before I added the !) And to my dismay, it started up with no error. I was shocked.

I have used swift, where ! will trap at runtime if it is null. I expected c# not to trap (Exit), but throw a null exception.

If I wanted to turn off a compiler warning and do something unsafe, I would have used a pragma or unsafe keyword. Not this.

I have been slogging through asp.net for a new project, and have been willing to overlook a lot of struggles because I think C# is a decent language, but this has me worried I'm making a bad bet.

[ghost]

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I wrote a line of code today,

<table>
  <tr>
    <th align="left">Author:</th>
    <td>kjpgit</td>
  </tr>
  <tr>
    <th align="left">Assignees:</th>
    <td>-</td>
  </tr>
  <tr>
    <th align="left">Labels:</th>
    <td>

`area-Infrastructure-libraries`, `untriaged`

</td>
  </tr>
  <tr>
    <th align="left">Milestone:</th>
    <td>-</td>
  </tr>
</table>
</details>

[kjpgit]

I seem to have hit ctrl-enter or shfit-enter very early when typing my code (annoying feature to be honest), so it prematurely called the bots.

[bartonjs]

! in C# is just the Jedi mind trick for "there's no null here...". It doesn't cause a throw to happen, just makes a "this might be null..." warning go away.

If it's used in the middle of a member invocation expression (e.g. Environment.GetEnvironmentVariable("CPS_TABLE_NAME")!.Substring(0)) then the member invocation is what will throw a NullReferenceException, not the !.

For the line of code as written I'd expect the CPStorage constructor to throw an ArgumentNullException... unless the CPStorage constructor wasn't marked as non-null-accepting, in which case the suffix ! has no effect in code.

In C# the suffix ! is called the null-forgiving operator: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-forgiving

Available in C# 8.0 and later, the unary postfix ! operator is the null-forgiving, or null-suppression, operator. ... The null-forgiving operator has no effect at run time. It only affects the compiler's static flow analysis ...

[danmoseley]

@kjpgit what tools are you using? If you use Visual Studio I suggest you hit debug to see what's happening.

[kjpgit]

So let's see how to write safe, modern c# code in 2022:

class Storage
{
    public Storage(string required_table_name) { }
}

// Not this way
new Storage(Environment.GetEnvironmentVariable("CPS_TABLE_NAME")!);

// Not this way
new Storage((string)Environment.GetEnvironmentVariable("CPS_TABLE_NAME"));

// Yep, this is clearly the idiomatic and simple way
using static correctness_is_always_job_one;
class correctness_is_always_job_one
{
    public static T __unwrap<T>(T? obj) {
        if (obj is null) {
            throw new ArgumentNullException();
        }
        return obj;
    }
}
// Errors do not pass silently
// Do not violate principle of least surprise
// Correctness is job 1 of business software
new Storage(__unwrap(Environment.GetEnvironmentVariable("CPS_TABLE_NAME")));

I vote for a #pragma bang_enforces directive (or compiler option preferably) where ! does not violate the preconditions of the callee. If I wanted to YOLO I'd use python.

The SHORTEST code had better be the correct thing, because that's what everyone will do. I'm guessing people are just using ! everywhere because the compiler isn't don't good enough control flow analysis, but there should be a different, longer symbol (or system function) for that and it should be marked as unsafe in modern codebases.

I can not express how disappointed I am with this "new philosophy" of the c# stewards, where unsafe code is now the easiest and shortest path. Apple's Swift language got this right with the ! unwrap operator (although I'd prefer an exception instead of terminating the thread, for server software obviously).

Please transfer this issue to the language repo if appropriate. Thank you.

[kjpgit]

For the record, the "global usings" with "static=True" feature did make it convenient to expose our __unwrap<T> helper function to all source code files. And I do commend the effort toward increasing null safety. I will look at the roslyn links when I have time. Lastly, in what repo should I file a request to add an official compiler warning?

[CyrusNajmabadi]

Lastly, in what repo should I file a request to add an official compiler warning?

github.com/dotnet/roslyn-analyzers would be the repo. However, it is highly unlikely we would add a feature to warn on a language feature being used for its intended purpose.

[kjpgit]

Thanks. Filed dotnet/roslyn-analyzers#5934

[RikkiGibson]

The closest analogue to C#'s postfix ! that I'm aware of is probably the non-null assertion operator in TypeScript.

[kjpgit]

FYI This project with 7000 nuget downloads does what I want... warns about all uses of ! unless you suppress it with a special comment. So we heavily discourage obj! (which removes safety, clearly some language features are not as safe as others) and use our __unwrap(obj) instead. Yay, I have a working solution and feel the Zen now. (Well, other than relying on a random stranger's nuget package...WCGW?)

I don't have a strong opinion on the proposed !! thing. For marking / declaring arguments, I can see how it would be really short for existing code that has to support legacy callers. For use in code/expressions, meh, I like our our __unwrap looks -- easy to grep, just the right amount of friction that makes you think "should I do an explicit check here?", but at least "!!" might be able to be grepped for... except it would overlap with uses of !! in arguments... so meh, I don't care either way about the !!.

[danmoseley]

@kjpgit nobody here is saying you should write that code. We don't yet understand what the problem is.

[danmoseley]

Starting at the start again.
var db = new CPStorage(Environment.GetEnvironmentVariable("CPS_TABLE_NAME"));

If this environment variable does not exist, GetEnvironmentVariable will return null as you know. What happens now depends on the implementation of CPStorage:

1. If it accepts null then execution continues normally
2. If it does not, but it was built with a nullable annotation context (which essentially means it's API has been annotated) then you will get a compile time warning, because GetEnvironmentVariable has been annotated to potentially return null. You can either suppress this warning with ! or change your code so that it can't pass in null.
3. If it does not, but it's not participating in nullable annotations, then you will get no warning and only a run time error.

Does that match your expectations? In no circumstances do you need to write the extended code you have pasted.

[huoyaoyuan]

You can use ?? throw expression, or you can make the __unwrap extension method.

! is designed for when nullability is obvious to human, but not provable for the compiler. It's intentionally not checking for performance reason, for example the usage in ImmutableArray<T>.

The nullable reference type feature of C# 8 is designed to be pure compile time check, to avoid breaking existing code as possible.

[kjpgit]

Just to be clear, I assume you are suggesting obj ?? throw new ArgumentNullException() ? Given the choice between that and obj!, which do you think the average programmer is going to do? Which do you think programmers who care about trapping errors early are going to feel happy about?

My suggestion is simple:

• Make obj! actually enforce at runtime
• Make a T System.__assume_not_null<T>(T? t) for the "elite 1% performance junkies" who want to disable a warning, e.g. __assume_not_null(obj)

We have a language for performance junkies, it's C++. I've lived in that world, where the defaults are unsafe. It's terrible.

[CyrusNajmabadi]

Make obj! actually enforce at runtime

This would defeat the purpose of nrt and make it's literal non-starter for a vast section of our user base. Nrt was accepted because the claim can honestly and accurately be made that it has no runtime impact, and this can be adopted into existing codebases without risk.

We have a language for performance junkies, it's C++

You are fundamentally misunderstanding. This is not about performance. This is about risk management for teams contemplating using nrt.

If they adopt nrt and now get runtime exceptions for code that worked before just by using nrt, then that is a complete non-starter.

[sharwell]

This is the correct answer. The language provides ! as a suppression for cases where nullable reference types doesn't support the semantics of the program and the value is known to not be null, and provides ?? throw ... for cases where a truly nullable value is being converted for use in a non-nullable context. The option used an any given location depends on the program semantics.

[B1Z0N]

Also, there's supposedly will be !! operator in dotnet 7.0/c# 11 that solves just what you were talking about here. See this for more info.

[TahirAhmadov]

Why not just write an extension method?

public static class _Object_Extensions
{
  [MethodImpl(AggressiveInlining)]
  public static T NotNull<T>(this T? This) where T : class => This ?? throw new NullReferenceException();
}

string? s = null;
Foo(s.NotNull());

[CyrusNajmabadi]

But would it potentially conflict in the future if microsoft adds its own method with the same name?

This is likely never going to happen. And this would make ever using an extension method problematic, and that issue exists today.

I'm a bit shocked I was able to use NotNull from another source file without adding a using.

Is the containing static class in scope in the location where you're using the extension method?

[kjpgit]

My bad, yeah they were both in the default namespace. After I put the NotNull class in a different namespace, I got an error, and had to add a using, which is good.

I guess I'm not 100% clear what you're saying about "issue that exists today", but just for the sake of argument -- if microsoft did add their own extension method, or direct method (edit: probably not possible for something where this is null?), with the same name, would I be able to disambiguate and still prefer my own extension method that predated theirs?

[jmarolf]

I am assuming that if we allowed you to put !! and it meant "Throw null reference exception" You would be in favor?

Essentially this:

var db = new CPStorage(Environment.GetEnvironmentVariable("CPS_TABLE_NAME")!!);

Would be shorthand for this:

var db = new CPStorage(Environment.GetEnvironmentVariable("CPS_TABLE_NAME") ?? throw new NullReferenceException());

If so, good news! this is something the language design team is thinking about doing.

[kjpgit]

@jmarolf I don't have a strong opinion on the proposed !! thing (as long as I can warn/discourage ! in my codebase). For marking / declaring arguments, I can see how it would be really short for existing code that has to support legacy callers. For use in code/expressions, meh, I like how our __unwrap looks -- super easy to grep, just the right amount of friction that makes you think "should I do an explicit check here?" "Am I ok with this small runtime performance hit?" The symbol "!!" might be able to be grepped for, unlike '!', ... except it would overlap with uses of !! to annotate arguments... so meh, I don't care much either way about the !!.

It seems like bikeshedding, what syntax you want, you'd have to make a lot of real example code to see if you like the look of !! yelling !! at !! you !! vs unwrap() or NotNull -- which as this thread is showing, users have the option on what they want to call it... which may or not be a good thing, but I'm cautiously optimistic :-)

[TahirAhmadov]

I guess I'm not 100% clear what you're saying about "issue that exists today", but just for the sake of argument -- if microsoft did add their own extension method, or direct method (edit: probably not possible for something where this is null?), with the same name, would I be able to disambiguate and still prefer my own extension method that predated theirs?

Please refer to the docs on method resolution, or just experiment - create a sample assembly with this method, then create another project referencing the first assembly and also defining this method, and see what happens.