Merge pull request #3885 from som-snytt/issue/8525-cleanup

SI-8525 Clarify usage of -Xlint:_,flag

9 files changed, 133 insertions, 110 deletions

diff --git a/src/compiler/scala/tools/nsc/CompilerCommand.scala b/src/compiler/scala/tools/nsc/CompilerCommand.scala
index a1d0d52dcf..3ded456378 100644
--- a/src/compiler/scala/tools/nsc/CompilerCommand.scala
+++ b/src/compiler/scala/tools/nsc/CompilerCommand.scala
@@ -103,7 +103,15 @@ class CompilerCommand(arguments: List[String], val settings: Settings) {
       val components = global.phaseNames  // global.phaseDescriptors // one initializes
       s"Phase graph of ${components.size} components output to ${genPhaseGraph.value}*.dot."
     }
-    else                    ""
+    // would be nicer if we could ask all the options for their helpful messages
+    else {
+      val sb = new StringBuilder
+      allSettings foreach {
+        case s: MultiChoiceSetting if s.isHelping => sb append s.help
+        case _ =>
+      }
+      sb.toString
+    }
   }
 
   /**
diff --git a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala
index ef9695a594..c73e7ce00e 100644
--- a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala
@@ -29,7 +29,7 @@ trait AbsScalaSettings {
   def ChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: String): ChoiceSetting
   def IntSetting(name: String, descr: String, default: Int, range: Option[(Int, Int)], parser: String => Option[Int]): IntSetting
   def MultiStringSetting(name: String, helpArg: String, descr: String): MultiStringSetting
-  def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit): MultiChoiceSetting
+  def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit)(helper: MultiChoiceSetting => String): MultiChoiceSetting
   def OutputSetting(outputDirs: OutputDirs, default: String): OutputSetting
   def PathSetting(name: String, descr: String, default: String): PathSetting
   def PhasesSetting(name: String, descr: String, default: String): PhasesSetting
diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala
index dc49e8b822..3f41ede3ad 100644
--- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala
@@ -211,8 +211,10 @@ 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 = () => ()) =
-    add(new MultiChoiceSetting(name, helpArg, descr, choices, default))
+  def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit = () => ())(
+      helper: MultiChoiceSetting => String = _ => choices.mkString(f"$descr:%n", f"%n  ", f"%n")
+  ) =
+    add(new MultiChoiceSetting(name, helpArg, descr, choices, default, helper))
   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))
@@ -552,7 +554,7 @@ class MutableSettings(val errorFn: String => Unit)
   }
 
   /** A setting that receives any combination of enumerated values,
-   *  including "_" to mean all values.
+   *  including "_" to mean all values and "help" for verbose info.
    *  In non-colonated mode, stops consuming args at the first
    *  non-value, instead of at the next option, as for a multi-string.
    */
@@ -561,19 +563,21 @@ class MutableSettings(val errorFn: String => Unit)
     arg: String,
     descr: String,
     override val choices: List[String],
-    val default: () => Unit
-  ) extends MultiStringSetting(name, arg, s"$descr${ choices.mkString(": ", ",", ".") }") {
+    val default: () => Unit,
+    helper: MultiChoiceSetting => String
+  ) extends MultiStringSetting(name, s"_,$arg,-$arg", s"$descr: `_' for all, `$name:help' to list") {
 
-    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)
+    private def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'")
+    private def choosing = choices.nonEmpty
+    private def isChoice(s: String) = (s == "_") || (choices contains (s stripPrefix "-"))
+    private var sawHelp = false
 
     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 "help" if choosing            => sawHelp = true
+        case s if !choosing || isChoice(s) => added += s
         case s                             => badChoice(s, name)
       }
       def stoppingAt(arg: String) = (arg startsWith "-") || (choosing && !isChoice(arg))
@@ -584,9 +588,12 @@ 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)
     }
+
+    def isHelping: Boolean = sawHelp
+    def help: String = helper(this)
   }
 
   /** A setting that accumulates all strings supplied to it,
diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
index d22dcacad6..5c3eadbd60 100644
--- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
@@ -44,8 +44,11 @@ trait ScalaSettings extends AbsScalaSettings
   /** If any of these settings is enabled, the compiler should print a message and exit.  */
   def infoSettings = List[Setting](version, help, Xhelp, Yhelp, showPlugins, showPhases, genPhaseGraph)
 
+  /** Any -multichoice:help? Nicer if any option could report that it had help to offer. */
+  private def multihelp = allSettings exists { case s: MultiChoiceSetting => s.isHelping case _ => false }
+
   /** Is an info setting set? */
-  def isInfo = infoSettings exists (_.isSetByUser)
+  def isInfo = (infoSettings exists (_.isSetByUser)) || multihelp
 
   /** Disable a setting */
   def disable(s: Setting) = allSettings -= s
@@ -67,8 +70,27 @@ trait ScalaSettings extends AbsScalaSettings
   // Would be nice to build this dynamically from scala.languageFeature.
   // The two requirements: delay error checking until you have symbols, and let compiler command build option-specific help.
   val language      = {
-    val features = List("dynamics", "postfixOps", "reflectiveCalls", "implicitConversions", "higherKinds", "existentials", "experimental.macros")
-    MultiChoiceSetting("-language", "feature", "Enable one or more language features", features)
+    val features = List(
+      "dynamics"            -> "Allow direct or indirect subclasses of scala.Dynamic",
+      "postfixOps"          -> "Allow postfix operator notation, such as `1 to 10 toList'",
+      "reflectiveCalls"     -> "Allow reflective access to members of structural types",
+      "implicitConversions" -> "Allow definition of implicit functions called views",
+      "higherKinds"         -> "Allow higher-kinded types",  // "Ask Adriaan, but if you have to ask..."
+      "existentials"        -> "Existential types (besides wildcard types) can be written and inferred",
+      "experimental.macros" -> "Allow macro defintion (besides implementation and application)"
+    )
+    val description = "Enable or disable language features"
+    MultiChoiceSetting(
+      name    = "-language",
+      helpArg = "feature",
+      descr   = description,
+      choices = features map (_._1)
+    ) { s =>
+      val helpline: ((String, String)) => String = {
+        case (name, text) => f"  $name%-25s $text%n"
+      }
+      features map helpline mkString (f"$description:%n", "", f"%n")
+    }
   }
 
   /*
diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala
index 3ff2369f86..9989d1d188 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,65 @@ 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 = {
+    val description = "Enable or disable specific warnings"
+    val choices     = (lintWarnings map (_.name)).sorted
+    MultiChoiceSetting(
+      name    = "-Xlint",
+      helpArg = "warning",
+      descr   = description,
+      choices = choices,
+      default = () => xlint.value = true
+    ) { s =>
+      def helpline(n: String) = lintWarnings.find(_.name == n).map(w => f"  ${w.name}%-25s ${w.helpDescription}%n")
+      choices flatMap (helpline(_)) mkString (f"$description:%n", "", f"%n")
+    } 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))
   }
 }