Fix #525: Spuriously duplicate options (#536)

Co-authored-by: Alexandre Archambault <alexandre.archambault@gmail.com>
VirtusLab · Jan 11, 2022 · 5644277 · 5644277
1 parent ffc86cc
commit 5644277
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 20 deletions.
diff --git a/modules/build/src/main/scala/scala/build/Build.scala b/modules/build/src/main/scala/scala/build/Build.scala
@@ -163,8 +163,8 @@ object Build {
 
       val crossSources0 = crossSources.withVirtualDir(inputs0, scope, baseOptions)
 
-      val sources = value(crossSources0.scopedSources(baseOptions))
-        .sources(scope, baseOptions)
+      val scopedSources = value(crossSources0.scopedSources(baseOptions))
+      val sources       = scopedSources.sources(scope, baseOptions)
 
       val generatedSources = sources.generateSources(inputs0.generatedSrcRoot(scope))
       val buildOptions     = sources.buildOptions

diff --git a/modules/build/src/main/scala/scala/build/CrossSources.scala b/modules/build/src/main/scala/scala/build/CrossSources.scala
@@ -67,6 +67,7 @@ final case class CrossSources(
         .flatMap(_.withPlatform(platform.value).toSeq)
         .map(_.scopedValue(defaultScope)),
       buildOptions
+        .filter(!_.requirements.isEmpty)
         .flatMap(_.withScalaVersion(retainedScalaVersion).toSeq)
         .flatMap(_.withPlatform(platform.value).toSeq)
         .map(_.scopedValue(defaultScope))

diff --git a/modules/build/src/main/scala/scala/build/bsp/BspImpl.scala b/modules/build/src/main/scala/scala/build/bsp/BspImpl.scala
@@ -68,8 +68,8 @@ final class BspImpl(
     if (verbosity >= 3)
       pprint.stderr.log(scopedSources)
 
-    val sourcesMain = scopedSources.sources(Scope.Main, buildOptions)
-    val sourcesTest = scopedSources.sources(Scope.Test, buildOptions)
+    val sourcesMain = scopedSources.sources(Scope.Main, crossSources.sharedOptions(buildOptions))
+    val sourcesTest = scopedSources.sources(Scope.Test, crossSources.sharedOptions(buildOptions))
 
     if (verbosity >= 3)
       pprint.stderr.log(sourcesMain)

diff --git a/modules/build/src/main/scala/scala/build/options/HasBuildRequirements.scala b/modules/build/src/main/scala/scala/build/options/HasBuildRequirements.scala
@@ -14,4 +14,6 @@ final case class HasBuildRequirements[+T](
     }
   def scopedValue(defaultScope: Scope): HasScope[T] =
     HasScope(requirements.scope.map(_.scope).getOrElse(defaultScope), value)
+  def map[U](f: T => U): HasBuildRequirements[U] =
+    copy(value = f(value))
 }
diff --git a/modules/build/src/main/scala/scala/build/options/HasScope.scala b/modules/build/src/main/scala/scala/build/options/HasScope.scala
@@ -10,4 +10,6 @@ final case class HasScope[+T](
   def valueForInheriting(currentScope: Scope): Option[T] =
     if (currentScope.allScopes.contains(scope)) Some(value)
     else None
+  def map[U](f: T => U): HasScope[U] =
+    copy(value = f(value))
 }
diff --git a/modules/build/src/test/scala/scala/build/tests/BuildTests.scala b/modules/build/src/test/scala/scala/build/tests/BuildTests.scala
@@ -775,4 +775,30 @@ class BuildTests extends munit.FunSuite {
       assert(maybeBuild.toOption.get.options.scalaNativeOptions.linkingOptions.isEmpty)
     }
   }
+
+  // Issue #525
+  test("scalac options not spuriously duplicating") {
+    val inputs = TestInputs(
+      os.rel / "foo.scala" ->
+        """// using scala "2.13"
+          |// using options "-deprecation", "-feature", "-Xmaxwarns", "1"
+          |// using option "-Xdisable-assertions"
+          |
+          |def foo = "bar"
+          |""".stripMargin
+    )
+
+    val buildOptions: BuildOptions = defaultOptions.copy(
+      internal = defaultOptions.internal.copy(
+        keepDiagnostics = true
+      )
+    )
+
+    inputs.withBuild(buildOptions, buildThreads, bloopConfig) { (_, _, maybeBuild) =>
+      val expectedOptions =
+        Seq("-deprecation", "-feature", "-Xmaxwarns", "1", "-Xdisable-assertions")
+      val scalacOptions = maybeBuild.toOption.get.options.scalaOptions.scalacOptions
+      expect(scalacOptions == expectedOptions)
+    }
+  }
 }
