Skip to content

non-null use after null check should not generate an error #1029

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

Closed
bsutton opened this issue Jun 14, 2020 · 4 comments
Closed

non-null use after null check should not generate an error #1029

bsutton opened this issue Jun 14, 2020 · 4 comments
Labels
bug There is a mistake in the language specification or in an active document

Comments

@bsutton
Copy link

bsutton commented Jun 14, 2020

I have the following code:

https://nullsafety.dartpad.dev/cf6d285f6b0dc34e2db23e9a40ffbd01

The code does a conditional check that then nullable field is non-null.
I would expect the call to exists() to work as the I've just checked that the value is non null.

class A
{
  String? get startScriptPath => null;

  bool addToPath(String path) {
    if (startScriptPath != null) {
        if (exists(startScriptPath)) {
          print('hi');
        }
    } else {
      throw UnsupportedError(
          "The shell doesn't support a start script so we can't configure the path");
    }
    return true;
  }
  
  bool exists(String path) => true;
}
@bsutton bsutton added the bug There is a mistake in the language specification or in an active document label Jun 14, 2020
@srawlins
Copy link
Member

Unfortunately type promotion doesn't work on fields. #104

The field could return a non-null value the first time, and null the second.

@bsutton
Copy link
Author

bsutton commented Jun 14, 2020

So type promotion is done at complie time?

@srawlins
Copy link
Member

That's correct. I've modified your example:

class A {
  static int calls = 0;

  String? get startScriptPath => (calls++).isEven ? 'hey' : null;

  void addToPath(String path) {
    if (startScriptPath != null) {
      if (exists(startScriptPath!)) {
        print('hi');
      }
    }
  }

  bool exists(String path) => true;
}

main() => A().addToPath('');

to show that startScriptPath can return a non-nullable, then null, producing a null-check error:

$ ~/code/dart-sdk/sdk/xcodebuild/ReleaseX64/dart-sdk/bin/dart --enable-experiment=non-nullable 1029.dart 
Unhandled exception:
Null check operator used on a null value
#0      A.addToPath (file:///Users/srawlins/code/dart-csslib/1029.dart:8:33)
#1      main (file:///Users/srawlins/code/dart-csslib/1029.dart:17:15)
#2      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:301:19)
#3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

@leafpetersen
Copy link
Member

@srawlins explanation is correct. This is one of the sharper corners of the null safety release, and I'm hoping that we can provide better patterns for working with fields in the future (possibly using something inspired by swift style let guards?). On balance though, we felt that having implicit runtime checks when fields were promoted wasn't serving our users well - implicit runtime errors are generally an unpleasant surprise when they fail.

Binding the field to a local variable will allow type promotion to kick in, or you can simply use an explicit null assertion operator instead (which is what the compiler would have to do if we allowed promotion here).

class A
{
  String? get startScriptPath => null;

  bool addToPath(String path) {
    var path = startScriptPath; // Bind to a local so promotion works
    if (path != null) { 
        if (exists(path)) { // or use startScriptPath!
          print('hi');
        }
    } else {
      throw UnsupportedError(
          "The shell doesn't support a start script so we can't configure the path");
    }
    return true;
  }
  
  bool exists(String path) => true;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug There is a mistake in the language specification or in an active document
Projects
None yet
Development

No branches or pull requests

3 participants