Merge pull request #2098 from Gedochao/fix-export-directive-file-duplicates

Fix `export` failing on input duplicates

Author: Gedochao

modules/build/src/main/scala/scala/build/CollectionOps.scala

@@ -0,0 +1,29 @@
+package scala.build
+import scala.collection.mutable
+object CollectionOps {
+  extension [T](items: Seq[T]) {
+
+    /** Works the same standard lib's `distinct`, but only differentiates based on the key extracted
+      * by the passed function. If more than one value exists for the same key, only the first one
+      * is kept, the rest is filtered out.
+      *
+      * @param f
+      *   function to extract the key used for distinction
+      * @tparam K
+      *   type of the key used for distinction
+      * @return
+      *   the sequence of items with distinct [[items]].map(f)
+      */
+    def distinctBy[K](f: T => K): Seq[T] =
+      if items.length == 1 then items
+      else
+        val seen = mutable.HashSet.empty[K]
+        items.filter { item =>
+          val key = f(item)
+          if seen(key) then false
+          else
+            seen += key
+            true
+        }
+  }
+}

modules/build/src/main/scala/scala/build/CrossSources.scala

@@ -2,6 +2,7 @@ package scala.build
 
 import java.io.File
 
+import scala.build.CollectionOps.*
 import scala.build.EitherCps.{either, value}
 import scala.build.Ops.*
 import scala.build.Positioned
@@ -266,9 +267,21 @@ object CrossSources {
       inputs.withElements(elements = filteredElements)
     )
 
-    val preprocessedSources =
+    val preprocessedSources: Seq[PreprocessedSource] =
       (preprocessedInputFromArgs ++ preprocessedSourcesFromDirectives).distinct
-        .pipe(sources => value(validateExcludeDirectives(sources, allInputs.workspace)))
+        .pipe { sources =>
+          val validatedSources: Seq[PreprocessedSource] =
+            value(validateExcludeDirectives(sources, allInputs.workspace))
+          val distinctSources = validatedSources.distinctBy(_.distinctPathOrSource)
+          val diff            = validatedSources.diff(distinctSources)
+          if diff.nonEmpty then
+            val diffString = diff.map(_.distinctPathOrSource).mkString(s"${System.lineSeparator}  ")
+            logger.message(
+              s"""[${Console.YELLOW}warn${Console.RESET}] Skipped duplicate sources:
+                 |  $diffString""".stripMargin
+            )
+          distinctSources
+        }
 
     val scopedRequirements       = preprocessedSources.flatMap(_.scopedRequirements)
     val scopedRequirementsByRoot = scopedRequirements.groupBy(_.path.root)

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

@@ -9,10 +9,15 @@ sealed abstract class PreprocessedSource extends Product with Serializable {
   def optionsWithTargetRequirements: List[WithBuildRequirements[BuildOptions]]
   def requirements: Option[BuildRequirements]
   def mainClassOpt: Option[String]
-
   def scopedRequirements: Seq[Scoped[BuildRequirements]]
   def scopePath: ScopePath
   def directivesPositions: Option[Position.File]
+  def distinctPathOrSource: String = this match {
+    case PreprocessedSource.OnDisk(p, _, _, _, _, _, _)                   => p.toString
+    case PreprocessedSource.InMemory(op, rp, _, _, _, _, _, _, _, _, _)   => s"$op; $rp"
+    case PreprocessedSource.UnwrappedScript(p, _, _, _, _, _, _, _, _, _) => p.toString
+    case PreprocessedSource.NoSourceCode(_, _, _, _, p)                   => p.toString
+  }
 }
 
 object PreprocessedSource {

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

@@ -0,0 +1,45 @@
+package scala.build.tests
+
+import com.eed3si9n.expecty.Expecty.expect
+
+import scala.build.CollectionOps.distinctBy
+class DistinctByTests extends munit.FunSuite {
+  case class Message(a: String, b: Int)
+  val distinctData = Seq(
+    Message(a = "1", b = 4),
+    Message(a = "2", b = 3),
+    Message(a = "3", b = 2),
+    Message(a = "4", b = 1)
+  )
+  val repeatingData = Seq(
+    Message(a = "1", b = 4),
+    Message(a = "1", b = 44),
+    Message(a = "2", b = 3),
+    Message(a = "22", b = 3),
+    Message(a = "3", b = 22),
+    Message(a = "33", b = 2),
+    Message(a = "4", b = 1),
+    Message(a = "4", b = 11)
+  )
+
+  test("distinctBy where data is already distinct") {
+    val distinctByA     = distinctData.distinctBy(_.a)
+    val distinctByB     = distinctData.distinctBy(_.b)
+    val generalDistinct = distinctData.distinct
+    expect(distinctData == generalDistinct)
+    expect(distinctData == distinctByA)
+    expect(distinctData == distinctByB)
+  }
+
+  test("distinctBy doesn't change data order") {
+    val expectedData = Seq(
+      Message(a = "1", b = 4),
+      Message(a = "2", b = 3),
+      Message(a = "22", b = 3),
+      Message(a = "3", b = 22),
+      Message(a = "33", b = 2),
+      Message(a = "4", b = 1)
+    )
+    expect(repeatingData.distinctBy(_.a) == expectedData)
+  }
+}

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

@@ -6,10 +6,8 @@ import coursier.util.{Artifact, Task}
 import dependency.*
 
 import scala.build.Ops.*
-import scala.build.Sources
+import scala.build.{CrossSources, Position, Sources, UnwrappedCrossSources}
 import scala.build.internal.ObjectCodeWrapper
-import scala.build.CrossSources
-import scala.build.Position
 import scala.build.errors.{UsingDirectiveValueNumError, UsingDirectiveWrongValueTypeError}
 import scala.build.input.ScalaCliInvokeData
 import scala.build.options.{BuildOptions, Scope, SuppressWarningOptions}
@@ -536,4 +534,42 @@ class SourcesTests extends munit.FunSuite {
     }
   }
 
