[NNBD] Analyser missing explicit null check for class fields

[esDotDev]

class Foo {
  String? label1;
  dynamic f() {
    String? label2;
    if (label1 != null) {
      return Text(label1); // compile error
    }
    if (label2 != null) {
      return Text(label2); // no compiler error
    }
  }
}

Why does the compiler complain here? Compiler should know that this is a final field? This pattern is used very often in flutter, where we will check a property of the widget to see if it's null.

final String label;

build(){
....
[
   if(label != null) Text(label), // Really don't want to have to use ! here, or put in a fallback that will never be used.
]
...
}

I think I understand this in general, because we can't be sure some subsequent function does not change a field. But, in this case its in the immediately following line. With something like if(value == null) return; how can there possibly be an issue? Additionally many times these fields are final so they actually can not be changed.

To work around this, we need to add local version of each field:

build(){
   String? label = widget.label;
   ...
   [
       if(label != null) Text(label), // works now
   ]
}

[esDotDev]

Another use case, that is also coming from this same limitation:

// I want this:
void _handleSelectAllPressed() async {
    if (controller == null) return;
    controller.selection = TextSelection(baseOffset: 0, extentOffset: controller.text.length);
  }

Does not work, I have to write either:

void _handleSelectAllPressed() async {
    final c = controller;
    if (c == null) return;
    c.selection = TextSelection(baseOffset: 0, extentOffset: c.text.length);
  }

Or

void _handleSelectAllPressed() async {
    controller?.selection = TextSelection(baseOffset: 0, extentOffset: controller?.text?.length ?? 0);
  }

Or

void _handleSelectAllPressed() async {
    if(controller == null) return;
    controller!.selection = TextSelection(baseOffset: 0, extentOffset: controller!.text.length);
  }

The early exit is quite a bit nicer to read and to write, especially if the method body is long and references controller many times.

For example, this will look rather gross full of ? and ??, when all I really want to do is say: if controller is null, skip this method., in fact my class already avoids wiring up this handler when controller is null anyways, so there is no way this method can even be called.

void _handlePastePressed() async {
    int start = controller.selection.start;
    removeTextRange(controller.selection.start, controller.selection.end);
    String? value = (await Clipboard.getData("text/plain"))?.text;
    if (value != null) {
      addTextAtOffset(controller.selection.start, value);
      // Move cursor to end on paste, as one does on desktop :)
      controller.selection = TextSelection.fromPosition(TextPosition(offset: start + value.length));
    }
  }

In an ideal world, none of this would be necessary, as it's a private method, and in my build routine I have something like:

[
  if(controller != null)  SomeButton(onPressed: _handlePastePressed)
]

So compiler should actually know that this method can only be called when controller is non-null, and really leave all my private methods alone.

[esDotDev]

I can make some sort of workaround for this using a closure, but it's not great:

void useControllerIfNotNull(void Function(TextEditingController c) action) {
    final c = controller;
    if (c == null) return;
    action(c);
  }

Then I can do:

void _handlePastePressed() async {
    useControllerIfNotNull((controller){
      int start = controller.selection.start;
      removeTextRange(controller.selection.start, controller.selection.end);
      String? value = (await Clipboard.getData("text/plain"))?.text;
      if (value != null) {
        addTextAtOffset(controller.selection.start, value);
        // Move cursor to end on paste, as one does on desktop :)
        controller.selection = TextSelection.fromPosition(TextPosition(offset: start + value.length));
      }
   });
  }

[srawlins]

Fields are not promotable elements. Subclasses may contain getters which return different objects on each access.

@stereotype441 do we have a link where we can point folks to which describes why the Dart flow analysis does not promote fields?

[stereotype441]

Fields are not promotable elements. Subclasses may contain getters which return different objects on each access.

@stereotype441 do we have a link where we can point folks to which describes why the Dart flow analysis does not promote fields?

I believe either @munificent or @kwalrath was planning to write something up?

[kwalrath]

There's a mention here: https://dart.dev/guides/language/language-tour#late-variables

I don't know whether we've published more detail elsewhere, but if we have, I'd be happy to link to it.

The thing is, though... how would people find this doc? Can the tool point them to the explanation, somehow?

[esDotDev]

Oh, I didn't realize that something like this would actually compile:

class FooA {
  final int? bar = 0;
}

class FooB extends FooA {
  @override
  int get bar => 0;
}

It's really quite odd, I can "override" a nullable final to a non-nullable non-final, and a field to an accessor, as long as the base type remains the same. That's really unfortunate, and I can see how that prevents you from doing virtually anything with class fields, even in seemingly the most obvious cases.

This is a rather large pain point in flutter, all kinds of common use cases are made cludgier:

[
  if(widget.icon != null)  Icon(icon),
   Spacer(),
  if(widget.label != null) Text(label),
]

Using ! creates some gaping holes in the compiler, because now the null check could be removed without any error, which we wouldn't want. So really the only way we can write this soundly is:

final i = icon;
final l = label;
[
  if(i!= null)  Icon(icon),
  if(l!= null) Text(label),
]

The real kicker is this would be used 99% of the time in Flutter with Stateless or Stateful Widget, which are, in practice, virtually never overridden in the way this would require.

I wonder if there is some sort of quality of life annotation that could exist here, so StatefulWidget and StatelessWidget can be exempted from this aggressive checking, allowing us to not fill our layouts with this rather redundant code.

[srawlins]

@leafpetersen also has great links in a comment he made at dart-lang/language#1494 (comment):

Unfortunately, this is working as intended. See here for some general discussion about working with fields, here for some discussion of why even final fields can't be promoted soundly, and here for a list of issues discussing potential ways of making fields easier to work with in null safety.