Merge pull request #10369 from som-snytt/forward/int-div

Lint for integral divisions that are widened to a float [forward port from 2.12]

src/compiler/scala/tools/nsc/Reporting.scala

@@ -408,6 +408,7 @@ object Reporting {
     object LintUnitSpecialization extends Lint; add(LintUnitSpecialization)
     object LintMultiargInfix extends Lint; add(LintMultiargInfix)
     object LintPerformance extends Lint; add(LintPerformance)
+    object LintIntDivToFloat extends Lint; add(LintIntDivToFloat)
 
     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)

src/compiler/scala/tools/nsc/settings/Warnings.scala

@@ -214,6 +214,7 @@ trait Warnings {
     val ImplicitRecursion      = LintWarning("implicit-recursion",        "Implicit resolves to an enclosing definition.")
     val UniversalMethods       = LintWarning("universal-methods",         "Require arg to is/asInstanceOf.")
     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`.")
 
     def allLintWarnings = values.toSeq.asInstanceOf[Seq[LintWarning]]
   }
@@ -248,6 +249,7 @@ trait Warnings {
   def lintImplicitRecursion      = lint.contains(ImplicitRecursion) || (warnSelfImplicit.value: @nowarn("cat=deprecation"))
   def lintUniversalMethods       = lint.contains(UniversalMethods)
   def lintArgDiscard             = lint.contains(ArgDiscard)
+  def lintIntDivToFloat          = lint.contains(IntDivToFloat)
 
   // The Xlint warning group.
   val lint = MultiChoiceSetting(

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -837,7 +837,6 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
                       // refuses to re-attempt typechecking, and presumes that someone
                       // else was responsible for issuing the related type error!
                       fun.setSymbol(NoSymbol)
-                  case _ =>
                 }
                 debuglog(s"fallback on implicits: $tree/$resetTree")
                 // scala/bug#10066 Need to patch the enclosing tree in the context to make translation of Dynamic
@@ -1124,17 +1123,31 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           @inline def warnValueDiscard(): Unit =
             if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !treeInfo.hasExplicitUnit(tree))
               context.warning(tree.pos, s"discarded non-Unit value of type ${tree.tpe}", WarningCategory.WFlagValueDiscard)
-          @inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit =
-            if (!isPastTyper) {
-              val isInharmonic = (
-                (tpSym == IntClass && ptSym == FloatClass)
-                  || (tpSym == LongClass && (ptSym == FloatClass || ptSym == DoubleClass))
-              )
-              if (isInharmonic)
-                // not `context.deprecationWarning` because they are not buffered in silent mode
-                context.warning(tree.pos, s"Widening conversion from ${tpSym.name} to ${ptSym.name} is deprecated because it loses precision. Write `.to${ptSym.name}` instead.", WarningCategory.Deprecation)
-              else if (settings.warnNumericWiden.value) context.warning(tree.pos, "implicit numeric widening", WarningCategory.WFlagNumericWiden)
+          @inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit = if (!isPastTyper) {
+            val targetIsWide = ptSym == FloatClass || ptSym == DoubleClass
+            val isInharmonic = {
+              def intWidened = tpSym == IntClass && ptSym == FloatClass
+              def longWidened = tpSym == LongClass && targetIsWide
+              intWidened || longWidened
+            }
+            if (isInharmonic)
+              // not `context.deprecationWarning` because they are not buffered in silent mode
+              context.warning(tree.pos, s"Widening conversion from ${tpSym.name} to ${ptSym.name} is deprecated because it loses precision. Write `.to${ptSym.name}` instead.", WarningCategory.Deprecation)
+            else {
+              object warnIntDiv extends Traverser {
+                def isInt(t: Tree) = ScalaIntegralValueClasses(t.tpe.typeSymbol)
+                override def traverse(tree: Tree): Unit = tree match {
+                  case Apply(Select(q, nme.DIV), _) if isInt(q) =>
+                    context.warning(tree.pos, s"integral division is implicitly converted (widened) to floating point. Add an explicit `.to${ptSym.name}`.", WarningCategory.LintIntDivToFloat)
+                  case Apply(Select(a1, _), List(a2)) if isInt(tree) && isInt(a1) && isInt(a2) => traverse(a1); traverse(a2)
+                  case Select(q, _) if isInt(tree) && isInt(q) => traverse(q)
+                  case _ =>
+                }
+              }
+              if (targetIsWide && settings.lintIntDivToFloat) warnIntDiv(tree)
+              if (settings.warnNumericWiden.value) context.warning(tree.pos, "implicit numeric widening", WarningCategory.WFlagNumericWiden)
             }
+          }
 
           // The <: Any requirement inhibits attempts to adapt continuation types to non-continuation types.
           val anyTyped = tree.tpe <:< AnyTpe

src/library/scala/math/BigInt.scala

@@ -278,6 +278,7 @@ final class BigInt private (private var _bigInteger: BigInteger, private val _lo
     (shifted.signum != 0) && !(shifted equals BigInt.minusOne)
   }
 
+  @deprecated("isWhole on an integer type is always true", "2.12.15")
   def isWhole: Boolean = true
   def underlying: BigInteger = bigInteger
 

src/library/scala/runtime/RichInt.scala

@@ -31,6 +31,7 @@ final class RichInt(val self: Int) extends AnyVal with ScalaNumberProxy[Int] wit
   /** Returns `'''true'''` if this number has no decimal component.
     * Always `'''true'''` for `RichInt`.
     */
+  @deprecated("isWhole on an integer type is always true", "2.12.15")
   def isWhole = true
 
   override def isValidInt   = true

src/library/scala/runtime/ScalaNumberProxy.scala

@@ -50,6 +50,7 @@ trait ScalaNumberProxy[T] extends Any with ScalaNumericAnyConversions with Typed
   @deprecated("use `sign` method instead", since = "2.13.0") def signum: Int = num.signum(self)
 }
 trait ScalaWholeNumberProxy[T] extends Any with ScalaNumberProxy[T] {
+  @deprecated("isWhole on an integer type is always true", "2.12.15")
   def isWhole = true
 }
 trait IntegralProxy[T] extends Any with ScalaWholeNumberProxy[T] with RangedProxy[T] {

src/reflect/scala/reflect/internal/Definitions.scala

@@ -182,6 +182,8 @@ trait Definitions extends api.StandardDefinitions {
     }
     def ScalaPrimitiveValueClasses: List[ClassSymbol] = ScalaValueClasses
 
+    lazy val ScalaIntegralValueClasses: Set[Symbol] = Set(CharClass, ByteClass, ShortClass, IntClass, LongClass)
+
     def underlyingOfValueClass(clazz: Symbol): Type =
       clazz.derivedValueClassUnbox.tpe.resultType
 

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

@@ -530,6 +530,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     definitions.ScalaValueClasses
     definitions.ScalaValueClassesSet
     definitions.ScalaNumericValueClassesSet
+    definitions.ScalaIntegralValueClasses
 
     uncurry.VarargsSymbolAttachment
     uncurry.DesugaredParameterType

test/files/neg/lint-int-div-to-float.check

@@ -0,0 +1,18 @@
+lint-int-div-to-float.scala:6: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
+  def w1: Double = f / 2
+                     ^
+lint-int-div-to-float.scala:7: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
+  def w2: Double = (f / 2) * 3
+                      ^
+lint-int-div-to-float.scala:8: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
+  def w3: Double = -(f / 2)
+                       ^
+lint-int-div-to-float.scala:9: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
+  def w4: Double = (new C).f / (new C).f * 3
+                             ^
+lint-int-div-to-float.scala:10: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
+  def w5: Double = f - f.abs / 2
+                             ^
+error: No warnings can be incurred under -Werror.
+5 warnings
+1 error

test/files/neg/lint-int-div-to-float.scala

@@ -0,0 +1,18 @@
+// scalac: -Xlint -Xfatal-warnings
+
+class C {
+  def f = 1
+
+  def w1: Double = f / 2
+  def w2: Double = (f / 2) * 3
+  def w3: Double = -(f / 2)
+  def w4: Double = (new C).f / (new C).f * 3
+  def w5: Double = f - f.abs / 2
+
+  def o1: Double = (f / 2).toDouble
+  def o2: Double = f.toDouble / 2
+  def o3: Double = f / 2.toDouble
+  def o4: Double = f / 2d
+  def o5: Double = (new C).f.toDouble / (new C).f * 3
+  def o6: Long = f / 2 + 3 // only warn if widening to a floating point, not when widening int to long
+}

test/files/run/is-valid-num.scala

@@ -1,6 +1,5 @@
-/*
- * filter: inliner warnings; re-run with
- */
+// scalac: -Xlint -Werror
+@annotation.nowarn("cat=deprecation&msg=isWhole")
 object Test {
   def x = BigInt("10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
   def y = BigDecimal("" + (Short.MaxValue + 1) + ".0")
@@ -11,7 +10,7 @@ object Test {
   def l2 = Int.MinValue.toLong - 1
 
   def main(args: Array[String]): Unit = {
-//    assert(x.isWhole, x)
+    assert(x.isWhole, x)
     assert(!x.isValidDouble, x)
     assert(!x.isValidFloat, x)
     assert(!x.isValidLong, x)
@@ -171,7 +170,7 @@ object Test {
 
     if (!d.isInfinity) {
       val bd = BigDecimal(new java.math.BigDecimal(d))
-//      assert(!bd.isWhole, bd)
+      assert(!bd.isWhole, bd)
       assert(bd.isExactDouble, bd)
       assert(bd.isExactFloat == isFloat, bd)
       assert(!bd.isValidLong, bd)
@@ -221,7 +220,7 @@ object Test {
     assert(bd.isValidShort == isShort, bd)
     assert(bd.isValidByte == isByte, bd)
 
-//    assert(bi.isWhole, bi)
+    assert(bi.isWhole, bi)
     assert(bi.isValidDouble == isDouble, bi)
     assert(bi.isValidFloat == isFloat, bi)
     assert(bi.isValidLong == isLong, bi)

test/scalacheck/scala/math/BigIntProperties.scala

@@ -5,6 +5,7 @@ import Arbitrary.arbitrary
 import org.scalacheck.Prop._
 import java.math.BigInteger
 import java.lang.Character
+import scala.annotation.nowarn
 import scala.util.Random
 
 object BigIntProperties extends Properties("BigInt") {
@@ -95,7 +96,7 @@ object BigIntProperties extends Properties("BigInt") {
   property("isValidLong") = forAll { (l: Long) => BigInt(l).isValidLong }
   property("isValidLong") = !BigInt("9223372036854775808").isValidLong
   property("isValidLong") = !BigInt("-9223372036854775809").isValidLong
-  property("isWhole") = forAll { (bi: BigInt) => bi.isWhole }
+  property("isWhole") = forAll { (bi: BigInt) => bi.isWhole: @nowarn }
   property("underlying") = forAll(bigInteger) { bi => BigInt(bi).underlying ?= bi }
   property("equals") = forAll(bigInteger, bigInteger) { (x, y) => (x == y) ?= (BigInt(x) equals BigInt(y)) }
   property("compare") = forAll(bigInteger, bigInteger) { (x, y) => x.compareTo(y) ?= BigInt(x).compare(y) }