Skip to content

"Null check operator used on a null value" error message should use better terminology #47185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jamesderlin opened this issue Sep 10, 2021 · 13 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-enhancement A request for a change that isn't a bug

Comments

@jamesderlin
Copy link
Contributor

jamesderlin commented Sep 10, 2021

Steps to reproduce:

  1. Run the following program in the Dart VM:
    void main() {
      print(null!);
    }

Result:

Unhandled exception:
Null check operator used on a null value
#0      main (file:///path/to/source_file.dart:2:13)
...

I dislike that this error message refers to postfix ! as the "Null check operator":

  • It is referred to as the "null assertion operator" in other documentation (https://dart.dev/null-safety/understanding-null-safety#null-assertion-operator), which is inconsistent.
  • "Null check operator" implies that it's performing a null check. Calling it a "null check operator" makes it confusing explaining to users that they should perform a null check first when their variable turns out to be null.

I think that the error message instead should be: "Null assertion operator used on a null value".

I'm using: Dart SDK version: 2.14.1 (stable) (Unknown timestamp) on "linux_x64".

@jamesderlin
Copy link
Contributor Author

I suppose while I'm nitpicking the terminology, while I prefer "null assertion operator" over "null check operator", I'm not a huge fan of "null assertion operator" either. Assertions assert that a condition is true, so "null assertion operator" implies to me that something is null. IMO "non-null assertion operator" would be better. But even then, assertions are typically conditionally enabled, so my ideal preference would be something like "null failure operator" or "non-null cast operator".

@lrhn lrhn added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-enhancement A request for a change that isn't a bug labels Sep 10, 2021
@bwilkerson
Copy link
Member

"Null check operator" implies that it's performing a null check.

The null-safety specification, in the section on runtime semantics, includes the following

Null check operator

When evaluating an expression of the form e!, where e evaluates to a value v, a dynamic type error occurs if v is null, and otherwise the expression evaluates to v.

This implies that there is, in fact, a runtime check involved with the use of the operator, so I believe that the word "check" is conveying the correct semantics.

@jamesderlin
Copy link
Contributor Author

I don't dispute that it performs a runtime check nor that that's how the specification refers to it; I'm saying that the term is confusing and makes it harder to explain to new users what they're doing wrong. Calling it a "null check" is not useful because it introduces ambiguity. Each of the following things can be called a "null check", but one of them is not like the others:

  • x ?? defaultValue
  • if (x != null)
  • x!

@eernstg
Copy link
Member

eernstg commented Jan 4, 2022

The specification uses the phrase 'type check' about expressions of the form e as T, which could be spelled out as "perform a dynamic check that the result v of evaluating e has type T; evaluate to v if this is true, otherwise throw".

We could use that as a starting point for the bang selector: e! could be spelled out as "perform a dynamic check that the result v of evaluating e is non-null; evaluate to v if this is true, otherwise throw", and we could then call e! a 'non-null check', and a ! which is used as a selector could be called a 'non-null check selector'.

It may seem overly pedantic to call it a 'selector' (because that word might not be very familiar to some developers who would consider 'operator' to be an obvious choice), but it does give more precise hints about the semantics. For instance, a?.b!.c will do null shorting: if a is null then the ! will never be evaluated. This is because ! is a link in a chain of selectors, and we're skipping the whole chain when we encounter a ?. with a null receiver. Conversely, a + b! will check that b isn't null, it will not check a + b isn't null. So I tend to think that it's helpful to use the word 'selector' explicitly when talking about the non-null check selector.

@bwilkerson
Copy link
Member

I'm not aware of any user studies that have been done on terminology (though that might be useful), but I suspect that very few users will understand the word "selector". As far as I can tell, the term "selector" is only used to mean "CSS selector" on dart.dev. I suspect that most users refer to it as an operator, and think (incorrectly perhaps) of a + b! being equivalent to a + (b!) as a result of operator precedence.

I'm ok with calling it a "non-null check [operator]" and will be happy to update messages and documentation for the analyzer to use that terminology if that's what we're settling on.

@jamesderlin
Copy link
Contributor Author

  1. "Selector" seems very unusual. It would not give me any insight about semantics since that term does not have any meaning to me, and therefore it would be more confusing to me than helpful.

  2. While "non-null check selector/operator/whatever" would be a slight improvement over "null check operator", my main complaint is about the term "check". While it technically performs a check, I do not consider that to be a "check" in a meaningful sense. That is, it is used where a value is expected to be unconditionally non-null. Failure is expected to be fatal.

    To reiterate: "check" is problematic because someone writes code that uses x!, encounters an error, asks about it, is told "You need to check that x is non-null first!", but wait, they used something literally called a "null check operator", so didn't they already perform a "check"? That's awkward at best and confusing at worst.

    Is there any real difference between x! and a x as NonnullableType cast? Why not call it something like a "non-null cast operator"?

@eernstg
Copy link
Member

eernstg commented Jan 4, 2022

Is there any real difference between x! and a x as NonnullableType cast?

The NonnullableType may not be denotable, which may or may not count as a "real" difference. ;-)

