Linter: Add a lint rule for using `Null` as a `bool`

[natebosch]

if (someVar = null) {
   print('Not what I wanted!');
}

We could consider also linting if if dynamic is used as a bool.

[rrousselGit]

This should also works with ternary operations.

(someVar = null) ? foo : bar;

[srawlins]

Since this fails at runtime, shouldn't this be an analyzer static analysis warning/error?

void main() {
  dynamic someVar = false;
  if (someVar = null) {
    print('not what i wanted');
  }
}

$ dart a.dart 
Unhandled exception:
Failed assertion: boolean expression must not be null
#0      main (file:///Users/srawlins/code/dart-linter/a.dart)
#1      _startIsolate.<anonymous closure> (dart:isolate/runtime/libisolate_patch.dart:279:19)
#2      _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:165:12)

[natebosch]

I'd also be in favor of this being an analyzer warning instead of a lint if we think we can get the false positives to zero.

[bwilkerson]

Should we have a more general check that hints/lints when code that is subject to boolean conversion (such as conditions in control structures) has a non-bool type?

[natebosch]

AFAICT dynamic and Null are the two cases that we allow through to conditionals without warning. Other non-bool types get a warning.

In what way do we think we can make this check more general?

[bwilkerson]

If those cover all the cases, then there isn't any reason to be more general.

[rrousselGit]

An alternative would be to have a lint rule against assignments inside condition blocks such as if/ternary/while/for
Something to prevent if (bool = null) but also if ((var = new Var()).isOk)

As this is a fairly bad practice to assign variables inside condition blocks

[pq]

@rrousselGit: I really like this proposal 👍

[natebosch]

If we can make it a Warning we wouldn't want to prevent any assignment, only look for the cases we know would fail at runtime.

If we want to limit it to a lint we could consider preventing any assignment.

[JHBitencourt]

Regarding the recent changes in the language behavior, couldn't this issue be closed?
The analyzer now produces a non_bool_condition error.

[natebosch]

yeah I think this has been made obsolete by null safety.

There is likely still a risk around dynamic, but I think that might also be covered by implicit-casts.