Don't differentiate between - and -- prefixes in compiler options

VirtusLab · Oct 31, 2024 · 67a8043 · 67a8043
1 parent 6abc7cc
commit 67a8043
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 66 deletions.
diff --git a/modules/cli/src/main/scala/scala/cli/commands/ScalaCommand.scala b/modules/cli/src/main/scala/scala/cli/commands/ScalaCommand.scala
@@ -21,6 +21,7 @@ import scala.build.input.{ScalaCliInvokeData, SubCommand}
 import scala.build.internal.util.WarningMessages
 import scala.build.internal.{Constants, Runner}
 import scala.build.internals.{EnvVar, FeatureType}
+import scala.build.options.ScalacOpt.noDashPrefixes
 import scala.build.options.{BuildOptions, ScalacOpt, Scope}
 import scala.build.{Artifacts, Directories, Logger, Positioned, ReplArtifacts}
 import scala.cli.commands.default.LegacyScalaOptions
@@ -175,9 +176,13 @@ abstract class ScalaCommand[T <: HasGlobalOptions](implicit myParser: Parser[T],
     val logger = options.global.logging.logger
     sharedOptions(options).foreach { so =>
       val scalacOpts = so.scalacOptions.toScalacOptShadowingSeq
-      if scalacOpts.keys.contains(ScalacOpt(YScriptRunnerOption)) then
-        logger.message(
-          LegacyScalaOptions.yScriptRunnerWarning(scalacOpts.getOption(YScriptRunnerOption))
+      scalacOpts.keys
+        .find(k =>
+          k == ScalacOpt(s"-$YScriptRunnerOption") || k == ScalacOpt(s"--$YScriptRunnerOption")
+        )
+        .map(_.value)
+        .foreach(k =>
+          logger.message(LegacyScalaOptions.yScriptRunnerWarning(k, scalacOpts.getOption(k)))
         )
     }
   }
@@ -192,7 +197,7 @@ abstract class ScalaCommand[T <: HasGlobalOptions](implicit myParser: Parser[T],
       shared <- sharedOptions(options)
       scalacOptions        = shared.scalacOptions
       updatedScalacOptions = scalacOptions.withScalacExtraOptions(shared.scalacExtra)
-      if updatedScalacOptions.exists(ScalacOptions.ScalacPrintOptions)
+      if updatedScalacOptions.map(_.noDashPrefixes).exists(ScalacOptions.ScalacPrintOptions)
       logger = shared.logger
       fixedBuildOptions = buildOptions.copy(scalaOptions =
         buildOptions.scalaOptions.copy(defaultScalaVersion = Some(ScalaCli.getDefaultScalaVersion))

diff --git a/modules/cli/src/main/scala/scala/cli/commands/default/LegacyScalaOptions.scala b/modules/cli/src/main/scala/scala/cli/commands/default/LegacyScalaOptions.scala
@@ -11,7 +11,6 @@ import scala.cli.commands.default.LegacyScalaOptions.*
 import scala.cli.commands.package0.Package
 import scala.cli.commands.shared.HelpGroup
 import scala.cli.commands.shared.HelpMessages.PowerString
-import scala.cli.commands.shared.ScalacOptions.YScriptRunnerOption
 import scala.cli.commands.tags
 
 /** Options covering backwards compatibility with the old scala runner.
@@ -168,7 +167,7 @@ object LegacyScalaOptions {
   implicit lazy val parser: Parser[LegacyScalaOptions] = Parser.derive
   implicit lazy val help: Help[LegacyScalaOptions]     = Help.derive
 
-  def yScriptRunnerWarning(yScriptRunnerValue: Option[String]): String = {
+  def yScriptRunnerWarning(yScriptRunnerKey: String, yScriptRunnerValue: Option[String]): String = {
     val valueSpecificMsg = yScriptRunnerValue match {
       case Some(v @ "default") =>
         s"scala.tools.nsc.DefaultScriptRunner (the $v script runner) is no longer available."
@@ -185,7 +184,7 @@ object LegacyScalaOptions {
         s"Using $className as the script runner is no longer supported and will not be attempted."
       case _ => ""
     }
-    s"""Deprecated option '$YScriptRunnerOption' is ignored.
+    s"""Deprecated option '$yScriptRunnerKey' is ignored.
        |The script runner can no longer be picked as before.
        |$valueSpecificMsg""".stripMargin
   }

diff --git a/modules/cli/src/main/scala/scala/cli/commands/shared/ScalacOptions.scala b/modules/cli/src/main/scala/scala/cli/commands/shared/ScalacOptions.scala
@@ -8,6 +8,7 @@ import caseapp.core.{Arg, Error}
 import com.github.plokhotnyuk.jsoniter_scala.core.*
 import com.github.plokhotnyuk.jsoniter_scala.macros.*
 
+import scala.build.options.ScalacOpt.noDashPrefixes
 import scala.cli.commands.tags
 
 // format: off
@@ -26,6 +27,13 @@ final case class ScalacOptions(
 // format: on
 
 object ScalacOptions {
+  extension (opt: String) {
+    private def hasValidScalacOptionDashes: Boolean =
+      opt.startsWith("-") && opt.length > 1 && (
+        if opt.length > 2 then opt.charAt(2) != '-'
+        else opt.charAt(1) != '-'
+      )
+  }
 
   private val scalacOptionsArg = Arg("scalacOption").copy(
     extraNames = Seq(Name("scala-opt"), Name("O"), Name("scala-option")),
@@ -37,56 +45,54 @@ object ScalacOptions {
     origin = Some("ScalacOptions")
   )
   // .withIsFlag(true) // The scalac options we handle accept no value after the -… argument
-  val YScriptRunnerOption = "-Yscriptrunner"
-  private val scalacOptionsPurePrefixes =
-    Set("-V", "-W", "-X", "-Y")
-  private val scalacOptionsPrefixes =
-    Set("-P") ++ scalacOptionsPurePrefixes
+  val YScriptRunnerOption               = "Yscriptrunner"
+  private val scalacOptionsPurePrefixes = Set("V", "W", "X", "Y")
+  private val scalacOptionsPrefixes     = Set("P") ++ scalacOptionsPurePrefixes
   private val scalacAliasedOptions = // these options don't require being passed after -O and accept an arg
     Set(
-      "-encoding",
-      "-release",
-      "-color",
-      "-g",
-      "-language",
-      "-opt",
-      "-target",
-      "-source",
+      "encoding",
+      "release",
+      "color",
+      "g",
+      "language",
+      "opt",
+      "target",
+      "source",
       YScriptRunnerOption
     )
   private val scalacNoArgAliasedOptions = // these options don't require being passed after -O and don't accept an arg
     Set(
-      "-unchecked",
-      "-nowarn",
-      "-feature",
-      "-deprecation",
-      "-rewrite",
-      "-old-syntax",
-      "-new-syntax",
-      "-indent",
-      "-no-indent"
+      "unchecked",
+      "nowarn",
+      "feature",
+      "deprecation",
+      "rewrite",
+      "old-syntax",
+      "new-syntax",
+      "indent",
+      "no-indent"
     )
 
   /** This includes all the scalac options which disregard inputs and print a help and/or context
     * message instead.
     */
   val ScalacPrintOptions: Set[String] =
     scalacOptionsPurePrefixes ++ Set(
-      "-help",
-      "-opt:help",
-      "-Xshow-phases",
-      "-Xsource:help",
-      "-Xplugin-list",
-      "-Xmixin-force-forwarders:help",
-      "-Xlint:help",
-      "-Vphases"
+      "help",
+      "opt:help",
+      "Xshow-phases",
+      "Xsource:help",
+      "Xplugin-list",
+      "Xmixin-force-forwarders:help",
+      "Xlint:help",
+      "Vphases"
     )
 
   /** This includes all the scalac options which are redirected to native Scala CLI options. */
-  val ScalaCliRedirectedOptions = Set(
-    "-classpath",
-    "-cp", // redirected to --extra-jars
-    "-d"   // redirected to --compilation-output
+  val ScalaCliRedirectedOptions: Set[String] = Set(
+    "classpath",
+    "cp", // redirected to --extra-jars
+    "d"   // redirected to --compilation-output
   )
   val ScalacDeprecatedOptions: Set[String] = Set(
     YScriptRunnerOption // old 'scala' runner specific, no longer supported
@@ -109,15 +115,20 @@ object ScalacOptions {
       ): Either[(Error, List[String]), Option[(Option[List[String]], List[String])]] =
         args match {
           case h :: t
-              if scalacOptionsPrefixes.exists(h.startsWith) &&
-              !ScalacDeprecatedOptions.contains(h) =>
+              if h.hasValidScalacOptionDashes &&
+              scalacOptionsPrefixes.exists(h.noDashPrefixes.startsWith) &&
+              !ScalacDeprecatedOptions.contains(h.noDashPrefixes) =>
             Right(Some((Some(acc.getOrElse(Nil) :+ h), t)))
-          case h :: t if scalacNoArgAliasedOptions.contains(h) =>
+          case h :: t
+              if h.hasValidScalacOptionDashes &&
+              scalacNoArgAliasedOptions.contains(h.noDashPrefixes) =>
             Right(Some((Some(acc.getOrElse(Nil) :+ h), t)))
           case h :: t
-              if scalacAliasedOptions.exists(o => h.startsWith(o + ":")) &&
+              if h.hasValidScalacOptionDashes &&
+              scalacAliasedOptions.exists(o => h.noDashPrefixes.startsWith(o + ":")) &&
               h.count(_ == ':') == 1 => Right(Some((Some(acc.getOrElse(Nil) :+ h), t)))
-          case h :: t if scalacAliasedOptions.contains(h) =>
+          case h :: t
+              if h.hasValidScalacOptionDashes && scalacAliasedOptions.contains(h.noDashPrefixes) =>
             // check if the next scalac arg is a different option or a param to the current option
             val maybeOptionArg = t.headOption.filter(!_.startsWith("-"))
             // if it's a param, it'll be treated as such and considered already parsed
@@ -131,8 +142,8 @@ object ScalacOptions {
     }
 
   implicit lazy val parser: Parser[ScalacOptions] = {
-    val baseParser = scalacOptionsArgument :: NilParser
-    implicit val p = ArgFileOption.parser
+    val baseParser                              = scalacOptionsArgument :: NilParser
+    implicit val p: Parser[List[ArgFileOption]] = ArgFileOption.parser
     baseParser.addAll[List[ArgFileOption]].to[ScalacOptions]
   }
 
@@ -143,7 +154,7 @@ object ScalacOptions {
 case class ArgFileOption(file: String) extends AnyVal
 
 object ArgFileOption {
-  val arg = Arg(
+  val arg: Arg = Arg(
     name = Name("args-file"),
     valueDescription = Some(ValueDescription("@arguments-file")),
     helpMessage = Some(HelpMessage("File with scalac options.")),

diff --git a/modules/cli/src/main/scala/scala/cli/commands/util/ScalacOptionsUtil.scala b/modules/cli/src/main/scala/scala/cli/commands/util/ScalacOptionsUtil.scala
@@ -1,7 +1,7 @@
 package scala.cli.commands.util
 
 import scala.build.Logger
-import scala.build.options.ScalacOpt.filterScalacOptionKeys
+import scala.build.options.ScalacOpt.{filterScalacOptionKeys, noDashPrefixes}
 import scala.build.options.{ScalacOpt, ShadowingSeq}
 import scala.cli.commands.bloop.BloopExit
 import scala.cli.commands.default.LegacyScalaOptions
@@ -33,9 +33,13 @@ object ScalacOptionsUtil {
 
   extension (opts: ShadowingSeq[ScalacOpt]) {
     def filterNonRedirected: ShadowingSeq[ScalacOpt] =
-      opts.filterScalacOptionKeys(!ScalacOptions.ScalaCliRedirectedOptions.contains(_))
+      opts.filterScalacOptionKeys(k =>
+        !ScalacOptions.ScalaCliRedirectedOptions.contains(k.noDashPrefixes)
+      )
     def filterNonDeprecated: ShadowingSeq[ScalacOpt] =
-      opts.filterScalacOptionKeys(!ScalacOptions.ScalacDeprecatedOptions.contains(_))
+      opts.filterScalacOptionKeys(k =>
+        !ScalacOptions.ScalacDeprecatedOptions.contains(k.noDashPrefixes)
+      )
     def getOption(key: String): Option[String] =
       opts.get(ScalacOpt(key)).headOption.map(_.value)
   }

diff --git a/...integration/src/test/scala/scala/cli/integration/CompileScalacCompatTestDefinitions.scala b/...integration/src/test/scala/scala/cli/integration/CompileScalacCompatTestDefinitions.scala
@@ -40,15 +40,26 @@ trait CompileScalacCompatTestDefinitions { _: CompileTestDefinitions =>
     for {
       mode <- modes
       if actualScalaVersion == Constants.scala3Next
+      dashPrefix <- Seq("-", "--")
       syntaxVariant <- Seq(
-        Seq(Seq("-color:never"), Seq("-language:noAutoTupling"), Seq("-language:strictEquality")),
         Seq(
-          Seq("-color", "never"),
-          Seq("-language", "noAutoTupling"),
-          Seq("-language", "strictEquality")
+          Seq(s"${dashPrefix}color:never"),
+          Seq(s"${dashPrefix}language:noAutoTupling"),
+          Seq(s"${dashPrefix}language:strictEquality")
         ),
-        Seq(Seq("-color:never"), Seq("\"-language:noAutoTupling,strictEquality\"")),
-        Seq(Seq("-color", "never"), Seq("-language", "\"noAutoTupling,strictEquality\""))
+        Seq(
+          Seq(s"${dashPrefix}color", "never"),
+          Seq(s"${dashPrefix}language", "noAutoTupling"),
+          Seq(s"${dashPrefix}language", "strictEquality")
+        ),
+        Seq(
+          Seq(s"${dashPrefix}color:never"),
+          Seq(s"\"${dashPrefix}language:noAutoTupling,strictEquality\"")
+        ),
+        Seq(
+          Seq(s"${dashPrefix}color", "never"),
+          Seq(s"${dashPrefix}language", "\"noAutoTupling,strictEquality\"")
+        )
       )
       (cliOpts, directiveOpts) = {
         val (initialCliOpts, initialDirectiveOpts) = mode match {

diff --git a/modules/options/src/main/scala/scala/build/options/ScalacOpt.scala b/modules/options/src/main/scala/scala/build/options/ScalacOpt.scala
@@ -1,25 +1,33 @@
 package scala.build.options
 
+import scala.build.options.ScalacOpt.noDashPrefixes
+
 final case class ScalacOpt(value: String) {
 
   /** @return raw key for the option (if valid) */
   private[options] def key: Option[String] =
-    if value.startsWith("-") then Some(value.takeWhile(_ != ':'))
+    if value.startsWith("-") || value.startsWith("--") then Some(value.takeWhile(_ != ':'))
     else Some("@").filter(value.startsWith)
 
   /** @return raw key for the option (only if the key can be shadowed from the CLI) */
   private[options] def shadowableKey: Option[String] = key match
     case Some(key)
-        if ScalacOpt.repeatingKeys.exists(rKey => rKey.startsWith(key + ":") || rKey == key) => None
+        if ScalacOpt.repeatingKeys
+          .exists(rKey =>
+            rKey.startsWith(key.noDashPrefixes + ":") || rKey == key.noDashPrefixes
+          ) => None
     case otherwise => otherwise
 }
 
 object ScalacOpt {
+  extension (opt: String) {
+    def noDashPrefixes: String = opt.stripPrefix("--").stripPrefix("-")
+  }
   private val repeatingKeys = Set(
-    "-Xplugin",
-    "-P", // plugin options
-    "-language",
-    "-Wconf"
+    "Xplugin",
+    "P", // plugin options
+    "language",
+    "Wconf"
   )
 
   implicit val hashedType: HashedType[ScalacOpt] = {
@@ -32,12 +40,12 @@ object ScalacOpt {
       seq => groupCliOptions(seq.map(_.value))
     )
 
-  // Groups options (starting with `-` or `@`) with option arguments that follow
+  // Groups options (starting with `-`, `--` or `@`) with option arguments that follow
   def groupCliOptions(opts: Seq[String]): Seq[Int] =
     opts
       .zipWithIndex
       .collect {
-        case (opt, idx) if opt.startsWith("-") || opt.startsWith("@") =>
+        case (opt, idx) if opt.startsWith("-") || opt.startsWith("--") || opt.startsWith("@") =>
           idx
       }