Don't let fix built-in rules to omit test dependencies in test sources (#3425)

Author: Gedochao

modules/cli/src/main/scala/scala/cli/commands/fix/BuiltInRules.scala

@@ -28,6 +28,15 @@ object BuiltInRules extends CommandHelpers {
     DirectivesPreprocessingUtils.usingDirectiveWithReqsHandlers
       .flatMap(_.keys)
 
+  private lazy val directiveTestPrefix = "test."
+  extension (strictDirective: StrictDirective) {
+    private def hasTestPrefix: Boolean = strictDirective.key.startsWith(directiveTestPrefix)
+    private def existsTestEquivalent: Boolean =
+      !strictDirective.hasTestPrefix &&
+      usingDirectivesWithTestPrefixKeysGrouped
+        .exists(_.nameAliases.contains(directiveTestPrefix + strictDirective.key))
+  }
+
   private val newLine: String = System.lineSeparator()
 
   def runRules(
@@ -114,7 +123,13 @@ object BuiltInRules extends CommandHelpers {
 
         val allDirectives = for {
           directivesWithTestPrefix <- transformedTestDirectives.map(_.withTestPrefix)
-          directive                <- directivesWithTestPrefix ++ testDirectivesFromMain
+          directivesWithNoTestPrefixEquivalents <-
+            transformedTestDirectives.map {
+              _.noTestPrefixAvailable
+                .filter(_.existsTestEquivalent)
+            }
+          directive <-
+            directivesWithTestPrefix ++ directivesWithNoTestPrefixEquivalents ++ testDirectivesFromMain
         } yield directive
 
         createFormattedLinesAndAppend(allDirectives, projectFileContents, isTest = true)
@@ -139,7 +154,12 @@ object BuiltInRules extends CommandHelpers {
       .foreach(d => removeDirectivesFrom(d.position))
     directivesFromWritableTestInputs
       .filterNot(ttd => isProjectFile(ttd.positions))
-      .foreach(ttd => removeDirectivesFrom(ttd.positions, toKeep = ttd.noTestPrefixAvailable))
+      .foreach(ttd =>
+        removeDirectivesFrom(
+          position = ttd.positions,
+          toKeep = ttd.noTestPrefixAvailable.filterNot(_.existsTestEquivalent)
+        )
+      )
   }
 
   private def getProjectSources(inputs: Inputs, logger: Logger)(using
@@ -262,18 +282,15 @@ object BuiltInRules extends CommandHelpers {
       extractedFromSingleElement <- extractedDirectives
       directives = extractedFromSingleElement.directives
     } yield {
-      val (withTestEquivalent, noTestEquivalent) = directives.partition { directive =>
-        usingDirectivesWithTestPrefixKeysGrouped.exists(
-          _.nameAliases.contains("test." + directive.key)
-        )
-      }
-
+      val (withInitialTestPrefix, noInitialTestPrefix) = directives.partition(_.hasTestPrefix)
+      val (withTestEquivalent, noTestEquivalent) =
+        noInitialTestPrefix.partition(_.existsTestEquivalent)
       val transformedToTestEquivalents = withTestEquivalent.map {
         case StrictDirective(key, values, _) => StrictDirective("test." + key, values)
       }
 
       TransformedTestDirectives(
-        withTestPrefix = transformedToTestEquivalents,
+        withTestPrefix = transformedToTestEquivalents ++ withInitialTestPrefix,
         noTestPrefixAvailable = noTestEquivalent,
         positions = extractedFromSingleElement.position
       )
@@ -328,7 +345,9 @@ object BuiltInRules extends CommandHelpers {
           StrictDirective(key, directives.flatMap(_.values))
         }
         // group by key prefixes to create splits between groups
-        .groupBy(dir => (if (isTest) dir.key.stripPrefix("test.") else dir.key).takeWhile(_ != '.'))
+        .groupBy(dir =>
+          (if (isTest) dir.key.stripPrefix(directiveTestPrefix) else dir.key).takeWhile(_ != '.')
+        )
         .map { (_, directives) =>
           directives.flatMap(_.explodeToStringsWithColLimit()).toSeq.sorted
         }

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

@@ -481,4 +481,50 @@ trait FixBuiltInRulesTestDefinitions { _: FixTestDefinitions =>
         }
       }
   }
+
+  if (!Properties.isWin)
+    test("all test directives get extracted into project.scala") {
+      val osLibDep               = "com.lihaoyi::os-lib:0.11.3"
+      val munitDep               = "org.scalameta::munit:1.0.4"
+      val pprintDep              = "com.lihaoyi::pprint:0.9.0"
+      val osLibDepDirective      = s"//> using dependency $osLibDep"
+      val osLibTestDepDirective  = s"//> using test.dependency $osLibDep"
+      val munitTestDepDirective  = s"//> using test.dependency $munitDep"
+      val pprintTestDepDirective = s"//> using test.dependency $pprintDep"
+      val mainFilePath           = os.rel / "Main.scala"
+      val testFilePath           = os.rel / "MyTests.test.scala"
+      TestInputs(
+        mainFilePath -> s"""$munitTestDepDirective
+                           |object Main extends App {
+                           |  def hello: String = "Hello, world!"
+                           |  println(hello)
+                           |}
+                           |""".stripMargin,
+        testFilePath -> s"""$osLibDepDirective
+                           |$pprintTestDepDirective
+                           |import munit.FunSuite
+                           |
+                           |class MyTests extends FunSuite {
+                           |  test("hello") {
+                           |    pprint.pprintln(os.pwd)
+                           |    assert(Main.hello == "Hello, world!")
+                           |  }
+                           |}
+                           |""".stripMargin
+      ).fromRoot { root =>
+        os.proc(TestUtil.cli, "--power", "fix", ".", extraOptions).call(cwd = root)
+        val expectedProjectFileContents =
+          s"""// Test
+             |$osLibTestDepDirective
+             |$pprintTestDepDirective
+             |$munitTestDepDirective""".stripMargin
+        val projectFileContents = os.read(root / projectFileName)
+        expect(projectFileContents.trim() == expectedProjectFileContents)
+        val mainFileContents = os.read(root / mainFilePath)
+        expect(!mainFileContents.contains("//> using"))
+        val testFileContents = os.read(root / testFilePath)
+        expect(!testFileContents.contains("//> using"))
+        os.proc(TestUtil.cli, "test", ".", extraOptions).call(cwd = root)
+      }
+    }
 }

website/docs/commands/fix.md

@@ -30,7 +30,9 @@ Files containing (experimental) `using target` directives, e.g. `//> using targe
 The original scope (`main` or `test`) of each extracted directive is respected. `main` scope directives are transformed 
 them into their `test.*` equivalent when needed.
 
-Note: directives won't be extracted for single-file projects.
+Exceptions:
+- directives won't be extracted for single-file projects;
+- directives in test inputs with no test scope equivalents won't be extracted to preserve their initial scope.
 
 ## `scalafix` integration