import 'dart:async';

void main() {
  FutureOr<int?> x = 1 as dynamic; // The cast just ensures that we don't promote `x` to `int` here.
  ...
  foo(x!); // For some reason we insist that `x` isn't null at this point.
}

There is no type T such that x! has the same meaning as x as T in the above example.

I don't really see why it should be misleading or confusing to call it a 'non-null check': A 'check' is an operation that may fail or succeed, and if it fails then we get a dynamic error. That error is specified to be the act of throwing an object, so even though it is bad style then it is not impossible to catch it, which is a property that it shares with other dynamic errors. If the check succeeds then it is transparent (that is, everything behaves as if the check never occurred).

Granted, I might consider this terminology normal and unsurprising because that's the terminology we have used in the specification for many years when it comes to type checks (that is, as expressions). But there are no other interpretations of the word 'check' that comes to mind for me, and which makes the terminology misleading or confusing.

someone writes code that uses x!, encounters an error, asks about it, is told
"You need to check that x is non-null first!", but wait, they used something
literally called a "null check operator", so didn't they already perform a "check"?

But it wouldn't work that way, there is no "check that x is non-null first" error for an occurrence of x!, that would only happen when x is used without !.

It might be confusing that we can use x! and then, maybe just a few characters later in the source code, x, and the lone x gives rise to a "check that x is non-null first" error. But this is simply because the first check does not suffice to ensure that x is still non-null when it is evaluated again. So the second occurrence may also need to be x!, such that we "non-null check" both usages.

@jamesderlin
Copy link
Contributor Author

The NonnullableType may not be denotable, which may or may not count as a "real" difference. ;-)

That's a good example, but I still think it'd be fair to call it the "non-null cast operator". (I also still like "null failure operator".)

That error is specified to be the act of throwing an object, so even though it is bad style then it is not impossible to catch it, which is a property that it shares with other dynamic errors.

I recall being told that for some optimization levels when compiling to JavaScript that (some?) Errors are presumed to never be thrown and therefore will not be caught. I therefore was told to prefer using a LBYL (look before you leap) style of code over EAFP (easier to ask for forgiveness than permission). (Significantly, I think that I was told this in the context of type casts.)

But it wouldn't work that way, there is no "check that x is non-null first" error for an occurrence of x!, that would only happen when x is used without !.

You have the perspective of someone who is already intimately familiar with the language. I'm talking about people who very much aren't, who sprinkle in ! usage in a misguided attempt to fix compile-time errors but who then become bewildered when they run their code and encounter runtime errors instead.

While "check" is technically correct, I don't think it does us any favors. It's broad and ambiguous; there are other operators that check whether a value is null, and I don't think that using a definite article is an obvious distinction.

@eernstg
Copy link
Member

eernstg commented Jan 5, 2022

I agree, of course, that we need to be careful about the terminology, and also that it's important to strive for consistency.

Dictionary definitions of 'check' include examine (something) in order to determine its accuracy, quality, or condition, or to detect the presence of something and to make certain that something or someone is correct, safe, or suitable by examining it, him, or her quickly. They both fit the framework of (1) performing a measurement, and (2) comparing the given outcome with an expected outcome. They leave it implicit what happens if the outcome isn't as expected, but I think it's reasonable to say that it is a failure of sorts.

'Assertion' is actually better than 'check' because it very directly implies that there is a measurement with a certain expected outcome, and any different outcome is a failure. But I bought the argument that 'assertion' in Dart almost exclusively refers to the language construct assert, and it's expected to be an optional mechanism. So that word is already taken.

'Check' then seems to be a reasonable choice, and it seems to support the underlying structure "OK => just continue" vs. "ERROR => throw".

@jamesderlin wrote:

for some optimization levels when compiling to JavaScript that (some?) Errors
are presumed to never be thrown and therefore will not be caught

When I said:

it is not impossible to catch it

I was just responding to your remark

Failure is expected to be fatal.

