Skip to content

Commit

Permalink
Improved warning for insensible comparisons.
Browse files Browse the repository at this point in the history
Utilize knowledge of case class synthetic equals to rule out
some comparisons statically.  Closes SI-5426.
  • Loading branch information
paulp committed Jan 31, 2012
1 parent 263aa2e commit 147e9ea
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 5 deletions.
11 changes: 7 additions & 4 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Expand Up @@ -1034,18 +1034,21 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
/** Symbols which limit the warnings we can issue since they may be value types */
val isMaybeValue = Set(AnyClass, AnyRefClass, AnyValClass, ObjectClass, ComparableClass, JavaSerializableClass)

// Whether def equals(other: Any) is overridden
def isUsingDefaultEquals = {
// Whether def equals(other: Any) is overridden or synthetic
def isUsingWarnableEquals = {
val m = receiver.info.member(nme.equals_)
(m == Object_equals) || (m == Any_equals)
(m == Object_equals) || (m == Any_equals) || (m.isSynthetic && m.owner.isCase)
}
// Whether this == or != is one of those defined in Any/AnyRef or an overload from elsewhere.
def isUsingDefaultScalaOp = {
val s = fn.symbol
(s == Object_==) || (s == Object_!=) || (s == Any_==) || (s == Any_!=)
}
// Whether the operands+operator represent a warnable combo (assuming anyrefs)
def isWarnable = isReferenceOp || (isUsingDefaultEquals && isUsingDefaultScalaOp)
// Looking for comparisons performed with ==/!= in combination with either an
// equals method inherited from Object or a case class synthetic equals (for
// which we know the logic.)
def isWarnable = isReferenceOp || (isUsingDefaultScalaOp && isUsingWarnableEquals)
def isEitherNullable = (NullClass.tpe <:< receiver.info) || (NullClass.tpe <:< actual.info)
def isBoolean(s: Symbol) = unboxedValueClass(s) == BooleanClass
def isUnit(s: Symbol) = unboxedValueClass(s) == UnitClass
Expand Down
5 changes: 4 additions & 1 deletion test/files/neg/checksensible.check
Expand Up @@ -28,6 +28,9 @@ checksensible.scala:27: error: comparing values of types Int and Unit using `=='
checksensible.scala:29: error: comparing values of types Int and String using `==' will always yield false
1 == "abc"
^
checksensible.scala:33: error: comparing values of types Some[Int] and Int using `==' will always yield false
Some(1) == 1 // as above
^
checksensible.scala:38: error: comparing a fresh object using `==' will always yield false
new AnyRef == 1
^
Expand Down Expand Up @@ -94,4 +97,4 @@ checksensible.scala:84: error: comparing values of types EqEqRefTest.this.C3 and
checksensible.scala:95: error: comparing values of types Unit and Int using `!=' will always yield true
while ((c = in.read) != -1)
^
32 errors found
33 errors found
13 changes: 13 additions & 0 deletions test/files/neg/t5426.check
@@ -0,0 +1,13 @@
t5426.scala:2: error: comparing values of types Some[Int] and Int using `==' will always yield false
def f1 = Some(5) == 5
^
t5426.scala:3: error: comparing values of types Int and Some[Int] using `==' will always yield false
def f2 = 5 == Some(5)
^
t5426.scala:8: error: comparing values of types Int and Some[Int] using `==' will always yield false
(x1 == x2)
^
t5426.scala:9: error: comparing values of types Some[Int] and Int using `==' will always yield false
(x2 == x1)
^
four errors found
1 change: 1 addition & 0 deletions test/files/neg/t5426.flags
@@ -0,0 +1 @@
-Xfatal-warnings
10 changes: 10 additions & 0 deletions test/files/neg/t5426.scala
@@ -0,0 +1,10 @@
class A {
def f1 = Some(5) == 5
def f2 = 5 == Some(5)

val x1 = 5
val x2 = Some(5)

(x1 == x2)
(x2 == x1)
}

0 comments on commit 147e9ea

Please sign in to comment.