Don't print the spread directives warning if there's only a single fi…

…le per scope
VirtusLab · Apr 5, 2023 · 9a5ba7d · 9a5ba7d
1 parent 0226124
commit 9a5ba7d
Show file tree

Hide file tree

Showing 3 changed files with 119 additions and 42 deletions.
diff --git a/modules/build/src/main/scala/scala/build/CrossSources.scala b/modules/build/src/main/scala/scala/build/CrossSources.scala
@@ -170,31 +170,6 @@ object CrossSources {
     val preprocessedSources =
       (preprocessedInputFromArgs ++ preprocessedSourcesFromDirectives).distinct
 
-    val preprocessedWithUsingDirs = preprocessedSources.filter(_.directivesPositions.isDefined)
-    if preprocessedWithUsingDirs.length > 1 &&
-      !suppressWarningOptions.suppressDirectivesInMultipleFilesWarning.getOrElse(false)
-    then {
-      val projectFilePath = inputs.elements.projectSettingsFiles.headOption match
-        case Some(s) => s.path
-        case _       => inputs.workspace / Constants.projectFileName
-      preprocessedWithUsingDirs
-        .filter(_.scopePath != ScopePath.fromPath(projectFilePath))
-        .foreach { source =>
-          source.directivesPositions match
-            case Some(positions) =>
-              val pos = Seq(
-                positions.codeDirectives,
-                positions.specialCommentDirectives,
-                positions.plainCommentDirectives
-              ).maxBy(p => p.endPos._1)
-              logger.diagnostic(
-                s"Using directives detected in multiple files. It is recommended to keep them centralized in the $projectFilePath file.",
-                positions = Seq(pos)
-              )
-            case _ => ()
-        }
-    }
-
     val scopedRequirements       = preprocessedSources.flatMap(_.scopedRequirements)
     val scopedRequirementsByRoot = scopedRequirements.groupBy(_.path.root)
     def baseReqs(path: ScopePath): BuildRequirements = {
@@ -234,22 +209,26 @@ object CrossSources {
       mainClass          <- processedMainClass.mainClassOpt
     } yield mainClass
 
-    val paths = preprocessedSources.collect {
-      case d: PreprocessedSource.OnDisk =>
-        val baseReqs0 = baseReqs(d.scopePath)
-        WithBuildRequirements(
-          d.requirements.fold(baseReqs0)(_ orElse baseReqs0),
-          (d.path, d.path.relativeTo(allInputs.workspace))
-        )
-    }
-    val inMemory = preprocessedSources.collect {
-      case m: PreprocessedSource.InMemory =>
-        val baseReqs0 = baseReqs(m.scopePath)
-        WithBuildRequirements(
-          m.requirements.fold(baseReqs0)(_ orElse baseReqs0),
-          Sources.InMemory(m.originalPath, m.relPath, m.code, m.ignoreLen)
-        )
-    }
+    val pathsWithDirectivePositions
+      : Seq[(WithBuildRequirements[(os.Path, os.RelPath)], Option[DirectivesPositions])] =
+      preprocessedSources.collect {
+        case d: PreprocessedSource.OnDisk =>
+          val baseReqs0 = baseReqs(d.scopePath)
+          WithBuildRequirements(
+            d.requirements.fold(baseReqs0)(_ orElse baseReqs0),
+            (d.path, d.path.relativeTo(allInputs.workspace))
+          ) -> d.directivesPositions
+      }
+    val inMemoryWithDirectivePositions
+      : Seq[(WithBuildRequirements[Sources.InMemory], Option[DirectivesPositions])] =
+      preprocessedSources.collect {
+        case m: PreprocessedSource.InMemory =>
+          val baseReqs0 = baseReqs(m.scopePath)
+          WithBuildRequirements(
+            m.requirements.fold(baseReqs0)(_ orElse baseReqs0),
+            Sources.InMemory(m.originalPath, m.relPath, m.code, m.ignoreLen)
+          ) -> m.directivesPositions
+      }
 
     val resourceDirs = allInputs.elements.collect {
       case r: ResourceDirectory =>
@@ -258,6 +237,44 @@ object CrossSources {
       WithBuildRequirements(BuildRequirements(), _)
     )
 
+    val allPathsWithDirectivesByScope: Map[Scope, Seq[(os.Path, DirectivesPositions)]] =
+      (pathsWithDirectivePositions ++ inMemoryWithDirectivePositions)
+        .flatMap { (withBuildRequirements, directivesPositions) =>
+          val scope = withBuildRequirements.scopedValue(Scope.Main).scope
+          val path: os.Path = withBuildRequirements.value match
+            case im: Sources.InMemory =>
+              im.originalPath match
+                case Right((_, p: os.Path)) => p
+                case _                      => inputs.workspace / im.generatedRelPath
+            case (p: os.Path, _) => p
+          directivesPositions.map((path, scope, _))
+        }
+        .groupBy((_, scope, _) => scope)
+        .view
+        .mapValues(_.map((path, _, directivesPositions) => path -> directivesPositions))
+        .toMap
+    lazy val anyScopeHasMultipleSourcesWithDirectives =
+      Scope.all.exists(allPathsWithDirectivesByScope.get(_).map(_.length).getOrElse(0) > 1)
+    val shouldSuppressWarning =
+      suppressWarningOptions.suppressDirectivesInMultipleFilesWarning.getOrElse(false)
+    if !shouldSuppressWarning && anyScopeHasMultipleSourcesWithDirectives then {
+      val projectFilePath = inputs.elements.projectSettingsFiles.headOption match
+        case Some(s) => s.path
+        case _       => inputs.workspace / Constants.projectFileName
+      allPathsWithDirectivesByScope
+        .values
+        .flatten
+        .filter((path, _) => ScopePath.fromPath(path) != ScopePath.fromPath(projectFilePath))
+        .foreach { (_, directivesPositions) =>
+          logger.diagnostic(
+            s"Using directives detected in multiple files. It is recommended to keep them centralized in the $projectFilePath file.",
+            positions = Seq(directivesPositions.all.maxBy(_.endPos._1))
+          )
+        }
+    }
+
+    val paths    = pathsWithDirectivePositions.map(_._1)
+    val inMemory = inMemoryWithDirectivePositions.map(_._1)
     (CrossSources(paths, inMemory, defaultMainClassOpt, resourceDirs, buildOptions), allInputs)
   }
 

diff --git a/modules/build/src/main/scala/scala/build/preprocessing/ExtractedDirectives.scala b/modules/build/src/main/scala/scala/build/preprocessing/ExtractedDirectives.scala
@@ -30,7 +30,10 @@ case class DirectivesPositions(
   codeDirectives: Position.File,
   specialCommentDirectives: Position.File,
   plainCommentDirectives: Position.File
-)
+) {
+  def all: Seq[Position.File] =
+    Seq(codeDirectives, specialCommentDirectives, plainCommentDirectives)
+}
 
 object ExtractedDirectives {
 

diff --git a/modules/integration/src/test/scala/scala/cli/integration/CompileTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/CompileTestDefinitions.scala
@@ -77,6 +77,63 @@ abstract class CompileTestDefinitions(val scalaVersionOpt: Option[String])
     }
   }
 
