New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report suspicious use of '==' #35617

Open
rakudrama opened this Issue Jan 9, 2019 · 2 comments

Comments

Projects
None yet
3 participants
@rakudrama
Copy link
Member

rakudrama commented Jan 9, 2019

This program has no warning, hint or lint:

foo(int id, String name) {
  print(name == id);
  print(id == name);
}

main() {
  foo(1, 'hello');
  foo(null, null);
}

It is suspicious because the static type of the receiver and argument are disjoint. The only way name == id can be true is if both are null. If that is the intention it is clearer to write name == null && id == null. It is more likely to be a bug.

I recently spent an hour debugging a bug of this form, where I had value == 0 but value was a BigInt, so the test was always false.
There are some cases where this is not an error, for example package fixnum's Int64.== that converts int values, so value == 0 is fine when value can be an Int64 (but 0 == value is still silently wrong).

Given the presence of definitions of == that do conversions, a blanket warning would not be appropriate. But we want to detect as many errors as possible, so a lint is too weak. I suggest:

  • A warning for corelib types where we know the behaviour of ==. This would have caught my BigInt case, and is worthwhile even if the rest of this idea is too hard.

  • An annotation for describing the accepted types that could be used for Int64.== to warn on excluded types. An accepted type of Object disable the diagnostic.

  @pragma('analyzer:accepts', int) @pragma('analyzer:accepts', fixnum.Int32)
  operator == (Object other) ...

We can usually tell from the body of the == method when == tests within-domain. Bodies that do not contain code like other is Foo && ... could have a hint/lint to annotate with the above annotation.

As a bonus, it would be useful to extend this as far as possible into parametric types, e.g.

   <T extends num>(T x, String y) => x == y

should warn.

@matanlurey

This comment has been minimized.

Copy link
Contributor

matanlurey commented Jan 9, 2019

This is, at least partially covered, by unrelated_type_equality_checks in the Linter:
https://dart-lang.github.io/linter/lints/unrelated_type_equality_checks.html.

That being said, I'd be very supportive of the language or tools tightening the definition of ==.

@stereotype441

This comment has been minimized.

Copy link
Member

stereotype441 commented Jan 14, 2019

@pq FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment