diff options
author | Som Snytt <som.snytt@gmail.com> | 2014-05-24 06:08:17 -0700 |
---|---|---|
committer | Som Snytt <som.snytt@gmail.com> | 2014-07-08 21:28:35 -0700 |
commit | 44855dcd3c2e19d5dbaf01b2165ea8dc9fb287d3 (patch) | |
tree | 02bc8677560fb0e78b6b2eb67487ec6e29790ac1 | |
parent | 91670d91824493e8bbbd88e9861e04e710311cbb (diff) | |
download | scala-44855dcd3c2e19d5dbaf01b2165ea8dc9fb287d3.tar.gz scala-44855dcd3c2e19d5dbaf01b2165ea8dc9fb287d3.tar.bz2 scala-44855dcd3c2e19d5dbaf01b2165ea8dc9fb287d3.zip |
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.
-rw-r--r-- | src/compiler/scala/tools/nsc/settings/MutableSettings.scala | 61 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/settings/Warnings.scala | 24 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Typers.scala | 8 | ||||
-rw-r--r-- | test/files/neg/forgot-interpolator.check | 16 | ||||
-rw-r--r-- | test/files/neg/t7848-interp-warn.check | 6 | ||||
-rw-r--r-- | test/files/neg/t8525.check | 6 | ||||
-rw-r--r-- | test/files/neg/t8525.flags | 1 | ||||
-rw-r--r-- | test/files/neg/t8525.scala | 10 | ||||
-rw-r--r-- | test/files/neg/t8610-arg.check | 9 | ||||
-rw-r--r-- | test/files/neg/t8610-arg.scala | 5 | ||||
-rw-r--r-- | test/files/neg/t8610.check | 11 | ||||
-rw-r--r-- | test/files/neg/t8610.scala | 5 | ||||
-rw-r--r-- | test/files/run/t8610.check | 2 | ||||
-rw-r--r-- | test/junit/scala/tools/nsc/settings/SettingsTest.scala | 18 |
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)) + } } |