SI-8525 Add -Xlint:-warn-missing-interpolator

Turn off lint warnings with negating prefix, and add a lint-only
warning for the infamously nagging "Did you forget the interpolator?"

That message is made more dignified.

Without `-Xlint:false`, there is no mechanism to turn off
anonymous linters once `-Xlint` is selected.

14 files changed, 123 insertions, 59 deletions

diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala
index b399c0e024..dc49e8b822 100644
--- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala
@@ -211,11 +211,8 @@ class MutableSettings(val errorFn: String => Unit)
     add(new ChoiceSetting(name, helpArg, descr, choices, default))
   def IntSetting(name: String, descr: String, default: Int, range: Option[(Int, Int)], parser: String => Option[Int]) = add(new IntSetting(name, descr, default, range, parser))
   def MultiStringSetting(name: String, arg: String, descr: String) = add(new MultiStringSetting(name, arg, descr))
-  def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit = () => ()): MultiChoiceSetting = {
-    val fullChoix = choices.mkString(": ", ",", ".")
-    val fullDescr = s"$descr$fullChoix"
-    add(new MultiChoiceSetting(name, helpArg, fullDescr, choices, default))
-  }
+  def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit = () => ()) =
+    add(new MultiChoiceSetting(name, helpArg, descr, choices, default))
   def OutputSetting(outputDirs: OutputDirs, default: String) = add(new OutputSetting(outputDirs, default))
   def PhasesSetting(name: String, descr: String, default: String = "") = add(new PhasesSetting(name, descr, default))
   def StringSetting(name: String, arg: String, descr: String, default: String) = add(new StringSetting(name, arg, descr, default))
@@ -564,8 +561,33 @@ class MutableSettings(val errorFn: String => Unit)
     arg: String,
     descr: String,
     override val choices: List[String],
