Lint for unintentional assignment in `if`/`do`/`while`

[dnfield]

Similar to #58546

BAD:

bool foo = false;

// ...

if (foo = true) {}
else if (foo = false) {}

while (foo = true) {}  // or false

do {} while (foo = true); // or false

GOOD:

bool foo = false;

// ...

if (foo == true) {}

while (foo == true) {}

do {} while (foo == true);

These lints should apply if the literal true or false is used. I'm not sure if they should apply when a const expression that evaluates to true or false is used. I think they should not apply if some non-const expression is used, e.g. while (keepGoing = checkIfIShouldKeepGoing()) would not trigger the lint.

[dnfield]

May also be worth considering the same kind of assignments in the middle part of for loop clauses.

[pq]

Honestly I'm sort of surprised we don't already have a lint discouraging assignment in conditions.

Some prior art:

• https://docs.rubocop.org/rubocop/cops_lint.html#lintassignmentincondition
• https://eslint.org/docs/rules/no-cond-assign

@munificent: did you ever consider this advice for Effective Dart?

/fyi @natebosch @jakemac53 @goderbauer @lrhn

[jakemac53]

+1 this seems like a high quality lint to me as long as it applies only to the literal true/false assignments. I can't think of a valid use case.

[munificent]

GOOD:

bool foo = false;

// ...

if (foo == true) {}

while (foo == true) {}

do {} while (foo == true);

This looks bad to me too. I think the code should be:

bool foo = false;

// ...

if (foo) {}

while (foo) {}

do {} while (foo);

We have a lint for Boolean literals in conditional expressions but I don't see one for equality expressions, which is a little surprising to me. Maybe that's the real lint we should have here?

I'm also on board with a general lint against = inside condition expressions.

[dnfield]

The actual case that brought this to my attention was using == true because of a nullable type - so it'd be bool? foo = false. But I agree.

[natebosch]

FWIW ?? false is a more idiomatic pattern than == true for handling bool? which would make the GOOD examples

bool? foo = false;

// ...

if (foo ?? false) {}

while (foo ?? false) {}

do {} while (foo ?? false);

[pq]

FWIW the ?? false should be covered by use_if_null_to_convert_nulls_to_bools.

[pq]

We have a lint for Boolean literals in conditional expressions but I don't see one for equality expressions, which is a little surprising to me. Maybe that's the real lint we should have here?

💯

#58533

Thanks for chiming in!

[jamesderlin]

I agree that == true and == false checks are a code smell and should be avoided. However, see https://stackoverflow.com/q/70793299/ for another case where someone accidentally did if (booleanVariable = anotherBooleanVariable), which perhaps is a bit more compelling (although also uncommon).