Warn when arms of if or match lub to Any

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

@@ -601,7 +601,7 @@ trait Infer extends Checkable {
               if (!result && !seen(t)) t.dealiasWidenChain.foreach(saw)
             }
           }
-          @`inline` def containsAny(t: Type) = collector.collect(t)
+          @inline def containsAny(t: Type): Boolean = collector.collect(t)
           val hasAny = containsAny(pt) || containsAny(restpe) ||
             formals.exists(containsAny) ||
             argtpes.exists(containsAny) ||

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

@@ -18,7 +18,6 @@ import scala.collection.mutable
 import symtab.Flags._
 import scala.reflect.internal.util.ListOfNil
 import scala.tools.nsc.Reporting.WarningCategory
-import scala.util.chaining._
 
 /** This trait declares methods to create symbols and to enter them into scopes.
  *
@@ -697,7 +696,7 @@ trait Namers extends MethodSynthesis {
             // There are two ways in which we exclude the symbol from being added in typedStats::addSynthetics,
             // because we don't know when the completer runs with respect to this loop in addSynthetics
             //  for (sym <- scope)
-            //    for (tree <- context.unit.synthetics.get(sym) if shouldAdd(sym)) {
+            //    for (tree <- context.unit.synthetics.get(sym) if shouldAdd(sym))
             //      if (!sym.initialize.hasFlag(IS_ERROR))
             //        newStats += typedStat(tree)
             // If we're already in the loop, set the IS_ERROR flag and trigger the condition `sym.initialize.hasFlag(IS_ERROR)`
@@ -734,7 +733,7 @@ trait Namers extends MethodSynthesis {
         /* @M! TypeDef's type params are handled differently, e.g., in `type T[A[x <: B], B]`, A and B are entered
          * first as both are in scope in the definition of x. x is only in scope in `A[x <: B]`.
          * No symbols are created for the abstract type's params at this point, i.e. the following assertion holds:
-         *     !tree.symbol.isAbstractType || { tparams.forall(_.symbol == NoSymbol)
+         *     !tree.symbol.isAbstractType || tparams.forall(_.symbol == NoSymbol)
          * (tested with the above example, `trait C { type T[A[X <: B], B] }`). See also comment in PolyTypeCompleter.
          */
         if (!tree.symbol.isAbstractType) //@M TODO: change to isTypeMember ?
@@ -1169,7 +1168,11 @@ trait Namers extends MethodSynthesis {
           }
           pt
         }
-        else legacy.tap(InferredImplicitError(tree, _, context))
+        else {
+          InferredImplicitError(tree, legacy, context)
+          context.unit.transformed.get(tree.rhs).foreach(rhs => checkLub(rhs, legacy))
+          legacy
+        }
       }.setPos(tree.pos.focus): @nowarn("cat=w-flag-value-discard")
       tree.tpt.tpe
     }
@@ -2113,6 +2116,23 @@ trait Namers extends MethodSynthesis {
       // @PP: I added this as a check because these flags are supposed to be converted to ABSOVERRIDE before arriving here.
       checkNoConflict(ABSTRACT, OVERRIDE)
     }
