Merge pull request #3646 from Gedochao/feature/fix-tweaks

Gedochao · web-flow · commit 081f009aaec0 · 2025-04-28T16:00:24.000+02:00
`fix` sub-command tweaks
diff --git a/modules/cli/src/main/scala/scala/cli/commands/compile/Compile.scala b/modules/cli/src/main/scala/scala/cli/commands/compile/Compile.scala
@@ -102,6 +102,7 @@ object Compile extends ScalaCommand[CompileOptions] with BuildCommandHelpers {
         configDb.get(Keys.actions).getOrElse(None)
       )
 
+    val shouldBuildTestScope = options.shared.scope.test.getOrElse(false)
     if (options.watch.watchMode) {
       val watcher = Build.watch(
         inputs,
@@ -110,7 +111,7 @@ object Compile extends ScalaCommand[CompileOptions] with BuildCommandHelpers {
         None,
         logger,
         crossBuilds = cross,
-        buildTests = options.shared.scope.test,
+        buildTests = shouldBuildTestScope,
         partial = None,
         actionableDiagnostics = actionableDiagnostics,
         postAction = () => WatchUtil.printWatchMessage()
@@ -129,7 +130,7 @@ object Compile extends ScalaCommand[CompileOptions] with BuildCommandHelpers {
         None,
         logger,
         crossBuilds = cross,
-        buildTests = options.shared.scope.test,
+        buildTests = shouldBuildTestScope,
         partial = None,
         actionableDiagnostics = actionableDiagnostics
       )
diff --git a/modules/cli/src/main/scala/scala/cli/commands/dependencyupdate/DependencyUpdate.scala b/modules/cli/src/main/scala/scala/cli/commands/dependencyupdate/DependencyUpdate.scala
@@ -25,7 +25,7 @@ object DependencyUpdate extends ScalaCommand[DependencyUpdateOptions] {
     args: RemainingArgs,
     logger: Logger
   ): Unit = {
-    if options.shared.scope.test then
+    if options.shared.scope.test.nonEmpty then
       logger.message(
         s"""$warnPrefix Including the test scope does not change the behaviour of this command. 
            |$warnPrefix Test dependencies are updated regardless.""".stripMargin
diff --git a/modules/cli/src/main/scala/scala/cli/commands/doc/Doc.scala b/modules/cli/src/main/scala/scala/cli/commands/doc/Doc.scala
@@ -52,7 +52,7 @@ object Doc extends ScalaCommand[DocOptions] {
         configDb.get(Keys.actions).getOrElse(None)
       )
 
-    val withTestScope = options.shared.scope.test
+    val withTestScope = options.shared.scope.test.getOrElse(false)
     Build.build(
       inputs,
       initialBuildOptions,
diff --git a/modules/cli/src/main/scala/scala/cli/commands/export0/Export.scala b/modules/cli/src/main/scala/scala/cli/commands/export0/Export.scala
@@ -136,7 +136,7 @@ object Export extends ScalaCommand[ExportOptions] {
   override def sharedOptions(opts: ExportOptions): Option[SharedOptions] = Some(opts.shared)
 
   override def runCommand(options: ExportOptions, args: RemainingArgs, logger: Logger): Unit = {
-    if options.shared.scope.test then {
+    if options.shared.scope.test.nonEmpty then {
       logger.error(
         s"""Including the test scope sources together with the main scope is currently not supported.
            |Note that test scope sources will still be exported as per the output build tool tests definition demands.""".stripMargin
diff --git a/modules/cli/src/main/scala/scala/cli/commands/fix/ScalafixRules.scala b/modules/cli/src/main/scala/scala/cli/commands/fix/ScalafixRules.scala
@@ -10,6 +10,7 @@ import scala.build.compiler.ScalaCompilerMaker
 import scala.build.errors.BuildException
 import scala.build.input.{Inputs, ScalaCliInvokeData}
 import scala.build.internal.{Constants, Runner}
+import scala.build.internals.ConsoleUtils.ScalaCliConsole.warnPrefix
 import scala.build.options.{BuildOptions, Scope}
 import scala.build.{Build, Logger, Os, ScalafixArtifacts}
 import scala.cli.commands.fix.ScalafixOptions
@@ -32,24 +33,44 @@ object ScalafixRules extends CommandHelpers {
     actionableDiagnostics: Option[Boolean],
     logger: Logger
   )(using ScalaCliInvokeData): Either[BuildException, Int] = {
-    val buildOptionsWithSemanticDb = buildOptions.copy(scalaOptions =
-      buildOptions.scalaOptions.copy(semanticDbOptions =
-        buildOptions.scalaOptions.semanticDbOptions.copy(generateSemanticDbs = Some(true))
-      )
-    )
-
-    val scalaVersion =
-      buildOptions.scalaParams.orExit(logger).map(_.scalaVersion)
-        .getOrElse(Constants.defaultScalaVersion)
+    sharedOptions.semanticDbOptions.semanticDb match {
+      case Some(false) =>
+        logger.message(
+          s"""$warnPrefix SemanticDB files' generation was explicitly set to false.
+             |$warnPrefix Some scalafix rules require .semanticdb files and may not work properly."""
+            .stripMargin
+        )
+      case Some(true) =>
+        logger.debug("SemanticDB files' generation enabled.")
+      case None =>
+        logger.debug("Defaulting SemanticDB files' generation to true, to satisfy scalafix needs.")
+    }
+    val buildOptionsWithSemanticDb =
+      if buildOptions.scalaOptions.semanticDbOptions.generateSemanticDbs.isEmpty then
+        buildOptions.copy(scalaOptions =
+          buildOptions.scalaOptions.copy(semanticDbOptions =
+            buildOptions.scalaOptions.semanticDbOptions.copy(generateSemanticDbs =
+              Some(true)
+            )
+          )
+        )
+      else buildOptions
 
+    val shouldBuildTestScope = sharedOptions.scope.test.getOrElse(true)
+    if !shouldBuildTestScope then
+      logger.message(
+        s"""$warnPrefix Building test scope was explicitly disabled.
+           |$warnPrefix Some scalafix rules may not work correctly with test scope inputs."""
+          .stripMargin
+      )
     val res = Build.build(
       inputs,
       buildOptionsWithSemanticDb,
       compilerMaker,
       None,
       logger,
       crossBuilds = false,
-      buildTests = true,
+      buildTests = shouldBuildTestScope,
       partial = None,
       actionableDiagnostics = actionableDiagnostics
     )
@@ -64,6 +85,13 @@ object ScalafixRules extends CommandHelpers {
           successfulBuilds.headOption.toSeq
             .flatMap(_.options.scalaOptions.scalacOptions.toSeq.map(_.value.value))
 
+        val scalaVersion = {
+          for {
+            b           <- successfulBuilds.headOption
+            scalaParams <- b.scalaParams
+          } yield scalaParams.scalaVersion
+        }.getOrElse(Constants.defaultScalaVersion)
+
         either {
           val artifacts =
             value(
diff --git a/modules/cli/src/main/scala/scala/cli/commands/fmt/Fmt.scala b/modules/cli/src/main/scala/scala/cli/commands/fmt/Fmt.scala
@@ -43,7 +43,7 @@ object Fmt extends ScalaCommand[FmtOptions] {
   override def runCommand(options: FmtOptions, args: RemainingArgs, logger: Logger): Unit = {
     val buildOptions = buildOptionsOrExit(options)
 
-    if options.shared.scope.test then
+    if options.shared.scope.test.nonEmpty then
       logger.message(
         s"""$warnPrefix Including the test scope does not change the behaviour of this command. 
            |$warnPrefix Test scope inputs are formatted, regardless.""".stripMargin
diff --git a/modules/cli/src/main/scala/scala/cli/commands/package0/Package.scala b/modules/cli/src/main/scala/scala/cli/commands/package0/Package.scala
@@ -88,7 +88,7 @@ object Package extends ScalaCommand[PackageOptions] with BuildCommandHelpers {
         configDb.get(Keys.actions).getOrElse(None)
       )
 
-    val withTestScope = options.shared.scope.test
+    val withTestScope = options.shared.scope.test.getOrElse(false)
     if options.watch.watchMode then {
       var expectedModifyEpochSecondOpt = Option.empty[Long]
       val watcher = Build.watch(
diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala
@@ -266,7 +266,7 @@ object Publish extends ScalaCommand[PublishOptions] with BuildCommandHelpers {
       () => configDb,
       options.mainClass,
       dummy = options.sharedPublish.dummy,
-      buildTests = options.shared.scope.test
+      buildTests = options.shared.scope.test.getOrElse(false)
     )
   }
 
diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/PublishLocal.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/PublishLocal.scala
@@ -86,7 +86,7 @@ object PublishLocal extends ScalaCommand[PublishLocalOptions] {
       configDb = () => ConfigDb.empty, // shouldn't be used, no need of repo credentials here
       mainClassOptions = options.mainClass,
       dummy = options.sharedPublish.dummy,
-      buildTests = options.shared.scope.test
+      buildTests = options.shared.scope.test.getOrElse(false)
     )
   }
 }
diff --git a/modules/cli/src/main/scala/scala/cli/commands/repl/Repl.scala b/modules/cli/src/main/scala/scala/cli/commands/repl/Repl.scala
@@ -190,10 +190,11 @@ object Repl extends ScalaCommand[ReplOptions] with BuildCommandHelpers {
         configDb.get(Keys.actions).getOrElse(None)
       )
 
+    val shouldBuildTestScope = options.shared.scope.test.getOrElse(false)
     if (inputs.isEmpty) {
       val allArtifacts =
         Seq(initialBuildOptions.artifacts(logger, Scope.Main).orExit(logger)) ++
-          (if options.shared.scope.test
+          (if shouldBuildTestScope
            then Seq(initialBuildOptions.artifacts(logger, Scope.Test).orExit(logger))
            else Nil)
       // synchronizing, so that multiple presses to enter (handled by WatchUtil.waitForCtrlC)
@@ -224,7 +225,7 @@ object Repl extends ScalaCommand[ReplOptions] with BuildCommandHelpers {
         None,
         logger,
         crossBuilds = cross,
-        buildTests = options.shared.scope.test,
+        buildTests = shouldBuildTestScope,
         partial = None,
         actionableDiagnostics = actionableDiagnostics,
         postAction = () => WatchUtil.printWatchMessage()
@@ -252,7 +253,7 @@ object Repl extends ScalaCommand[ReplOptions] with BuildCommandHelpers {
           None,
           logger,
           crossBuilds = cross,
-          buildTests = options.shared.scope.test,
+          buildTests = shouldBuildTestScope,
           partial = None,
           actionableDiagnostics = actionableDiagnostics
         )
diff --git a/modules/cli/src/main/scala/scala/cli/commands/run/Run.scala b/modules/cli/src/main/scala/scala/cli/commands/run/Run.scala
@@ -233,6 +233,7 @@ object Run extends ScalaCommand[RunOptions] with BuildCommandHelpers {
         configDb.get(Keys.actions).getOrElse(None)
       )
 
+    val shouldBuildTestScope = options.shared.scope.test.getOrElse(false)
     if options.sharedRun.watch.watchMode then {
 
       /** A handle to the Runner process, used to kill the process if it's still alive when a change
@@ -258,7 +259,7 @@ object Run extends ScalaCommand[RunOptions] with BuildCommandHelpers {
         docCompilerMakerOpt = None,
         logger = logger,
         crossBuilds = cross,
-        buildTests = options.shared.scope.test,
+        buildTests = shouldBuildTestScope,
         partial = None,
         actionableDiagnostics = actionableDiagnostics,
         postAction = () =>
@@ -331,7 +332,7 @@ object Run extends ScalaCommand[RunOptions] with BuildCommandHelpers {
         None,
         logger,
         crossBuilds = cross,
-        buildTests = options.shared.scope.test,
+        buildTests = shouldBuildTestScope,
         partial = None,
         actionableDiagnostics = actionableDiagnostics
       )
diff --git a/modules/cli/src/main/scala/scala/cli/commands/shared/ScopeOptions.scala b/modules/cli/src/main/scala/scala/cli/commands/shared/ScopeOptions.scala
@@ -12,7 +12,7 @@ case class ScopeOptions(
   @Name("testScope")
   @Name("withTestScope")
   @Name("withTest")
-  test: Boolean = false
+  test: Option[Boolean] = None
 )
 object ScopeOptions {
   implicit lazy val parser: Parser[ScopeOptions] = Parser.derive
diff --git a/modules/integration/src/test/scala/scala/cli/integration/FixScalafixRulesTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/FixScalafixRulesTestDefinitions.scala
@@ -2,6 +2,8 @@ package scala.cli.integration
 
 import com.eed3si9n.expecty.Expecty.expect
 
+import scala.util.Properties
+
 trait FixScalafixRulesTestDefinitions {
   _: FixTestDefinitions =>
   protected val scalafixConfFileName: String = ".scalafix.conf"
@@ -293,37 +295,58 @@ trait FixScalafixRulesTestDefinitions {
     }
   }
 
-  test("scalafix rules requiring SemanticDB run correctly with test scope sources") {
-    val compilerOptions =
-      if (actualScalaVersion.startsWith("2.12")) Seq("-Ywarn-unused-import")
-      else Seq("-Wunused:imports")
-    TestInputs(
-      os.rel / scalafixConfFileName ->
-        """rules = [
-          |  DisableSyntax,
-          |  LeakingImplicitClassVal,
-          |  NoValInForComprehension,
-          |  ExplicitResultTypes,
-          |  OrganizeImports
-          |]
-          |ExplicitResultTypes.fetchScala3CompilerArtifactsOnVersionMismatch = true
-          |""".stripMargin,
-      os.rel / projectFileName ->
-        s"""//> using test.dep org.scalameta::munit::${Constants.munitVersion}
-           |//> using options ${compilerOptions.mkString(" ")}
-           |""".stripMargin,
-      os.rel / "example.test.scala" ->
-        """import munit.FunSuite
-          |
-          |class Munit extends FunSuite {
-          |  test("foo") {
-          |    assert(2 + 2 == 4)
-          |    println("Hello from Munit")
-          |  }
-          |}
-          |""".stripMargin
-    ).fromRoot { root =>
-      os.proc(TestUtil.cli, "fix", ".", "--power", extraOptions).call(cwd = root)
-    }
+  for {
+    (semanticDbOptions, expectedSuccess) <- Seq(
+      Nil -> true, // .semanticdb files should be implicitly generated when Scalafix is run
+      Seq("--semanticdb")       -> true,
+      Seq("--semanticdb=false") -> false
+    )
+    semanticDbOptionsDescription =
+      if (semanticDbOptions.nonEmpty) s" (${semanticDbOptions.mkString(" ")})"
+      else ""
+    verb = if (expectedSuccess) "run" else "fail"
+    if !Properties.isWin || expectedSuccess
   }
+    test(
+      s"scalafix rules requiring SemanticDB $verb correctly with test scope sources$semanticDbOptionsDescription"
+    ) {
+      val compilerOptions =
+        if (actualScalaVersion.startsWith("2.12")) Seq("-Ywarn-unused-import")
+        else Seq("-Wunused:imports")
+      TestInputs(
+        os.rel / scalafixConfFileName ->
+          """rules = [
+            |  DisableSyntax,
+            |  LeakingImplicitClassVal,
+            |  NoValInForComprehension,
+            |  ExplicitResultTypes,
+            |  OrganizeImports
+            |]
+            |ExplicitResultTypes.fetchScala3CompilerArtifactsOnVersionMismatch = true
+            |""".stripMargin,
+        os.rel / projectFileName ->
+          s"""//> using test.dep org.scalameta::munit::${Constants.munitVersion}
+             |//> using options ${compilerOptions.mkString(" ")}
+             |""".stripMargin,
+        os.rel / "example.test.scala" ->
+          """import munit.FunSuite
+            |
+            |class Munit extends FunSuite {
+            |  test("foo") {
+            |    assert(2 + 2 == 4)
+            |    println("Hello from Munit")
+            |  }
+            |}
+            |""".stripMargin
+      ).fromRoot { root =>
+        val res = os.proc(TestUtil.cli, "fix", ".", "--power", semanticDbOptions, extraOptions)
+          .call(cwd = root, check = false, stderr = os.Pipe)
+        val successful = res.exitCode == 0
+        expect(successful == expectedSuccess)
+        if (!expectedSuccess)
+          expect(
+            res.err.trim().contains("SemanticDB files' generation was explicitly set to false")
+          )
+      }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ object Doc extends ScalaCommand[DocOptions] {`
`52`	`52`	`configDb.get(Keys.actions).getOrElse(None)`
`53`	`53`	`)`
`54`	`54`
`55`		`- val withTestScope = options.shared.scope.test`
	`55`	`+ val withTestScope = options.shared.scope.test.getOrElse(false)`
`56`	`56`	`Build.build(`
`57`	`57`	`inputs,`
`58`	`58`	`initialBuildOptions,`
Original file line number	Diff line number	Diff line change
`@@ -88,7 +88,7 @@ object Package extends ScalaCommand[PackageOptions] with BuildCommandHelpers {`
`88`	`88`	`configDb.get(Keys.actions).getOrElse(None)`
`89`	`89`	`)`
`90`	`90`
`91`		`- val withTestScope = options.shared.scope.test`
	`91`	`+ val withTestScope = options.shared.scope.test.getOrElse(false)`
`92`	`92`	`if options.watch.watchMode then {`
`93`	`93`	`var expectedModifyEpochSecondOpt = Option.empty[Long]`
`94`	`94`	`val watcher = Build.watch(`
Original file line number	Diff line number	Diff line change
`@@ -266,7 +266,7 @@ object Publish extends ScalaCommand[PublishOptions] with BuildCommandHelpers {`
`266`	`266`	`() => configDb,`
`267`	`267`	`options.mainClass,`
`268`	`268`	`dummy = options.sharedPublish.dummy,`
`269`		`- buildTests = options.shared.scope.test`
	`269`	`+ buildTests = options.shared.scope.test.getOrElse(false)`
`270`	`270`	`)`
`271`	`271`	`}`
`272`	`272`
Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ object PublishLocal extends ScalaCommand[PublishLocalOptions] {`
`86`	`86`	`configDb = () => ConfigDb.empty, // shouldn't be used, no need of repo credentials here`
`87`	`87`	`mainClassOptions = options.mainClass,`
`88`	`88`	`dummy = options.sharedPublish.dummy,`
`89`		`- buildTests = options.shared.scope.test`
	`89`	`+ buildTests = options.shared.scope.test.getOrElse(false)`
`90`	`90`	`)`
`91`	`91`	`}`
`92`	`92`	`}`