+  test("with one file per scope, no warning about spread directives should be printed") {
+    TestInputs(
+      os.rel / "Bar.scala" ->
+        """//> using dep "com.lihaoyi::os-lib:0.9.1"
+          |
+          |object Bar extends App {
+          |  println(os.pwd)
+          |}
+          |""".stripMargin,
+      os.rel / "Foo.test.scala" ->
+        """//> using dep "org.scalameta::munit:0.7.29"
+          |
+          |class Foo extends munit.FunSuite {
+          |  test("Hello") {
+          |    assert(true)
+          |  }
+          |}
+          |""".stripMargin
+    ).fromRoot { root =>
+      val warningMessage = "Using directives detected in multiple files"
+      val output = os.proc(TestUtil.cli, "compile", ".", "--test", extraOptions)
+        .call(cwd = root, stderr = os.Pipe).err.trim()
+      expect(!output.contains(warningMessage))
+    }
+  }
+
+  test("with >1 file per scope, the warning about spread directives should be printed") {
+    TestInputs(
+      os.rel / "Bar.scala" ->
+        """//> using dep "com.lihaoyi::os-lib:0.9.1"
+          |
+          |object Bar extends App {
+          |  pprint.pprintln(Foo(os.pwd.toString).value)
+          |}
+          |""".stripMargin,
+      os.rel / "Foo.scala" ->
+        """//> using dep "com.lihaoyi::pprint:0.8.1"
+          |
+          |case class Foo(value: String)
+          |""".stripMargin,
+      os.rel / "Foo.test.scala" ->
+        """//> using dep "org.scalameta::munit:0.7.29"
+          |
+          |class FooTest extends munit.FunSuite {
+          |  test("Hello") {
+          |    assert(true)
+          |  }
+          |}
+          |""".stripMargin
+    ).fromRoot { root =>
+      val warningMessage = "Using directives detected in multiple files"
+      val output = os.proc(TestUtil.cli, "compile", ".", "--test", extraOptions)
+        .call(cwd = root, stderr = os.Pipe).err.trim()
+      expect(output.contains(warningMessage))
+    }
+  }
+
   test(
     "target directives in files should not produce warnings about using directives in multiple files"
   ) {