Support Scalafix migrations in Scala CLI projects

[fthomas]

Scala CLI 1.5.4+ will contain a scalafix command (VirtusLab/scala-cli#2968) which we should be able to use for implementing ScalaCliAlg#runMigration which is currently a NOP.

Full support for Scalafix migrations is currently only available for sbt projects. The corresponding code in SbtAlg can be used as inspiration for supporting them in Scala CLI.

[Gedochao]

Actually, it will ship in Scala CLI 1.6.0, just FYI.
It's available in the recent nightly, but the syntax is still likely to change considerably (track VirtusLab/scala-cli#3327) before I release 1.6.x

[fthomas]

I just opened a PR for this feature: #3564

[bjaglin]

Looking at the intial implementation, there seems to be an undocumented limitation: the dependency: syntax allowing to pass maven coordinates (used here for example) is implemented in sbt-scalafix, but not in scalafix (which only supports source files, that it compiles on the fly), so it won't work as-is with scala-cli.

It is probably possible, but far from trivial to upstream the feature from sbt-scalafix to scalafix because of the classloading structure, so maybe the easiest would be to leverage that feature (that I haven't tested myself) and create an empty file with the //> using scalafix.dep when a dependency: is found? Until/unless a central flag is exposed...

[fthomas]

so maybe the easiest would be to leverage that feature (that I haven't tested myself) and create an empty file with the //> using scalafix.dep when a dependency: is found?

Ok, let me play this through for the example you pointed at:

rewriteRules: ["dependency:MigrateToServerBuilder@com.typesafe.akka:akka-http-scalafix-rules:10.2.0"]

Scala Steward would detect here that the rule starts with dependency: and creates a temporary file (e.g. scala-steward-scala-cli.sc) with the content:

//> using scalafix.dep "com.typesafe.akka:akka-http-scalafix-rules:10.2.0"

(Does it need to replace the first : with :: here?)

Then it invokes Scala CLI with scala-cli --power fix --scalafix-rules MigrateToServerBuilder to run the migration. That doesn't sound to hard to implement.

[bjaglin]

Exactly, I just tried it out and it does seem to work 🤞

One thing that this test showed is that --enable-built-in=false is missing to prevent https://scala-cli.virtuslab.org/docs/commands/fix#built-in-rules to kick-in when several files are present (which will then always be the case, but may already be the case for some builds?). On that topic, I provided feedback about the current UX in VirtusLab/scala-cli#3400 (comment) .

And, yes, we should replace the first : with :: in all existing references, as scala-cli is more strict that sbt-scalafix. sbt-scalafix does support both syntax for a while, so this won't impact sbt builds.

// MigrateToServerBuilderTest.scala 

// https://github.com/akka/akka-http/blob/main/akka-http-scalafix/scalafix-test-input/src/main/scala/akka/http/fix/MigrateToServerBuilderTest.scala

//> using scala 2.13
//> using dependency com.typesafe.akka::akka-http:10.1.15
//> using dependency com.typesafe.akka::akka-actor-typed:2.5.31
//> using dependency com.typesafe.akka::akka-stream:2.5.31

package akka.http.fix

import akka.actor._
import akka.event.LoggingAdapter
import akka.http.scaladsl._
import akka.http.scaladsl.server._
import akka.http.scaladsl.settings.ServerSettings
import akka.http.scaladsl.model._
import akka.stream.Materializer
import akka.stream.scaladsl.{ Flow, Sink }

import scala.concurrent.Future

object MigrateToServerBuilderTest {
  // Add code that needs fixing here.
  implicit def actorSystem: ActorSystem = ???
  def customMaterializer: Materializer = ???
  def http: HttpExt = ???
  implicit def log: LoggingAdapter = ???
  def settings: ServerSettings = ???
  def httpContext: HttpConnectionContext = ???
  def context: HttpsConnectionContext = ???
  def handler: HttpRequest => Future[HttpResponse] = ???
  def syncHandler: HttpRequest => HttpResponse = ???
  def flow: Flow[HttpRequest, HttpResponse, Any] = ???
  def route: Route = ???
  trait ServiceRoutes {
    def route: Route = ???
  }
  def service: ServiceRoutes = ???

  Http().bindAndHandleAsync(handler, "127.0.0.1", 8080, log = log)
  Http().bindAndHandleAsync(handler, "127.0.0.1", log = log, port = 8080)
  Http().bindAndHandleAsync(handler, "127.0.0.1", settings = settings)
  Http().bindAndHandleAsync(
    handler,
    interface = "localhost",
    port = 8443,
    context)
  Http().bindAndHandleAsync(
    handler,
    interface = "localhost",
    port = 8080,
    httpContext)
  Http().bindAndHandleAsync(
    handler,
    interface = "localhost",
    port = 8080,
    HttpConnectionContext)
  Http().bindAndHandle(flow, "127.0.0.1", port = 8080)
  Http().bindAndHandle(route, "127.0.0.1", port = 8080)
  Http().bindAndHandle(service.route, "127.0.0.1", port = 8080)
  Http().bindAndHandleSync(syncHandler, "127.0.0.1", log = log)

  Http().bind("127.0.0.1", settings = settings).runWith(Sink.ignore)

  // format: OFF
  Http().bindAndHandle(route, "127.0.0.1", port = 8080)(customMaterializer)// assert: MigrateToServerBuilder.custom-materializer-warning
  Http().bindAndHandleAsync(handler, "127.0.0.1", 8080)(customMaterializer)// assert: MigrateToServerBuilder.custom-materializer-warning
  Http().bindAndHandleSync(syncHandler, "127.0.0.1", 8080)(customMaterializer)// assert: MigrateToServerBuilder.custom-materializer-warning
  Http() // needed to appease formatter
  // format: ON

  http.bindAndHandle(route, "127.0.0.1", port = 8080)
  http.bindAndHandleAsync(handler, "127.0.0.1", 8080)
  http.bindAndHandleSync(syncHandler, "127.0.0.1", 8080)

  Http(actorSystem).bindAndHandle(route, "127.0.0.1", port = 8080)
  Http(actorSystem).bindAndHandleAsync(handler, "127.0.0.1", 8080)
  Http(actorSystem).bindAndHandleSync(syncHandler, "127.0.0.1", 8080)
}

// steward.scala 

//> using scalafix.dep "com.typesafe.akka::akka-http-scalafix-rules:10.2.0"

brice@t14:/tmp/cli$ scala-cli --power fix --enable-built-in=false --scalafix-rules MigrateToServerBuilder --check .
Some utilized features are marked as experimental:
 - `fix` sub-command
 - `--enable-built-in=false` option
 - `--scalafix-rules` option
 - `--check` option
Please bear in mind that non-ideal user experience should be expected.
If you encounter any bugs or have feedback to share, make sure to reach out to the maintenance team at https://github.com/VirtusLab/scala-cli
Running scalafix rules...
[warn]  Using directives detected in multiple files:
 - MigrateToServerBuilderTest.scala:1:1-4:59
 - steward.scala:1:1-76
It is recommended to keep them centralized in the /tmp/cli/project.scala file.

Compiling project (Scala 2.13.16, JVM (21))
Compiled project (Scala 2.13.16, JVM (21))
[hint] ./MigrateToServerBuilderTest.scala:2:22
[hint] "akka-http is outdated, update to 10.5.3"
[hint]      akka-http 10.1.15 -> com.typesafe.akka::akka-http:10.5.3
[hint] //> using dependency com.typesafe.akka::akka-http:10.1.15
[hint]                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[hint] ./MigrateToServerBuilderTest.scala:3:22
[hint] "akka-actor-typed is outdated, update to 2.8.8"
[hint]      akka-actor-typed 2.5.31 -> com.typesafe.akka::akka-actor-typed:2.8.8
[hint] //> using dependency com.typesafe.akka::akka-actor-typed:2.5.31
[hint]                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[hint] ./MigrateToServerBuilderTest.scala:4:22
[hint] "akka-stream is outdated, update to 2.8.8"
[hint]      akka-stream 2.5.31 -> com.typesafe.akka::akka-stream:2.8.8
[hint] //> using dependency com.typesafe.akka::akka-stream:2.5.31
[hint]                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/tmp/cli/MigrateToServerBuilderTest.scala:65:57: warning: [MigrateToServerBuilder.custom-materializer-warning] Custom materializers are often not needed any more. You can often remove custom materializers and use the system materializer which is supplied automatically.
  Http().bindAndHandle(route, "127.0.0.1", port = 8080)(customMaterializer)// assert: MigrateToServerBuilder.custom-materializer-warning
                                                        ^^^^^^^^^^^^^^^^^^
/tmp/cli/MigrateToServerBuilderTest.scala:66:57: warning: [MigrateToServerBuilder.custom-materializer-warning] Custom materializers are often not needed any more. You can often remove custom materializers and use the system materializer which is supplied automatically.
  Http().bindAndHandleAsync(handler, "127.0.0.1", 8080)(customMaterializer)// assert: MigrateToServerBuilder.custom-materializer-warning
                                                        ^^^^^^^^^^^^^^^^^^
/tmp/cli/MigrateToServerBuilderTest.scala:67:60: warning: [MigrateToServerBuilder.custom-materializer-warning] Custom materializers are often not needed any more. You can often remove custom materializers and use the system materializer which is supplied automatically.
  Http().bindAndHandleSync(syncHandler, "127.0.0.1", 8080)(customMaterializer)// assert: MigrateToServerBuilder.custom-materializer-warning
                                                           ^^^^^^^^^^^^^^^^^^
--- /tmp/cli/MigrateToServerBuilderTest.scala
+++ <expected fix>
@@ -36,43 +36,31 @@
   }
   def service: ServiceRoutes = ???
 
-  Http().bindAndHandleAsync(handler, "127.0.0.1", 8080, log = log)
-  Http().bindAndHandleAsync(handler, "127.0.0.1", log = log, port = 8080)
-  Http().bindAndHandleAsync(handler, "127.0.0.1", settings = settings)
-  Http().bindAndHandleAsync(
-    handler,
-    interface = "localhost",
-    port = 8443,
-    context)
-  Http().bindAndHandleAsync(
-    handler,
-    interface = "localhost",
-    port = 8080,
-    httpContext)
-  Http().bindAndHandleAsync(
-    handler,
-    interface = "localhost",
-    port = 8080,
-    HttpConnectionContext)
-  Http().bindAndHandle(flow, "127.0.0.1", port = 8080)
-  Http().bindAndHandle(route, "127.0.0.1", port = 8080)
-  Http().bindAndHandle(service.route, "127.0.0.1", port = 8080)
-  Http().bindAndHandleSync(syncHandler, "127.0.0.1", log = log)
+  Http().newServerAt("127.0.0.1", 8080).logTo(log).bind(handler)
+  Http().newServerAt("127.0.0.1", 8080).logTo(log).bind(handler)
+  Http().newServerAt("127.0.0.1", 0).withSettings(settings).bind(handler)
+  Http().newServerAt(interface = "localhost", port = 8443).enableHttps(context).bind(handler)
+  Http().newServerAt(interface = "localhost", port = 8080).bind(handler)
+  Http().newServerAt(interface = "localhost", port = 8080).bind(handler)
+  Http().newServerAt("127.0.0.1", 8080).bindFlow(flow)
+  Http().newServerAt("127.0.0.1", 8080).bind(route)
+  Http().newServerAt("127.0.0.1", 8080).bind(service.route)
+  Http().newServerAt("127.0.0.1", 0).logTo(log).bindSync(syncHandler)
 
-  Http().bind("127.0.0.1", settings = settings).runWith(Sink.ignore)
+  Http().newServerAt("127.0.0.1", 0).withSettings(settings).connectionSource().runWith(Sink.ignore)
 
   // format: OFF
-  Http().bindAndHandle(route, "127.0.0.1", port = 8080)(customMaterializer)// assert: MigrateToServerBuilder.custom-materializer-warning
-  Http().bindAndHandleAsync(handler, "127.0.0.1", 8080)(customMaterializer)// assert: MigrateToServerBuilder.custom-materializer-warning
-  Http().bindAndHandleSync(syncHandler, "127.0.0.1", 8080)(customMaterializer)// assert: MigrateToServerBuilder.custom-materializer-warning
+  Http().newServerAt("127.0.0.1", 8080).withMaterializer(customMaterializer).bind(route)// assert: MigrateToServerBuilder.custom-materializer-warning
+  Http().newServerAt("127.0.0.1", 8080).withMaterializer(customMaterializer).bind(handler)// assert: MigrateToServerBuilder.custom-materializer-warning
+  Http().newServerAt("127.0.0.1", 8080).withMaterializer(customMaterializer).bindSync(syncHandler)// assert: MigrateToServerBuilder.custom-materializer-warning
   Http() // needed to appease formatter
   // format: ON
 
-  http.bindAndHandle(route, "127.0.0.1", port = 8080)
-  http.bindAndHandleAsync(handler, "127.0.0.1", 8080)
-  http.bindAndHandleSync(syncHandler, "127.0.0.1", 8080)
+  http.newServerAt("127.0.0.1", 8080).bind(route)
+  http.newServerAt("127.0.0.1", 8080).bind(handler)
+  http.newServerAt("127.0.0.1", 8080).bindSync(syncHandler)
 
-  Http(actorSystem).bindAndHandle(route, "127.0.0.1", port = 8080)
-  Http(actorSystem).bindAndHandleAsync(handler, "127.0.0.1", 8080)
-  Http(actorSystem).bindAndHandleSync(syncHandler, "127.0.0.1", 8080)
+  Http(actorSystem).newServerAt("127.0.0.1", 8080).bind(route)
+  Http(actorSystem).newServerAt("127.0.0.1", 8080).bind(handler)
+  Http(actorSystem).newServerAt("127.0.0.1", 8080).bindSync(syncHandler)
 }

[bjaglin]

It is probably possible, but far from trivial to upstream the feature from sbt-scalafix to scalafix because of the classloading structure

I just opened scalacenter/scalafix#2189 (more for tracking/visibility than any promise to get it implemented) in case you want to document what led to the hack we are discussing.

[fthomas]

Many thanks for trying this out, @bjaglin! I'll open a new issue to track supporting dependency: rules with Scala CLI (#3575). The double colons are added in #3573 and I'll also take care of adding the --enable-built-in=false option (#3574).