dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
627 stars 172 forks source link

proposal: `avoidable_null_assertion ` #4804

Open scheglov opened 7 months ago

scheglov commented 7 months ago

avoidable_null_assertion

Replace above with your proposed rule name. (See notes on naming in Writing Lints.)

Description

AVOID null assertions when they can be avoided.

You can instead use null check patters, or extract a local variable.

Details

Give a detailed description that should provide context and motivation. Ideally this could be used directly in the rule's documentation.

Kind

This guards against unsafe null assertions. Potentially faster performance, because null assertion costs something.

Bad Examples

class A {
  int? foo;

  void f() {
    if (foo != null) {
      foo!.isEven;
    }
  }
}

Good Examples

class A {
  int? foo;

  void f() {
    if (foo case final foo?) {
      foo.isEven;
    }
  }
}
class A {
  int? foo;

  void f() {
    final foo = this.foo;
    if (foo != null) {
      foo.isEven;
    }
  }
}

Discussion

We have quite a few instances of such code in the analyzer, that would be reported by this lint, and hopefully automatically fixed.

Discussion checklist

Not yet ready CL.

bwilkerson commented 7 months ago

It would be nice to be a little more explicit by describing when they can be avoided, or at least the conditions under which this lint will fire. Is the example you gave the only code pattern that should be caught, or are there others?

Maybe this is less about using the non-null-assert operator and more about the redundant test for null (using both != null and !?

bwilkerson commented 7 months ago

Either way, do we want it to catch more complex cases, such as

void f(int? x, int? y) {
  if (x != null && y != null) {
    x! + y!;
  }
}

Is this something that would benefit from flow analysis? (@stereotype441)

scheglov commented 7 months ago

Yes, I thought that there might be more cases when this lint could be reported. But this specific example that I provided is often used in our code base, as the failures here show.

The example that you provided, with formal parameters, actually is not what we want to cover by this lint. We specifically want it for places where we don't get type inference, because the checked value is returned by a getter, and we implicitly expect that the send read will return non-nullable value. So, this would be:

class A {
  int? foo;
  int? bar;

  void f() {
    if (foo != null && bar != null) {
      foo! + bar!;
    }
  }
}