-    override val default: () => Unit
-  ) extends MultiStringSetting(name, arg, descr)
+    val default: () => Unit
+  ) extends MultiStringSetting(name, arg, s"$descr${ choices.mkString(": ", ",", ".") }") {
+
+    def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'")
+    def choosing = choices.nonEmpty
+    def isChoice(s: String) = (s == "_") || (choices contains (s stripPrefix "-"))
+    def wildcards = choices // filter (!_.isSetByUser)
+
+    override protected def tts(args: List[String], halting: Boolean) = {
+      val total = collection.mutable.ListBuffer.empty[String] ++ value
+      def tryArg(arg: String) = arg match {
+        case "_" if choosing               => wildcards foreach (total += _)
+        case s if !choosing || isChoice(s) => total += s
+        case s                             => badChoice(s, name)
+      }
+      def stoppingAt(arg: String) = (arg startsWith "-") || (choosing && !isChoice(arg))
+      def loop(args: List[String]): List[String] = args match {
+        case arg :: _ if halting && stoppingAt(arg) => args
+        case arg :: rest                            => tryArg(arg) ; loop(rest)
+        case Nil                                    => Nil
+      }
+      val rest = loop(args)
+      if (rest.size == args.size) default()       // if no arg consumed, trigger default action
+      else value = total.toList                   // update once
+      Some(rest)
+    }
+  }
 
   /** A setting that accumulates all strings supplied to it,
    *  until it encounters one starting with a '-'.
@@ -577,29 +599,18 @@ class MutableSettings(val errorFn: String => Unit)
   extends Setting(name, descr) with Clearable {
     type T = List[String]
     protected var v: T = Nil
-    val default: () => Unit = () => ()            // no natural default
-    def appendToValue(str: String) { value ++= List(str) }
-    def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'")
+    def appendToValue(str: String) = value ++= List(str)
 
-    private def tts(args: List[String], verify: Boolean) = {
-      def tryArg(arg: String) = arg match {
-        case "_" if choices.nonEmpty                      => choices foreach appendToValue
-        case s if choices.isEmpty || (choices contains s) => appendToValue(s)
-        case s                                            => badChoice(s, name)
-      }
+    // try to set. halting means halt at first non-arg
+    protected def tts(args: List[String], halting: Boolean) = {
       def loop(args: List[String]): List[String] = args match {
+        case arg :: rest => if (halting && (arg startsWith "-")) args else { appendToValue(arg) ; loop(rest) }
         case Nil         => Nil
-        case arg :: _
-          if (arg startsWith "-") || (!verify && choices.nonEmpty && !(choices contains arg))
-                         => args
-        case arg :: rest => tryArg(arg) ; loop(rest)
       }
-      val rest = loop(args)
-      if (rest.size == args.size) default()       // if no arg consumed, trigger default action
-      Some(rest)
+      Some(loop(args))
     }
-    def tryToSet(args: List[String])                  = tts(args, verify = false)
-    override def tryToSetColon(args: List[String])    = tts(args, verify = true)
+    def tryToSet(args: List[String])                  = tts(args, halting = true)
+    override def tryToSetColon(args: List[String])    = tts(args, halting = false)
     override def tryToSetFromPropertyValue(s: String) = tryToSet(s.trim.split(',').toList) // used from ide
 
     def clear(): Unit         = (v = Nil)
diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala
index ae2696accf..f1ed45a2a0 100644
--- a/src/compiler/scala/tools/nsc/settings/Warnings.scala
+++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala
@@ -31,9 +31,10 @@ trait Warnings {
     warnNullaryOverride,
     warnNullaryUnit,
     warnAdaptedArgs,
-    warnInferAny
+    warnInferAny,
     // warnUnused       SI-7712, SI-7707 warnUnused not quite ready for prime-time
     // warnUnusedImport currently considered too noisy for general use
+    warnMissingInterpolator
   )
 
   private lazy val warnSelectNullable = BooleanSetting("-Xcheck-null", "This option is obsolete and does nothing.")
@@ -50,23 +51,32 @@ trait Warnings {
   val warnUnused           = BooleanSetting   ("-Ywarn-unused", "Warn when local and private vals, vars, defs, and types are are unused")
   val warnUnusedImport     = BooleanSetting   ("-Ywarn-unused-import", "Warn when imports are unused")
 
+  // Lint warnings that are not -Y
+  val warnMissingInterpolator = new BooleanSetting("warn-missing-interpolator", "Warn when a string literal appears to be missing an interpolator id.")
+
   // Warning groups.
   val lint                 = {
     // Boolean setting for testing if lint is on; not "added" to option processing
     val xlint = new BooleanSetting("-Xlint", "Enable recommended additional warnings.")
-    val lintables = (lintWarnings map (_.name drop 2)).sorted
+    def lintables = (lintWarnings map (_.name stripPrefix "-Y")).sorted
+    def isAnon(b: BooleanSetting) = !(b.name startsWith "-")
+    def setPolitely(b: BooleanSetting, v: Boolean) = if (!b.isSetByUser) b.value = v
+    def set(w: String, v: Boolean) = lintWarnings find (_.name.stripPrefix("-Y") == w) foreach (b => setPolitely(b, v))
+    val Neg = "-"
     def propagate(ss: List[String]): Unit = ss match {
-      case w :: rest => lintWarnings find (_.name == s"-Y$w") foreach (_.value = true) ; propagate(rest)
-      case Nil         => ()
+      case w :: rest => if (w startsWith Neg) set(w stripPrefix Neg, false) else set(w, true) ; propagate(rest)
+      case Nil       => ()
     }
-    def enableAll(): Unit = {   // enable lint and the group
+    // enable lint and the group, honoring previous -Y settings
+    def enableAll(): Unit = {
       xlint.value = true
-      for (s <- lintWarnings if !s.isSetByUser) s.value = true
+      for (s <- lintWarnings) setPolitely(s, true)
     }
     // The command option
-    MultiChoiceSetting("-Xlint", "warning", "Enable recommended additional warnings", lintables, enableAll) withPostSetHook { x =>
+    MultiChoiceSetting("-Xlint", "warning", "Enable recommended additional warnings", choices = lintables, default = enableAll) withPostSetHook { x =>
       propagate(x.value)      // enabling the selections (on each append to value)
       xlint.value = true      // only enables lint, not the group
+      for (b <- lintWarnings if isAnon(b) && !b.isSetByUser) b.value = true   // init anonymous settings (but not if disabled)
     }
     xlint
   }
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 2c31ef2e8e..efc121d479 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -5099,7 +5099,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         def isPlausible(m: Symbol) = m.alternatives exists (m => requiresNoArgs(m.info))
 
         def maybeWarn(s: String): Unit = {
-          def warn(message: String)         = context.warning(lit.pos, s"$message Did you forget the interpolator?")
+          def warn(message: String)         = context.warning(lit.pos, s"possible missing interpolator: $message")
           def suspiciousSym(name: TermName) = context.lookupSymbol(name, _ => true).symbol
           def suspiciousExpr                = InterpolatorCodeRegex findFirstIn s
           def suspiciousIdents              = InterpolatorIdentRegex findAllIn s map (s => suspiciousSym(s drop 1))
@@ -5107,9 +5107,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           // heuristics - no warning on e.g. a string with only "$asInstanceOf"
           if (s contains ' ') (
             if (suspiciousExpr.nonEmpty)
-              warn("That looks like an interpolated expression!") // "${...}"
+              warn("detected an interpolated expression") // "${...}"
             else
-              suspiciousIdents find isPlausible foreach (sym => warn(s"`$$${sym.name}` looks like an interpolated identifier!")) // "$id"
+              suspiciousIdents find isPlausible foreach (sym => warn(s"detected interpolated identifier `$$${sym.name}`")) // "$id"
           )
         }
         lit match {
@@ -5119,7 +5119,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
       }
 
       def typedLiteral(tree: Literal) = {
-        if (settings.lint) warnMissingInterpolator(tree)
+        if (settings.warnMissingInterpolator) warnMissingInterpolator(tree)
 
         tree setType (if (tree.value.tag == UnitTag) UnitTpe else ConstantType(tree.value))
       }
diff --git a/test/files/neg/forgot-interpolator.check b/test/files/neg/forgot-interpolator.check
index 8988458982..8e75350518 100644
--- a/test/files/neg/forgot-interpolator.check
+++ b/test/files/neg/forgot-interpolator.check
@@ -1,25 +1,25 @@
-forgot-interpolator.scala:4: warning: `$bippy` looks like an interpolated identifier! Did you forget the interpolator?
+forgot-interpolator.scala:4: warning: possible missing interpolator: detected interpolated identifier `$bippy`
   def f = "Put the $bippy in the $bippy!" // warn 1
           ^
-forgot-interpolator.scala:14: warning: That looks like an interpolated expression! Did you forget the interpolator?
+forgot-interpolator.scala:14: warning: possible missing interpolator: detected an interpolated expression
   def f = """Put the ${println("bippy")} in the bippy!""" // warn 2
           ^
-forgot-interpolator.scala:30: warning: `$beppo` looks like an interpolated identifier! Did you forget the interpolator?
+forgot-interpolator.scala:30: warning: possible missing interpolator: detected interpolated identifier `$beppo`
       def f = "$beppo was a marx bros who saw dollars."  // warn 3
               ^
-forgot-interpolator.scala:34: warning: `$aleppo` looks like an interpolated identifier! Did you forget the interpolator?
+forgot-interpolator.scala:34: warning: possible missing interpolator: detected interpolated identifier `$aleppo`
     def f = "$aleppo is a pepper and a city."     // warn 4
             ^
-forgot-interpolator.scala:47: warning: `$hippo` looks like an interpolated identifier! Did you forget the interpolator?
+forgot-interpolator.scala:47: warning: possible missing interpolator: detected interpolated identifier `$hippo`
     def h = "$hippo takes an implicit"  // warn 6
             ^
-forgot-interpolator.scala:88: warning: `$groucho` looks like an interpolated identifier! Did you forget the interpolator?
+forgot-interpolator.scala:88: warning: possible missing interpolator: detected interpolated identifier `$groucho`
     def f2 = "I salute $groucho" // warn 7
              ^
-forgot-interpolator.scala:89: warning: `$dingo` looks like an interpolated identifier! Did you forget the interpolator?
+forgot-interpolator.scala:89: warning: possible missing interpolator: detected interpolated identifier `$dingo`
     def f3 = "I even salute $dingo" // warn 8
              ^
-forgot-interpolator.scala:90: warning: `$calico` looks like an interpolated identifier! Did you forget the interpolator?
+forgot-interpolator.scala:90: warning: possible missing interpolator: detected interpolated identifier `$calico`
     def f4 = "I also salute $calico" // warn 9
              ^
 error: No warnings can be incurred under -Xfatal-warnings.
diff --git a/test/files/neg/t7848-interp-warn.check b/test/files/neg/t7848-interp-warn.check
index b7df6d8ce2..4cf9d55ffd 100644
--- a/test/files/neg/t7848-interp-warn.check
+++ b/test/files/neg/t7848-interp-warn.check
@@ -1,10 +1,10 @@
-t7848-interp-warn.scala:8: warning: `$foo` looks like an interpolated identifier! Did you forget the interpolator?
+t7848-interp-warn.scala:8: warning: possible missing interpolator: detected interpolated identifier `$foo`
     "An important $foo message!"
     ^
-t7848-interp-warn.scala:12: warning: That looks like an interpolated expression! Did you forget the interpolator?
+t7848-interp-warn.scala:12: warning: possible missing interpolator: detected an interpolated expression
     "A doubly important ${foo * 2} message!"
     ^
-t7848-interp-warn.scala:16: warning: `$bar` looks like an interpolated identifier! Did you forget the interpolator?
+t7848-interp-warn.scala:16: warning: possible missing interpolator: detected interpolated identifier `$bar`
   def j = s"Try using '${ "something like $bar" }' instead."  // warn
                           ^
 error: No warnings can be incurred under -Xfatal-warnings.
diff --git a/test/files/neg/t8525.check b/test/files/neg/t8525.check
new file mode 100644
index 0000000000..554ab23253
--- /dev/null
+++ b/test/files/neg/t8525.check
@@ -0,0 +1,6 @@
+t8525.scala:9: warning: private[this] value name in class X shadows mutable name inherited from class Named.  Changes to name will not be visible within class X - you may want to give them distinct names.
+  override def toString = name // shadowing mutable var name
+                          ^
+error: No warnings can be incurred under -Xfatal-warnings.
+one warning found
+one error found
diff --git a/test/files/neg/t8525.flags b/test/files/neg/t8525.flags
new file mode 100644
index 0000000000..f19af1f717
--- /dev/null
+++ b/test/files/neg/t8525.flags
@@ -0,0 +1 @@
+-Xfatal-warnings -Xlint:-warn-missing-interpolator
diff --git a/test/files/neg/t8525.scala b/test/files/neg/t8525.scala
new file mode 100644
index 0000000000..7bed04904f
--- /dev/null
+++ b/test/files/neg/t8525.scala
@@ -0,0 +1,10 @@
+
+class Named(var name: String)
+
+class X(name: String) extends Named(name) {
+  def x = "Hi, $name"   // missing interp
+  def f(p: (Int, Int)): Int = p._1 * p._2
+  def g = f(3, 4)       // adapted
+  def u: Unit = ()      // unitarian universalist
+  override def toString = name // shadowing mutable var name
+}
diff --git a/test/files/neg/t8610-arg.check b/test/files/neg/t8610-arg.check
index f2879b0d45..4f194ce84a 100644
--- a/test/files/neg/t8610-arg.check
+++ b/test/files/neg/t8610-arg.check
@@ -1,9 +1,12 @@
-t8610-arg.scala:3: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator?
+t8610-arg.scala:5: warning: possible missing interpolator: detected interpolated identifier `$name`
   def x = "Hi, $name"   // missing interp
           ^
-t8610-arg.scala:6: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead
+t8610-arg.scala:9: warning: private[this] value name in class X shadows mutable name inherited from class Named.  Changes to name will not be visible within class X - you may want to give them distinct names.
+  override def toString = name // shadowing mutable var name
+                          ^
+t8610-arg.scala:8: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead
   def u: Unit = ()      // unitarian universalist
       ^
 error: No warnings can be incurred under -Xfatal-warnings.
-two warnings found
+three warnings found
 one error found
diff --git a/test/files/neg/t8610-arg.scala b/test/files/neg/t8610-arg.scala
index 494a3354da..7bed04904f 100644
--- a/test/files/neg/t8610-arg.scala
+++ b/test/files/neg/t8610-arg.scala
@@ -1,7 +1,10 @@
 
-case class X(name: String) {
+class Named(var name: String)
+
+class X(name: String) extends Named(name) {
   def x = "Hi, $name"   // missing interp
   def f(p: (Int, Int)): Int = p._1 * p._2
   def g = f(3, 4)       // adapted
   def u: Unit = ()      // unitarian universalist
+  override def toString = name // shadowing mutable var name
 }
diff --git a/test/files/neg/t8610.check b/test/files/neg/t8610.check
index 7a18e55127..334a947549 100644
--- a/test/files/neg/t8610.check
+++ b/test/files/neg/t8610.check
@@ -1,15 +1,18 @@
-t8610.scala:3: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator?
+t8610.scala:5: warning: possible missing interpolator: detected interpolated identifier `$name`
   def x = "Hi, $name"   // missing interp
           ^
-t8610.scala:5: warning: Adapting argument list by creating a 2-tuple: this may not be what you want.
+t8610.scala:7: warning: Adapting argument list by creating a 2-tuple: this may not be what you want.
         signature: X.f(p: (Int, Int)): Int
   given arguments: 3, 4
  after adaptation: X.f((3, 4): (Int, Int))
   def g = f(3, 4)       // adapted
            ^
-t8610.scala:6: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead
+t8610.scala:9: warning: private[this] value name in class X shadows mutable name inherited from class Named.  Changes to name will not be visible within class X - you may want to give them distinct names.
+  override def toString = name // shadowing mutable var name
+                          ^
+t8610.scala:8: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead
   def u: Unit = ()      // unitarian universalist
       ^
 error: No warnings can be incurred under -Xfatal-warnings.
-three warnings found
+four warnings found
 one error found
diff --git a/test/files/neg/t8610.scala b/test/files/neg/t8610.scala
index 494a3354da..7bed04904f 100644
--- a/test/files/neg/t8610.scala
+++ b/test/files/neg/t8610.scala
@@ -1,7 +1,10 @@
 
-case class X(name: String) {
+class Named(var name: String)
+
+class X(name: String) extends Named(name) {
   def x = "Hi, $name"   // missing interp
   def f(p: (Int, Int)): Int = p._1 * p._2
   def g = f(3, 4)       // adapted
   def u: Unit = ()      // unitarian universalist
+  override def toString = name // shadowing mutable var name
 }
diff --git a/test/files/run/t8610.check b/test/files/run/t8610.check
index 077382fb1c..0e0dfc2cd3 100644
--- a/test/files/run/t8610.check
+++ b/test/files/run/t8610.check
@@ -1,4 +1,4 @@
-t8610.scala:4: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator?
+t8610.scala:4: warning: possible missing interpolator: detected interpolated identifier `$name`
   def x = "Hi, $name"   // missing interp
           ^
 t8610.scala:6: warning: Adapting argument list by creating a 2-tuple: this may not be what you want.
diff --git a/test/junit/scala/tools/nsc/settings/SettingsTest.scala b/test/junit/scala/tools/nsc/settings/SettingsTest.scala
index e4b5ecc7c3..29591727a3 100644
--- a/test/junit/scala/tools/nsc/settings/SettingsTest.scala
+++ b/test/junit/scala/tools/nsc/settings/SettingsTest.scala
@@ -26,7 +26,7 @@ class SettingsTest {
     assertThrows[IllegalArgumentException](check("-Ytest-setting:rubbish"))
   }
 
-  @Test def userSettingsHavePredecenceOverOptimize() {
+  @Test def userSettingsHavePrecedenceOverOptimize() {
     def check(args: String*): MutableSettings#BooleanSetting = {
       val s = new MutableSettings(msg => throw new IllegalArgumentException(msg))
       val (ok, residual) = s.processArguments(args.toList, processAll = true)
@@ -38,7 +38,7 @@ class SettingsTest {
     assertFalse(check("-Yinline:false", "-optimise").value)
   }
 
-  @Test def userSettingsHavePredecenceOverLint() {
+  @Test def userSettingsHavePrecedenceOverLint() {
     def check(args: String*): MutableSettings#BooleanSetting = {
       val s = new MutableSettings(msg => throw new IllegalArgumentException(msg))
       val (ok, residual) = s.processArguments(args.toList, processAll = true)
@@ -49,4 +49,18 @@ class SettingsTest {
     assertFalse(check("-Xlint", "-Ywarn-adapted-args:false").value)
     assertFalse(check("-Ywarn-adapted-args:false", "-Xlint").value)
   }
+
+  def check(args: String*)(b: MutableSettings => MutableSettings#BooleanSetting): MutableSettings#BooleanSetting = {
+    val s = new MutableSettings(msg => throw new IllegalArgumentException(msg))
+    val (ok, residual) = s.processArguments(args.toList, processAll = true)
+    assert(residual.isEmpty)
+    b(s)
+  }
+  @Test def anonymousLintersCanBeNamed() {
+    assertTrue(check("-Xlint")(_.warnMissingInterpolator)) // among Xlint
+    assertFalse(check("-Xlint:-warn-missing-interpolator")(_.warnMissingInterpolator))
+    assertFalse(check("-Xlint:-warn-missing-interpolator", "-Xlint")(_.warnMissingInterpolator))
+    // two lint settings are first come etc; unlike -Y
+    assertTrue(check("-Xlint", "-Xlint:-warn-missing-interpolator")(_.warnMissingInterpolator))
+  }
 }