+
+    private val topTypes: Set[Symbol] = Set(AnyClass, AnyValClass, ObjectClass)
+    // check whether the given type is an unhealthy lub of arms of if/else or match
+    def checkLub(tree: Tree, res: Type): res.type = {
+      if (settings.warnInferAny && topTypes(res.typeSymbol)) {
+        val dubious = tree match {
+          case If(_, thenp, elsep) => treeInfo.isInterestingExpression(thenp) || treeInfo.isInterestingExpression(elsep)
+          case Match(_, cases) => cases.exists(k => treeInfo.isInterestingExpression(k.body))
+          //case Block(_, res) => treeInfo.isInterestingExpression(res)
+          //case _ => treeInfo.isInterestingExpression(tree)
+          case _ => false
+        }
+        if (dubious)
+          context.warning(tree.pos, s"a type was inferred to be `${res.typeSymbol.name}`; this may indicate a programming error.", WarningCategory.LintInferAny)
+      }
+      res
+    }
   }
 
   abstract class TypeCompleter extends LazyType {

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

@@ -1763,50 +1763,8 @@ abstract class RefChecks extends Transform {
 
     // if expression in statement position (of template or block)
     // looks like a useful value that should not be ignored, warn and return true
-    // User specifies that an expression is boring by ascribing `e: Unit`.
-    // The subtree `e` will bear an attachment, but may be wrapped in adaptations.
-    private def checkInterestingResultInStatement(t: Tree): Boolean = {
-      def isUninterestingSymbol(sym: Symbol): Boolean =
-        sym != null && (
-          sym.isConstructor ||
-          sym.hasPackageFlag ||
-          sym.isPackageObjectOrClass ||
-          sym == BoxedUnitClass ||
-          sym == AnyClass ||
-          sym == AnyRefClass ||
-          sym == AnyValClass
-        )
-      def isUninterestingType(tpe: Type): Boolean =
-        tpe != null && (
-          isUnitType(tpe) ||
-          tpe.typeSymbol.isBottomClass ||
-          tpe =:= UnitTpe ||
-          tpe =:= BoxedUnitTpe ||
-          isTrivialTopType(tpe)
-        )
-      // The quirk of typechecking if is that the LUB often results in boring types.
-      // Parser adds suppressing attachment on `if (b) expr` when user has `-Wnonunit-if:false`.
-      def checkInterestingShapes(t: Tree): Boolean =
-        t match {
-          case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) || checkInterestingShapes(elsepart) // either or
-          //case Block(_, Apply(label, Nil)) if label.symbol != null && nme.isLoopHeaderLabel(label.symbol.name) => false
-          case Block(_, res) => checkInterestingShapes(res)
-          case Match(_, cases) => cases.exists(k => checkInterestingShapes(k.body))
-          case _ => checksForInterestingResult(t)
-        }
-      // tests for various flavors of blandness in expressions.
-      def checksForInterestingResult(t: Tree): Boolean = (
-           !t.isDef && !treeInfo.isPureDef(t)     // ignore defs
-        && !isUninterestingSymbol(t.symbol)       // ctors, package, Unit, Any
-        && !isUninterestingType(t.tpe)            // bottom types, Unit, Any
-        && !treeInfo.isThisTypeResult(t)          // buf += x
-        && !treeInfo.isSuperConstrCall(t)         // just a thing
-        && !treeInfo.hasExplicitUnit(t)           // suppressed by explicit expr: Unit
-        && !treeInfo.isJavaApplication(t)         // Java methods are inherently side-effecting
-        && !treeInfo.isLazyMember(t)              // side effect forcing a lazy val
-      )
-      // begin checkInterestingResultInStatement
-      settings.warnNonUnitStatement.value && checkInterestingShapes(t) && {
+    private def checkInterestingResultInStatement(t: Tree): Boolean =
+      settings.warnNonUnitStatement.value && treeInfo.isInterestingExpression(t) && {
         val where = t match {
           case Block(_, res) => res
           case If(_, thenpart, Literal(Constant(()))) =>
@@ -1820,7 +1778,6 @@ abstract class RefChecks extends Transform {
         refchecksWarning(where.pos, msg, WarningCategory.WFlagValueDiscard)
         true
       }
-    } // end checkInterestingResultInStatement
 
     override def transform(tree: Tree): Tree = {
       val savedLocalTyper = localTyper

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

@@ -1131,33 +1131,10 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           @inline def tpdPos(transformed: Tree) = typedPos(tree.pos, mode, pt)(transformed)
           @inline def tpd(transformed: Tree)    = typed(transformed, mode, pt)
 
-          def isUninterestingSymbol(sym: Symbol): Boolean =
-            sym != null && (
-              sym.isConstructor ||
-              sym.hasPackageFlag ||
-              sym.isPackageObjectOrClass ||
-              sym == BoxedUnitClass ||
-              sym == AnyClass ||
-              sym == AnyRefClass ||
-              sym == AnyValClass
-            )
-          def isUninterestingType(tpe: Type): Boolean =
-            tpe != null && (
-              isUnitType(tpe) ||
-              tpe.typeSymbol.isBottomClass ||
-              tpe =:= UnitTpe ||
-              tpe =:= BoxedUnitTpe ||
-              isTrivialTopType(tpe)
-            )
-          // true for a value that may be discarded without compunction
-          @inline def excludeValueDiscard(): Boolean =
-            isUninterestingSymbol(tree.symbol) || isUninterestingType(tree.tpe) || treeInfo.isThisTypeResult(tree) ||
-            treeInfo.hasExplicitUnit(tree) || treeInfo.isJavaApplication(tree) ||
-            treeInfo.isLazyMember(tree) /*|| tree.exists(treeInfo.hasExplicitUnit(_))*/
-          @inline def warnValueDiscard(): Unit =
-            if (!isPastTyper && settings.warnValueDiscard.value && !excludeValueDiscard())
+          def warnValueDiscard(): Unit =
+            if (!isPastTyper && settings.warnValueDiscard.value && treeInfo.isInterestingExpression(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) {
+          def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit = if (!isPastTyper) {
             val targetIsWide = ptSym == FloatClass || ptSym == DoubleClass
             val isInharmonic = {
               def intWidened = tpSym == IntClass && ptSym == FloatClass
@@ -2742,7 +2719,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         case packed if sameWeakLubAsLub(packed) => finish(casesTyped, lub(packed))
         case packed                             =>
           val lub = weakLub(packed)
-          finish(casesTyped map (adaptCase(_, mode, lub)), lub)
+          finish(casesTyped.map(adaptCase(_, mode, lub)), lub)
       }
     }
 

src/reflect/scala/reflect/internal/TreeInfo.scala

@@ -26,15 +26,7 @@ abstract class TreeInfo {
   val global: SymbolTable
 
   import global._
-  import definitions.{
-    isBlackboxMacroBundleType,
-    isCastSymbol,
-    isUniversalMember,
-    isVarArgsList,
-    isWhiteboxContextType,
-    ThrowableClass,
-    uncheckedStableClass,
-  }
+  import definitions._
 
   /* Does not seem to be used. Not sure what it does anyway.
   def isOwnerDefinition(tree: Tree): Boolean = tree match {
@@ -411,6 +403,51 @@ abstract class TreeInfo {
   // lazy val x and object x may entail a forcing reference x, a side-effecting operation that results in x.type
   def isLazyMember(t: Tree): Boolean = t.symbol != null && (t.symbol.isLazy || t.symbol.isModule)
 
+  // For the purpose of warning about discarded values and expressions in statement position, is the tree interesting?
+  def isInterestingExpression(t: Tree): Boolean = {
+    def isUninterestingSymbol(sym: Symbol): Boolean =
+      sym != null && (
+        sym.isConstructor ||
+        sym.hasPackageFlag ||
+        sym.isPackageObjectOrClass ||
+        sym == BoxedUnitClass ||
+        sym == AnyClass ||
+        sym == AnyRefClass ||
+        sym == AnyValClass
+      )
+    def isUninterestingType(tpe: Type): Boolean =
+      tpe != null && (
+        isUnitType(tpe) ||
+        tpe.typeSymbol.isBottomClass ||
+        tpe =:= UnitTpe ||
+        tpe =:= BoxedUnitTpe ||
+        isTrivialTopType(tpe)
+      )
+    // tests for various flavors of blandness in expressions.
+    def checksForInterestingResult(t: Tree): Boolean = (
+         !t.isDef && !treeInfo.isPureDef(t)     // ignore defs
+      && !isUninterestingSymbol(t.symbol)       // ctors, package, Unit, Any
+      && !isUninterestingType(t.tpe)            // bottom types, Unit, Any
+      && !treeInfo.isThisTypeResult(t)          // buf += x
+      && !treeInfo.isSuperConstrCall(t)         // just a thing
+      && !treeInfo.hasExplicitUnit(t)           // suppressed by explicit expr: Unit or heuristic
+      && !treeInfo.isJavaApplication(t)         // Java methods are inherently side-effecting
+      && !treeInfo.isLazyMember(t)              // side effect forcing a lazy val
+    )
+    // The quirk of typechecking if is that the LUB often results in boring types.
+    // Parser adds suppressing attachment on `if (b) expr` when user has `-Wnonunit-if:false`.
+    def checkInterestingShapes(t: Tree): Boolean =
+      t match {
+        case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) || checkInterestingShapes(elsepart) // either or
+        //case Block(_, Apply(label, Nil)) if label.symbol != null && nme.isLoopHeaderLabel(label.symbol.name) => false
+        case Block(_, res) => checkInterestingShapes(res)
+        case Match(_, cases) => cases.exists(k => checkInterestingShapes(k.body))
+        case _ => checksForInterestingResult(t)
+      }
+    // begin isInterestingExpression
+    checkInterestingShapes(t)
+  }
+
   /**
    * Named arguments can transform a constructor call into a block, e.g.
    *   <init>(b = foo, a = bar)

src/reflect/scala/reflect/internal/tpe/GlbLubs.scala

@@ -253,14 +253,14 @@ private[internal] trait GlbLubs {
 
   /** Does this set of types have the same weak lub as
    *  it does regular lub? This is exposed so lub callers
-   *  can discover whether the trees they are typing will
+   *  can discover whether the trees they are typing
    *  may require further adaptation. It may return false
    *  negatives, but it will not return false positives.
    */
   def sameWeakLubAsLub(tps: List[Type]) = tps match {
     case Nil       => true
     case tp :: Nil => tp.annotations.isEmpty
-    case tps       => tps.forall(_.annotations.isEmpty) && !(tps forall isNumericValueType)
+    case tps       => tps.forall(_.annotations.isEmpty) && !tps.forall(isNumericValueType)
   }
 
   /** If the arguments are all numeric value types, the numeric

test/files/neg/t12829.check

@@ -0,0 +1,15 @@
+t12829.scala:6: warning: a type was inferred to be `Any`; this may indicate a programming error.
+  def f(i: Int, t: Boolean) = if (t) b += i else 42 // warn because if in expr position
+                              ^
+t12829.scala:11: warning: a type was inferred to be `Any`; this may indicate a programming error.
+    t match { // warn expr
+      ^
+t12829.scala:15: warning: a type was inferred to be `Any`; this may indicate a programming error.
+  def z = List(42, "hello") // warn type inference as usual
+          ^
+t12829.scala:23: warning: a type was inferred to be `Object`; this may indicate a programming error.
+    xs match { // warn because interesting type of arm
+       ^
+error: No warnings can be incurred under -Werror.
+4 warnings
+1 error

test/files/neg/t12829.scala

@@ -0,0 +1,28 @@
+
+// scalac: -Werror -Xlint:infer-any
+
+class C {
+  val b = collection.mutable.ListBuffer.empty[Int]
+  def f(i: Int, t: Boolean) = if (t) b += i else 42 // warn because if in expr position
+  def g[A](a: A): A = a
+  def h(i: Int, t: Boolean) = g(if (t) k) // warn because expr
+  def k: Int = 27
+  def n(i: Int, t: Boolean) =
+    t match { // warn expr
+      case true => b += i
+      case _ => 42
+    }
+  def z = List(42, "hello") // warn type inference as usual
+
+  def `arms are any`(xs: List[String]) =
+    (xs: Any) match { // nowarn because only top/bottom types
+      case null => throw new NullPointerException // null-check first helps static analysis of instanceOf
+      case y: Any => y
+    }
+  def `arms are any but`(xs: List[String]) =
+    xs match { // warn because interesting type of arm
+      case null => throw new NullPointerException // null-check first helps static analysis of instanceOf
+      case _ :: Nil => new Object
+      case List(_ @ _*) => ""
+    }
+}