`use_if_null_to_convert_nulls_to_bools` points to outdated guidance

[MaryaBelanger]

This lint refers to a section in Effect Dart that is changing quite a bit (based on this issue), to the point where the lint using it as a reference doesn't really make sense.

To summarize, the guidance will no longer be "PREFER using ?? to convert null to a boolean value", but just "DON'T use true or false in equality operations".

• if (nullableBool ?? false) isn't necessarily "good" (like the lint promotes) because "Using a null-aware operator on a variable inside a condition doesn’t promote the variable to a non-nullable type".
• Is "converting" null to boolean really even a use-case? The new guidance is more precise and applicable

The lint should change its name, description, or just not point to Effective Dart anymore.

[bwilkerson]

I'm confused. The advice appears to be to use ?? when a bool? variable is used as a condition, which is what I think this lint is encouraging. So I think the reference back to Effective Dart is still valid.

But if the name of the advice has changed then we might want to rename the lint to make it easier for users to find.

@pq

[MaryaBelanger]

@bwilkerson My mistake, I didn't explain that well. Yes, the name of the advice is changing and renaming the lint would be ideal.

use_if_null is no longer the main point, because while it's still "good", the better option is an explicit != null check

And to_convert_nulls_to_bools is no longer the main point. It's now just the more general idea that using boolean literals in equality expressions is never a good idea (null check or not). It encompasses the old lint but includes more cases.

Here's the staged new section so it's easier to get the full picture.

I don't know what the lint name should change to, though.

[pq]

I'm confused.

Looking at dart-lang/site-www#4388, it seems like @munificent is emphasizing nuance. The current lint is pretty blunt and I agree with @MaryaBelanger not really reflective of the nuance in the new advice. Look down the bug and Bob notes:

Now the ?? operator makes it clear that something to do with null is happening. But it's equally valid (and often better because of type promotion) to do:

if (foo != null && foo) { ... }
This guideline shouldn't be that ?? is good, it's more that == true etc. are bad.

But the lint advises:

Use if-null operators to convert nulls to bools.

😬

To summarize, the guidance will no longer be "PREFER using ?? to convert null to a boolean value", but just "DON'T use true or false in equality operations".

I think this should likely be a new/different lint.

We do have avoid_bool_literals_in_conditional_expressions but that's cases like condition ? true : boolExpression. We might expand that or consider a separate no_bool_literals_in_conditions to handle stuff like:

if (foo == true) { ... }
if (foo != true) { ... }
if (foo == false) { ... }
if (foo != false) { ... }

while (foo == true) { ... }
while (foo != true) { ... }
while (foo == false) { ... }
while (foo != false) { ... }

And I guess maybe

for ( ; foo != false; )

?

[pq]

I don't know what the lint name should change to, though.

Alternatively, we might consider deprecating the lint.

[pq]

PS: thanks for filing this @MaryaBelanger!

[bwilkerson]

... or consider a separate no_bool_literals_in_conditions ...

Or no_literal_bool_comparisons (not sure whether the _in_conditions part is relevant, other than that's where promotion happens).

Follow-up question, maybe for @munificent: If the expression being compared to a literal value isn't a local variable, or if it is a local variable but the variable isn't referenced inside the block, would we still prefer an equality test over ???

[munificent]

Or no_literal_bool_comparisons

This is what I was thinking. If you have true or false in a != or == expression, then you're doing something wrong. If the other operand is bool, then it's pointless but harmless. If the other operand is bool? then it looks pointless and harmless but isn't and a naïve refactoring to not use the bool literal will change its behavior.

Follow-up question, maybe for @munificent: If the expression being compared to a literal value isn't a local variable, or if it is a local variable but the variable isn't referenced inside the block, would we still prefer an equality test over ???

If you don't care about getting type promotion, then I think it's fine to use ?? if you want, but that's just my personal opinion.

[lrhn]

and a naïve refactoring to not use the bool literal will change its behavior.

It will immediately fail to compile if you just remove == true or change b == false to !b, if the value is actually nullable.
With null safety, it's no longer something that you can accidentally change, and not notice that it no longer works correctly.

I'm not that worried about comparisons to literals any more, mainly because of that.
If the value is actually nullable, I'm becoming fine with using any of ?? literal, == literal and != literal.
If it's not nullable, it's fair to yell about unnecessarily complicated code, and that would also enforce that the == literal is only used when the value is nullable.

Anything which involves ?? and interpreting the value of the first operand is hard to read. Adding boolean negation makes it worse.

For a nullable value b, the closest alternative to b != true using ?? is !(b ?? false), which I just can't read at any reasonable speed. The (b == null || !b) isn't particularly readable either,
I think, scarily enough, that (b == null || b == false) is actually more readable.
It's a tri-state variable, with values true, false and null, and giving special rules for two of the values, just because they are booleans, doesn't feel warranted.
I wouldn't argue against enum Choice { accept, reject; } ... Choice? maybeChoice .... if (maybeChoice != Choice.accept) ....

(I wrote code today using != false and != true on a nullable boolean, which I found easier to read than using ??.)

Ob-hack:

typedef Choice = bool?;
const Choice yes = true;
const Choice no = false;
const Choice maybe = null;
// ..
  if (choice != yes) print("😝");

[MaryaBelanger]

If anyone thinks the changes to Effective Dart should be reversed or revised, even just temporarily while this conversation happens, please let me know (can't quite assess the nuance here myself, at least without spending a lot of time sinking back into the topic :))

For reference: PR here, the new section is on the site right now

[pq]

More and more I'm feeling like this lint should be deprecated. If we want to nudge people towards judicious use of ?? we can start fresh with a new lint.

Thoughts?

@lrhn @bwilkerson @munificent ?

[munificent]

If anyone thinks the changes to Effective Dart should be reversed or revised, even just temporarily while this conversation happens, please let me know (can't quite assess the nuance here myself, at least without spending a lot of time sinking back into the topic :))

For what it's worth, I'm entirely happy with what you ended up with on Effective Dart.

More and more I'm feeling like this lint should be deprecated.

SGTM.