Sdk: Analyzer is missing an error case for "if (null)"

[lukepighetti]

Consider the following:

final String foo = null;

void main()  {
  if(foo?.isNotEmpty){
    print("true");
  } else {
    print("not true");
  }
}

Even though this executes and prints not true, there is a compiler warning that states:

The value of the '?.' operator can be 'null', which isn't appropriate in a condition.

However, if we change it to explicitly null, there is no warning!

void main()  {
  if(null){
    print("true");
  } else {
    print("not true");
  }
}

I would argue that if(foo?.isNotEmpty) should not produce a compiler warning since the conditional checks if the value is true. It doesn't check if the value is not false.

A common developer solution to this problem seems to be the following:

final String foo = null;

void main()  {
  if(foo?.isNotEmpty ?? false){
    print("true");
  } else {
    print("not true");
  }
}

I think this little lint/warning promotes codegolf when it could silently fail without issue.

[matanlurey]

Sorry, this warning is not unnecessary, as Dart does not allow null in conditionals since Dart 2:

void main() {
  if (null) {
    print('Hello');
  }
}

$ dart /tmp/null.dart

Unhandled exception:
Failed assertion: boolean expression must not be null
#0      main (file:///tmp/null.dart)
#1      _startIsolate.<anonymous closure> (dart:isolate/runtime/libisolate_patch.dart:300:19)
#2      _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:171:12)

If you are not seeing this error, it is a bug. Analyzer should be reporting an error as well.

[lukepighetti]

For reference:

[eernstg]

This will be a bug at the point where the plain type bool does no longer mean bool | Null (that is, when we have introduced non-null-by-default), but currently the static analysis requires only that

It is a compile-time error if the type of the expression $b$
may not be assigned to \code{bool}.

where $b$ is the condition of an if statement. Null is assignable to bool today, so there is no compile-time error (not even with --no-implicit-casts, because this is an upcast).

However, it has always been a _dynamic_ error for such a condition to be null at run time (in Dart 1 checked mode, and in Dart 2), so there is a bug here: dart2js compiled code prints 'Are you sure?', and that is a bug. As @matanlurey mentioned, the VM raises a dynamic error and prints 'boolean expression must not be null'. So the bug does not exist in the VM.

I'll change the labels of this issue such that it targets dart2js and not the analyzer.

We're working hard on support for non-null types, so it will be a compile-time error as well, just not immediately. Of course, it could be the target of a lint in case we think it's sufficiently common.

[lrhn]

There is alread a open issue for dart2js, so let's keep this issue open as an analyzer feature request to do better.

The spec (here from the if statement condition) says that

It is a compile-time error if the type of the expression b may not be assigned to bool.

but it is actaully guaraneteed to be a runtime error if the type of the condition is not a supertype of bool. The analyzer could give a warning in that case, even if the specification allows it as written.

[eernstg]

I added a reference to this issue in #34147, such that this known dart2js bug and a couple of similar ones aren't forgotten due to the 'area-analyzer' label here.

[srawlins]

Good news, dartanalyzer --enable-experiment=non-nullable reports:

error • Conditions must have a static type of 'bool'. • 36193.dart:2:6 • non_bool_condition