scalafix: add scalafix.dep, more tests

dos65 · dos65 · commit 9d908a2a335d · 2024-10-21T23:30:44.000+03:00
diff --git a/modules/build/src/main/scala/scala/build/ScalafixArtifacts.scala b/modules/build/src/main/scala/scala/build/ScalafixArtifacts.scala
@@ -25,7 +25,7 @@ object ScalafixArtifacts {
 
   def artifacts(
     scalaVersion: String,
-    compileOnlyDeps: Seq[Dependency],
+    externalRulesDeps: Seq[Positioned[AnyDependency]],
     extraRepositories: Seq[Repository],
     logger: Logger,
     cache: FileCache[Task]
@@ -39,6 +39,7 @@ object ScalafixArtifacts {
 
       val scalafixDeps =
         Seq(dep"ch.epfl.scala:scalafix-cli_$fetchScalaVersion:${Constants.scalafixVersion}")
+
       val scalafix =
         value(
           Artifacts.artifacts(
@@ -50,16 +51,23 @@ object ScalafixArtifacts {
           )
         )
 
+      val scalaParameters =
+        // Scalafix for scala 3 uses 2.13-published community rules
+        // https://github.com/scalacenter/scalafix/issues/2041
+        if (scalaVersion.startsWith("3")) ScalaParameters(Constants.defaultScala213Version)
+        else ScalaParameters(scalaVersion)
+
       val tools =
         value(
           Artifacts.artifacts(
-            compileOnlyDeps.map(Positioned.none),
+            externalRulesDeps,
             extraRepositories,
-            None,
+            Some(scalaParameters),
             logger,
-            cache.withMessage(s"Downloading tools classpath for scalafix")
+            cache.withMessage(s"Downloading scalafix.deps")
           )
         )
+
       ScalafixArtifacts(scalafix.map(_._2), tools.map(_._2))
     }
 
@@ -81,7 +89,6 @@ object ScalafixArtifacts {
           propsData
         }
         else os.read(cachePath)
-
       val props  = new Properties()
       val stream = new ByteArrayInputStream(content.getBytes())
       props.load(stream)
diff --git a/modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala b/modules/cli/src/main/scala/scala/cli/commands/scalafix/Scalafix.scala
@@ -5,7 +5,12 @@ import caseapp.core.help.HelpFormat
 import coursier.cache.FileCache
 import dependency.*
 import scalafix.interfaces.ScalafixError.*
-import scalafix.interfaces.{Scalafix => ScalafixInterface, ScalafixError, ScalafixException, ScalafixRule}
+import scalafix.interfaces.{
+  Scalafix => ScalafixInterface,
+  ScalafixError,
+  ScalafixException,
+  ScalafixRule
+}
 
 import java.io.File
 import java.util.Optional
@@ -96,12 +101,6 @@ object Scalafix extends ScalaCommand[ScalafixOptions] {
       case None => sys.exit(1)
       case Some(build) =>
         val classPaths = build.fullClassPath
-        val compileOnlyDeps = {
-          val params = ScalaParameters(scalaVersion)
-          build.options.classPathOptions.extraCompileOnlyDependencies.values.flatten.map(
-            _.value.applyParams(params)
-          )
-        }
 
         val scalacOptions = options.shared.scalac.scalacOption ++
           build.options.scalaOptions.scalacOptions.toSeq.map(_.value.value)
@@ -111,18 +110,17 @@ object Scalafix extends ScalaCommand[ScalafixOptions] {
             value(
               ScalafixArtifacts.artifacts(
                 scalaVersion,
-                compileOnlyDeps,
+                build.options.classPathOptions.scalafixDependencies.values.flatten,
                 value(buildOptions.finalRepositories),
                 logger,
                 buildOptions.internal.cache.getOrElse(FileCache())
               )
             )
 
           val scalafixOptions =
-            configFilePathOpt.map(file => Seq("-c", file.toString)).getOrElse(Nil) ++
+            options.scalafixConf.toList.flatMap(scalafixConf => List("--config", scalafixConf)) ++
               Seq("--sourceroot", workspace.toString) ++
               Seq("--classpath", classPaths.mkString(java.io.File.pathSeparator)) ++
-              options.scalafixConf.toList.flatMap(scalafixConf => List("--config", scalafixConf)) ++
               (if (options.check) Seq("--test") else Nil) ++
               (if (scalacOptions.nonEmpty) scalacOptions.flatMap(Seq("--scalac-options", _))
                else Nil) ++
diff --git a/modules/cli/src/main/scala/scala/cli/commands/scalafix/ScalafixOptions.scala b/modules/cli/src/main/scala/scala/cli/commands/scalafix/ScalafixOptions.scala
@@ -34,7 +34,6 @@ final case class ScalafixOptions(
   @Tag(tags.experimental)
   @HelpMessage("Run rule(s) explicitly, overriding the configuration file default.")
   @Tag(tags.inShortHelp)
-  @Name("r")
   rules: List[String] = Nil,
 
   @Group(HelpGroup.Format.toString)
diff --git a/modules/directives/src/main/scala/scala/build/preprocessing/directives/Dependency.scala b/modules/directives/src/main/scala/scala/build/preprocessing/directives/Dependency.scala
@@ -14,6 +14,7 @@ import scala.build.options.{
   ShadowingSeq,
   WithBuildRequirements
 }
+import scala.build.preprocessing.directives.Dependency.DependencyType
 import scala.build.preprocessing.directives.DirectiveUtil.*
 import scala.cli.commands.SpecificationLevel
 
@@ -46,27 +47,48 @@ final case class Dependency(
   @DirectiveName("compileOnly.dep")
   @DirectiveName("compileOnly.deps")
   @DirectiveName("compileOnly.dependencies")
-  compileOnlyDependency: List[Positioned[String]] = Nil
+  compileOnlyDependency: List[Positioned[String]] = Nil,
+  @DirectiveName("scalafix.dep")
+  @DirectiveName("scalafix.deps")
+  @DirectiveName("scalafix.dependencies")
+  scalafixDependency: List[Positioned[String]] = Nil
 ) extends HasBuildOptionsWithRequirements {
   def buildOptionsList: List[Either[BuildException, WithBuildRequirements[BuildOptions]]] = List(
-    Dependency.buildOptions(dependency).map(_.withEmptyRequirements),
-    Dependency.buildOptions(testDependency).map(_.withScopeRequirement(Scope.Test)),
-    Dependency.buildOptions(compileOnlyDependency, isCompileOnly = true)
+    Dependency.buildOptions(dependency, DependencyType.Runtime).map(_.withEmptyRequirements),
+    Dependency.buildOptions(testDependency, DependencyType.Runtime).map(
+      _.withScopeRequirement(Scope.Test)
+    ),
+    Dependency.buildOptions(compileOnlyDependency, DependencyType.CompileOnly)
+      .map(_.withEmptyRequirements),
+    Dependency.buildOptions(scalafixDependency, DependencyType.Scalafix)
       .map(_.withEmptyRequirements)
   )
 }
 
 object Dependency {
   val handler: DirectiveHandler[Dependency] = DirectiveHandler.derive
+
+  sealed trait DependencyType
+  object DependencyType {
+    case object Runtime     extends DependencyType
+    case object CompileOnly extends DependencyType
+    case object Scalafix    extends DependencyType
+  }
+
   def buildOptions(
     ds: List[Positioned[String]],
-    isCompileOnly: Boolean = false
+    tpe: DependencyType
   ): Either[BuildException, BuildOptions] = either {
     val dependencies: ShadowingSeq[Positioned[AnyDependency]] =
       value(ds.asDependencies.map(ShadowingSeq.from))
     val classPathOptions =
-      if (isCompileOnly) ClassPathOptions(extraCompileOnlyDependencies = dependencies)
-      else ClassPathOptions(extraDependencies = dependencies)
+      tpe match {
+        case DependencyType.Runtime => ClassPathOptions(extraDependencies = dependencies)
+        case DependencyType.CompileOnly =>
+          ClassPathOptions(extraCompileOnlyDependencies = dependencies)
+        case DependencyType.Scalafix => ClassPathOptions(scalafixDependencies = dependencies)
+      }
+
     BuildOptions(classPathOptions = classPathOptions)
   }
 }
diff --git a/modules/integration/src/test/scala/scala/cli/integration/ScalafixTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/ScalafixTestDefinitions.scala
@@ -101,7 +101,7 @@ abstract class ScalafixTestDefinitions extends ScalaCliSuite with TestScalaVersi
     }
   }
 
-  test("rule args") {
+  test("--rules args") {
     val input = TestInputs(
       os.rel / confFileName ->
         s"""|rules = [
@@ -149,4 +149,155 @@ abstract class ScalafixTestDefinitions extends ScalaCliSuite with TestScalaVersi
 
     }
   }
+
+  test("--scalafix-arg arg") {
+    val original: String =
+      """|package foo
+         |
+         |final object Hello { // keep `final` beucase of parameter finalObject=false
+         |  s"Foo"
+         |}
+         |""".stripMargin
+    val inputs: TestInputs = TestInputs(
+      os.rel / confFileName ->
+        s"""|rules = [
+            |  RedundantSyntax
+            |]
+            |""".stripMargin,
+      os.rel / "Hello.scala" -> original
+    )
+    val expectedContent: String = noCrLf {
+      """|package foo
+         |
+         |final object Hello { // keep `final` beucase of parameter finalObject=false
+         |  "Foo"
+         |}
+         |""".stripMargin
+    }
+
+    inputs.fromRoot { root =>
+      os.proc(
+        TestUtil.cli,
+        "scalafix",
+        ".",
+        "--scalafix-arg=--settings.RedundantSyntax.finalObject=false",
+        "--power",
+        scalaVersionArgs
+      ).call(cwd = root)
+      val updatedContent = noCrLf(os.read(root / "Hello.scala"))
+      expect(updatedContent == expectedContent)
+    }
+  }
+
+  test("--scalafix-conf arg") {
+    val original: String =
+      """|package foo
+         |
+         |final object Hello {
+         |  s"Foo"
+         |}
+         |""".stripMargin
+
+    val confFileName = "unusual-scalafix-filename"
+    val inputs: TestInputs = TestInputs(
+      os.rel / confFileName ->
+        s"""|rules = [
+            |  RedundantSyntax
+            |]
+            |""".stripMargin,
+      os.rel / "Hello.scala" -> original
+    )
+    val expectedContent: String = noCrLf {
+      """|package foo
+         |
+         |object Hello {
+         |  "Foo"
+         |}
+         |""".stripMargin
+    }
+
+    inputs.fromRoot { root =>
+      os.proc(
+        TestUtil.cli,
+        "scalafix",
+        ".",
+        s"--scalafix-conf=$confFileName",
+        "--power",
+        scalaVersionArgs
+      ).call(cwd = root)
+      val updatedContent = noCrLf(os.read(root / "Hello.scala"))
+      expect(updatedContent == expectedContent)
+    }
+  }
+
+  test("external rule") {
+    val original: String =
+      """|//> using scalafix.dep "io.github.ghostbuster91.scalafix-unified::unified:0.0.8"
+         |
+         |package foo
+         |
+         |object Hello {
+         |  val a = List[Int]()
+         |}
+         |""".stripMargin
+    val inputs: TestInputs = TestInputs(
+      os.rel / confFileName ->
+        s"""|rules = [
+            |  EmptyCollectionsUnified
+            |]
+            |""".stripMargin,
+      os.rel / "Hello.scala" -> original
+    )
+    val expectedContent: String = noCrLf {
+      """|//> using scalafix.dep "io.github.ghostbuster91.scalafix-unified::unified:0.0.8"
+         |
+         |package foo
+         |
+         |object Hello {
+         |  val a = List.empty[Int]
+         |}
+         |""".stripMargin
+    }
+
+    inputs.fromRoot { root =>
+      os.proc(TestUtil.cli, "scalafix", ".", "--power", scalaVersionArgs).call(cwd = root)
+      val updatedContent = noCrLf(os.read(root / "Hello.scala"))
+      expect(updatedContent == expectedContent)
+    }
+  }
+
+  test {
+    val name = "explicit-result-types"
+    if (actualScalaVersion.startsWith("3")) name.ignore else munit.TestOptions(name)
+  } {
+    val original: String =
+      """|package foo
+         |
+         |object Hello {
+         |  def a(a: Int) = "asdasd" + a.toString
+         |}
+         |""".stripMargin
+    val inputs: TestInputs = TestInputs(
+      os.rel / confFileName ->
+        s"""|rules = [
+            |  ExplicitResultTypes
+            |]
+            |""".stripMargin,
+      os.rel / "Hello.scala" -> original
+    )
+    val expectedContent: String = noCrLf {
+      """|package foo
+         |
+         |object Hello {
+         |  def a(a: Int): String = "asdasd" + a.toString
+         |}
+         |""".stripMargin
+    }
+
+    inputs.fromRoot { root =>
+      os.proc(TestUtil.cli, "scalafix", ".", "--power", scalaVersionArgs).call(cwd = root)
+      val updatedContent = noCrLf(os.read(root / "Hello.scala"))
+      expect(updatedContent == expectedContent)
+    }
+  }
 }
diff --git a/modules/integration/src/test/scala/scala/cli/integration/ScalafixTests213.scala b/modules/integration/src/test/scala/scala/cli/integration/ScalafixTests213.scala
@@ -1,61 +1,6 @@
 package scala.cli.integration
 
-import com.eed3si9n.expecty.Expecty.expect
-
 class ScalafixTests213 extends ScalafixTestDefinitions with Test213 {
   override val unusedRuleOption: String = "-Wunused"
 
-  test("external rule") {
-    val unnamedParamsInputsContent: String =
-      """//> using options -P:semanticdb:synthetics:on
-        |//> using compileOnly.dep "com.github.jatcwang::scalafix-named-params:0.2.5"
-        |
-        |package foo
-        |
-        |object Hello {
-        |  def greetMany(name: String, times: Int) =
-        |    for {
-        |      i <- 0 to times
-        |      _ = println(s"Hello $name")
-        |    } yield ()
-        |
-        |  def main(args: Array[String]): Unit =
-        |    greetMany("John", 42)
-        |}
-        |""".stripMargin
-    val externalRuleInputs: TestInputs = TestInputs(
-      os.rel / confFileName ->
-        s"""|rules = [
-            |  UseNamedParameters
-            |]
-            |
-            |UseNamedParameters.minParams = 2
-            |""".stripMargin,
-      os.rel / "Hello.scala" -> unnamedParamsInputsContent
-    )
-    val expectedContent: String = noCrLf {
-      """//> using options -P:semanticdb:synthetics:on
-        |//> using compileOnly.dep "com.github.jatcwang::scalafix-named-params:0.2.5"
-        |
-        |package foo
-        |
-        |object Hello {
-        |  def greetMany(name: String, times: Int) =
-        |    for {
-        |      i <- 0 to times
-        |      _ = println(s"Hello $name")
-        |    } yield ()
-        |
-        |  def main(args: Array[String]): Unit =
-        |    greetMany(name = "John", times = 42)
-        |}
-        |""".stripMargin
-    }
-
-    externalRuleInputs.fromRoot { root =>
-      os.proc(TestUtil.cli, "scalafix", "--power", ".", "-S", "2").call(cwd = root)
-      val updatedContent = noCrLf(os.read(root / "Hello.scala"))
-      expect(updatedContent == expectedContent)
-    }
-  }
 }
diff --git a/modules/options/src/main/scala/scala/build/options/ClassPathOptions.scala b/modules/options/src/main/scala/scala/build/options/ClassPathOptions.scala
diff --git a/website/docs/commands/scalafix.md b/website/docs/commands/scalafix.md
diff --git a/website/docs/reference/commands.md b/website/docs/reference/commands.md
