Override using directives scalac, java, and dependency options with t…

…heir cli counterparts (VirtusLab#612)

* Add initial String options and dependency shadowing mechanism

* Move implicits to seperate object

* Make scalac options Positioned and add prefixed option handling

* Shadow using directives java options with cli options

* Add tests for shadowing

* Apply scalafix

* scalafix

* Redesign shadowing for a more generic approach

* Revert changes to map ConfigMonoid

* scalafix

* fix compiler plugin option handling

* Simplify JavaOpt / ScalacOpt

They now only wrap a single option

* Better hashing for ShadowingSeq

* NIT / clean-up / diff minimization

Co-authored-by: Alexandre Archambault <alexandre.archambault@gmail.com>

modules/build/src/main/scala/scala/build/Build.scala

@@ -554,7 +554,7 @@ object Build {
     }
     else if (options.javaOptions.bloopJvmVersion.exists(_.value == 8))
       None
-    else if (options.scalaOptions.scalacOptions.contains("-release"))
+    else if (options.scalaOptions.scalacOptions.toSeq.exists(_.value.value == "-release"))
       None
     else
       Some(javaHome.value.version)
@@ -635,11 +635,13 @@ object Build {
     val scalacReleaseV = releaseFlagVersion.map(v => List("-release", v)).getOrElse(Nil)
     val javacReleaseV  = releaseFlagVersion.map(v => List("--release", v)).getOrElse(Nil)
 
-    val scalacOptions = options.scalaOptions.scalacOptions ++
-      pluginScalacOptions ++
-      semanticDbScalacOptions ++
-      sourceRootScalacOptions ++
-      scalaJsScalacOptions ++ scalacReleaseV
+    val scalacOptions =
+      options.scalaOptions.scalacOptions.toSeq.map(_.value.value) ++
+        pluginScalacOptions ++
+        semanticDbScalacOptions ++
+        sourceRootScalacOptions ++
+        scalaJsScalacOptions ++
+        scalacReleaseV
 
     val scalaCompiler = ScalaCompiler(
       scalaVersion = params.scalaVersion,

modules/build/src/main/scala/scala/build/Positioned.scala

@@ -1,6 +1,6 @@
 package scala.build
 
-import scala.build.options.{ConfigMonoid, HashedType}
+import scala.build.options.{ConfigMonoid, HashedType, ShadowingSeq}
 
 final case class Positioned[+T](
   positions: Seq[Position],
@@ -50,4 +50,12 @@ object Positioned {
 
   implicit def ordering[T](implicit underlying: Ordering[T]): Ordering[Positioned[T]] =
     Ordering.by(_.value)
+
+  implicit def keyOf[T](implicit
+    underlying: ShadowingSeq.KeyOf[T]
+  ): ShadowingSeq.KeyOf[Positioned[T]] =
+    ShadowingSeq.KeyOf(
+      p => underlying.get(p.value),
+      seq => underlying.groups(seq.map(_.value))
+    )
 }

modules/build/src/main/scala/scala/build/options/BuildOptions.scala

@@ -85,7 +85,7 @@ final case class BuildOptions(
     value(maybeJsDependencies).map(Positioned.none(_)) ++
       value(maybeNativeDependencies).map(Positioned.none(_)) ++
       value(scalaLibraryDependencies).map(Positioned.none(_)) ++
-      classPathOptions.extraDependencies
+      classPathOptions.extraDependencies.toSeq
   }
 
   private def semanticDbPlugins: Either[BuildException, Seq[AnyDependency]] = either {

modules/build/src/main/scala/scala/build/options/ClassPathOptions.scala

@@ -10,7 +10,7 @@ final case class ClassPathOptions(
   extraCompileOnlyJars: Seq[os.Path] = Nil,
   extraSourceJars: Seq[os.Path] = Nil,
   fetchSources: Option[Boolean] = None,
-  extraDependencies: Seq[Positioned[AnyDependency]] = Nil,
+  extraDependencies: ShadowingSeq[Positioned[AnyDependency]] = ShadowingSeq.empty,
   resourcesDir: Seq[os.Path] = Nil,
   resourcesVirtualDir: Seq[os.SubPath] = Nil
 )

modules/build/src/main/scala/scala/build/options/JavaOpt.scala

@@ -0,0 +1,25 @@
+package scala.build.options
+
+final case class JavaOpt(value: String) {
+  def key: Option[String] =
+    JavaOpt.optionPrefixes.find(value.startsWith)
+      .orElse {
+        if (value.startsWith("-")) Some(value.takeWhile(_ != ':'))
+        else if (value.startsWith("@")) Some("@")
+        else None
+      }
+}
+
+object JavaOpt {
+  /* Hardcoded prefixes for java options */
+  private val optionPrefixes = Set("-Xmn", "-Xms", "-Xmx", "-Xss")
+
+  implicit val hashedType: HashedType[JavaOpt] = {
+    opt => opt.value
+  }
+  implicit val keyOf: ShadowingSeq.KeyOf[JavaOpt] =
+    ShadowingSeq.KeyOf(
+      _.key,
+      seq => ScalacOpt.groupCliOptions(seq.map(_.value))
+    )
+}

modules/build/src/main/scala/scala/build/options/JavaOptions.scala

@@ -10,7 +10,7 @@ final case class JavaOptions(
   jvmIndexOpt: Option[String] = None,
   jvmIndexOs: Option[String] = None,
   jvmIndexArch: Option[String] = None,
-  javaOpts: Seq[Positioned[String]] = Nil,
+  javaOpts: ShadowingSeq[Positioned[JavaOpt]] = ShadowingSeq.empty,
   bloopJvmVersion: Option[Positioned[Int]] = None,
   javacPluginDependencies: Seq[Positioned[AnyDependency]] = Nil,
   javacPlugins: Seq[Positioned[os.Path]] = Nil,

modules/build/src/main/scala/scala/build/options/ScalaOptions.scala

@@ -9,7 +9,7 @@ final case class ScalaOptions(
   scalaBinaryVersion: Option[String] = None,
   addScalaLibrary: Option[Boolean] = None,
   generateSemanticDbs: Option[Boolean] = None,
-  scalacOptions: Seq[String] = Nil,
+  scalacOptions: ShadowingSeq[Positioned[ScalacOpt]] = ShadowingSeq.empty,
   extraScalaVersions: Set[String] = Set.empty,
   compilerPlugins: Seq[Positioned[AnyDependency]] = Nil,
   platform: Option[Positioned[Platform]] = None,

modules/build/src/main/scala/scala/build/options/ScalacOpt.scala

@@ -0,0 +1,37 @@
+package scala.build.options
+
+final case class ScalacOpt(value: String) {
+  def key: Option[String] =
+    if (value.startsWith("-"))
+      Some(value.takeWhile(_ != ':'))
+        .filterNot(ScalacOpt.repeatingKeys.contains)
+    else if (value.startsWith("@"))
+      Some("@")
+    else
+      None
+}
+
+object ScalacOpt {
+  private val repeatingKeys = Set(
+    "-Xplugin:",
+    "-P" // plugin options
+  )
+
+  implicit val hashedType: HashedType[ScalacOpt] = {
+    opt => opt.value
+  }
+  implicit val keyOf: ShadowingSeq.KeyOf[ScalacOpt] =
+    ShadowingSeq.KeyOf(
+      _.key,
+      seq => groupCliOptions(seq.map(_.value))
+    )
+
+  // 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("@") =>
+          idx
+      }
+}

modules/build/src/main/scala/scala/build/options/ShadowingSeq.scala

@@ -0,0 +1,73 @@
+package scala.build.options
+
+import dependency.AnyDependency
+import shapeless.Lazy
+
+import scala.collection.mutable
+
+/** Seq ensuring some of its values are unique according to some key */
+final case class ShadowingSeq[T] private (values: Seq[Seq[T]]) {
+  lazy val toSeq: Seq[T] = values.flatten
+  def ++(other: Seq[T])(implicit key: ShadowingSeq.KeyOf[T]): ShadowingSeq[T] =
+    addGroups(ShadowingSeq.groups(other, key.groups(other)))
+  private def addGroups(other: Seq[Seq[T]])(implicit key: ShadowingSeq.KeyOf[T]): ShadowingSeq[T] =
+    if (other.isEmpty) this
+    else {
+      val l    = new mutable.ListBuffer[Seq[T]]
+      val seen = new mutable.HashSet[String]
+
+      for (group <- values.iterator ++ other.iterator) {
+        assert(group.nonEmpty)
+        val keyOpt = key.get(group.head)
+        if (!keyOpt.exists(seen.contains)) {
+          l += group
+          for (key <- keyOpt)
+            seen += key
+        }
+      }
+
+      ShadowingSeq(l.toList)
+    }
+}
+
+object ShadowingSeq {
+
+  final case class KeyOf[T](
+    get: T => Option[String],
+    /** The indices at which sub-groups of values start */
+    groups: Seq[T] => Seq[Int]
+  )
+  object KeyOf {
+    implicit val keyOfAnyDependency: KeyOf[AnyDependency] =
+      KeyOf(dep => Some(dep.module.render), _.indices)
+  }
+
+  implicit def monoid[T](implicit key: KeyOf[T]): ConfigMonoid[ShadowingSeq[T]] =
+    ConfigMonoid.instance(ShadowingSeq.empty[T]) { (a, b) =>
+      a.addGroups(b.values)
+    }
+  implicit def hashedField[T](implicit
+    hasher: Lazy[HashedType[T]]
+  ): HashedField[ShadowingSeq[T]] = {
+    (name, seq, update) =>
+      for (t <- seq.toSeq)
+        update(s"$name+=${hasher.value.hashedValue(t)}")
+  }
+
+  def empty[T]: ShadowingSeq[T] = ShadowingSeq(Nil)
+
+  def from[T](values: Seq[T])(implicit key: KeyOf[T]): ShadowingSeq[T] =
+    empty[T] ++ values
+
+  private def groups[T](values: Seq[T], indices: Seq[Int]): Seq[Seq[T]] = {
+    val safeIndices = Seq(0) ++ indices ++ Seq(values.length)
+    safeIndices
+      .sliding(2)
+      .map {
+        case Seq(start, end) =>
+          values.slice(start, end)
+      }
+      .filter(_.nonEmpty)
+      .toVector
+  }
+}

modules/build/src/main/scala/scala/build/options/validation/BuildOptionsRule.scala

@@ -17,12 +17,12 @@ class ValidationException(
 
 object JvmOptionsForNonJvmBuild extends BuildOptionsRule {
   def validate(options: BuildOptions): List[Diagnostic] = {
-    val jvmOptions = options.javaOptions.javaOpts.find(p => p.value.nonEmpty)
+    val jvmOptions = options.javaOptions.javaOpts.toSeq
     if (jvmOptions.nonEmpty && options.platform.value != scala.build.options.Platform.JVM)
       List(Diagnostic(
         "Conflicting options. Jvm Options are valid only for jvm platform.",
         Severity.Warning,
-        options.platform.positions ++ jvmOptions.get.positions
+        options.platform.positions ++ jvmOptions.flatMap(_.positions)
       ))
     else Nil
   }

modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala

@@ -17,7 +17,7 @@ import scala.build.EitherCps.{either, value}
 import scala.build.Ops._
 import scala.build.errors.{BuildException, CompositeBuildException, DependencyFormatError}
 import scala.build.internal.{AmmUtil, Util}
-import scala.build.options.{BuildOptions, BuildRequirements, ClassPathOptions}
+import scala.build.options.{BuildOptions, BuildRequirements, ClassPathOptions, ShadowingSeq}
 import scala.build.preprocessing.directives._
 import scala.build.{Inputs, Logger, Position, Positioned}
 import scala.jdk.CollectionConverters._
@@ -261,7 +261,7 @@ case object ScalaPreprocessor extends Preprocessor {
       }
       val options = BuildOptions(
         classPathOptions = ClassPathOptions(
-          extraDependencies = deps
+          extraDependencies = ShadowingSeq.from(deps)
         )
       )
       Some(SpecialImportsProcessingOutput(BuildRequirements(), options, newCode))

modules/build/src/main/scala/scala/build/preprocessing/directives/UsingDependencyDirectiveHandler.scala

@@ -5,7 +5,7 @@ import dependency.parser.DependencyParser
 import scala.build.EitherCps.{either, value}
 import scala.build.Ops._
 import scala.build.errors.{BuildException, DependencyFormatError}
-import scala.build.options.{BuildOptions, ClassPathOptions}
+import scala.build.options.{BuildOptions, ClassPathOptions, ShadowingSeq}
 import scala.build.preprocessing.ScopePath
 import scala.build.{Logger, Positioned}
 
@@ -48,7 +48,7 @@ case object UsingDependencyDirectiveHandler extends UsingDirectiveHandler {
     ProcessedDirective(
       Some(BuildOptions(
         classPathOptions = ClassPathOptions(
-          extraDependencies = extraDependencies
+          extraDependencies = ShadowingSeq.from(extraDependencies)
         )
       )),
       Seq.empty

modules/build/src/main/scala/scala/build/preprocessing/directives/UsingJavaOptionsDirectiveHandler.scala

@@ -1,8 +1,9 @@
 package scala.build.preprocessing.directives
-import scala.build.Logger
+
 import scala.build.errors.BuildException
-import scala.build.options.{BuildOptions, JavaOptions}
+import scala.build.options.{BuildOptions, JavaOpt, JavaOptions, ShadowingSeq}
 import scala.build.preprocessing.ScopePath
+import scala.build.{Logger, Positioned}
 
 case object UsingJavaOptionsDirectiveHandler extends UsingDirectiveHandler {
   def name        = "Java options"
@@ -26,7 +27,12 @@ case object UsingJavaOptionsDirectiveHandler extends UsingDirectiveHandler {
     val javaOpts = DirectiveUtil.stringValues(directive.values, path, cwd)
     val options = BuildOptions(
       javaOptions = JavaOptions(
-        javaOpts = javaOpts.map { case (v, pos, _) => scala.build.Positioned(Seq(pos), v) }
+        javaOpts = ShadowingSeq.from(
+          javaOpts.map {
+            case (v, pos, _) =>
+              Positioned(Seq(pos), JavaOpt(v))
+          }
+        )
       )
     )
     Right(ProcessedDirective(Some(options), Seq.empty))

modules/build/src/main/scala/scala/build/preprocessing/directives/UsingJavaPropsDirectiveHandler.scala

@@ -1,6 +1,6 @@
 package scala.build.preprocessing.directives
 import scala.build.errors.BuildException
-import scala.build.options.{BuildOptions, JavaOptions}
+import scala.build.options.{BuildOptions, JavaOpt, JavaOptions, ShadowingSeq}
 import scala.build.preprocessing.ScopePath
 import scala.build.{Logger, Positioned}
 
@@ -25,11 +25,15 @@ case object UsingJavaPropsDirectiveHandler extends UsingDirectiveHandler {
     val javaProps = DirectiveUtil.stringValues(directive.values, path, cwd)
     val javaOpts = javaProps.map { case (value, position, _) =>
       value.split("=") match {
-        case Array(k)    => Positioned(position, s"-D$k")
-        case Array(k, v) => Positioned(position, s"-D$k=$v")
+        case Array(k)    => Positioned(position, JavaOpt(s"-D$k"))
+        case Array(k, v) => Positioned(position, JavaOpt(s"-D$k=$v"))
       }
     }
-    val options = BuildOptions(javaOptions = JavaOptions(javaOpts = javaOpts))
+    val options = BuildOptions(
+      javaOptions = JavaOptions(
+        javaOpts = ShadowingSeq.from(javaOpts)
+      )
+    )
     Right(ProcessedDirective(Some(options), Seq.empty))
   }
 }

modules/build/src/main/scala/scala/build/preprocessing/directives/UsingOptionDirectiveHandler.scala

@@ -1,8 +1,8 @@
 package scala.build.preprocessing.directives
-import scala.build.Logger
 import scala.build.errors.BuildException
-import scala.build.options.{BuildOptions, ScalaOptions}
+import scala.build.options.{BuildOptions, ScalaOptions, ScalacOpt, ShadowingSeq}
 import scala.build.preprocessing.ScopePath
+import scala.build.{Logger, Positioned}
 
 case object UsingOptionDirectiveHandler extends UsingDirectiveHandler {
   def name        = "Compiler options"
@@ -24,11 +24,14 @@ case object UsingOptionDirectiveHandler extends UsingDirectiveHandler {
     cwd: ScopePath,
     logger: Logger
   ): Either[BuildException, ProcessedUsingDirective] = {
-    val values        = directive.values
-    val scalacOptions = DirectiveUtil.stringValues(values, path, cwd)
+    val values = directive.values
+    val scalacOptions = DirectiveUtil.stringValues(values, path, cwd).map {
+      case (value, pos, _) =>
+        Positioned(Seq(pos), ScalacOpt(value))
+    }
     val options = BuildOptions(
       scalaOptions = ScalaOptions(
-        scalacOptions = scalacOptions.map(_._1)
+        scalacOptions = ShadowingSeq.from(scalacOptions)
       )
     )
     Right(ProcessedDirective(Some(options), Seq.empty))

modules/build/src/test/scala/scala/build/tests/BuildProjectTests.scala

@@ -4,7 +4,7 @@ import scala.build.errors.Severity
 import com.eed3si9n.expecty.Expecty.expect
 import scala.build.options.{BuildOptions, InternalOptions, JavaOptions, Scope}
 import scala.build.{Build, Inputs, LocalRepo, Positioned, Sources}
-import scala.build.options.ScalaOptions
+import scala.build.options.{ScalacOpt, ScalaOptions, ShadowingSeq}
 import scala.build.Logger
 import scala.build.Ops._
 import scala.build.blooprifle.BloopRifleLogger
@@ -66,7 +66,11 @@ class BuildProjectTests extends munit.FunSuite {
           Some(Positioned(bloopJavaPath, bloopJvmVersion)),
         javaHomeOpt = Some(Positioned.none(os.Path(javaHome)))
       ),
-      scalaOptions = ScalaOptions(scalacOptions = scalacOptions)
+      scalaOptions = ScalaOptions(
+        scalacOptions = ShadowingSeq.from(
+          scalacOptions.map(ScalacOpt(_)).map(Positioned.commandLine(_))
+        )
+      )
     )
 
     val inputs  = Inputs(Nil, None, os.pwd, "project", false)