SI-8040 Warn unused flags

Introduce `-Ywarn-unused:x,y,z` and exploit `-Ywarn-unused:patvars`.

Although the tree attachment for shielding patvars from warnings
is not structural, sneaking the settings flag into the reflection
internal TreeGen is awkward.

Add test to ensure isolation of patvars warning from others.

`-Ywarn-unused-import` is an alias for `-Ywarn-unused:imports`.

`-Xlint:unused` is an alias for `-Ywarn-unused`, but not enabled
yet. The help text advises to use `-Ywarn-unused`. The future can
decide if `-Xlint:unused-imports` is warranted.

12 files changed, 126 insertions, 20 deletions

diff --git a/src/compiler/scala/reflect/reify/codegen/GenAnnotationInfos.scala b/src/compiler/scala/reflect/reify/codegen/GenAnnotationInfos.scala
index 2dded48251..089f07de06 100644
--- a/src/compiler/scala/reflect/reify/codegen/GenAnnotationInfos.scala
+++ b/src/compiler/scala/reflect/reify/codegen/GenAnnotationInfos.scala
@@ -10,7 +10,6 @@ trait GenAnnotationInfos {
   // however, when reifying free and tough types, we're forced to reify annotation infos as is
   // why is that bad? take a look inside
   def reifyAnnotationInfo(ann: AnnotationInfo): Tree = {
-    //val reifiedArgs = ann.args map   // SI-8915
     ann.args.foreach { arg =>
       val saved1 = reifyTreeSymbols
       val saved2 = reifyTreeTypes
diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
index 762456c9c9..a2c37b59fb 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
@@ -371,4 +371,6 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL {
         Typed(New(anonClass.tpe), TypeTree(fun.tpe)))
     }
   }
+
+  override def isPatVarWarnable = settings.warnUnusedPatVars
 }
diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
index ba1659d274..68bd663ab8 100644
--- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
+++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
@@ -1950,7 +1950,7 @@ self =>
       def pattern2(): Tree = {
         val p = pattern3()
 
-        if (in.token != AT) p
+        if (in.token != AT) p             // var pattern upgraded later to x @ _
         else p match {
           case Ident(nme.WILDCARD) =>
             in.nextToken()
@@ -1962,6 +1962,7 @@ self =>
               val t = Bind(name, body)
               body match {
                 case Ident(nme.WILDCARD) => t updateAttachment AtBoundIdentifierAttachment
+                case _ if !settings.warnUnusedPatVars => t updateAttachment AtBoundIdentifierAttachment
                 case _ => t
               }
             }
diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala
index 87534656f9..1a16480149 100644
--- a/src/compiler/scala/tools/nsc/settings/Warnings.scala
+++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala
@@ -20,10 +20,35 @@ trait Warnings {
   val warnDeadCode         = BooleanSetting("-Ywarn-dead-code", "Warn when dead code is identified.")
   val warnValueDiscard     = BooleanSetting("-Ywarn-value-discard", "Warn when non-Unit expression results are unused.")
   val warnNumericWiden     = BooleanSetting("-Ywarn-numeric-widen", "Warn when numerics are widened.")
-  // SI-7712, SI-7707 warnUnused not quite ready for prime-time
-  val warnUnused           = BooleanSetting("-Ywarn-unused", "Warn when local and private vals, vars, defs, and types are unused.")
-  // currently considered too noisy for general use
-  val warnUnusedImport     = BooleanSetting("-Ywarn-unused-import", "Warn when imports are unused.")
+
+  object UnusedWarnings extends MultiChoiceEnumeration {
+    val Imports   = Choice("imports",   "Warn if an import selector is not referenced.")
+    val PatVars   = Choice("patvars",   "Warn if a variable bound in a pattern is unused.")
+    val Privates  = Choice("privates",  "Warn if a private member is unused.")
+    val Locals    = Choice("locals",    "Warn if a local definition is unused.")
+    val Params    = Choice("params",    "Warn if a value parameter is unused.")
+    val Implicits = Choice("implicits", "Warn if an implicit parameter is unused.")
+  }
+
+  // The -Ywarn-unused warning group.
+  val warnUnused = MultiChoiceSetting(
+    name    = "-Ywarn-unused",
+    helpArg = "warning",
+    descr   = "Enable or disable specific `unused' warnings",
+    domain  = UnusedWarnings,
+    default = Some(List("_"))
+  )
+
+  def warnUnusedImport    = warnUnused contains UnusedWarnings.Imports
+  def warnUnusedPatVars   = warnUnused contains UnusedWarnings.PatVars
+  def warnUnusedPrivates  = warnUnused contains UnusedWarnings.Privates
+  def warnUnusedLocals    = warnUnused contains UnusedWarnings.Locals
+  def warnUnusedParams    = warnUnused contains UnusedWarnings.Params
+  def warnUnusedImplicits = warnUnused contains UnusedWarnings.Implicits
+
+  BooleanSetting("-Ywarn-unused-import", "Warn when imports are unused.") withPostSetHook { s =>
+    warnUnused.add(s"${if (s) "" else "-"}imports")
+  } // withDeprecationMessage s"Enable -Ywarn-unused:imports"
 
   val warnExtraImplicit    = BooleanSetting("-Ywarn-extra-implicit", "Warn when more than one implicit parameter section is defined.")
 
@@ -60,6 +85,8 @@ trait Warnings {
     val UnsoundMatch           = LintWarning("unsound-match",             "Pattern match may not be typesafe.")
     val StarsAlign             = LintWarning("stars-align",               "Pattern sequence wildcard must align with sequence component.")
     val Constant               = LintWarning("constant",                  "Evaluation of a constant arithmetic expression results in an error.")
+    //val Unused                 = LintWarning("unused",                    "Warn when private and local definitions are unused.")
+    val Unused                 = LintWarning("unused",                    "Use -Ywarn-unused to warn when private and local definitions are unused.")
 
     def allLintWarnings = values.toSeq.asInstanceOf[Seq[LintWarning]]
   }
@@ -82,6 +109,7 @@ trait Warnings {
   def warnUnsoundMatch           = lint contains UnsoundMatch
   def warnStarsAlign             = lint contains StarsAlign
   def warnConstant               = lint contains Constant
+  def lintUnused                 = lint contains Unused
 
   // Lint warnings that are currently -Y, but deprecated in that usage
   @deprecated("Use warnAdaptedArgs", since="2.11.2")
@@ -101,7 +129,9 @@ trait Warnings {
     helpArg = "warning",
     descr   = "Enable or disable specific warnings",
     domain  = LintWarnings,
-    default = Some(List("_")))
+    default = Some(List("_"))) //.withPostSetHook (s => if (s contains Unused) warnUnused.add("_"))
+
+  // restore -Xlint:unused hook when SI-8040 is complete
 
   allLintWarnings foreach {
     case w if w.yAliased =>
diff --git a/src/compiler/scala/tools/nsc/typechecker/Analyzer.scala b/src/compiler/scala/tools/nsc/typechecker/Analyzer.scala
index 323fe1c171..b8ef439e03 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Analyzer.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Analyzer.scala
@@ -104,7 +104,7 @@ trait Analyzer extends AnyRef
           for (workItem <- unit.toCheck) workItem()
           if (settings.warnUnusedImport)
             warnUnusedImports(unit)
-          if (settings.warnUnused)
+          if (settings.warnUnused.isSetByUser)
             typer checkUnused unit
         }
         finally {
diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
index f344364a75..b263a0fffd 100644
--- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
@@ -505,7 +505,7 @@ trait TypeDiagnostics {
           // 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)) {
+            for (tp <- t.tpe if !treeTypes(tp) && !currentOwner.ownerChain.contains(tp.typeSymbol)) {
               tp match {
                 case NoType | NoPrefix    =>
                 case NullaryMethodType(_) =>
@@ -557,12 +557,17 @@ trait TypeDiagnostics {
         def unsetVars = localVars filter (v => !setVars(v) && !isUnusedTerm(v))
       }
 
-      def apply(unit: CompilationUnit) = {
+      private def warningsEnabled: Boolean = {
+        val ss = settings
+        import ss._
+        warnUnusedPatVars || warnUnusedPrivates || warnUnusedLocals || warnUnusedParams || warnUnusedImplicits
+      }
+
+      def apply(unit: CompilationUnit): Unit = if (warningsEnabled) {
         val p = new UnusedPrivates
         p traverse unit.body
-        val unused = p.unusedTerms
-        unused foreach { defn: DefTree =>
-          val sym             = defn.symbol
+        for (defn: DefTree <- p.unusedTerms) {
+          val sym = defn.symbol
           val pos = (
             if (defn.pos.isDefined) defn.pos
             else if (sym.pos.isDefined) sym.pos
@@ -591,10 +596,10 @@ trait TypeDiagnostics {
           )
           reporter.warning(pos, s"$why $what in ${sym.owner} is never used")
         }
-        p.unsetVars foreach { v =>
+        for (v <- p.unsetVars) {
           reporter.warning(v.pos, s"local var ${v.name} in ${v.owner} is never set - it could be a val")
         }
-        p.unusedTypes foreach { t =>
+        for (t <- p.unusedTypes) {
           val sym = t.symbol
           val why = if (sym.isPrivate) "private" else "local"
           reporter.warning(t.pos, s"$why ${sym.fullLocationString} is never used")
diff --git a/src/compiler/scala/tools/reflect/FormatInterpolator.scala b/src/compiler/scala/tools/reflect/FormatInterpolator.scala
index bbb2987679..857b733f59 100644
--- a/src/compiler/scala/tools/reflect/FormatInterpolator.scala
+++ b/src/compiler/scala/tools/reflect/FormatInterpolator.scala
@@ -19,7 +19,6 @@ abstract class FormatInterpolator {
   @inline private def truly(body: => Unit): Boolean = { body ; true }
   @inline private def falsely(body: => Unit): Boolean = { body ; false }
 
-  //private def fail(msg: String) = c.abort(c.enclosingPosition, msg)
   private def bail(msg: String) = global.abort(msg)
 
   def interpolate: Tree = c.macroApplication match {
@@ -94,7 +93,7 @@ abstract class FormatInterpolator {
               case '\f' => "\\f"
               case '\r' => "\\r"
               case '\"' => "$" /* avoid lint warn */ +
-                "{'\"'} or a triple-quoted literal \"\"\"with embedded \" or \\u0022\"\"\""  // $" in future?
+                "{'\"'} or a triple-quoted literal \"\"\"with embedded \" or \\u0022\"\"\""
               case '\'' => "'"
               case '\\' => """\\"""
               case x    => "\\u%04x" format x
diff --git a/src/reflect/scala/reflect/internal/TreeGen.scala b/src/reflect/scala/reflect/internal/TreeGen.scala
index 373758d0a5..ade9ee84ac 100644
--- a/src/reflect/scala/reflect/internal/TreeGen.scala
+++ b/src/reflect/scala/reflect/internal/TreeGen.scala
@@ -899,8 +899,8 @@ abstract class TreeGen {
       case Ident(name) if treeInfo.isVarPattern(tree) && name != nme.WILDCARD =>
         atPos(tree.pos) {
           val b = Bind(name, atPos(tree.pos.focus) (Ident(nme.WILDCARD)))
-          if (forFor) b updateAttachment AtBoundIdentifierAttachment
-          else b
+          if (!forFor && isPatVarWarnable) b
+          else b updateAttachment AtBoundIdentifierAttachment
         }
       case Typed(id @ Ident(name), tpt) if treeInfo.isVarPattern(id) && name != nme.WILDCARD =>
         atPos(tree.pos.withPoint(id.pos.point)) {
@@ -922,7 +922,13 @@ abstract class TreeGen {
         tree
     }
   }
+
+  /** Can be overridden to depend on settings.warnUnusedPatvars. */
+  def isPatVarWarnable: Boolean = true
+
+  /** Not in for comprehensions, whether to warn unused pat vars depends on flag. */
   object patvarTransformer       extends PatvarTransformer(forFor = false)
+
   /** Tag pat vars in for comprehensions. */
   object patvarTransformerForFor extends PatvarTransformer(forFor = true)
 
diff --git a/test/files/neg/warn-unused-imports/warn-unused-imports_2.scala b/test/files/neg/warn-unused-imports/warn-unused-imports_2.scala
index ded1186209..56ad3393a1 100644
--- a/test/files/neg/warn-unused-imports/warn-unused-imports_2.scala
+++ b/test/files/neg/warn-unused-imports/warn-unused-imports_2.scala
@@ -96,7 +96,7 @@ trait Warn {
 trait Nested {
   {
     import p1._   // warn
-    trait Warn {  // warn about unused local trait for good measure
+    trait Warn {  // don't warn about unused local trait with -Ywarn-unused:imports
       import p2._
       println(new A)
       println("abc".bippy)
diff --git a/test/files/neg/warn-unused-patvars.check b/test/files/neg/warn-unused-patvars.check
new file mode 100644
index 0000000000..002f7a07ca
--- /dev/null
+++ b/test/files/neg/warn-unused-patvars.check
@@ -0,0 +1,12 @@
+warn-unused-patvars.scala:7: warning: private val x in trait Boundings is never used
+  private val x = 42                      // warn, sanity check
+              ^
+warn-unused-patvars.scala:26: warning: local val x in method v is never used
+    val D(x) = d                          // warn, fixme
+         ^
+warn-unused-patvars.scala:30: warning: local val x in method w is never used
+    val D(x @ _) = d                      // warn, fixme (valdef pos is different)
+         ^
+error: No warnings can be incurred under -Xfatal-warnings.
+three warnings found
+one error found
diff --git a/test/files/neg/warn-unused-patvars.flags b/test/files/neg/warn-unused-patvars.flags
new file mode 100644
index 0000000000..d5bd86a658
--- /dev/null
+++ b/test/files/neg/warn-unused-patvars.flags
@@ -0,0 +1 @@
+-Ywarn-unused:-patvars,_ -Xfatal-warnings
diff --git a/test/files/neg/warn-unused-patvars.scala b/test/files/neg/warn-unused-patvars.scala
new file mode 100644
index 0000000000..6f4620c0c7
--- /dev/null
+++ b/test/files/neg/warn-unused-patvars.scala
@@ -0,0 +1,51 @@
+
+case class C(a: Int, b: String, c: Option[String])
+case class D(a: Int)
+
+trait Boundings {
+
+  private val x = 42                      // warn, sanity check
+
+  def c = C(42, "hello", Some("world"))
+  def d = D(42)
+
+  def f() = {
+    val C(x, y, Some(z)) = c              // no warn
+    17
+  }
+  def g() = {
+    val C(x @ _, y @ _, Some(z @ _)) = c  // no warn
+    17
+  }
+  def h() = {
+    val C(x @ _, y @ _, z @ Some(_)) = c  // no warn for z?
+    17
+  }
+
+  def v() = {
+    val D(x) = d                          // warn, fixme
+    17
+  }
+  def w() = {
+    val D(x @ _) = d                      // warn, fixme (valdef pos is different)
+    17
+  }
+
+}
+
+trait Forever {
+  def f = {
+    val t = Option((17, 42))
+    for {
+      ns <- t
+      (i, j) = ns                        // no warn
+    } yield (i + j)
+  }
+  def g = {
+    val t = Option((17, 42))
+    for {
+      ns <- t
+      (i, j) = ns                        // no warn
+    } yield 42
+  }
+}