diff --git a/modules/build/src/test/scala/scala/build/tests/SourcesTests.scala b/modules/build/src/test/scala/scala/build/tests/SourcesTests.scala
@@ -44,7 +44,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       expect(sources.buildOptions.classPathOptions.extraDependencies.map(_.value) == expectedDeps)
       expect(sources.paths.isEmpty)
@@ -78,7 +78,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       expect(sources.buildOptions.classPathOptions.extraDependencies.map(
         _.value
@@ -111,7 +111,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       expect(sources.buildOptions.classPathOptions.extraDependencies.map(_.value) == expectedDeps)
       expect(sources.paths.isEmpty)
@@ -140,7 +140,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       expect(sources.buildOptions.classPathOptions.extraDependencies.map(_.value) == expectedDeps)
       expect(sources.paths.isEmpty)
@@ -169,7 +169,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       expect(sources.buildOptions.classPathOptions.extraDependencies.map(_.value).isEmpty)
       expect(sources.paths.isEmpty)
@@ -203,7 +203,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       expect(sources.buildOptions.classPathOptions.extraDependencies.map(_.value) == expectedDeps)
       expect(sources.paths.length == 1)
@@ -236,7 +236,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       expect(sources.buildOptions.classPathOptions.extraDependencies.map(_.value) == expectedDeps)
       expect(sources.paths.isEmpty)
@@ -299,7 +299,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       val parsedCodes: Seq[String] = sources.inMemory.map(_._3)
 
@@ -331,7 +331,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       expect(sources.buildOptions.classPathOptions.extraDependencies.map(_.value) == expectedDeps)
       expect(sources.paths.isEmpty)
@@ -364,7 +364,7 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
+      val sources = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
 
       expect(sources.buildOptions.classPathOptions.extraDependencies.map(_.value) == expectedDeps)
       expect(sources.paths.isEmpty)
@@ -388,8 +388,8 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
-      val javaOpts      = sources.buildOptions.javaOptions.javaOpts.sortBy(_.toString())
+      val sources  = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
+      val javaOpts = sources.buildOptions.javaOptions.javaOpts.sortBy(_.toString())
 
       expect(
         javaOpts(0).value == "-Dfoo1",
@@ -419,8 +419,8 @@ class SourcesTests extends munit.FunSuite {
           TestLogger()
         ).orThrow
       val scopedSources = crossSources.scopedSources(BuildOptions()).orThrow
-      val sources       = scopedSources.sources(Scope.Main, BuildOptions())
-      val jsOptions     = sources.buildOptions.scalaJsOptions
+      val sources   = scopedSources.sources(Scope.Main, crossSources.sharedOptions(BuildOptions()))
+      val jsOptions = sources.buildOptions.scalaJsOptions
       expect(
         jsOptions.version == Some("1.8.0"),
         jsOptions.mode == Some("mode"),

diff --git a/modules/cli/src/main/scala/scala/cli/commands/Export.scala b/modules/cli/src/main/scala/scala/cli/commands/Export.scala
@@ -33,7 +33,7 @@ object Export extends ScalaCommand[ExportOptions] {
       )
     }
     val scopedSources = value(crossSources.scopedSources(buildOptions))
-    val sources       = scopedSources.sources(Scope.Main, buildOptions)
+    val sources       = scopedSources.sources(Scope.Main, crossSources.sharedOptions(buildOptions))
 
     if (verbosity >= 3)
       pprint.stderr.log(sources)

diff --git a/modules/cli/src/main/scala/scala/cli/commands/SetupIde.scala b/modules/cli/src/main/scala/scala/cli/commands/SetupIde.scala
@@ -38,7 +38,7 @@ object SetupIde extends ScalaCommand[SetupIdeOptions] {
       }
 
       value(crossSources.scopedSources(options))
-        .sources(Scope.Main, options)
+        .sources(Scope.Main, crossSources.sharedOptions(options))
         .buildOptions
     }