diff options
| -rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 65 | ||||
| -rw-r--r-- | test/files/neg/t5663-badwarneq.check | 22 | ||||
| -rw-r--r-- | test/files/neg/t5663-badwarneq.flags | 1 | ||||
| -rw-r--r-- | test/files/neg/t5663-badwarneq.scala | 76 |
4 files changed, 142 insertions, 22 deletions
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 806ee480f0..c4b08f6ea2 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1059,12 +1059,10 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R // equals. def isUsingWarnableEquals = { val m = receiver.info.member(nme.equals_) - def n = actual.info.member(nme.equals_) - ( (m == Object_equals) - || (m == Any_equals) - || (m.isSynthetic && m.owner.isCase && !n.owner.isCase) - ) + ((m == Object_equals) || (m == Any_equals) || isMethodCaseEquals(m)) } + def isMethodCaseEquals(m: Symbol) = m.isSynthetic && m.owner.isCase + def isCaseEquals = isMethodCaseEquals(receiver.info.member(nme.equals_)) // Whether this == or != is one of those defined in Any/AnyRef or an overload from elsewhere. def isUsingDefaultScalaOp = { val s = fn.symbol @@ -1087,9 +1085,11 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R val msg = alwaysEqual == (name == nme.EQ || name == nme.eq) unit.warning(pos, "comparing "+what+" using `"+name.decode+"' will always yield " + msg) } - def nonSensible(pre: String, alwaysEqual: Boolean) = nonSensibleWarning(pre+"values of types "+typesString, alwaysEqual) + def nonSensiblyEq() = nonSensible("", true) + def nonSensiblyNeq() = nonSensible("", false) + def nonSensiblyNew() = nonSensibleWarning("a fresh object", false) def unrelatedTypes() = { val msg = if (name == nme.EQ || name == nme.eq) @@ -1097,52 +1097,73 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R unit.warning(pos, typesString + " are unrelated: they will most likely " + msg) } - if (nullCount == 2) - nonSensible("", true) // null == null + if (nullCount == 2) // null == null + nonSensiblyEq() else if (nullCount == 1) { if (onSyms(_ exists isPrimitiveValueClass)) // null == 5 - nonSensible("", false) + nonSensiblyNeq() else if (onTrees( _ exists isNew)) // null == new AnyRef - nonSensibleWarning("a fresh object", false) + nonSensiblyNew() } else if (isBoolean(receiver)) { if (!isBoolean(actual) && !isMaybeValue(actual)) // true == 5 - nonSensible("", false) + nonSensiblyNeq() } else if (isUnit(receiver)) { if (isUnit(actual)) // () == () - nonSensible("", true) + nonSensiblyEq() else if (!isUnit(actual) && !isMaybeValue(actual)) // () == "abc" - nonSensible("", false) + nonSensiblyNeq() } else if (isNumeric(receiver)) { if (!isNumeric(actual) && !forMSIL) if (isUnit(actual) || isBoolean(actual) || !isMaybeValue(actual)) // 5 == "abc" - nonSensible("", false) + nonSensiblyNeq() } - else if (isWarnable) { + else if (isWarnable && !isCaseEquals) { if (isNew(qual)) // new X == y - nonSensibleWarning("a fresh object", false) + nonSensiblyNew() else if (isNew(args.head) && (receiver.isEffectivelyFinal || isReferenceOp)) // object X ; X == new Y - nonSensibleWarning("a fresh object", false) + nonSensiblyNew() else if (receiver.isEffectivelyFinal && !(receiver isSubClass actual)) { // object X, Y; X == Y if (isEitherNullable) nonSensible("non-null ", false) else - nonSensible("", false) + nonSensiblyNeq() } } // possibleNumericCount is insufficient or this will warn on e.g. Boolean == j.l.Boolean if (isWarnable && nullCount == 0 && !(isSpecial(receiver) && isSpecial(actual))) { - if (actual isSubClass receiver) () - else if (receiver isSubClass actual) () - // warn only if they have no common supertype below Object - else { + // better to have lubbed and lost + def warnIfLubless(): Unit = { val common = global.lub(List(actual.tpe, receiver.tpe)) if (ObjectClass.tpe <:< common) unrelatedTypes() } + def eitherSubclasses = (actual isSubClass receiver) || (receiver isSubClass actual) + // warn if actual has a case parent that is not same as receiver's; + // if actual is not a case, then warn if no common supertype, as below + if (isCaseEquals) { + def thisCase = receiver.info.member(nme.equals_).owner + actual.info.baseClasses.find(_.isCase) match { + case Some(p) if (p != thisCase) => nonSensible("case class ", false) + case None => + // stronger message on (Some(1) == None) + //if (receiver.isCase && receiver.isEffectivelyFinal && !(receiver isSubClass actual)) nonSensiblyNeq() + //else + // if a class, it must be super to thisCase (and receiver) since not <: thisCase + if (!actual.isTrait && !(receiver isSubClass actual)) nonSensiblyNeq() + else if (!eitherSubclasses) warnIfLubless() + case _ => + } + } + else if (actual isSubClass receiver) () + else if (receiver isSubClass actual) () + // warn only if they have no common supertype below Object + else { + warnIfLubless() + } } case _ => } diff --git a/test/files/neg/t5663-badwarneq.check b/test/files/neg/t5663-badwarneq.check new file mode 100644 index 0000000000..00c2234e9d --- /dev/null +++ b/test/files/neg/t5663-badwarneq.check @@ -0,0 +1,22 @@ +t5663-badwarneq.scala:42: error: comparing case class values of types Some[Int] and None.type using `==' will always yield false + println(new Some(1) == None) // Should complain on type, was: spuriously complains on fresh object + ^ +t5663-badwarneq.scala:43: error: comparing case class values of types Some[Int] and Thing using `==' will always yield false + println(Some(1) == new Thing(1)) // Should complain on type, was: spuriously complains on fresh object + ^ +t5663-badwarneq.scala:51: error: ThingOne and Thingy are unrelated: they will most likely never compare equal + println(t1 == t2) // true, but apparently unrelated, a compromise warning + ^ +t5663-badwarneq.scala:52: error: ThingThree and Thingy are unrelated: they will most likely never compare equal + println(t4 == t2) // true, complains because ThingThree is final and Thingy not a subclass, stronger claim than unrelated + ^ +t5663-badwarneq.scala:55: error: comparing case class values of types ThingTwo and Some[Int] using `==' will always yield false + println(t3 == Some(1)) // false, warn on different cases + ^ +t5663-badwarneq.scala:56: error: comparing values of types ThingOne and Cousin using `==' will always yield false + println(t1 == c) // should warn + ^ +t5663-badwarneq.scala:64: error: comparing case class values of types Simple and SimpleSibling.type using `==' will always yield false + println(new Simple() == SimpleSibling) // like Some(1) == None, but needn't be final case + ^ +7 errors found diff --git a/test/files/neg/t5663-badwarneq.flags b/test/files/neg/t5663-badwarneq.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/neg/t5663-badwarneq.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/neg/t5663-badwarneq.scala b/test/files/neg/t5663-badwarneq.scala new file mode 100644 index 0000000000..56ec389c03 --- /dev/null +++ b/test/files/neg/t5663-badwarneq.scala @@ -0,0 +1,76 @@ + +// alias +trait Thingy + +class Gramps + +// sibling classes that extend a case class +case class Thing(i: Int) extends Gramps +class ThingOne(x:Int) extends Thing(x) +class ThingTwo(y:Int) extends Thing(y) with Thingy +final class ThingThree(z:Int) extends Thing(z) + +// not case cousin +class Cousin extends Gramps + +class SimpleParent +case class Simple() extends SimpleParent +case object SimpleSibling extends SimpleParent + +/* It's not possible to run partest without -deprecation. + * Since detecting the warnings requires a neg test with + * -Xfatal-warnings, and deprecation terminates the compile, + * we'll just comment out the nasty part. The point was + * just to show there's nothing special about a trait + * that extends a case class, which is only permitted + * (deprecatingly) by omitting the parens. + * +// common ancestor is something else +class AnyThing +case class SomeThing extends AnyThing // deprecation +class OtherThing extends AnyThing + +// how you inherit caseness doesn't matter +trait InThing extends SomeThing +class MyThing extends InThing +*/ + +object Test { + def main(a: Array[String]) { + // nothing to do with Gavin + println(new Some(1) == new Some(1)) // OK, true + println(new Some(1) == None) // Should complain on type, was: spuriously complains on fresh object + println(Some(1) == new Thing(1)) // Should complain on type, was: spuriously complains on fresh object + + val t1 = new ThingOne(11) + val t2: Thingy = new ThingTwo(11) + val t3 = new ThingTwo(11) + val t4 = new ThingThree(11) + val c = new Cousin + + println(t1 == t2) // true, but apparently unrelated, a compromise warning + println(t4 == t2) // true, complains because ThingThree is final and Thingy not a subclass, stronger claim than unrelated + println(t2 == t3) // OK, two Thingy + println(t3 == t2) // ditto with case receiver + println(t3 == Some(1)) // false, warn on different cases + println(t1 == c) // should warn + + // don't warn on fresh cases + println(new ThingOne(11) == t1) // OK, was: two cases not warnable on trunk + println(new ThingTwo(11) == t2) // true, was: spuriously complains on fresh object + println(new ThingOne(11) == t3) // two cases not warnable on trunk + println(new ThingTwo(11) == t3) // ditto + + println(new Simple() == SimpleSibling) // like Some(1) == None, but needn't be final case + + /* + val mine = new MyThing + val some = new SomeThing + val other = new OtherThing + println(mine == some) // OK, two Something + println(some == mine) + println(mine == other) // OK, two Anything? + println(mine == t1) // false + */ + } +} |