[linter] New lint to suggest avoid bool == true and bool == false

[FMorschel]

We have use_if_null_to_convert_nulls_to_bools to trigger the first case, but no diagnostic in the other two cases:

void foo(bool? nullable, bool boolean) {
  if (nullable == true) {}
  if (boolean == true) {}
  if (boolean == false) {}
}

CC @srawlins @bwilkerson

[bwilkerson]

@devoncarew @lrhn @natebosch Is this a lint that we'd be likely to add to the recommended set?

[lrhn]

Yes. It's a classic unnecessary code lint.

I don't remember if we have something similar for && and || already, but the following at all unnecessarily overlong:

• bool == true, true == bool => bool
• bool == false, false == bool => !bool
• bool1 ? true : bool2 => bool1 || bool2
• bool1 ? false : bool2 => !bool1 && bool2
• bool1 ? bool2 : true => !bool1 || bool2
• bool1 ? bool2 : false => bool1 && bool2
• bool1 || true => true
• bool1 || false => bool1
• bool1 && true => bool1
• bool1 && false => false

[FMorschel]

To note, I found this when working on https://dart-review.googlesource.com/c/sdk/+/424560 at pkg/analysis_server/lib/src/services/correction/dart/create_getter.dart.

sdk/pkg/analysis_server/lib/src/services/correction/dart/create_getter.dart

Line 197 in b573667

if (targetElement.library2.isInSdk == true) {

[FMorschel]

• bool1 || true => true
• bool1 || false => bool1
• bool1 && true => bool1
• bool1 && false => false

This reminds me that (I'm not sure we should, but) we can test boolean operations with the same expression on both sides (immutable expressions, of course - private final fields, local variables, etc):

• bool1 == bool1 => true
• bool1 != bool1 => false
• bool1 || bool1 => bool1
• bool1 && bool1 => bool1
• bool1 ^ bool1 => false

Also:

• bool1 ? bool1 : bool2 => bool1 || bool2
• bool1 ? bool2 : bool1 => bool1 && bool2

This one is more complex but it can help if we are willing to implement (found at flutter/flutter#156928 (comment))

• bool1 ? (bool2 ? exp1 : exp2) : (bool2 ? exp2 : exp1) => bool1 == bool2 ? exp1 : exp2

[bwilkerson]

It's a classic unnecessary code lint.

Most of the unnecessary code diagnostics are warnings so that they don't need to be enabled by users. Sounds like we should consider making these warnings too.

... we can test boolean operations with the same expression on both sides (immutable expressions, of course - private final fields, local variables, etc)

At one time we had a lint that attempted to identify conditions whose value was always the same. It ended up being a nightmare, so it's going to take a lot of work to convince me that we want to go down that path again.

This one is more complex ...

More complex, but not necessarily harder to read. I don't think I'd want to flag this kind of case, but I could imagine adding an assist to 'Simplify expression' for some cases like this. I'm not convinced that it has enough value to warrant the cost of implementation and maintenance, though, so I'm not proposing that we add it.

[lrhn]

The bool1== bool1 can be generalized to most types that do not contain a double. (You should make == reflexive, but NaN breaks that. Bloody NaN.)

Everything that applies to || and&& should also apply to | and &. They're the same operation, just not short-circuiting.