Skip to content

Commit 147e9ea

Browse files
committed
Improved warning for insensible comparisons.
Utilize knowledge of case class synthetic equals to rule out some comparisons statically. Closes SI-5426.
1 parent 263aa2e commit 147e9ea

File tree

5 files changed

+35
-5
lines changed

5 files changed

+35
-5
lines changed

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,18 +1034,21 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
10341034
/** Symbols which limit the warnings we can issue since they may be value types */
10351035
val isMaybeValue = Set(AnyClass, AnyRefClass, AnyValClass, ObjectClass, ComparableClass, JavaSerializableClass)
10361036

1037-
// Whether def equals(other: Any) is overridden
1038-
def isUsingDefaultEquals = {
1037+
// Whether def equals(other: Any) is overridden or synthetic
1038+
def isUsingWarnableEquals = {
10391039
val m = receiver.info.member(nme.equals_)
1040-
(m == Object_equals) || (m == Any_equals)
1040+
(m == Object_equals) || (m == Any_equals) || (m.isSynthetic && m.owner.isCase)
10411041
}
10421042
// Whether this == or != is one of those defined in Any/AnyRef or an overload from elsewhere.
10431043
def isUsingDefaultScalaOp = {
10441044
val s = fn.symbol
10451045
(s == Object_==) || (s == Object_!=) || (s == Any_==) || (s == Any_!=)
10461046
}
10471047
// Whether the operands+operator represent a warnable combo (assuming anyrefs)
1048-
def isWarnable = isReferenceOp || (isUsingDefaultEquals && isUsingDefaultScalaOp)
1048+
// Looking for comparisons performed with ==/!= in combination with either an
1049+
// equals method inherited from Object or a case class synthetic equals (for
1050+
// which we know the logic.)
1051+
def isWarnable = isReferenceOp || (isUsingDefaultScalaOp && isUsingWarnableEquals)
10491052
def isEitherNullable = (NullClass.tpe <:< receiver.info) || (NullClass.tpe <:< actual.info)
10501053
def isBoolean(s: Symbol) = unboxedValueClass(s) == BooleanClass
10511054
def isUnit(s: Symbol) = unboxedValueClass(s) == UnitClass

test/files/neg/checksensible.check

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ checksensible.scala:27: error: comparing values of types Int and Unit using `=='
2828
checksensible.scala:29: error: comparing values of types Int and String using `==' will always yield false
2929
1 == "abc"
3030
^
31+
checksensible.scala:33: error: comparing values of types Some[Int] and Int using `==' will always yield false
32+
Some(1) == 1 // as above
33+
^
3134
checksensible.scala:38: error: comparing a fresh object using `==' will always yield false
3235
new AnyRef == 1
3336
^
@@ -94,4 +97,4 @@ checksensible.scala:84: error: comparing values of types EqEqRefTest.this.C3 and
9497
checksensible.scala:95: error: comparing values of types Unit and Int using `!=' will always yield true
9598
while ((c = in.read) != -1)
9699
^
97-
32 errors found
100+
33 errors found

test/files/neg/t5426.check

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
t5426.scala:2: error: comparing values of types Some[Int] and Int using `==' will always yield false
2+
def f1 = Some(5) == 5
3+
^
4+
t5426.scala:3: error: comparing values of types Int and Some[Int] using `==' will always yield false
5+
def f2 = 5 == Some(5)
6+
^
7+
t5426.scala:8: error: comparing values of types Int and Some[Int] using `==' will always yield false
8+
(x1 == x2)
9+
^
10+
t5426.scala:9: error: comparing values of types Some[Int] and Int using `==' will always yield false
11+
(x2 == x1)
12+
^
13+
four errors found

test/files/neg/t5426.flags

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-Xfatal-warnings

test/files/neg/t5426.scala

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
class A {
2+
def f1 = Some(5) == 5
3+
def f2 = 5 == Some(5)
4+
5+
val x1 = 5
6+
val x2 = Some(5)
7+
8+
(x1 == x2)
9+
(x2 == x1)
10+
}

0 commit comments

Comments
 (0)