diff options
| -rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 88 | ||||
| -rw-r--r-- | test/files/neg/checksensible.check | 125 | ||||
| -rw-r--r-- | test/files/neg/checksensible.flags | 1 | ||||
| -rw-r--r-- | test/files/neg/checksensible.scala | 96 |
4 files changed, 208 insertions, 102 deletions
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 03da1a4f09..c0510dd33d 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -777,51 +777,59 @@ abstract class RefChecks extends InfoTransform { } def checkSensible(pos: Position, fn: Tree, args: List[Tree]) = fn match { - case Select(qual, name) if (args.length == 1) => + case Select(qual, name @ (nme.EQ | nme.NE | nme.eq | nme.ne)) if args.length == 1 => + def isReferenceOp = name == nme.eq || name == nme.ne def isNew(tree: Tree) = tree match { case Function(_, _) | Apply(Select(New(_), nme.CONSTRUCTOR), _) => true case _ => false } - name match { - case nme.EQ | nme.NE | nme.LT | nme.GT | nme.LE | nme.GE => - def underlyingClass(tp: Type): Symbol = { - var sym = tp.widen.typeSymbol - while (sym.isAbstractType) - sym = sym.info.bounds.hi.widen.typeSymbol - sym - } - val formal = underlyingClass(fn.tpe.params.head.tpe) - val actual = underlyingClass(args.head.tpe) - val receiver = underlyingClass(qual.tpe) - def nonSensibleWarning(what: String, alwaysEqual: Boolean) = - unit.warning(pos, "comparing "+what+" using `"+name.decode+"' will always yield "+ - (alwaysEqual == (name == nme.EQ || name == nme.LE || name == nme.GE))) - def nonSensible(pre: String, alwaysEqual: Boolean) = - nonSensibleWarning(pre+"values of types "+normalizeAll(qual.tpe.widen)+" and "+normalizeAll(args.head.tpe.widen), - alwaysEqual) // @MAT normalize for consistency in error message, otherwise part is normalized due to use of `typeSymbol', but the rest isn't - def hasObjectEquals = receiver.info.member(nme.equals_) == Object_equals - // see if it has any == methods beyond Any's and AnyRef's: ticket #2240 - def hasOverloadedEqEq = receiver.info.member(nme.EQ).alternatives.length > 2 - - if (formal == UnitClass && actual == UnitClass) - nonSensible("", true) - else if ((receiver == BooleanClass || receiver == UnitClass) && - !(receiver isSubClass actual)) - nonSensible("", false) - else if (isNumericValueClass(receiver) && - !isNumericValueClass(actual) && - !(forMSIL || (actual isSubClass BoxedNumberClass)) && - !(receiver isSubClass actual)) - nonSensible("", false) - else if ((receiver hasFlag FINAL) && hasObjectEquals && !isValueClass(receiver) && - !(receiver isSubClass actual) && receiver != NullClass && actual != NullClass && - (name == nme.EQ || name == nme.LE)) - nonSensible("non-null ", false) - else if ((isNew(qual) || isNew(args.head)) && hasObjectEquals && - (name == nme.EQ || name == nme.NE) && !hasOverloadedEqEq) - nonSensibleWarning("a fresh object", false) - case _ => + def underlyingClass(tp: Type): Symbol = { + var sym = tp.widen.typeSymbol + while (sym.isAbstractType) + sym = sym.info.bounds.hi.widen.typeSymbol + sym + } + val actual = underlyingClass(args.head.tpe) + val receiver = underlyingClass(qual.tpe) + + // Whether def equals(other: Any) is overridden + def isUsingDefaultEquals = { + val m = receiver.info.member(nme.equals_) + (m == Object_equals) || (m == Any_equals) + } + // Whether this == or != is actually an overloaded version + def isUsingDefaultScalaOp = { + val s = fn.symbol + (s == Object_==) || (s == Object_!=) || (s == Any_==) || (s == Any_!=) || (s == Object_eq) || (s == Object_ne) + } + // Whether the operands+operator represent a warnable combo (assuming anyrefs) + def isWarnable = isReferenceOp || (isUsingDefaultEquals && isUsingDefaultScalaOp) + def isValueOrBoxed(s: Symbol) = isNumericValueClass(s) || (s isSubClass BoxedNumberClass) + def isEitherNull = (receiver == NullClass) || (actual == NullClass) + def isEitherNullable = (NullClass.tpe <:< receiver.info) || (NullClass.tpe <:< actual.info) + + def nonSensibleWarning(what: String, alwaysEqual: Boolean) = { + val msg = alwaysEqual == (name == nme.EQ || name == nme.eq) + unit.warning(pos, "comparing "+what+" using `"+name.decode+"' will always yield " + msg) + } + + // @MAT normalize for consistency in error message, otherwise only part is normalized due to use of `typeSymbol' + def nonSensible(pre: String, alwaysEqual: Boolean) = + nonSensibleWarning(pre+"values of types "+normalizeAll(qual.tpe.widen)+" and "+normalizeAll(args.head.tpe.widen), + alwaysEqual) + + if (receiver == BooleanClass && !(receiver isSubClass actual)) // true == 5 + nonSensible("", false) + else if (receiver == UnitClass && actual == UnitClass) // () == () + nonSensible("", true) + else if (isNumericValueClass(receiver) && !isValueOrBoxed(actual) && !forMSIL) // 5 == "abc" + nonSensible("", false) + else if (isWarnable) { + if (receiver.isFinal && !isEitherNull && !(receiver isSubClass actual)) // object X, Y; X == Y + nonSensible((if (isEitherNullable) "non-null " else ""), false) + else if (isNew(qual) || (isNew(args.head) && (receiver.isFinal || isReferenceOp))) // new X == y or object X ; X == new Y + nonSensibleWarning("a fresh object", false) } case _ => } diff --git a/test/files/neg/checksensible.check b/test/files/neg/checksensible.check index 9d0487f9a2..b36b11bf98 100644 --- a/test/files/neg/checksensible.check +++ b/test/files/neg/checksensible.check @@ -1,44 +1,91 @@ -checksensible.scala:1: error: class Test needs to be abstract, since method isabstract is not defined -class Test { +checksensible.scala:13: error: comparing a fresh object using `eq' will always yield false + (new AnyRef) eq (new AnyRef) + ^ +checksensible.scala:14: error: comparing a fresh object using `ne' will always yield true + (new AnyRef) ne (new AnyRef) + ^ +checksensible.scala:15: error: comparing a fresh object using `eq' will always yield false + Shmoopie eq (new AnyRef) + ^ +checksensible.scala:16: error: comparing a fresh object using `eq' will always yield false + (Shmoopie: AnyRef) eq (new AnyRef) + ^ +checksensible.scala:17: error: comparing a fresh object using `eq' will always yield false + (new AnyRef) eq Shmoopie + ^ +checksensible.scala:18: error: comparing a fresh object using `eq' will always yield false + (new AnyRef) eq null + ^ +checksensible.scala:19: error: comparing a fresh object using `eq' will always yield false + null eq new AnyRef + ^ +checksensible.scala:26: error: comparing values of types Unit and Int using `==' will always yield false + (c = 1) == 0 + ^ +checksensible.scala:27: error: comparing values of types Int and Unit using `==' will always yield false + 0 == (c = 1) + ^ +checksensible.scala:29: error: comparing values of types Int and java.lang.String using `==' will always yield false + 1 == "abc" + ^ +checksensible.scala:32: error: comparing a fresh object using `==' will always yield false + new AnyRef == 1 + ^ +checksensible.scala:33: error: comparing values of types Int and java.lang.Object using `==' will always yield false + 1 == new AnyRef + ^ +checksensible.scala:35: error: comparing values of types Int and Boolean using `!=' will always yield true + 1 != true + ^ +checksensible.scala:36: error: comparing values of types Unit and Boolean using `==' will always yield false + () == true + ^ +checksensible.scala:37: error: comparing values of types Unit and Unit using `==' will always yield true + () == () + ^ +checksensible.scala:38: error: comparing values of types Unit and Unit using `==' will always yield true + () == println + ^ +checksensible.scala:40: error: comparing values of types Int and Unit using `!=' will always yield true + (1 != println) + ^ +checksensible.scala:41: error: comparing values of types Int and Symbol using `!=' will always yield true + (1 != 'sym) + ^ +checksensible.scala:47: error: comparing a fresh object using `==' will always yield false + ((x: Int) => x + 1) == null + ^ +checksensible.scala:48: error: comparing values of types object Bep and (Int) => Int using `==' will always yield false + Bep == ((_: Int) + 1) ^ -checksensible.scala:3: warning: comparing values of types Unit and Int using `==' will always yield false - println((c = 1) == 0) +checksensible.scala:50: error: comparing a fresh object using `==' will always yield false + new Object == new Object + ^ +checksensible.scala:51: error: comparing a fresh object using `==' will always yield false + new Object == "abc" + ^ +checksensible.scala:52: error: comparing a fresh object using `!=' will always yield true + new Exception() != new Exception() ^ -checksensible.scala:5: warning: comparing values of types Int and java.lang.String using `==' will always yield false - println(1 == "abc") - ^ -checksensible.scala:6: warning: comparing values of types Int and Boolean using `!=' will always yield true - println(1 != true) - ^ -checksensible.scala:8: warning: comparing a fresh object using `==' will always yield false - println(((x: Int) => x + 1) == null) - ^ -checksensible.scala:9: warning: comparing a fresh object using `==' will always yield false - println(new Object == new Object) - ^ -checksensible.scala:10: warning: comparing a fresh object using `!=' will always yield true - println(new Exception() != new Exception()) - ^ -checksensible.scala:13: warning: comparing values of types Int and Null using `==' will always yield false - if (foo.length == null) // == 0 makes more sense, but still +checksensible.scala:55: error: comparing values of types Int and Null using `==' will always yield false + if (foo.length == null) "plante" else "plante pas" ^ -checksensible.scala:22: warning: comparing values of types Unit and Int using `!=' will always yield true +checksensible.scala:60: error: comparing values of types Bip and Bop using `==' will always yield false + (x1 == x2) + ^ +checksensible.scala:71: error: comparing values of types EqEqRefTest.this.C3 and EqEqRefTest.this.Z1 using `==' will always yield false + c3 == z1 + ^ +checksensible.scala:72: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `==' will always yield false + z1 == c3 + ^ +checksensible.scala:73: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `!=' will always yield true + z1 != c3 + ^ +checksensible.scala:74: error: comparing values of types EqEqRefTest.this.C3 and java.lang.String using `!=' will always yield true + c3 != "abc" + ^ +checksensible.scala:83: error: comparing values of types Unit and Int using `!=' will always yield true while ((c = in.read) != -1) ^ -checksensible.scala:28: warning: comparing values of types Unit and Boolean using `==' will always yield false - println({} == true) - ^ -checksensible.scala:30: warning: comparing a fresh object using `==' will always yield false - println(new Object == 1) - ^ -checksensible.scala:31: warning: comparing values of types Int and java.lang.Object using `==' will always yield false - println(1 == (new Object)) - ^ -checksensible.scala:35: warning: comparing values of types Int and Unit using `!=' will always yield true - println(1 != println) - ^ -checksensible.scala:36: warning: comparing values of types Int and Symbol using `!=' will always yield true - println(1 != 'sym) - ^ -13 warnings found -one error found +30 errors found diff --git a/test/files/neg/checksensible.flags b/test/files/neg/checksensible.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/neg/checksensible.flags @@ -0,0 +1 @@ +-Xfatal-warnings
\ No newline at end of file diff --git a/test/files/neg/checksensible.scala b/test/files/neg/checksensible.scala index 092c08592f..f67e9ea57f 100644 --- a/test/files/neg/checksensible.scala +++ b/test/files/neg/checksensible.scala @@ -1,38 +1,88 @@ -class Test { +final class Bip { def <=(other: Bop) = true } +final class Bop { } +object Bep { } + +final class Zing { + def !=(other: Zing) = false +} + +// 7 warnings +class RefEqTest { + object Shmoopie + + (new AnyRef) eq (new AnyRef) + (new AnyRef) ne (new AnyRef) + Shmoopie eq (new AnyRef) + (Shmoopie: AnyRef) eq (new AnyRef) + (new AnyRef) eq Shmoopie + (new AnyRef) eq null + null eq new AnyRef +} + +// 11 warnings +class EqEqValTest { var c = 0 - println((c = 1) == 0) - println(1 == "abc") - println(1 != true) + (c = 1) == 0 + 0 == (c = 1) + + 1 == "abc" + "abc" == 1 // doesn't warn since String defines an equals method + + new AnyRef == 1 + 1 == new AnyRef + + 1 != true + () == true + () == () + () == println + + (1 != println) + (1 != 'sym) +} - println(((x: Int) => x + 1) == null) - println(new Object == new Object) - println(new Exception() != new Exception()) +// 12 warnings +class EqEqRefTest { + val ref = new Bop + ((x: Int) => x + 1) == null + Bep == ((_: Int) + 1) + + new Object == new Object + new Object == "abc" + new Exception() != new Exception() val foo: Array[String] = Array("1","2","3") - if (foo.length == null) // == 0 makes more sense, but still - println("plante") // this code leads to runtime crash - else - println("plante pas") + if (foo.length == null) "plante" else "plante pas" + + // final classes with default equals + val x1 = new Bip + val x2 = new Bop + (x1 == x2) + + class C1 { } + class C2 extends C1 { } + final class Z1 extends C2 { } + final class C3 extends C2 { def !=(other: Z1) = false } + val z1 = new Z1 + val c3 = new C3 + + // all but c3 != z1 should warn + c3 != z1 + c3 == z1 + z1 == c3 + z1 != c3 + c3 != "abc" + + // non-warners + (null: AnyRef) == (null: AnyRef) + (x1 <= x2) def main(args: Array[String]) = { val in = new java.io.FileInputStream(args(0)) - var c = 0 while ((c = in.read) != -1) print(c.toChar) in.close } - - println({} == true) - println("hello" == 2) - println(new Object == 1) - println(1 == (new Object)) - - def isabstract: Int - - println(1 != println) - println(1 != 'sym) - } |