From 72d86cbe8cb4792a2eae190aee3677854bffb89f Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 11 Apr 2012 17:29:49 -0700 Subject: SI-5663: Tweak warnings on case class equals Re-enable testing the sensibility of comparing instances of two case classes. In particular, warn if we detect that the two objects inherit from different case classes. In addition, avoid emitting misleading warnings when comparing "new C". --- .../scala/tools/nsc/typechecker/RefChecks.scala | 65 ++++++++++++++-------- 1 file changed, 43 insertions(+), 22 deletions(-) (limited to 'src/compiler') 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 _ => } -- cgit v1.2.3