Changes in parameter parsing & deprecation of -Xlint

Author: szymon-rd

compiler/src/dotty/tools/dotc/config/CliCommand.scala

@@ -86,7 +86,7 @@ trait CliCommand:
   protected def isVerbose(s: Setting[?])(using settings: ConcreteSettings)(using SettingsState): Boolean =
     s.name.startsWith("-V") && s.name != "-V"
   protected def isWarning(s: Setting[?])(using settings: ConcreteSettings)(using SettingsState): Boolean =
-    s.name.startsWith("-W") && s.name != "-W" || s.name == "-Xlint"
+    s.name.startsWith("-W") && s.name != "-W"
   protected def isAdvanced(s: Setting[?])(using settings: ConcreteSettings)(using SettingsState): Boolean =
     s.name.startsWith("-X") && s.name != "-X"
   protected def isPrivate(s: Setting[?])(using settings: ConcreteSettings)(using SettingsState): Boolean =

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -295,9 +295,9 @@ private sealed trait WarningSettings:
          |to prevent the shell from expanding patterns.""".stripMargin,
   )
 
-  val Xlint: Setting[List[ChoiceWithHelp[String]]] = UncompleteMultiChoiceHelpSetting(
-    AdvancedSetting,
-    name = "Xlint",
+  val Wshadow: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
+    WarningSetting,
+    name = "Wshadow",
     helpArg = "advanced warning",
     descr = "Enable or disable specific `lint` warnings",
     choices = List(
@@ -308,9 +308,9 @@ private sealed trait WarningSettings:
     default = Nil
   )
 
-  object XlintHas:
+  object WshadowHas:
     def allOr(s: String)(using Context) =
-      Xlint.value.pipe(us => us.contains("all") || us.contains(s))
+      Wshadow.value.pipe(us => us.contains("all") || us.contains(s))
     def privateShadow(using Context) =
       allOr("private-shadow")
     def typeParameterShadow(using Context) =
@@ -356,6 +356,10 @@ private sealed trait XSettings:
   }
 
   val XmacroSettings: Setting[List[String]] = MultiStringSetting(AdvancedSetting, "Xmacro-settings", "setting1,setting2,..settingN", "List of settings which exposed to the macros")
+
+  // Deprecated
+  val Xlint: Setting[_] = DeprecatedSetting(AdvancedSetting, "Xlint", "Enable or disable specific warnings", "Use -Wshadow to enable shadowing lints or -W:<opt> to enable specific sets of warnings.")
+
 end XSettings
 
 /** -Y "Forking" as in forked tongue or "Private" settings */

compiler/src/dotty/tools/dotc/config/Settings.scala

@@ -70,10 +70,15 @@ object Settings:
     aliases: List[String] = Nil,
     depends: List[(Setting[?], Any)] = Nil,
     ignoreInvalidArgs: Boolean = false,
-    propertyClass: Option[Class[?]] = None)(private[Settings] val idx: Int) {
+    propertyClass: Option[Class[?]] = None,
+    deprecationMsg: Option[String] = None)(private[Settings] val idx: Int) {
 
     assert(name.startsWith(s"-$category"), s"Setting $name does not start with category -$category")
 
+    // Without the following assertion, it would be easy to mistakenly try to pass a file to a setting that ignores invalid args.
+    // Example: -opt Main.scala would be interpreted as -opt:Main.scala, and the source file would be ignored.
+    assert(!(summon[ClassTag[T]] == ListTag && ignoreInvalidArgs), s"Ignoring invalid args is not supported for multivalue settings: $name")
+
     val allFullNames: List[String] = s"$name" :: s"-$name" :: aliases  
 
     def valueIn(state: SettingsState): T = state.value(idx).asInstanceOf[T]
@@ -150,45 +155,45 @@ object Settings:
               update(x, args)
         catch case _: NumberFormatException =>
           fail(s"$argValue is not an integer argument for $name", args)
+        
+      def setOutput(argValue: String, args: List[String]) = 
+        val path = Directory(argValue)
+        val isJar = path.extension == "jar"
+        if (!isJar && !path.isDirectory)
+          fail(s"'$argValue' does not exist or is not a directory or .jar file", args)
+        else {
+          val output = if (isJar) JarArchive.create(path) else new PlainDirectory(path)
+          update(output, args)
+        }
+
+      def appendList(strings: List[String], args: List[String]) =
+        choices match
+          case Some(valid) => strings.filterNot(valid.contains) match
+            case Nil => update(strings, args)
+            case invalid => invalidChoices(invalid)
+          case _ => update(strings, args)
+
 
       def doSet(argRest: String) = 
         ((summon[ClassTag[T]], args): @unchecked) match {
           case (BooleanTag, _) =>
             setBoolean(argRest, args)
           case (OptionTag, _) =>
             update(Some(propertyClass.get.getConstructor().newInstance()), args)
-          case (ListTag, _) =>
-            if (argRest.isEmpty) missingArg
-            else
-              val strings = argRest.split(",").toList
-              choices match
-                case Some(valid) => strings.filterNot(valid.contains) match
-                  case Nil => update(strings, args)
-                  case invalid => invalidChoices(invalid)
-                case _ => update(strings, args)
+          case (ListTag, args) if argRest.nonEmpty =>
+            val strings = argRest.split(",").toList
+            appendList(strings, args)
+          case (ListTag, arg2 :: args2) if !(arg2 startsWith "-")=>
+            appendList(arg2 :: Nil, args2)
           case (StringTag, _) if argRest.nonEmpty || choices.exists(_.contains("")) =>
             setString(argRest, args)
           case (StringTag, arg2 :: args2) =>
             if (arg2 startsWith "-") missingArg
             else setString(arg2, args2)
-          case (OutputTag, _) if argRest.nonEmpty =>
-            val path = Directory(argRest)
-            val isJar = path.extension == "jar"
-            if (!isJar && !path.isDirectory)
-              fail(s"'$argRest' does not exist or is not a directory or .jar file", args)
-            else {
-              val output = if (isJar) JarArchive.create(path) else new PlainDirectory(path)
-              update(output, args)
-            }
-          case (OutputTag, arg :: args) =>
-            val path = Directory(arg)
-            val isJar = path.extension == "jar"
-            if (!isJar && !path.isDirectory)
-              fail(s"'$arg' does not exist or is not a directory or .jar file", args)
-            else {
-              val output = if (isJar) JarArchive.create(path) else new PlainDirectory(path)
-              update(output, args)
-            }
+          case (OutputTag, args) if argRest.nonEmpty =>
+            setOutput(argRest, args)
+          case (OutputTag, arg2 :: args2) =>
+            setOutput(arg2, args2)
           case (IntTag, args) if argRest.nonEmpty =>
             setInt(argRest, args)
           case (IntTag, arg2 :: args2) =>
@@ -214,7 +219,10 @@ object Settings:
         if(prefix.isEmpty) arg.dropWhile(_ != ':').drop(1) else arg.drop(prefix.get.length)
 
       if matches(arg) then 
-        doSet(argValRest)
+        if deprecationMsg.isDefined then
+          warn(s"Option $name is deprecated: ${deprecationMsg.get}", args)
+        else 
+          doSet(argValRest)
       else
         state
     }
@@ -334,9 +342,6 @@ object Settings:
     def MultiChoiceHelpSetting(category: String, name: String, helpArg: String, descr: String, choices: List[ChoiceWithHelp[String]], default: List[ChoiceWithHelp[String]], aliases: List[String] = Nil): Setting[List[ChoiceWithHelp[String]]] =
       publish(Setting(category, validateAndPrependName(name), descr, default, helpArg, Some(choices), aliases = aliases.map(validateSetting)))
 
-    def UncompleteMultiChoiceHelpSetting(category: String, name: String, helpArg: String, descr: String, choices: List[ChoiceWithHelp[String]], default: List[ChoiceWithHelp[String]], aliases: List[String] = Nil): Setting[List[ChoiceWithHelp[String]]] =
-      publish(Setting(category, validateAndPrependName(name), descr, default, helpArg, Some(choices), aliases = aliases.map(validateSetting), ignoreInvalidArgs = true))
-
     def IntSetting(category: String, name: String, descr: String, default: Int, aliases: List[String] = Nil): Setting[Int] =
       publish(Setting(category, validateAndPrependName(name), descr, default, aliases = aliases.map(validateSetting)))
 
@@ -364,5 +369,8 @@ object Settings:
 
     def OptionSetting[T: ClassTag](category: String, name: String, descr: String, aliases: List[String] = Nil): Setting[Option[T]] =
       publish(Setting(category, validateAndPrependName(name), descr, None, propertyClass = Some(summon[ClassTag[T]].runtimeClass), aliases = aliases.map(validateSetting)))
+    
+    def DeprecatedSetting(category: String, name: String, descr: String, deprecationMsg: String): Setting[Boolean] =
+      publish(Setting(category, validateAndPrependName(name), descr, false, deprecationMsg = Some(deprecationMsg)))
   }
 end Settings

compiler/src/dotty/tools/dotc/transform/CheckShadowing.scala

@@ -51,7 +51,7 @@ class CheckShadowing extends MiniPhase:
 
   override def isRunnable(using Context): Boolean =
     super.isRunnable &&
-    ctx.settings.Xlint.value.nonEmpty &&
+    ctx.settings.Wshadow.value.nonEmpty &&
     !ctx.isJava
 
   // Setup before the traversal
@@ -266,12 +266,12 @@ object CheckShadowing:
     /** Get the shadowing analysis's result */
     def getShadowingResult(using Context): ShadowResult =
       val privateWarnings: List[ShadowWarning] =
-        if ctx.settings.XlintHas.privateShadow then
+        if ctx.settings.WshadowHas.privateShadow then
           privateShadowWarnings.toList
         else
           Nil
       val typeParamWarnings: List[ShadowWarning] =
-        if ctx.settings.XlintHas.typeParameterShadow then
+        if ctx.settings.WshadowHas.typeParameterShadow then
           typeParamShadowWarnings.toList
         else
           Nil

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -31,7 +31,7 @@ class CompilationTests {
   @Test def pos: Unit = {
     implicit val testGroup: TestGroup = TestGroup("compilePos")
     var tests = List(
-      compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init", "-Wunused:all", "-Xlint:private-shadow", "-Xlint:type-parameter-shadow"), FileFilter.include(TestSources.posLintingAllowlist)),
+      compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init", "-Wunused:all", "-Wshadow:private-shadow", "-Wshadow:type-parameter-shadow"), FileFilter.include(TestSources.posLintingAllowlist)),
       compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init"), FileFilter.exclude(TestSources.posLintingAllowlist)),
       compileFilesInDir("tests/pos-deep-subtype", allowDeepSubtypes),
       compileFilesInDir("tests/pos-special/sourcepath/outer", defaultOptions.and("-sourcepath", "tests/pos-special/sourcepath")),

tests/neg/i17612b/i17612b.scala

@@ -1,4 +1,4 @@
-//> using options -Xfatal-warnings -Xlint:private-shadow -source:3.3
+//> using options -Xfatal-warnings -Wshadow:private-shadow -source:3.3
 
 object i17612b:
 

tests/neg/i17613b/i17613b.scala

@@ -1,4 +1,4 @@
-//> using options -Xlint:type-parameter-shadow -Xfatal-warnings
+//> using options -Wshadow:type-parameter-shadow -Xfatal-warnings
 
 object i17613b:
 	import importTry._

tests/warn/i17612a.scala

@@ -1,4 +1,4 @@
-//> using options  -Xlint:private-shadow -source:3.3
+//> using options  -Wshadow:private-shadow -source:3.3
 
 object i17612a:
 	class Base(var x: Int, val y: Int, var z: Int):

tests/warn/i17613a.scala

@@ -1,4 +1,4 @@
-//> using options  -Xlint:type-parameter-shadow
+//> using options  -Wshadow:type-parameter-shadow
 
 object i17613a:
 	class B: