Expanded unused warnings.

Now warns on unused private and local terms and types.
In addition it warns when a local var is read-only past
the point of its creation - something I never would have
guessed would be such a gold mine. Over 100 vars in trunk
turn into vals.

3 files changed, 165 insertions, 24 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
index 34f736e047..7f46cdfb37 100644
--- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
@@ -432,34 +432,84 @@ trait TypeDiagnostics {
       class UnusedPrivates extends Traverser {
         val defnTrees = ListBuffer[MemberDef]()
         val targets   = mutable.Set[Symbol]()
+        val setVars   = mutable.Set[Symbol]()
+        val treeTypes = mutable.Set[Type]()
+
+        def defnSymbols = defnTrees.toList map (_.symbol)
+        def localVars   = defnSymbols filter (t => t.isLocal && t.isVar)
+
+        def qualifiesTerm(sym: Symbol) = (
+             (sym.isModule || sym.isMethod || sym.isPrivateLocal || sym.isLocal)
+          && !nme.isLocalName(sym.name)
+          && !sym.isParameter
+          && !sym.isParamAccessor       // could improve this, but it's a pain
+          && !sym.isEarlyInitialized    // lots of false positives in the way these are encoded
+          && !(sym.isGetter && sym.accessed.isEarlyInitialized)
+        )
+        def qualifiesType(sym: Symbol) = !sym.isDefinedInPackage
         def qualifies(sym: Symbol) = (
              (sym ne null)
-          && (sym.isMethod || sym.isPrivateLocal && !nme.isLocalName(sym.name))
-          && !sym.isParameter
-          && !sym.isParamAccessor  // could improve this, but it's a pain
+          && (sym.isTerm && qualifiesTerm(sym) || sym.isType && qualifiesType(sym))
         )
 
         override def traverse(t: Tree): Unit = {
           t match {
-            case t: ValOrDefDef if qualifies(t.symbol) => defnTrees += t
+            case t: MemberDef if qualifies(t.symbol)   => defnTrees += t
             case t: RefTree if t.symbol ne null        => targets += t.symbol
+            case Assign(lhs, _) if lhs.symbol != null  => setVars += lhs.symbol
             case _                                     =>
           }
+          // Only record type references which don't originate within the
+          // definition of the class being referenced.
+          if (t.tpe ne null) {
+            for (tp <- t.tpe ; if !treeTypes(tp) && !currentOwner.ownerChain.contains(tp.typeSymbol)) {
+              tp match {
+                case NoType | NoPrefix    =>
+                case NullaryMethodType(_) =>
+                case MethodType(_, _)     =>
+                case _                    =>
+                  log(s"$tp referenced from $currentOwner")
+                  treeTypes += tp
+              }
+            }
+            // e.g. val a = new Foo ; new a.Bar ; don't let a be reported as unused.
+            t.tpe.prefix foreach {
+              case SingleType(_, sym) => targets += sym
+              case _                 =>
+            }
+          }
           super.traverse(t)
         }
-        def isUnused(m: Symbol): Boolean = (
-             m.isPrivate
+        def isUnused(t: Tree): Boolean = (
+          if (t.symbol.isTerm) isUnusedTerm(t.symbol)
+          else isUnusedType(t.symbol)
+        )
+        def isUnusedType(m: Symbol): Boolean = (
+              m.isType
+          && !m.isTypeParameterOrSkolem // would be nice to improve this
+          && (m.isPrivate || m.isLocal)
+          && !(treeTypes.exists(tp => tp exists (t => t.typeSymbolDirect == m)))
+        )
+        def isUnusedTerm(m: Symbol): Boolean = (
+             (m.isTerm)
+          && (m.isPrivate || m.isLocal)
           && !targets(m)
-          && !ignoreNames(m.name)               // serialization methods
-          && !isConstantType(m.info.resultType) // subject to constant inlining
+          && !(m.name == nme.WILDCARD)              // e.g. val _ = foo
+          && !ignoreNames(m.name)                   // serialization methods
+          && !isConstantType(m.info.resultType)     // subject to constant inlining
+          && !treeTypes.exists(_ contains m)        // e.g. val a = new Foo ; new a.Bar
         )
-        def unused = defnTrees.toList filter (t => isUnused(t.symbol))
+        def unusedTypes = defnTrees.toList filter (t => isUnusedType(t.symbol))
+        def unusedTerms = defnTrees.toList filter (v => isUnusedTerm(v.symbol))
+        // local vars which are never set, except those already returned in unused
+        def unsetVars = localVars filter (v => !setVars(v) && !isUnusedTerm(v))
       }
 
       def apply(unit: CompilationUnit) = {
         val p = new UnusedPrivates
         p traverse unit.body
-        p.unused foreach { defn: DefTree =>
+        val unused = p.unusedTerms
+        unused foreach { defn: DefTree =>
           val sym             = defn.symbol
           val isDefaultGetter = sym.name containsName nme.DEFAULT_GETTER_STRING
           val pos = (
@@ -470,15 +520,26 @@ trait TypeDiagnostics {
               case _               => NoPosition
             }
           )
+          val why = if (sym.isPrivate) "private" else "local"
           val what = (
             if (isDefaultGetter) "default argument"
             else if (sym.isConstructor) "constructor"
+            else if (sym.isVar || sym.isGetter && sym.accessed.isVar) "var"
+            else if (sym.isVal || sym.isGetter && sym.accessed.isVal) "val"
             else if (sym.isSetter) "setter"
-            else if (sym.isGetter) "getter"
             else if (sym.isMethod) "method"
-            else "member"
+            else if (sym.isModule) "object"
+            else "term"
           )
-          unit.warning(pos, s"private $what in ${sym.owner} is never used")
+          unit.warning(pos, s"$why $what in ${sym.owner} is never used")
+        }
+        p.unsetVars foreach { v =>
+          unit.warning(v.pos, s"local var ${v.name} in ${v.owner} is never set - it could be a val")
+        }
+        p.unusedTypes foreach { t =>
+          val sym = t.symbol
+          val why = if (sym.isPrivate) "private" else "local"
+          unit.warning(t.pos, s"$why ${sym.fullLocationString} is never used")
         }
       }
     }
diff --git a/test/files/neg/warn-unused-privates.check b/test/files/neg/warn-unused-privates.check
index c37e01106c..9c41a33e8f 100644
--- a/test/files/neg/warn-unused-privates.check
+++ b/test/files/neg/warn-unused-privates.check
@@ -4,27 +4,60 @@ warn-unused-privates.scala:2: warning: private constructor in class Bippy is nev
 warn-unused-privates.scala:4: warning: private method in class Bippy is never used
   private def boop(x: Int)            = x+a+b     // warn
               ^
-warn-unused-privates.scala:6: warning: private getter in class Bippy is never used
+warn-unused-privates.scala:6: warning: private val in class Bippy is never used
   final private val MILLIS2: Int      = 1000      // warn
                     ^
-warn-unused-privates.scala:13: warning: private getter in object Bippy is never used
+warn-unused-privates.scala:13: warning: private val in object Bippy is never used
   private val HEY_INSTANCE: Int = 1000    // warn
               ^
-warn-unused-privates.scala:41: warning: private getter in trait Accessors is never used
+warn-unused-privates.scala:35: warning: private val in class Boppy is never used
+  private val hummer = "def" // warn
+              ^
+warn-unused-privates.scala:42: warning: private var in trait Accessors is never used
   private var v1: Int = 0 // warn
               ^
-warn-unused-privates.scala:42: warning: private setter in trait Accessors is never used
+warn-unused-privates.scala:43: warning: private setter in trait Accessors is never used
   private var v2: Int = 0 // warn, never set
               ^
-warn-unused-privates.scala:43: warning: private getter in trait Accessors is never used
+warn-unused-privates.scala:44: warning: private var in trait Accessors is never used
   private var v3: Int = 0 // warn, never got
               ^
-warn-unused-privates.scala:55: warning: private default argument in trait DefaultArgs is never used
+warn-unused-privates.scala:56: warning: private default argument in trait DefaultArgs is never used
   private def bippy(x1: Int, x2: Int = 10, x3: Int = 15): Int = x1 + x2 + x3
                              ^
-warn-unused-privates.scala:55: warning: private default argument in trait DefaultArgs is never used
+warn-unused-privates.scala:56: warning: private default argument in trait DefaultArgs is never used
   private def bippy(x1: Int, x2: Int = 10, x3: Int = 15): Int = x1 + x2 + x3
                                            ^
+warn-unused-privates.scala:67: warning: local var in method f0 is never used
+    var x = 1 // warn
+        ^
+warn-unused-privates.scala:74: warning: local val in method f1 is never used
+    val b = new Outer // warn
+        ^
+warn-unused-privates.scala:84: warning: private object in object Types is never used
+  private object Dongo { def f = this } // warn
+                 ^
+warn-unused-privates.scala:94: warning: local object in method l1 is never used
+    object HiObject { def f = this } // warn
+           ^
+warn-unused-privates.scala:78: warning: local var x in method f2 is never set - it could be a val
+    var x = 100 // warn about it being a var
+        ^
+warn-unused-privates.scala:85: warning: private class Bar1 in object Types is never used
+  private class Bar1 // warn
+                ^
+warn-unused-privates.scala:87: warning: private type Alias1 in object Types is never used
+  private type Alias1 = String // warn
+               ^
+warn-unused-privates.scala:95: warning: local class Hi is never used
+    class Hi { // warn
+          ^
+warn-unused-privates.scala:99: warning: local class DingDongDoobie is never used
+    class DingDongDoobie // warn
+          ^
+warn-unused-privates.scala:102: warning: local type OtherThing is never used
+    type OtherThing = String // warn
+         ^
 error: No warnings can be incurred under -Xfatal-warnings.
-9 warnings found
+20 warnings found
 one error found
diff --git a/test/files/neg/warn-unused-privates.scala b/test/files/neg/warn-unused-privates.scala
index 1ac272357f..cb6e946a34 100644
--- a/test/files/neg/warn-unused-privates.scala
+++ b/test/files/neg/warn-unused-privates.scala
@@ -18,8 +18,10 @@ class B1(msg: String) extends A(msg)
 class B2(msg0: String) extends A(msg0)
 class B3(msg0: String) extends A("msg")
 
-/*** Early defs full of noise due to SI-6595. ***/
-/***
+/*** Early defs warnings disabled primarily due to SI-6595.
+ *   The test case is here to assure we aren't issuing false positives;
+ *   the ones labeled "warn" don't warn.
+ ***/
 class Boppy extends {
   private val hmm: String = "abc"       // no warn, used in early defs
   private val hom: String = "def"       // no warn, used in body
@@ -35,7 +37,6 @@ class Boppy extends {
   private final val bum   = "ghi"       // no warn, might have been (was) inlined
   final val bum2          = "ghi"       // no warn, same
 }
-***/
 
 trait Accessors {
   private var v1: Int = 0 // warn
@@ -56,3 +57,49 @@ trait DefaultArgs {
 
   def boppy() = bippy(5, 100, 200)
 }
+
+class Outer {
+  class Inner
+}
+
+trait Locals {
+  def f0 = {
+    var x = 1 // warn
+    var y = 2
+    y = 3
+    y + y
+  }
+  def f1 = {
+    val a = new Outer // no warn
+    val b = new Outer // warn
+    new a.Inner
+  }
+  def f2 = {
+    var x = 100 // warn about it being a var
+    x
+  }
+}
+
+object Types {
+  private object Dongo { def f = this } // warn
+  private class Bar1 // warn
+  private class Bar2 // no warn
+  private type Alias1 = String // warn
+  private type Alias2 = String // no warn
+  def bippo = (new Bar2).toString
+
+  def f(x: Alias2) = x.length
+
+  def l1() = {
+    object HiObject { def f = this } // warn
+    class Hi { // warn
+      def f1: Hi = new Hi
+      def f2(x: Hi) = x
+    }
+    class DingDongDoobie // warn
+    class Bippy // no warn
+    type Something = Bippy // no warn
+    type OtherThing = String // warn
+    (new Bippy): Something
+  }
+}