[warning] proposal: do_not_check_equality_to_self

[filiph]

do_not_check_equality_to_self

Description

Checking equality to self will always return true and is likely a typo.

Details

Although good code tries to avoid it, sometimes two different variables or fields have similar names and types. It is then possible to mistakenly use one symbol where the programmer meant the other. This lint tries to prevent one such error, where a condition is in the form a == a or a != a.

Kind

This guards against hard-to-spot errors.

Bad Examples

class A {
  int _current = 0;

  void update(int current) {
    if (current != current) {
      // Do some work...
    }
  }
}

Good Examples

class A {
  int _current = 0;

  void update(int current) {
    if (current != _current) {
      // Do some work...
    }
  }
}

Discussion

Just found this exact bug in my code, and only after seeing it happen in the step debugger. This wasn't flagged in any way. I'm using package:flutter_lints/flutter.yaml.

I feel this is the kind of bug that linters were invented for. Stupid, hard to see as a human, easily detectable by the machine, and potentially infuriating ("why the hell doesn't this code run?").

I realize equality might be overridden for user types but even then, I think checking for a == a is probably a typo.

Discussion checklist

• List any existing rules this proposal modifies, complements, overlaps or conflicts with.
• List any relevant issues (reported here, the SDK Tracker, or elsewhere).
• If there's any prior art (e.g., in other linters), please add references here.
• If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn't any corresponding advice, should there be?)
• If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

[lrhn]

Doesn't sound like something that needs to be a lint, it can just be a general warning about unnecessary code that is always enabled. It's an unnecessarily long way to write true.
(Unless the type may contain double values, then it might be deliberate and necessary. Thanks IEEE-754!)

[filiph]

Yeah, I'm afraid there are relatively valid reasons for trying to check a == a, even outside double values. E.g. someone might want to have a == a in an assert if they've overriden ==.

Therefore, a lint that can be disabled would, in my opinion, be better than a dead code warning.

[lrhn]

A warning that can be // ignore: compare_to_self'ed for individual tests sounds better (to me) than a lint.

It's worth considering who should want to not enable a lint.
If the answer is "no-one", then it probably shouldn't be a lint to begin with.
The only real difference between a lint and a warning is whether it's enabled by default.

[filiph]

Oh! Somehow I thought warnings can't be ignored like that. Brainfart, sorry.

What's the best way to get the warning to be considered? Should I create an issue over at dart-lang/sdk?

[lrhn]

I think it would be an analyzer enhancement request, so yes.

[pq]

@filiph: I went ahead and moved this to the SDK so consider your request transferred!