dart-lang / linter

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

Add a lint rule for checking obvious assert mistakes #3583

Open rrousselGit opened 2 years ago

rrousselGit commented 2 years ago

Describe the rule you'd like to see implemented

It is fairly common for classes/functions to include asserts such as:

void fn({int? a, int? b}) {
  assert((a == null) == (b == null));
}

Which allows:

fn();
fn(a: 42, b: 21);

but rejects:

fn(a: 42);
fn(b: 21);

The downside with doing this is, the user experience isn't perfect because it's a runtime error. I wonder if a lint rule could be implemented to showcase assertion failures statically.

Of course, we can't support every single assert failure. But simple use-cases probably could be detected

Examples

I think focusing on combinations of x == null / x != null expressions could be enough as an MVP

Using the previous function:

void fn({int? a, int? b}) {
  assert((a == null) == (b == null));
}

we could statically decompose the (a == null) == (b == null) expression to determine that the nullability of a should be the same as the nullability of b

From there, we should be able to implement:

// all of the following lines produce a lint
fn(a: 42);
fn(b: 42);

// the followings would pass instead
fn()
fn(a: 42, b: 42)

int? a, b;
fn(a: a, b: b); // we can't statically know if this is valid or not, so we don't lint anything

Other cases that probably could be supported would be <> operators

void fn(int a) {
  assert(a >= 0);
}

fn(0); // ok
fn(42); // ok
fn(-1); // KO

int a;
fn(a); // we don't know, so ignored
bwilkerson commented 2 years ago

I'm not sure that a lint rule is the appropriate way to support this, but it's definitely an interesting feature request.

I think what you're suggesting essentially amounts to

This would be prohibitively expensive as a lint because it would require re-analyzing the defining library for every function invocation in every analyzed file (in order to find the asserts that needed to be evaluated).

To make it efficient we'd probably need to store a representation of the asserts we could attempt to evaluate in the element model, just like we do for const constructors. At that point I'd suggest making is an analyzer/language setting.

The other option would be to make it part of an off-line whole-world analysis. @srawlins

I suspect that neither of these are things we'd be able to staff any time soon, but it's definitely an interesting idea.