Skip to content

Commit

Permalink
Merge pull request #10446 from lrytz/p8120-backport
Browse files Browse the repository at this point in the history
[backport] Display warning on equals comparing non-references
  • Loading branch information
lrytz committed Jun 27, 2023
2 parents 466edaf + 3e418a8 commit 2b2ba17
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/compiler/scala/tools/nsc/Reporting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ object Reporting {
object OtherMigration extends Other; add(OtherMigration)
object OtherMatchAnalysis extends WarningCategory; add(OtherMatchAnalysis)
object OtherDebug extends WarningCategory; add(OtherDebug)
object OtherNonCooperativeEquals extends Other; add(OtherNonCooperativeEquals)

sealed trait WFlag extends WarningCategory { override def summaryCategory: WarningCategory = WFlag }
object WFlag extends WFlag { override def includes(o: WarningCategory): Boolean = o.isInstanceOf[WFlag] }; add(WFlag)
Expand Down
31 changes: 31 additions & 0 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1160,10 +1160,41 @@ abstract class RefChecks extends Transform {
else warnIfLubless()
}
}

private def checkSensibleAnyEquals(pos: Position, qual: Tree, name: Name, sym: Symbol, other: Tree) = {
def underlyingClass(tp: Type): Symbol = {
val sym = tp.widen.typeSymbol
if (sym.isAbstractType) underlyingClass(sym.info.upperBound)
else sym
}
val receiver = underlyingClass(qual.tpe)
val actual = underlyingClass(other.tpe)
def typesString = "" + normalizeAll(qual.tpe.widen) + " and " + normalizeAll(other.tpe.widen)
def nonSensiblyEquals() = {
refchecksWarning(pos, s"comparing values of types $typesString using `${name.decode}` unsafely bypasses cooperative equality; use `==` instead", WarningCategory.OtherNonCooperativeEquals)
}
def isScalaNumber(s: Symbol) = s isSubClass ScalaNumberClass
def isJavaNumber(s: Symbol) = s isSubClass JavaNumberClass
def isAnyNumber(s: Symbol) = isScalaNumber(s) || isJavaNumber(s)
def isNumeric(s: Symbol) = isNumericValueClass(unboxedValueClass(s)) || isAnyNumber(s)
def isReference(s: Symbol) = (unboxedValueClass(s) isSubClass AnyRefClass) || (s isSubClass ObjectClass)
def isUnit(s: Symbol) = unboxedValueClass(s) == UnitClass
def isNumOrNonRef(s: Symbol) = isNumeric(s) || (!isReference(s) && !isUnit(s))
if (isNumeric(receiver) && isNumOrNonRef(actual)) {
if (receiver == actual) ()
else nonSensiblyEquals()
}
else if ((sym == Any_equals || sym == Object_equals) && isNumOrNonRef(actual) && !isReference(receiver)) {
nonSensiblyEquals()
}
}

/** Sensibility check examines flavors of equals. */
def checkSensible(pos: Position, fn: Tree, args: List[Tree]) = fn match {
case Select(qual, name @ (nme.EQ | nme.NE | nme.eq | nme.ne)) if args.length == 1 && isObjectOrAnyComparisonMethod(fn.symbol) && (!currentOwner.isSynthetic || currentOwner.isAnonymousFunction) =>
checkSensibleEquals(pos, qual, name, fn.symbol, args.head)
case Select(qual, name@nme.equals_) if settings.isScala213 && args.length == 1 && (!currentOwner.isSynthetic || currentOwner.isAnonymousFunction) =>
checkSensibleAnyEquals(pos, qual, name, fn.symbol, args.head)
case _ =>
}

Expand Down
18 changes: 18 additions & 0 deletions test/files/neg/checksensible-equals.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
checksensible-equals.scala:4: warning: comparing values of types Long and Int using `equals` unsafely bypasses cooperative equality; use `==` instead
1L equals 1
^
checksensible-equals.scala:11: warning: comparing values of types Any and Int using `equals` unsafely bypasses cooperative equality; use `==` instead
(1L: Any) equals 1
^
checksensible-equals.scala:12: warning: comparing values of types AnyVal and Int using `equals` unsafely bypasses cooperative equality; use `==` instead
(1L: AnyVal) equals 1
^
checksensible-equals.scala:13: warning: comparing values of types AnyVal and AnyVal using `equals` unsafely bypasses cooperative equality; use `==` instead
(1L: AnyVal) equals (1: AnyVal)
^
checksensible-equals.scala:16: warning: comparing values of types A and Int using `equals` unsafely bypasses cooperative equality; use `==` instead
def foo[A](a: A) = a.equals(1)
^
error: No warnings can be incurred under -Xfatal-warnings.
5 warnings found
one error found
19 changes: 19 additions & 0 deletions test/files/neg/checksensible-equals.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// scalac: -Xsource:2.13 -Werror

class AnyEqualsTest {
1L equals 1
// ok, because it's between the same numeric types
1 equals 1
// ok
1L equals "string"
// ok
1L.equals(())
(1L: Any) equals 1
(1L: AnyVal) equals 1
(1L: AnyVal) equals (1: AnyVal)
// ok
"string" equals 1
def foo[A](a: A) = a.equals(1)
// ok
def bar[A <: AnyRef](a: A) = a.equals(1)
}

0 comments on commit 2b2ba17

Please sign in to comment.