in your definition of a 'check'. Dart check failures are always throwing, so they can always be caught. A low-level error like BUS_ERROR or SEGMENTATION_FAULT only occurs if we encounter a bug in the implementation of Dart. So I did not accept your premise that a check failure is always fatal.

ASIDE: dart2js allows for eliminating many dynamic checks that are specified for Dart, which means that it yields a running program whose semantics is different from the semantics which is specified for Dart. However, this does not create any practical problems in many cases: The different-from-spec semantics almost always occurs when the program encounters buggy code, and then the program generally crashes (for example, with a type error), and the only difference between the specified behavior and the behavior that occurs with the generated JavaScript from dart2js where the code for many checks has been omitted is that the program runs for a little bit longer, and then it may encounter a 'no such method' error rather than a 'type error'. Given that the performance benefit is huge, and the end-user hardly cares whether the program crashed because of a 'no such method' vs. because of a 'type error', this trade-off is typically preferred. Of course, the reason why the errors "don't get much worse" by having non-spec behavior is that JavaScript maintains its own invariants during execution, so we never destroy the low-level representation of objects, or mess up memory in other low-level ways. So the fact that some compilation modes will omit the code for some checks doesn't change the fact that the specified behavior of Dart includes a dynamic error in the case where a (non-null, or type, or index range, or some other) check fails.

there are other operators that check whether a value is null

Let's consider those other operators a bit:

  • e1 ?? e2 does behave differently when e1 evaluates to null respectively a non-null value. The grammar calls it an "if-null expression". We could call it the "null check operator", but it doesn't have an inherent failure semantics (e1 ?? e2 will not throw unless e1 throws or e2 throws), so it's more like having a backup or default value from e2 which will be used if e1 evaluates to null. Maybe ?? could be the "null default operator"?
  • e?.m() (and variants like e?.m<...>(...) and e?[...]) contains ?. which could be called the "null check member access operator", but this is again a construct that has no inherent failure semantics: We skip rather than throw when e evaluates to null. Cascades like e?..m() are similar. In this case the language specification uses 'conditional' (so e?.m() is a conditional method invocation), and other documents sometimes use 'null-aware', as in "a null-aware invocation". The word 'conditional' does not imply that there is any connection to null, and 'null-aware' does not imply that the semantics is to skip the invocation when the receiver is null, so none of these are particularly informative. However, we don't have invocations that are conditional based on any other condition, so it may be OK in practice.

@jamesderlin
Copy link
Contributor Author

@eernstg While everything you've written is technically correct, I'm not convinced that it is the best kind of correct. My desire was to use terminology less vague than "check" to make things less confusing for people learning Dart. Those people almost certainly will not do the careful and thoughtful analysis that you've done. (If they did, they wouldn't be misusing ! in the first place.)

That ! performs a check should not necessarily require that "check" be in its name. as performs a very similar runtime check, but I don't think that it is called a "type check operator" or an "as-check operator", and the Dart Language Specification refers to an expression e as T as a "cast expression".

Since ! and as are very similar, why not name ! to be more consistent with as?

@eernstg
Copy link
Member

eernstg commented Jan 6, 2022

@jamesderlin, it is true that e as T is denoted as a "type cast" in the spec, and we could say that e! is a "non-null cast". It would then be slightly confusing that there may not exist a type T such that e! and e as T is exactly the same thing. However, it would work smoothly in the rather typical case where the static type of e is of the form S? where S is non-nullable: e! would then be the same thing as e as S, with respect to both promotion and dynamic semantics. So I'd consider "non-null cast" as a slightly worse alternative to "non-null check".

But if you really, really don't like the word 'check', and we agree that 'assertion' is already taken, is there a better word? ;-)

@bwilkerson
Copy link
Member

In a different thread about the message for a different diagnostic, I suggested that we could eliminate the question of terminology by re-writing the message completely rather than by attempting to fix one phrase. That might be a useful approach here as well. What if the message were more like: "A value that was expected to not be null was null."? (We could probably find something even better, but it illustrates one wording that eliminates the need to have a name for the !.)

@jamesderlin
Copy link
Contributor Author

It would then be slightly confusing that there may not exist a type T such that e! and e as T is exactly the same thing. However, it would work smoothly in the rather typical case

I'd personally prefer to have a name that's clearer for the 99% use case than to make it worse 99% of the time to accommodate a 1% case.

I suggested that we could eliminate the question of terminology by re-writing the message completely rather than by attempting to fix one phrase. That might be a useful approach here as well. What if the message were more like: "A value that was expected to not be null was null."?

I'd be satisfied with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants