From 2103dbc5230ddf2a369389f179f4ef70eae344f2 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 22 May 2014 09:02:18 -0700 Subject: SI-8610 -Xlint is multichoice option Make -Xlint a "multichoice" option for purposes of option parsing. This allows turning on "lint with these warnings" instead of only "turn off these warnings but enable other lint warnings". ``` $ scalac -Xlint:warn-adapted-args linty.scala # lint plus a warning $ scalac -Xlint warn-adapted-args linty.scala # same $ scalac -Xlint linty.scala # same as now $ scalac -Xlint -- linty.scala # ok, not necessary $ scalac -Xlint _ -- linty.scala # another funky underscore ``` This would also enable Xlint options that are not standalone options, although that is not implemented in this commit. For example, `-Xlint:no-missing-interpolator` could be used to disable that warning. (There is no `-Xoption:flavor=off` syntax.) (`no-` switches would not be enabled by `_`.) --- test/files/run/t8610.check | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 test/files/run/t8610.check (limited to 'test/files/run/t8610.check') diff --git a/test/files/run/t8610.check b/test/files/run/t8610.check new file mode 100644 index 0000000000..077382fb1c --- /dev/null +++ b/test/files/run/t8610.check @@ -0,0 +1,10 @@ +t8610.scala:4: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator? + 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. + 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 + ^ +Hi, $name -- cgit v1.2.3 From 44855dcd3c2e19d5dbaf01b2165ea8dc9fb287d3 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 24 May 2014 06:08:17 -0700 Subject: 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. --- .../scala/tools/nsc/settings/MutableSettings.scala | 61 +++++++++++++--------- .../scala/tools/nsc/settings/Warnings.scala | 24 ++++++--- .../scala/tools/nsc/typechecker/Typers.scala | 8 +-- test/files/neg/forgot-interpolator.check | 16 +++--- test/files/neg/t7848-interp-warn.check | 6 +-- test/files/neg/t8525.check | 6 +++ test/files/neg/t8525.flags | 1 + test/files/neg/t8525.scala | 10 ++++ test/files/neg/t8610-arg.check | 9 ++-- test/files/neg/t8610-arg.scala | 5 +- test/files/neg/t8610.check | 11 ++-- test/files/neg/t8610.scala | 5 +- test/files/run/t8610.check | 2 +- .../scala/tools/nsc/settings/SettingsTest.scala | 18 ++++++- 14 files changed, 123 insertions(+), 59 deletions(-) create mode 100644 test/files/neg/t8525.check create mode 100644 test/files/neg/t8525.flags create mode 100644 test/files/neg/t8525.scala (limited to 'test/files/run/t8610.check') 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)) + } } -- cgit v1.2.3 From 3b89c168b4926139f7295183fdc1903f6f553798 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 1 Jul 2014 08:21:07 -0700 Subject: SI-8525 No anonymous lint Turn anonymous references to `settings.lint` into named settings. After that, trust to Adriaan to make them filterable. There are a few remaining top-level -Y lint warnings that are deprecated. --- src/compiler/scala/tools/nsc/plugins/Plugins.scala | 2 +- .../scala/tools/nsc/settings/Warnings.scala | 97 ++++++++++++++++++---- .../tools/nsc/transform/patmat/MatchAnalysis.scala | 2 +- .../scala/tools/nsc/typechecker/Namers.scala | 2 +- .../scala/tools/nsc/typechecker/RefChecks.scala | 8 +- .../tools/nsc/typechecker/SuperAccessors.scala | 2 +- .../tools/nsc/typechecker/SyntheticMethods.scala | 11 ++- .../scala/tools/nsc/doc/ScaladocAnalyzer.scala | 2 +- test/files/neg/t4851.flags | 2 +- test/files/neg/t8525.check | 11 ++- test/files/neg/t8525.flags | 2 +- test/files/neg/t8610-arg.check | 8 +- test/files/neg/warn-inferred-any.flags | 2 +- test/files/run/t8610.check | 3 + test/files/run/t8610.flags | 2 +- .../scala/tools/nsc/settings/SettingsTest.scala | 6 +- 16 files changed, 122 insertions(+), 40 deletions(-) (limited to 'test/files/run/t8610.check') diff --git a/src/compiler/scala/tools/nsc/plugins/Plugins.scala b/src/compiler/scala/tools/nsc/plugins/Plugins.scala index 12f9aeba27..6e3d013e52 100644 --- a/src/compiler/scala/tools/nsc/plugins/Plugins.scala +++ b/src/compiler/scala/tools/nsc/plugins/Plugins.scala @@ -33,7 +33,7 @@ trait Plugins { global: Global => } val maybes = Plugin.loadAllFrom(paths, dirs, settings.disable.value) val (goods, errors) = maybes partition (_.isSuccess) - // Explicit parameterization of recover to suppress -Xlint warning about inferred Any + // Explicit parameterization of recover to avoid -Xlint warning about inferred Any errors foreach (_.recover[Any] { // legacy behavior ignores altogether, so at least warn devs case e: MissingPluginException => if (global.isDeveloper) warning(e.getMessage) diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index f1ed45a2a0..3ff2369f86 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -34,34 +34,76 @@ trait Warnings { warnInferAny, // warnUnused SI-7712, SI-7707 warnUnused not quite ready for prime-time // warnUnusedImport currently considered too noisy for general use - warnMissingInterpolator + // warnValueOverrides + warnMissingInterpolator, + warnDocDetached, + warnPrivateShadow, + warnPolyImplicitOverload, + warnOptionImplicit, + warnDelayedInit, + warnByNameRightAssociative, + warnPackageObjectClasses, + warnUnsoundMatch ) private lazy val warnSelectNullable = BooleanSetting("-Xcheck-null", "This option is obsolete and does nothing.") // Individual warnings. - val warnAdaptedArgs = BooleanSetting ("-Ywarn-adapted-args", "Warn if an argument list is modified to match the receiver.") - 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.") - val warnNullaryUnit = BooleanSetting ("-Ywarn-nullary-unit", "Warn when nullary methods return Unit.") - val warnInaccessible = BooleanSetting ("-Ywarn-inaccessible", "Warn about inaccessible types in method signatures.") - val warnNullaryOverride = BooleanSetting ("-Ywarn-nullary-override", "Warn when non-nullary overrides nullary, e.g. `def foo()` over `def foo`.") - val warnInferAny = BooleanSetting ("-Ywarn-infer-any", "Warn when a type argument is inferred to be `Any`.") - 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") + 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.") + 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.") + // Lint warnings that are not -Y, created with new instead of autoregistering factory method + private def lintflag(name: String, text: String) = new BooleanSetting(name, text) + + val warnAdaptedArgs = lintflag("adapted-args", + "Warn if an argument list is modified to match the receiver.") + val warnNullaryUnit = lintflag("nullary-unit", + "Warn when nullary methods return Unit.") + val warnInaccessible = lintflag("inaccessible", + "Warn about inaccessible types in method signatures.") + val warnNullaryOverride = lintflag("nullary-override", + "Warn when non-nullary `def f()' overrides nullary `def f'.") + val warnInferAny = lintflag("infer-any", + "Warn when a type argument is inferred to be `Any`.") + val warnMissingInterpolator = lintflag("missing-interpolator", + "A string literal appears to be missing an interpolator id.") + val warnDocDetached = lintflag("doc-detached", + "A ScalaDoc comment appears to be detached from its element.") + val warnPrivateShadow = lintflag("private-shadow", + "A private field (or class parameter) shadows a superclass field.") + val warnPolyImplicitOverload = lintflag("poly-implicit-overload", + "Parameterized overloaded implicit methods are not visible as view bounds") + val warnOptionImplicit = lintflag("option-implicit", + "Option.apply used implicit view.") + val warnDelayedInit = lintflag("delayedinit-select", + "Selecting member of DelayedInit") + val warnByNameRightAssociative = lintflag("by-name-right-associative", + "By-name parameter of right associative operator") + val warnPackageObjectClasses = lintflag("package-object-classes", + "Class or object defined in package object") + val warnUnsoundMatch = lintflag("unsound-match", + "Pattern match may not be typesafe") + + // Lint warnings that are not enabled yet + val warnValueOverrides = lintflag("value-overrides", "Generated value class method overrides an implementation") // 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.") - def lintables = (lintWarnings map (_.name stripPrefix "-Y")).sorted + val xlint = lintflag("-Xlint", "Enable recommended additional warnings.") + val yprefix = "-Ywarn-" + def lintables = (lintWarnings map (_.name stripPrefix yprefix)).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)) + def set(w: String, v: Boolean) = lintWarnings find (s => (s.name stripPrefix yprefix) == w) foreach (b => setPolitely(b, v)) val Neg = "-" def propagate(ss: List[String]): Unit = ss match { case w :: rest => if (w startsWith Neg) set(w stripPrefix Neg, false) else set(w, true) ; propagate(rest) @@ -81,6 +123,29 @@ trait Warnings { xlint } + // Lint warnings that are currently -Y, but deprecated in that usage + // Alas, the -Yarg must have a doppelgaenger that is not deprecated + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnAdaptedArgs = BooleanSetting("-Ywarn-adapted-args", + "Warn if an argument list is modified to match the receiver.") withDeprecationMessage + "Enable -Xlint:adapted-args" enabling List(warnAdaptedArgs) + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnNullaryUnit = BooleanSetting("-Ywarn-nullary-unit", + "Warn when nullary methods return Unit.") withDeprecationMessage + "Enable -Xlint:nullary-unit" enabling List(warnNullaryUnit) + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnInaccessible = BooleanSetting("-Ywarn-inaccessible", + "Warn about inaccessible types in method signatures.") withDeprecationMessage + "Enable -Xlint:inaccessible" enabling List(warnInaccessible) + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnNullaryOverride = BooleanSetting("-Ywarn-nullary-override", + "Warn when non-nullary `def f()' overrides nullary `def f'.") withDeprecationMessage + "Enable -Xlint:nullary-override" enabling List(warnNullaryOverride) + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnInferAny = BooleanSetting("-Ywarn-infer-any", + "Warn when a type argument is inferred to be `Any`.") withDeprecationMessage + "Enable -Xlint:infer-any" enabling List(warnInferAny) + // Backward compatibility. @deprecated("Use fatalWarnings", "2.11.0") def Xwarnfatal = fatalWarnings // used by sbt @deprecated("This option is being removed", "2.11.0") def Xchecknull = warnSelectNullable // used by ide diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala index 6f81cbe152..b2dc6e4e52 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala @@ -50,7 +50,7 @@ trait TreeAndTypeAnalysis extends Debugging { // but the annotation didn't bubble up... // This is a pretty poor approximation. def unsoundAssumptionUsed = binder.name != nme.WILDCARD && !(pt <:< pat.tpe) - if (settings.lint && unsoundAssumptionUsed) + if (settings.warnUnsoundMatch && unsoundAssumptionUsed) reporter.warning(pat.pos, sm"""The value matched by $pat is bound to ${binder.name}, which may be used under the |unsound assumption that it has type ${pat.tpe}, whereas we can only safely diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 1328119aac..7bbd81118a 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -717,7 +717,7 @@ trait Namers extends MethodSynthesis { m.updateAttachment(new ConstructorDefaultsAttachment(tree, null)) } val owner = tree.symbol.owner - if (settings.lint && owner.isPackageObjectClass && !mods.isImplicit) { + if (settings.warnPackageObjectClasses && owner.isPackageObjectClass && !mods.isImplicit) { reporter.warning(tree.pos, "it is not recommended to define classes/objects inside of package objects.\n" + "If possible, define " + tree.symbol + " in " + owner.skipPackageObject + " instead." diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 1b3da26bf2..47465875e9 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -167,7 +167,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans } // This has become noisy with implicit classes. - if (settings.lint && settings.developer) { + if (settings.warnPolyImplicitOverload && settings.developer) { clazz.info.decls filter (x => x.isImplicit && x.typeParams.nonEmpty) foreach { sym => // implicit classes leave both a module symbol and a method symbol as residue val alts = clazz.info.decl(sym.name).alternatives filterNot (_.isModule) @@ -954,7 +954,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans def apply(tp: Type) = mapOver(tp).normalize } - def checkImplicitViewOptionApply(pos: Position, fn: Tree, args: List[Tree]): Unit = if (settings.lint) (fn, args) match { + def checkImplicitViewOptionApply(pos: Position, fn: Tree, args: List[Tree]): Unit = if (settings.warnOptionImplicit) (fn, args) match { case (tap@TypeApply(fun, targs), List(view: ApplyImplicitView)) if fun.symbol == currentRun.runDefinitions.Option_apply => reporter.warning(pos, s"Suspicious application of an implicit view (${view.fun}) in the argument to Option.apply.") // SI-6567 case _ => @@ -1320,7 +1320,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans && !qual.tpe.isInstanceOf[ThisType] && sym.accessedOrSelf.isVal ) - if (settings.lint.value && isLikelyUninitialized) + if (settings.warnDelayedInit && isLikelyUninitialized) reporter.warning(pos, s"Selecting ${sym} from ${sym.owner}, which extends scala.DelayedInit, is likely to yield an uninitialized value") } @@ -1387,7 +1387,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans private def checkByNameRightAssociativeDef(tree: DefDef) { tree match { case DefDef(_, name, _, params :: _, _, _) => - if (settings.lint && !treeInfo.isLeftAssoc(name.decodedName) && params.exists(p => isByName(p.symbol))) + if (settings.warnByNameRightAssociative && !treeInfo.isLeftAssoc(name.decodedName) && params.exists(p => isByName(p.symbol))) reporter.warning(tree.pos, "by-name parameters will be evaluated eagerly when called as a right-associative infix operator. For more details, see SI-1980.") case _ => diff --git a/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala b/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala index d3a41b9570..38b00a015b 100644 --- a/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala +++ b/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala @@ -247,7 +247,7 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT // also exists in a superclass, because they may be surprised // to find out that a constructor parameter will shadow a // field. See SI-4762. - if (settings.lint) { + if (settings.warnPrivateShadow) { if (sym.isPrivateLocal && sym.paramss.isEmpty) { qual.symbol.ancestors foreach { parent => parent.info.decls filterNot (x => x.isPrivate || x.isLocalToThis) foreach { m2 => diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala index 9516f94135..d0237fb468 100644 --- a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala +++ b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala @@ -339,12 +339,11 @@ trait SyntheticMethods extends ast.TreeDSL { !hasOverridingImplementation(m) || { clazz.isDerivedValueClass && (m == Any_hashCode || m == Any_equals) && { // Without a means to suppress this warning, I've thought better of it. - // - // if (settings.lint) { - // (clazz.info nonPrivateMember m.name) filter (m => (m.owner != AnyClass) && (m.owner != clazz) && !m.isDeferred) andAlso { m => - // currentUnit.warning(clazz.pos, s"Implementation of ${m.name} inherited from ${m.owner} overridden in $clazz to enforce value class semantics") - // } - // } + if (settings.warnValueOverrides) { + (clazz.info nonPrivateMember m.name) filter (m => (m.owner != AnyClass) && (m.owner != clazz) && !m.isDeferred) andAlso { m => + currentUnit.warning(clazz.pos, s"Implementation of ${m.name} inherited from ${m.owner} overridden in $clazz to enforce value class semantics") + } + } true } } diff --git a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala index 10c382e169..ccf18b76de 100644 --- a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala +++ b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala @@ -190,7 +190,7 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax typeParams.nonEmpty || version.nonEmpty || since.nonEmpty } def isDirty = unclean(unmooredParser parseComment doc) - if ((doc ne null) && (settings.lint || isDirty)) + if ((doc ne null) && (settings.warnDocDetached || isDirty)) reporter.warning(doc.pos, "discarding unmoored doc comment") } diff --git a/test/files/neg/t4851.flags b/test/files/neg/t4851.flags index ca0d0a0ba3..044ce22c84 100644 --- a/test/files/neg/t4851.flags +++ b/test/files/neg/t4851.flags @@ -1 +1 @@ --Ywarn-adapted-args -Xfatal-warnings -deprecation +-Xlint:adapted-args -Xfatal-warnings -deprecation diff --git a/test/files/neg/t8525.check b/test/files/neg/t8525.check index 554ab23253..5287e43b7a 100644 --- a/test/files/neg/t8525.check +++ b/test/files/neg/t8525.check @@ -1,6 +1,15 @@ +t8525.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 + ^ 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 ^ +t8525.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. -one warning found +three warnings found one error found diff --git a/test/files/neg/t8525.flags b/test/files/neg/t8525.flags index f19af1f717..53b2dfe7ec 100644 --- a/test/files/neg/t8525.flags +++ b/test/files/neg/t8525.flags @@ -1 +1 @@ --Xfatal-warnings -Xlint:-warn-missing-interpolator +-Xfatal-warnings -Xlint:-missing-interpolator -Xlint diff --git a/test/files/neg/t8610-arg.check b/test/files/neg/t8610-arg.check index 4f194ce84a..ea2805508d 100644 --- a/test/files/neg/t8610-arg.check +++ b/test/files/neg/t8610-arg.check @@ -1,6 +1,12 @@ t8610-arg.scala:5: warning: possible missing interpolator: detected interpolated identifier `$name` def x = "Hi, $name" // missing interp ^ +t8610-arg.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-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 ^ @@ -8,5 +14,5 @@ t8610-arg.scala:8: warning: side-effecting nullary methods are discouraged: sugg 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/warn-inferred-any.flags b/test/files/neg/warn-inferred-any.flags index a3127d392a..b580dfbbe3 100644 --- a/test/files/neg/warn-inferred-any.flags +++ b/test/files/neg/warn-inferred-any.flags @@ -1 +1 @@ --Xfatal-warnings -Ywarn-infer-any +-Xfatal-warnings -Xlint:infer-any diff --git a/test/files/run/t8610.check b/test/files/run/t8610.check index 0e0dfc2cd3..fde82d5109 100644 --- a/test/files/run/t8610.check +++ b/test/files/run/t8610.check @@ -7,4 +7,7 @@ t8610.scala:6: warning: Adapting argument list by creating a 2-tuple: this may n after adaptation: X.f((3, 4): (Int, Int)) def g = f(3, 4) // adapted ^ +t8610.scala:7: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead + def u: Unit = () // unitarian universalist + ^ Hi, $name diff --git a/test/files/run/t8610.flags b/test/files/run/t8610.flags index edcb5f4d0c..4195dec383 100644 --- a/test/files/run/t8610.flags +++ b/test/files/run/t8610.flags @@ -1 +1 @@ --Xlint:warn-adapted-args +-Xlint:adapted-args diff --git a/test/junit/scala/tools/nsc/settings/SettingsTest.scala b/test/junit/scala/tools/nsc/settings/SettingsTest.scala index 29591727a3..9203054d9a 100644 --- a/test/junit/scala/tools/nsc/settings/SettingsTest.scala +++ b/test/junit/scala/tools/nsc/settings/SettingsTest.scala @@ -58,9 +58,9 @@ class SettingsTest { } @Test def anonymousLintersCanBeNamed() { assertTrue(check("-Xlint")(_.warnMissingInterpolator)) // among Xlint - assertFalse(check("-Xlint:-warn-missing-interpolator")(_.warnMissingInterpolator)) - assertFalse(check("-Xlint:-warn-missing-interpolator", "-Xlint")(_.warnMissingInterpolator)) + assertFalse(check("-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) + assertFalse(check("-Xlint:-missing-interpolator", "-Xlint")(_.warnMissingInterpolator)) // two lint settings are first come etc; unlike -Y - assertTrue(check("-Xlint", "-Xlint:-warn-missing-interpolator")(_.warnMissingInterpolator)) + assertTrue(check("-Xlint", "-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) } } -- cgit v1.2.3 From f81ec8d1f6481ddacfb27e743c6c58961e765f0e Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 16 Jul 2014 11:01:31 -0700 Subject: SI-8525 Clarify usage of -Xlint:_,flag Also clarify usage of -Xlint flag. Align more with javac -Xlint:all,-flag,flag where once a flag is explicitly enabled it cannot be disabled, but where the wildcard is a backstop only. (There is no all option yet here.) -Xlint and -Xlint:_ just set a flag which is consulted by any unset lint warning. Xlint warnings consult the state of Xlint when they are unset. Individual -Ywarn-ings do not. Other warnings are explicitly set to false. They can only be enabled programmatically. Some tests are corrected. Also, option order is no longer significant, see the unit test. --- .../scala/tools/nsc/settings/MutableSettings.scala | 9 +- .../scala/tools/nsc/settings/Warnings.scala | 120 +++++++++++---------- test/files/neg/t8610-arg.check | 14 +-- test/files/neg/t8610-arg.flags | 2 +- test/files/run/t8610.check | 6 -- .../scala/tools/nsc/settings/SettingsTest.scala | 24 ++--- 6 files changed, 77 insertions(+), 98 deletions(-) (limited to 'test/files/run/t8610.check') diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala index dc49e8b822..0dd4ae0b3b 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -567,13 +567,12 @@ class MutableSettings(val errorFn: String => Unit) 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 + val added = collection.mutable.ListBuffer.empty[String] def tryArg(arg: String) = arg match { - case "_" if choosing => wildcards foreach (total += _) - case s if !choosing || isChoice(s) => total += s + case "_" if choosing => default() + case s if !choosing || isChoice(s) => added += s case s => badChoice(s, name) } def stoppingAt(arg: String) = (arg startsWith "-") || (choosing && !isChoice(arg)) @@ -584,7 +583,7 @@ class MutableSettings(val errorFn: String => Unit) } val rest = loop(args) if (rest.size == args.size) default() // if no arg consumed, trigger default action - else value = total.toList // update once + else value = added.toList // update all new settings at once Some(rest) } } diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 3ff2369f86..0b9ad80041 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -17,12 +17,15 @@ trait Warnings { // Warning semantics. val fatalWarnings = BooleanSetting("-Xfatal-warnings", "Fail the compilation if there are any warnings.") - // These warnings are all so noisy as to be useless in their + // These additional warnings are all so noisy as to be useless in their // present form, but have the potential to offer useful info. protected def allWarnings = lintWarnings ++ List( warnDeadCode, warnValueDiscard, - warnNumericWiden + warnNumericWiden, + warnUnused, // SI-7712, SI-7707 warnUnused not quite ready for prime-time + warnUnusedImport, // currently considered too noisy for general use + warnValueOverrides // currently turned off as experimental ) // These warnings should be pretty quiet unless you're doing // something inadvisable. @@ -32,9 +35,6 @@ trait Warnings { warnNullaryUnit, warnAdaptedArgs, warnInferAny, - // warnUnused SI-7712, SI-7707 warnUnused not quite ready for prime-time - // warnUnusedImport currently considered too noisy for general use - // warnValueOverrides warnMissingInterpolator, warnDocDetached, warnPrivateShadow, @@ -46,22 +46,26 @@ trait Warnings { warnUnsoundMatch ) - private lazy val warnSelectNullable = BooleanSetting("-Xcheck-null", "This option is obsolete and does nothing.") + // Individual warnings. They can be set with -Ywarn. + private def nonlintflag(name: String, text: String): BooleanSetting = BooleanSetting(name, text) - // Individual warnings. - val warnDeadCode = BooleanSetting("-Ywarn-dead-code", + val warnDeadCode = nonlintflag("-Ywarn-dead-code", "Warn when dead code is identified.") - val warnValueDiscard = BooleanSetting("-Ywarn-value-discard", + val warnValueDiscard = nonlintflag("-Ywarn-value-discard", "Warn when non-Unit expression results are unused.") - val warnNumericWiden = BooleanSetting("-Ywarn-numeric-widen", + val warnNumericWiden = nonlintflag("-Ywarn-numeric-widen", "Warn when numerics are widened.") - val warnUnused = BooleanSetting("-Ywarn-unused", + val warnUnused = nonlintflag("-Ywarn-unused", "Warn when local and private vals, vars, defs, and types are are unused") - val warnUnusedImport = BooleanSetting("-Ywarn-unused-import", + val warnUnusedImport = nonlintflag("-Ywarn-unused-import", "Warn when imports are unused") - // Lint warnings that are not -Y, created with new instead of autoregistering factory method - private def lintflag(name: String, text: String) = new BooleanSetting(name, text) + // Lint warnings that have no -Y avatar, created with new instead of the autoregistering factory method. + // They evaluate true if set to true or else are unset but -Xlint is true + private def lintflag(name: String, text: String): BooleanSetting = + new BooleanSetting(name, text) { + override def value = if (isSetByUser) super.value else xlint + } val warnAdaptedArgs = lintflag("adapted-args", "Warn if an argument list is modified to match the receiver.") @@ -92,59 +96,59 @@ trait Warnings { val warnUnsoundMatch = lintflag("unsound-match", "Pattern match may not be typesafe") - // Lint warnings that are not enabled yet - val warnValueOverrides = lintflag("value-overrides", "Generated value class method overrides an implementation") + // Experimental lint warnings that are turned off, but which could be turned on programmatically. + // These warnings are said to blind those who dare enable them. + // They are not activated by -Xlint and can't be enabled on the command line. + val warnValueOverrides = { + val flag = lintflag("value-overrides", "Generated value class method overrides an implementation") + flag.value = false + flag + } - // Warning groups. - val lint = { - // Boolean setting for testing if lint is on; not "added" to option processing - val xlint = lintflag("-Xlint", "Enable recommended additional warnings.") - val yprefix = "-Ywarn-" - def lintables = (lintWarnings map (_.name stripPrefix yprefix)).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 (s => (s.name stripPrefix yprefix) == w) foreach (b => setPolitely(b, v)) - val Neg = "-" - def propagate(ss: List[String]): Unit = ss match { - case w :: rest => if (w startsWith Neg) set(w stripPrefix Neg, false) else set(w, true) ; propagate(rest) - case Nil => () - } - // enable lint and the group, honoring previous -Y settings - def enableAll(): Unit = { - xlint.value = true - for (s <- lintWarnings) setPolitely(s, true) + // The Xlint warning group. + private val xlint = new BooleanSetting("-Zunused", "True if -Xlint or -Xlint:_") + // On -Xlint or -Xlint:_, set xlint, otherwise set the lint warning unless already set true + val lint = + MultiChoiceSetting( + name = "-Xlint", + helpArg = "warning", + descr = "Enable recommended additional warnings", + choices = (lintWarnings map (_.name)).sorted, + default = () => xlint.value = true + ) withPostSetHook { x => + val Neg = "-" + def setPolitely(b: BooleanSetting, v: Boolean) = if (!b.isSetByUser || !b) b.value = v + def set(w: String, v: Boolean) = lintWarnings find (_.name == w) foreach (setPolitely(_, v)) + def propagate(ss: List[String]): Unit = ss match { + case w :: rest => if (w startsWith Neg) set(w stripPrefix Neg, false) else set(w, true) ; propagate(rest) + case Nil => () + } + propagate(x.value) } - // The command option - 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 - } // Lint warnings that are currently -Y, but deprecated in that usage - // Alas, the -Yarg must have a doppelgaenger that is not deprecated - @deprecated("Use the Xlint flag", since="2.11.2") + @deprecated("Use warnAdaptedArgs", since="2.11.2") val YwarnAdaptedArgs = BooleanSetting("-Ywarn-adapted-args", - "Warn if an argument list is modified to match the receiver.") withDeprecationMessage - "Enable -Xlint:adapted-args" enabling List(warnAdaptedArgs) - @deprecated("Use the Xlint flag", since="2.11.2") + "Warn if an argument list is modified to match the receiver.") enabling List(warnAdaptedArgs) + //withDeprecationMessage "Enable -Xlint:adapted-args" + @deprecated("Use warnNullaryUnit", since="2.11.2") val YwarnNullaryUnit = BooleanSetting("-Ywarn-nullary-unit", - "Warn when nullary methods return Unit.") withDeprecationMessage - "Enable -Xlint:nullary-unit" enabling List(warnNullaryUnit) - @deprecated("Use the Xlint flag", since="2.11.2") + "Warn when nullary methods return Unit.") enabling List(warnNullaryUnit) + //withDeprecationMessage "Enable -Xlint:nullary-unit" + @deprecated("Use warnInaccessible", since="2.11.2") val YwarnInaccessible = BooleanSetting("-Ywarn-inaccessible", - "Warn about inaccessible types in method signatures.") withDeprecationMessage - "Enable -Xlint:inaccessible" enabling List(warnInaccessible) - @deprecated("Use the Xlint flag", since="2.11.2") + "Warn about inaccessible types in method signatures.") enabling List(warnInaccessible) + //withDeprecationMessage "Enable -Xlint:inaccessible" + @deprecated("Use warnNullaryOverride", since="2.11.2") val YwarnNullaryOverride = BooleanSetting("-Ywarn-nullary-override", - "Warn when non-nullary `def f()' overrides nullary `def f'.") withDeprecationMessage - "Enable -Xlint:nullary-override" enabling List(warnNullaryOverride) - @deprecated("Use the Xlint flag", since="2.11.2") + "Warn when non-nullary `def f()' overrides nullary `def f'.") enabling List(warnNullaryOverride) + //withDeprecationMessage "Enable -Xlint:nullary-override" + @deprecated("Use warnInferAny", since="2.11.2") val YwarnInferAny = BooleanSetting("-Ywarn-infer-any", - "Warn when a type argument is inferred to be `Any`.") withDeprecationMessage - "Enable -Xlint:infer-any" enabling List(warnInferAny) + "Warn when a type argument is inferred to be `Any`.") enabling List(warnInferAny) + //withDeprecationMessage "Enable -Xlint:infer-any" + + private lazy val warnSelectNullable = BooleanSetting("-Xcheck-null", "This option is obsolete and does nothing.") // Backward compatibility. @deprecated("Use fatalWarnings", "2.11.0") def Xwarnfatal = fatalWarnings // used by sbt diff --git a/test/files/neg/t8610-arg.check b/test/files/neg/t8610-arg.check index ea2805508d..d6fe207119 100644 --- a/test/files/neg/t8610-arg.check +++ b/test/files/neg/t8610-arg.check @@ -1,18 +1,6 @@ -t8610-arg.scala:5: warning: possible missing interpolator: detected interpolated identifier `$name` - def x = "Hi, $name" // missing interp - ^ -t8610-arg.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-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. -four warnings found +one warning found one error found diff --git a/test/files/neg/t8610-arg.flags b/test/files/neg/t8610-arg.flags index f8867a7b4e..f331ba9383 100644 --- a/test/files/neg/t8610-arg.flags +++ b/test/files/neg/t8610-arg.flags @@ -1 +1 @@ --Xfatal-warnings -Xlint warn-nullary-unit +-Xfatal-warnings -Xlint nullary-unit diff --git a/test/files/run/t8610.check b/test/files/run/t8610.check index fde82d5109..b3ab7a9cef 100644 --- a/test/files/run/t8610.check +++ b/test/files/run/t8610.check @@ -1,13 +1,7 @@ -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. 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:7: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead - def u: Unit = () // unitarian universalist - ^ Hi, $name diff --git a/test/junit/scala/tools/nsc/settings/SettingsTest.scala b/test/junit/scala/tools/nsc/settings/SettingsTest.scala index 9203054d9a..960d7f8ac1 100644 --- a/test/junit/scala/tools/nsc/settings/SettingsTest.scala +++ b/test/junit/scala/tools/nsc/settings/SettingsTest.scala @@ -38,29 +38,23 @@ class SettingsTest { assertFalse(check("-Yinline:false", "-optimise").value) } - @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) - assert(residual.isEmpty) - s.warnAdaptedArgs // among Xlint - } - assertTrue(check("-Xlint").value) - 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 = { + // for the given args, select the desired setting + private 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 userSettingsHavePrecedenceOverLint() { + assertTrue(check("-Xlint")(_.warnAdaptedArgs)) + assertFalse(check("-Xlint", "-Ywarn-adapted-args:false")(_.warnAdaptedArgs)) + assertFalse(check("-Ywarn-adapted-args:false", "-Xlint")(_.warnAdaptedArgs)) + } + @Test def anonymousLintersCanBeNamed() { assertTrue(check("-Xlint")(_.warnMissingInterpolator)) // among Xlint assertFalse(check("-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) assertFalse(check("-Xlint:-missing-interpolator", "-Xlint")(_.warnMissingInterpolator)) - // two lint settings are first come etc; unlike -Y - assertTrue(check("-Xlint", "-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) + assertFalse(check("-Xlint", "-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) } } -- cgit v1.2.3