+  test("CrossSources.forInputs respects the order of inputs passed") {
+    val inputArgs @ Seq(project, main, abc, message) =
+      Seq("project.scala", "Main.scala", "Abc.scala", "Message.scala")
+    val testInputs = TestInputs(
+      os.rel / project ->
+        """//> using dep "com.lihaoyi::os-lib::0.8.1"
+          |//> using file "Message.scala"
+          |""".stripMargin,
+      os.rel / main ->
+        """object Main extends App {
+          |  println(Message(Abc.hello))
+          |}
+          |""".stripMargin,
+      os.rel / abc ->
+        """object Abc {
+          |  val hello = "Hello"
+          |}
+          |""".stripMargin,
+      os.rel / message ->
+        """case class Message(value: String)
+          |""".stripMargin
+    )
+    testInputs.withInputs { (_, inputs) =>
+      val crossSourcesResult =
+        CrossSources.forInputs(
+          inputs,
+          preprocessors,
+          TestLogger(),
+          SuppressWarningOptions()
+        )
+      assert(crossSourcesResult.isRight)
+      val Right(CrossSources(onDiskSources, _, _, _, _)) =
+        crossSourcesResult.map(_._1.withWrappedScripts(BuildOptions()))
+      val onDiskPaths = onDiskSources.map(_.value._1.last)
+      expect(onDiskPaths == inputArgs)
+    }
+  }
+
 }

modules/cli/src/main/scala/scala/cli/commands/export0/Export.scala

@@ -40,7 +40,7 @@ object Export extends ScalaCommand[ExportOptions] {
 
     logger.log("Preparing build")
 
-    val (crossSources, _) = value {
+    val (crossSources: UnwrappedCrossSources, _) = value {
       CrossSources.forInputs(
         inputs,
         Sources.defaultPreprocessors(
@@ -56,8 +56,9 @@ object Export extends ScalaCommand[ExportOptions] {
 
     val wrappedScriptsSources = crossSources.withWrappedScripts(buildOptions)
 
-    val scopedSources = value(wrappedScriptsSources.scopedSources(buildOptions))
-    val sources = scopedSources.sources(scope, wrappedScriptsSources.sharedOptions(buildOptions))
+    val scopedSources: ScopedSources = value(wrappedScriptsSources.scopedSources(buildOptions))
+    val sources: Sources =
+      scopedSources.sources(scope, wrappedScriptsSources.sharedOptions(buildOptions))
 
     if (verbosity >= 3)
       pprint.err.log(sources)

modules/integration/src/test/scala/scala/cli/integration/ExportCommonTestDefinitions.scala

@@ -7,7 +7,8 @@ import java.nio.charset.Charset
 import scala.util.Properties
 
 trait ExportCommonTestDefinitions { _: ScalaCliSuite & TestScalaVersionArgs =>
-  protected lazy val extraOptions: Seq[String] = scalaVersionArgs ++ TestUtil.extraOptions
+  protected lazy val extraOptions: Seq[String] =
+    scalaVersionArgs ++ TestUtil.extraOptions ++ Seq("--suppress-experimental-warning")
 
   protected def runExportTests: Boolean = Properties.isLinux
 
@@ -61,6 +62,17 @@ trait ExportCommonTestDefinitions { _: ScalaCliSuite & TestScalaVersionArgs =>
       expect(output.contains("Hello"))
     }
 
+  def extraSourceFromDirectiveWithExtraDependency(inputs: String*): Unit =
+    prepareTestInputs(
+      ExportTestProjects.extraSourceFromDirectiveWithExtraDependency(actualScalaVersion)
+    ).fromRoot { root =>
+      exportCommand(inputs*).call(cwd = root, stdout = os.Inherit)
+      val res = buildToolCommand(root, runMainArgs*)
+        .call(cwd = root / outputDir)
+      val output = res.out.trim(Charset.defaultCharset())
+      expect(output.contains(root.toString))
+    }
+
   if (runExportTests) {
     test("JVM") {
       jvmTest()
@@ -74,5 +86,11 @@ trait ExportCommonTestDefinitions { _: ScalaCliSuite & TestScalaVersionArgs =>
     test("Ensure test framework NPE is not thrown when depending on logback") {
       logbackBugCase()
     }
+    test("extra source from a directive introducing a dependency") {
+      extraSourceFromDirectiveWithExtraDependency("Main.scala")
+    }
+    test("extra source passed both via directive and from command line") {
+      extraSourceFromDirectiveWithExtraDependency(".")
+    }
   }
 }

modules/integration/src/test/scala/scala/cli/integration/ExportTestProjects.scala

@@ -279,4 +279,19 @@ object ExportTestProjects {
          |//> using lib "ch.qos.logback:logback-classic:1.4.5"
          |println("Hello")
          |""".stripMargin)
+
+  def extraSourceFromDirectiveWithExtraDependency(scalaVersion: String): TestInputs =
+    TestInputs(
+      os.rel / "Main.scala" ->
+        s"""//> using scala "$scalaVersion"
+           |//> using file "Message.scala"
+           |object Main extends App {
+           |  println(Message(value = os.pwd.toString).value)
+           |}
+           |""".stripMargin,
+      os.rel / "Message.scala" ->
+        s"""//> using dep "com.lihaoyi::os-lib:0.9.1"
+           |case class Message(value: String)
+           |""".stripMargin
+    )
 }