Skip to content
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

warning on nonsense equality comparisons #4978

Closed
scabug opened this issue Sep 8, 2011 · 2 comments
Closed

warning on nonsense equality comparisons #4978

scabug opened this issue Sep 8, 2011 · 2 comments

Comments

@scabug
Copy link

scabug commented Sep 8, 2011

[Patch attached.]

My colleagues and I are porting a large codebase to Scala, and we're
finding we often use Option for fields or variables that are
"optional". (In the Python original of the codebase, these would be
variables where None is an acceptable value.) This results in a lot of
comparisons like
if (someUid == someOtherUid) ...
where it's easy to forget that, e.g., someUid is of type Option[Int]
while someOtherUid is of type Int. This comparison will always fail,
even in the case where the programmer intended it to succeed. I'm
therefore interested in compiler warnings that would help Scala
programmers avoid this kind of bug.

In nsc/typechecker/RefChecks.scala, we have a number of warnings along
these lines, but none of them quite manage to catch this case. There
is, however, this crude, effective, but disabled warning:

   // Warning on types without a parental relationship.  Uncovers a lot of
   // bugs, but not always right to warn.
   if (false) {
     if (nullCount == 0 && possibleNumericCount < 2 &&
         !(receiver isSubClass actual) && !(actual isSubClass receiver))
       unrelatedTypes()
   }

This warning is valuable enough, despite some false positives, that I
want to use it. I've attached a short patch which turns it on with a flag
-Ywarn-compare-unrelated and adds the flag to -Xlint.

I believe it should also be possible to warn on cases like Option[Int]
vs Int without any false positives, but it will take more work; I'll
open a separate ticket for that.

@scabug
Copy link
Author

scabug commented Sep 8, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4978?orig=1
Reporter: Greg Price (price)
Attachments:

@scabug
Copy link
Author

scabug commented Sep 9, 2011

@paulp said:
Merging with #4979.

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

No branches or pull requests

2 participants