Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint select from Unit, or Int that incurs widening #10372

Merged
merged 3 commits into from Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/compiler/scala/tools/nsc/Reporting.scala
Expand Up @@ -341,7 +341,7 @@ object Reporting {
private val insertDash = "(?=[A-Z][a-z])".r

val all: mutable.Map[String, WarningCategory] = mutable.Map.empty
private def add(c: WarningCategory): Unit = all += ((c.name, c))
private def add(c: WarningCategory): Unit = all.put(c.name, c).ensuring(_.isEmpty, s"lint '${c.name}' added twice")

object Deprecation extends WarningCategory; add(Deprecation)

Expand Down Expand Up @@ -409,6 +409,8 @@ object Reporting {
object LintMultiargInfix extends Lint; add(LintMultiargInfix)
object LintPerformance extends Lint; add(LintPerformance)
object LintIntDivToFloat extends Lint; add(LintIntDivToFloat)
object LintUniversalMethods extends Lint; add(LintUniversalMethods)
object LintNumericMethods extends Lint; add(LintNumericMethods)

sealed trait Feature extends WarningCategory { override def summaryCategory: WarningCategory = Feature }
object Feature extends Feature { override def includes(o: WarningCategory): Boolean = o.isInstanceOf[Feature] }; add(Feature)
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/scala/tools/nsc/settings/Warnings.scala
Expand Up @@ -212,7 +212,8 @@ trait Warnings {
val UnitSpecialization = LintWarning("unit-special", "Warn for specialization of Unit in parameter position.")
val MultiargInfix = LintWarning("multiarg-infix", "Infix operator was defined or used with multiarg operand.")
val ImplicitRecursion = LintWarning("implicit-recursion", "Implicit resolves to an enclosing definition.")
val UniversalMethods = LintWarning("universal-methods", "Require arg to is/asInstanceOf.")
val UniversalMethods = LintWarning("universal-methods", "Require arg to is/asInstanceOf. No Unit receiver.")
val NumericMethods = LintWarning("numeric-methods", "Dubious usages, such as `42.isNaN`.")
val ArgDiscard = LintWarning("arg-discard", "-Wvalue-discard for adapted arguments.")
val IntDivToFloat = LintWarning("int-div-to-float", "Warn when an integer division is converted (widened) to floating point: `(someInt / 2): Double`.")

Expand Down Expand Up @@ -248,6 +249,7 @@ trait Warnings {
def multiargInfix = lint contains MultiargInfix
def lintImplicitRecursion = lint.contains(ImplicitRecursion) || (warnSelfImplicit.value: @nowarn("cat=deprecation"))
def lintUniversalMethods = lint.contains(UniversalMethods)
def lintNumericMethods = lint.contains(NumericMethods)
def lintArgDiscard = lint.contains(ArgDiscard)
def lintIntDivToFloat = lint.contains(IntDivToFloat)

Expand Down
13 changes: 12 additions & 1 deletion src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -1265,7 +1265,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
adaptAfterOverloadResolution(tree, mode, pt, original)
case NullaryMethodType(restpe) => // (2)
if (hasUndets && settings.lintUniversalMethods && (isCastSymbol(tree.symbol) || isTypeTestSymbol(tree.symbol)) && context.undetparams.exists(_.owner == tree.symbol))
context.warning(tree.pos, s"missing type argument to ${tree.symbol}", WarningCategory.LintAdaptedArgs)
context.warning(tree.pos, s"missing type argument to ${tree.symbol}", WarningCategory.LintUniversalMethods)
val resTpDeconst = // keep constant types when they are safe to fold. erasure eliminates constant types modulo some exceptions, so keep those.
if (isBeforeErasure && tree.symbol.isAccessor && tree.symbol.hasFlag(STABLE) && treeInfo.isExprSafeToInline(tree)) restpe
else restpe.deconst
Expand Down Expand Up @@ -5292,16 +5292,26 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
def asDynamicCall = mkInvoke(context, tree, qual, name) map { t =>
wrapErrors(t, _.typed1(t, mode, pt))
}
def checkDubiousUnitSelection(result: Tree): Unit =
if (!isPastTyper && isUniversalMember(result.symbol))
context.warning(tree.pos, s"dubious usage of ${result.symbol} with unit value", WarningCategory.LintUniversalMethods)

val sym = tree.symbol orElse member(qual.tpe, name) orElse inCompanionForJavaStatic(qual.tpe.typeSymbol, name)
if ((sym eq NoSymbol) && name != nme.CONSTRUCTOR && mode.inAny(EXPRmode | PATTERNmode)) {
// symbol not found? --> try to convert implicitly to a type that does have the required
// member. Added `| PATTERNmode` to allow enrichment in patterns (so we can add e.g., an
// xml member to StringContext, which in turn has an unapply[Seq] method)

def checkDubiousAdaptation(sel: Tree): Unit = if (!isPastTyper && settings.lintNumericMethods) {
val dubious = ScalaIntegralValueClasses(qualTp.typeSymbol) && (
sel.symbol.owner.eq(BoxedFloatClass) || sel.symbol.owner.eq(RichFloatClass))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

long.isNaN goes through float2Float(long.toFloat), not double2Double, so this seems to cover it all.

if (dubious)
context.warning(tree.pos, s"dubious usage of ${sel.symbol} with integer value", WarningCategory.LintNumericMethods)
}
val qual1 = adaptToMemberWithArgs(tree, qual, name, mode)
if ((qual1 ne qual) && !qual1.isErrorTyped)
return typed(treeCopy.Select(tree, qual1, name), mode, pt)
.tap(checkDubiousAdaptation)
}

// This special-case complements the logic in `adaptMember` in erasure, it handles selections
Expand Down Expand Up @@ -5419,6 +5429,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
name
)
case _ =>
if (settings.lintUniversalMethods && qualTp.widen.eq(UnitTpe)) checkDubiousUnitSelection(result)
result
}
}
Expand Down
24 changes: 15 additions & 9 deletions src/partest/scala/tools/partest/nest/AbstractRunner.scala
Expand Up @@ -296,7 +296,8 @@ class AbstractRunner(val config: RunnerSpec.Config, protected final val testSour

val isRerun = config.optFailed
val rerunTests = if (isRerun) testKinds.failedTests else Nil
def miscTests = individualTests ++ greppedTests ++ branchedTests ++ rerunTests
val specialTests = if (realeasy) List(Path("test/files/run/t6240-universe-code-gen.scala")) else Nil
def miscTests = List(individualTests, greppedTests, branchedTests, rerunTests, specialTests).flatten

val givenKinds = standardKinds filter config.parsed.isSet
val kinds = (
Expand All @@ -312,16 +313,21 @@ class AbstractRunner(val config: RunnerSpec.Config, protected final val testSour

def testContributors = {
List(
if (rerunTests.isEmpty) "" else "previously failed tests",
if (kindsTests.isEmpty) "" else s"${kinds.size} named test categories",
if (greppedTests.isEmpty) "" else s"${greppedTests.size} tests matching '$grepExpr'",
if (branchedTests.isEmpty) "" else s"${branchedTests.size} tests modified on this branch",
if (individualTests.isEmpty) "" else "specified tests",
).filterNot(_.isEmpty).mkString(", ")
(rerunTests, "previously failed tests"),
(kindsTests, s"${kinds.size} named test categories"),
(greppedTests, s"${greppedTests.size} tests matching '$grepExpr'"),
(branchedTests, s"${branchedTests.size} tests modified on this branch"),
(individualTests, "specified tests"),
(specialTests, "other tests you might have forgotten"),
).filterNot(_._1.isEmpty).map(_._2) match {
case Nil => "the well of despair. I see you're not in a testing mood."
case one :: Nil => one
case all => all.init.mkString("", ", ", s", and ${all.last}")
}
}

val allTests: Array[Path] = distinctBy(miscTests ++ kindsTests)(_.toCanonical).sortBy(_.toString).toArray
val grouped = (allTests groupBy kindOf).toArray sortBy (x => standardKinds indexOf x._1)
val allTests: Array[Path] = distinctBy(miscTests ::: kindsTests)(_.toCanonical).sortBy(_.toString).toArray
val grouped = allTests.groupBy(kindOf).toArray.sortBy(x => standardKinds.indexOf(x._1))

onlyIndividualTests = individualTests.nonEmpty && rerunTests.isEmpty && kindsTests.isEmpty && greppedTests.isEmpty
totalTests = allTests.size
Expand Down
3 changes: 2 additions & 1 deletion src/partest/scala/tools/partest/nest/Runner.scala
Expand Up @@ -531,7 +531,8 @@ class Runner(val testInfo: TestInfo, val suiteRunner: AbstractRunner) {
else from.toInt <= currentJavaVersion && currentJavaVersion <= to.toInt
else
currentJavaVersion >= from.toInt
if (ok) None
if (ok && suiteRunner.realeasy && from.toInt > 8) Some(genSkip(s"skipped on Java $javaSpecVersion, compiling against JDK8 but must run on $v"))
else if (ok) None
else Some(genSkip(s"skipped on Java $javaSpecVersion, only running on $v"))
case v =>
Some(genFail(s"invalid javaVersion range in test comment: $v"))
Expand Down
1 change: 1 addition & 0 deletions src/reflect/scala/reflect/internal/Definitions.scala
Expand Up @@ -104,6 +104,7 @@ trait Definitions extends api.StandardDefinitions {
lazy val lazyHolders = symbolsMap(ScalaValueClasses, x => getClassIfDefined("scala.runtime.Lazy" + x))
lazy val LazyRefClass = getClassIfDefined("scala.runtime.LazyRef")
lazy val LazyUnitClass = getClassIfDefined("scala.runtime.LazyUnit")
lazy val RichFloatClass = getClassIfDefined("scala.runtime.RichFloat")

lazy val allRefClasses: Set[Symbol] = {
refClass.values.toSet ++ volatileRefClass.values.toSet ++ Set(VolatileObjectRefClass, ObjectRefClass)
Expand Down
1 change: 1 addition & 0 deletions src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
Expand Up @@ -506,6 +506,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse =>
definitions.lazyHolders
definitions.LazyRefClass
definitions.LazyUnitClass
definitions.RichFloatClass
definitions.allRefClasses
definitions.UnitClass
definitions.ByteClass
Expand Down
162 changes: 162 additions & 0 deletions test/files/neg/t12728.check
@@ -0,0 +1,162 @@
t12728.scala:10: warning: dubious usage of method != with unit value
println(u.!=(x))
^
t12728.scala:11: warning: dubious usage of method ## with unit value
println(u.##)
^
t12728.scala:12: warning: dubious usage of method == with unit value
println(u.==(x))
^
t12728.scala:13: warning: dubious usage of method asInstanceOf with unit value
println(u.asInstanceOf[Any])
^
t12728.scala:14: warning: dubious usage of method equals with unit value
println(u.equals(x))
^
t12728.scala:15: warning: dubious usage of method hashCode with unit value
println(u.hashCode)
^
t12728.scala:16: warning: dubious usage of method isInstanceOf with unit value
println(u.isInstanceOf[Any])
^
t12728.scala:17: warning: dubious usage of method toString with unit value
println(u.toString)
^
t12728.scala:20: warning: Widening conversion from Int to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(i.isNaN)
^
t12728.scala:20: warning: dubious usage of method isNaN with integer value
println(i.isNaN)
^
t12728.scala:21: warning: Widening conversion from Int to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(i.isInfinity)
^
t12728.scala:21: warning: dubious usage of method isInfinity with integer value
println(i.isInfinity)
^
t12728.scala:22: warning: Widening conversion from Int to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(i.isInfinite)
^
t12728.scala:22: warning: dubious usage of method isInfinite with integer value
println(i.isInfinite)
^
t12728.scala:23: warning: Widening conversion from Int to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(i.isFinite)
^
t12728.scala:23: warning: dubious usage of method isFinite with integer value
println(i.isFinite)
^
t12728.scala:24: warning: Widening conversion from Int to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(i.isPosInfinity)
^
t12728.scala:24: warning: dubious usage of method isPosInfinity with integer value
println(i.isPosInfinity)
^
t12728.scala:25: warning: Widening conversion from Int to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(i.isNegInfinity)
^
t12728.scala:25: warning: dubious usage of method isNegInfinity with integer value
println(i.isNegInfinity)
^
t12728.scala:27: warning: Widening conversion from Int to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(i.ceil)
^
t12728.scala:27: warning: dubious usage of method ceil with integer value
println(i.ceil)
^
t12728.scala:28: warning: Widening conversion from Int to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(i.floor)
^
t12728.scala:28: warning: dubious usage of method floor with integer value
println(i.floor)
^
t12728.scala:30: warning: Widening conversion from Long to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(l.isNaN)
^
t12728.scala:30: warning: dubious usage of method isNaN with integer value
println(l.isNaN)
^
t12728.scala:31: warning: Widening conversion from Long to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(l.isInfinity)
^
t12728.scala:31: warning: dubious usage of method isInfinity with integer value
println(l.isInfinity)
^
t12728.scala:32: warning: Widening conversion from Long to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(l.isInfinite)
^
t12728.scala:32: warning: dubious usage of method isInfinite with integer value
println(l.isInfinite)
^
t12728.scala:33: warning: Widening conversion from Long to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(l.isFinite)
^
t12728.scala:33: warning: dubious usage of method isFinite with integer value
println(l.isFinite)
^
t12728.scala:34: warning: Widening conversion from Long to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(l.isPosInfinity)
^
t12728.scala:34: warning: dubious usage of method isPosInfinity with integer value
println(l.isPosInfinity)
^
t12728.scala:35: warning: Widening conversion from Long to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(l.isNegInfinity)
^
t12728.scala:35: warning: dubious usage of method isNegInfinity with integer value
println(l.isNegInfinity)
^
t12728.scala:37: warning: Widening conversion from Long to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(l.ceil)
^
t12728.scala:37: warning: dubious usage of method ceil with integer value
println(l.ceil)
^
t12728.scala:38: warning: Widening conversion from Long to Float is deprecated because it loses precision. Write `.toFloat` instead.
println(l.floor)
^
t12728.scala:38: warning: dubious usage of method floor with integer value
println(l.floor)
^
t12728.scala:40: warning: dubious usage of method isNaN with integer value
println(c.isNaN)
^
t12728.scala:41: warning: dubious usage of method isInfinity with integer value
println(c.isInfinity)
^
t12728.scala:42: warning: dubious usage of method isInfinite with integer value
println(c.isInfinite)
^
t12728.scala:43: warning: dubious usage of method isFinite with integer value
println(c.isFinite)
^
t12728.scala:44: warning: dubious usage of method isPosInfinity with integer value
println(c.isPosInfinity)
^
t12728.scala:45: warning: dubious usage of method isNegInfinity with integer value
println(c.isNegInfinity)
^
t12728.scala:47: warning: dubious usage of method ceil with integer value
println(c.ceil)
^
t12728.scala:48: warning: dubious usage of method floor with integer value
println(c.floor)
^
t12728.scala:54: warning: dubious usage of method toString with unit value
def g = new java.lang.StringBuilder("hi").setCharAt(0, 'H').toString // "()"
^
t12728.scala:14: warning: comparing values of types Unit and Any using `equals` unsafely bypasses cooperative equality; use `==` instead
println(u.equals(x))
^
t12728.scala:26: warning: method round in class RichInt is deprecated (since 2.11.0): this is an integer type; there is no reason to round it. Perhaps you meant to call this on a floating-point value?
println(i.round)
^
t12728.scala:36: warning: method round in class RichLong is deprecated (since 2.11.0): this is an integer type; there is no reason to round it. Perhaps you meant to call this on a floating-point value?
println(l.round)
^
t12728.scala:46: warning: method round in class RichInt is deprecated (since 2.11.0): this is an integer type; there is no reason to round it. Perhaps you meant to call this on a floating-point value?
println(c.round)
^
error: No warnings can be incurred under -Werror.
53 warnings
1 error
55 changes: 55 additions & 0 deletions test/files/neg/t12728.scala
@@ -0,0 +1,55 @@
// scalac: -Werror -Xlint

class C {
val u = ()
val i = 42
val l = 42L
val c = 'c'
val x = null: Any

println(u.!=(x))
println(u.##)
println(u.==(x))
println(u.asInstanceOf[Any])
println(u.equals(x))
println(u.hashCode)
println(u.isInstanceOf[Any])
println(u.toString)
println(i.toString)

println(i.isNaN)
println(i.isInfinity)
println(i.isInfinite)
println(i.isFinite)
println(i.isPosInfinity)
println(i.isNegInfinity)
println(i.round)
println(i.ceil)
println(i.floor)

println(l.isNaN)
println(l.isInfinity)
println(l.isInfinite)
println(l.isFinite)
println(l.isPosInfinity)
println(l.isNegInfinity)
println(l.round)
println(l.ceil)
println(l.floor)

println(c.isNaN)
println(c.isInfinity)
println(c.isInfinite)
println(c.isFinite)
println(c.isPosInfinity)
println(c.isNegInfinity)
println(c.round)
println(c.ceil)
println(c.floor)
}

class UseCase {
def f = new scala.StringBuilder("hi").setCharAt(0, 'H').toString // "Hi"

def g = new java.lang.StringBuilder("hi").setCharAt(0, 'H').toString // "()"
}
11 changes: 11 additions & 0 deletions test/files/run/blank.scala
@@ -0,0 +1,11 @@
// javaVersion: 11+
//
// skalac: --release:8
// trivial manual test for partest --realeasy, which sets --release:8.
// under --realeasy, skip this test because of the javaVersion, irrespective of JDK in use.
// otherwise, this test passes trivially on JDK11+ and is skipped on lesser JDKs.
// note that explicit --release:8 asks to compile against JDK8 but only run on the requested version.

object Test extends App {
assert("".isBlank) // String#isBlank was added in JDK11
}
4 changes: 3 additions & 1 deletion test/junit/scala/ArrayTest.scala
Expand Up @@ -4,11 +4,13 @@ import org.junit.Assert.{ assertArrayEquals, assertFalse, assertTrue }
import org.junit.Test

import scala.runtime.BoxedUnit
import scala.util.chaining._

class ArrayTest {
@Test
def testArrayCopyOfUnit(): Unit = {
val expected = new Array[BoxedUnit](32).asInstanceOf[Array[AnyRef]]; java.util.Arrays.fill(expected, ().asInstanceOf[AnyRef])
val expected = new Array[BoxedUnit](32).asInstanceOf[Array[AnyRef]]
.tap(array => java.util.Arrays.fill(array, (): Any))
assertArrayEquals(expected, Array.copyOf(Array[Unit](), 32).asInstanceOf[Array[AnyRef]])
assertArrayEquals(expected, Array.copyAs[Unit](Array[Nothing](), 32).asInstanceOf[Array[AnyRef]])
assertArrayEquals(expected, Array.copyAs[Unit](Array[Unit](), 32).asInstanceOf[Array[AnyRef]])
Expand Down