Scalafix rules to rewrite to new ServerBuilder API automatically (#3407)

Co-authored-by: Enno <458526+ennru@users.noreply.github.com>

akka-http-scalafix/README.md

@@ -0,0 +1,17 @@
+The setup of the scalafix module roughly follows the example in https://github.com/scalacenter/scalafix.g8.
+
+## Adding new rules
+
+ * Add before/after test file in scalafix-test-input / scalafix-test-output
+ * Add rule in scalafix-rules
+ * run test in `akka-http-scalafix-tests`
+
+## Applying locally defined rules to docs examples
+
+ * run `scalafixEnable` on the sbt shell (this will unfortunately require a complete rebuild afterwards)
+ * run `set scalacOptions in ThisBuild += "-P:semanticdb:synthetics:on"` to allow access to synthetics
+ * e.g. run `docs/scalafixAll MigrateToServerBuilder`
+
+*Note:* There's some weird stuff going on regarding cross-publishing. The `scalafixScalaBinaryVersion` line in build.sbt
+should fix it but if running the rule fails with a weird error, try switching to Scala 2.12 first with `++2.12.11` (or
+whatever is now the current version).

akka-http-scalafix/scalafix-rules/src/main/resources/META-INF/services/scalafix.v1.Rule

@@ -0,0 +1 @@
+akka.http.fix.MigrateToServerBuilder

akka-http-scalafix/scalafix-rules/src/main/scala/akka/http/fix/MigrateToServerBuilder.scala

@@ -0,0 +1,111 @@
+/*
+ * Copyright (C) 2020 Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package akka.http.fix
+
+import scalafix.lint.LintSeverity
+import scalafix.v1._
+
+import scala.meta._
+import scala.util.Try
+
+class MigrateToServerBuilder extends SemanticRule("MigrateToServerBuilder") {
+
+  override def fix(implicit doc: SemanticDocument): Patch = {
+    def patch(t: Tree, http: Term, targetMethod: Term => String): Patch = {
+      val args = t.symbol.info.get.signature.asInstanceOf[MethodSignature].parameterLists.head.map(_.displayName)
+      val materializerAndTarget: Option[(Tree, Term)] =
+        Try {
+          val sig = t.parent.get.symbol.info.get.signature.asInstanceOf[MethodSignature]
+          require(sig.parameterLists(1)(0).signature.toString == "Materializer")
+          (t.parent.get, t.parent.get.asInstanceOf[Term.Apply].args.head)
+        }.toOption
+
+      val materializerLint: Option[Patch] = materializerAndTarget.map {
+        case (_, matArg) =>
+          Patch.lint(Diagnostic(
+            "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.",
+            matArg.pos,
+            severity = LintSeverity.Warning
+          ))
+      }
+
+      val argExps = namedArgMap(args, t.asInstanceOf[Term.Apply].args) ++ materializerAndTarget.map("materializer" -> _._2).toSeq
+      val targetTree = materializerAndTarget.map(_._1).getOrElse(t) // patch parent if materializer arg is found
+
+      patchTree(targetTree, http, argExps, targetMethod(argExps("handler"))) + materializerLint
+    }
+
+    def patchTree(t: Tree, http: Term, argExps: Map[String, Term], targetMethod: String): Patch =
+      Patch.replaceTree(t, s"${builder(http, argExps)}.$targetMethod(${argExps("handler")})")
+
+    def handlerIsRoute(handler: Term): Boolean =
+      handler.symbol.info.exists(_.signature.toString contains "Route") || // doesn't seem to work with synthetics on
+        (handler.synthetics match { // only works with `scalacOptions += "-P:semanticdb:synthetics:on"`
+          case ApplyTree(fun, _) :: Nil => fun.symbol.exists(_.displayName == "routeToFlow") // somewhat inaccurate, but that name should be unique enough for our purposes
+          case _                        => false
+        })
+    def bindAndHandleTargetMethod(handler: Term): String =
+      if (handlerIsRoute(handler)) "bind" else "bindFlow"
+
+    def builder(http: Term, argExps: Map[String, Term])(implicit doc: SemanticDocument): String = {
+      def clause(name: String, exp: String => String, onlyIf: Term => Boolean = _ => true): String =
+        if (argExps.contains(name) && onlyIf(argExps(name))) s".${exp(argExps(name).toString)}"
+        else ""
+
+      // This is an approximate test if the parameter might have type `HttpConnectionContext`.
+      // Due to limitations of scalafix (https://scalacenter.github.io/scalafix/docs/developers/semantic-type.html#test-for-subtyping)
+      // we cannot do accurate type tests against `HttpConnectionContext`. This will suffice for simple expressions,
+      // for more complicated ones we will just create an `enableHttps()` clause that will fail to compile if someone
+      // has done something weird which is fine for now.
+      def isNotHttpConnectionContext(term: Term): Boolean =
+        !term.symbol.info.exists(_.signature.toString.contains("HttpConnectionContext"))
+
+      val extraClauses =
+        clause("connectionContext", e => s"enableHttps($e)", isNotHttpConnectionContext) +
+          clause("settings", e => s"withSettings($e)") +
+          clause("log", e => s"logTo($e)") +
+          clause("materializer", e => s"withMaterializer($e)")
+
+      s"$http.newServerAt(${argExps("interface")}, ${argExps.getOrElse("port", 0)})$extraClauses"
+    }
+    def namedArgMap(names: Seq[String], exps: Seq[Term]): Map[String, Term] = {
+      val idx = exps.lastIndexWhere(!_.isInstanceOf[Term.Assign])
+      val positional = exps.take(idx + 1)
+      val named = exps.drop(idx + 1)
+      (positional.zipWithIndex.map {
+        case (expr, idx) => names(idx) -> expr
+      } ++
+        named.map {
+          case q"$name = $expr" => name.asInstanceOf[Term.Name].value -> expr
+        }
+      ).toMap
+    }
+    // still pretty inaccurate but scala meta doesn't support proper type information of terms in public API, so hard to
+    // do it better than this
+    def isHttpExt(http: Term): Boolean = http match {
+      case q"Http()" => true
+      case x if x.symbol.info.exists(_.signature.toString contains "HttpExt") => true
+      case _ => false
+    }
+
+    doc.tree.collect {
+      case t @ q"$http.bindAndHandleAsync(..$params)" if isHttpExt(http) =>
+        // FIXME: warn about parallelism if it exists
+
+        patch(t, http, _ => "bind")
+
+      case t @ q"$http.bindAndHandle(..$params)" if isHttpExt(http)     => patch(t, http, bindAndHandleTargetMethod)
+      case t @ q"$http.bindAndHandleSync(..$params)" if isHttpExt(http) => patch(t, http, _ => "bindSync")
+      case t @ q"$http.bind(..$params)" if isHttpExt(http) =>
+        val args = Seq("interface", "port", "connectionContext", "settings", "log")
+        val argExps = namedArgMap(args, params)
+
+        Patch.replaceTree(t, s"${builder(http, argExps)}.connectionSource()")
+
+    }.asPatch
+  }
+}

akka-http-scalafix/scalafix-test-input/src/main/scala/akka/http/fix/MigrateToServerBuilderTest.scala

@@ -0,0 +1,75 @@
+/*
+rule = MigrateToServerBuilder
+*/
+
+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)
+}

akka-http-scalafix/scalafix-test-output/src/main/scala/akka/http/fix/MigrateToServerBuilderTest.scala

@@ -0,0 +1,59 @@
+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().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().newServerAt("127.0.0.1", 0).withSettings(settings).connectionSource().runWith(Sink.ignore)
+
+  // format: OFF
+  Http().newServerAt("127.0.0.1", 8080).withMaterializer(customMaterializer).bind(route)
+  Http().newServerAt("127.0.0.1", 8080).withMaterializer(customMaterializer).bind(handler)
+  Http().newServerAt("127.0.0.1", 8080).withMaterializer(customMaterializer).bindSync(syncHandler)
+  Http() // needed to appease formatter
+  // format: ON
+
+  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).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)
+}

akka-http-scalafix/scalafix-tests/src/test/scala/akka/http/fix/RuleSuite.scala

@@ -0,0 +1,11 @@
+/*
+ * Copyright (C) 2020 Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package akka.http.fix
+
+import scalafix.testkit.SemanticRuleSuite
+
+class RuleSuite extends SemanticRuleSuite() {
+  runAllTests()
+}

build.sbt

@@ -38,6 +38,8 @@ inThisBuild(Def.settings(
     sLog.value.info(s"Building Akka HTTP ${version.value} against Akka ${AkkaDependency.akkaVersion} on Scala ${(scalaVersion in httpCore).value}")
     (onLoad in Global).value
   },
+
+  scalafixScalaBinaryVersion := scalaBinaryVersion.value,
 ))
 
 lazy val root = Project(
@@ -79,6 +81,7 @@ lazy val root = Project(
     httpTests,
     httpMarshallersScala,
     httpMarshallersJava,
+    httpScalafix,
     docs,
     compatibilityTests
   )
@@ -135,7 +138,7 @@ lazy val parsing = project("akka-parsing")
 lazy val httpCore = project("akka-http-core")
   .settings(commonSettings)
   .settings(AutomaticModuleName.settings("akka.http.core"))
-  .dependsOn(parsing)
+  .dependsOn(parsing, httpScalafixRules % ScalafixConfig)
   .addAkkaModuleDependency("akka-stream", "provided")
   .addAkkaModuleDependency("akka-stream-testkit", "test")
   .addAkkaModuleDependency(
@@ -233,7 +236,8 @@ lazy val httpTests = project("akka-http-tests")
   .settings(commonSettings)
   .settings(Dependencies.httpTests)
   .dependsOn(httpSprayJson, httpXml, httpJackson,
-    httpTestkit % "test", httpCore % "test->test")
+    httpTestkit % "test", httpCore % "test->test",
+    httpScalafixRules % ScalafixConfig)
   .enablePlugins(NoPublish).disablePlugins(BintrayPlugin) // don't release tests
   .enablePlugins(MultiNode)
   .disablePlugins(MimaPlugin) // this is only tests
@@ -316,6 +320,59 @@ def httpMarshallersJavaSubproject(name: String) =
   .enablePlugins(BootstrapGenjavadoc)
   .enablePlugins(ReproducibleBuildsPlugin)
 
+lazy val httpScalafix = project("akka-http-scalafix")
+  .enablePlugins(NoPublish)
+  .disablePlugins(BintrayPlugin, MimaPlugin)
+  .aggregate(httpScalafixRules, httpScalafixTestInput, httpScalafixTestOutput, httpScalafixTests)
+
+lazy val httpScalafixRules =
+  Project(id = "akka-http-scalafix-rules", base = file("akka-http-scalafix/scalafix-rules"))
+    .settings(
+      libraryDependencies += Dependencies.Compile.scalafix
+    )
+    .disablePlugins(MimaPlugin) // tooling, no bin compat guaranteed
+
+lazy val httpScalafixTestInput =
+  Project(id = "akka-http-scalafix-test-input", base = file("akka-http-scalafix/scalafix-test-input"))
+    .dependsOn(http)
+    .addAkkaModuleDependency("akka-stream")
+    .enablePlugins(NoPublish)
+    .disablePlugins(BintrayPlugin, MimaPlugin, HeaderPlugin /* because it gets confused about metaheader required for tests */)
+    .settings(
+      addCompilerPlugin(scalafixSemanticdb),
+      scalacOptions ++= List(
+        "-Yrangepos",
+        "-P:semanticdb:synthetics:on"
+      ),
+      scalacOptions := scalacOptions.value.filterNot(Set("-deprecation", "-Xlint").contains(_)), // we expect deprecated stuff in there
+    )
+
+lazy val httpScalafixTestOutput =
+  Project(id = "akka-http-scalafix-test-output", base = file("akka-http-scalafix/scalafix-test-output"))
+    .dependsOn(http)
+    .addAkkaModuleDependency("akka-stream")
+    .enablePlugins(NoPublish)
+    .disablePlugins(BintrayPlugin, MimaPlugin, HeaderPlugin /* because it gets confused about metaheader required for tests */)
+
+lazy val httpScalafixTests =
+  Project(id = "akka-http-scalafix-tests", base = file("akka-http-scalafix/scalafix-tests"))
+    .enablePlugins(NoPublish)
+    .disablePlugins(BintrayPlugin, MimaPlugin)
+    .settings(
+      skip in publish := true,
+      libraryDependencies += "ch.epfl.scala" % "scalafix-testkit" % Dependencies.scalafixVersion % Test cross CrossVersion.full,
+      compile.in(Compile) :=
+        compile.in(Compile).dependsOn(compile.in(httpScalafixTestInput, Compile)).value,
+      scalafixTestkitOutputSourceDirectories :=
+        sourceDirectories.in(httpScalafixTestOutput, Compile).value,
+      scalafixTestkitInputSourceDirectories :=
+        sourceDirectories.in(httpScalafixTestInput, Compile).value,
+      scalafixTestkitInputClasspath :=
+        fullClasspath.in(httpScalafixTestInput, Compile).value,
+    )
+    .dependsOn(httpScalafixRules)
+    .enablePlugins(ScalafixTestkitPlugin)
+
 lazy val docs = project("docs")
   .enablePlugins(AkkaParadoxPlugin, NoPublish, PublishRsyncPlugin)
   .disablePlugins(BintrayPlugin, MimaPlugin)
@@ -325,7 +382,7 @@ lazy val docs = project("docs")
   .addAkkaModuleDependency("akka-stream-testkit", "provided", AkkaDependency.docs)
   .dependsOn(
     httpCore, http, httpXml, http2Support, httpMarshallersJava, httpMarshallersScala, httpCaching,
-    httpTests % "compile;test->test", httpTestkit % "compile;test->test"
+    httpTests % "compile;test->test", httpTestkit % "compile;test->test", httpScalafixRules % ScalafixConfig
   )
   .settings(Dependencies.docs)
   .settings(
@@ -352,6 +409,7 @@ lazy val docs = project("docs")
       "akka.minimum.version25" -> AkkaDependency.minimumExpectedAkkaVersion,
       "akka.minimum.version26" -> AkkaDependency.minimumExpectedAkka26Version,
       "jackson.version" -> Dependencies.jacksonVersion,
+      "scalafix.version" -> _root_.scalafix.sbt.BuildInfo.scalafixVersion, // grab from scalafix plugin directly
       "extref.akka-docs.base_url" -> s"https://doc.akka.io/docs/akka/${AkkaDependency.docs.link}/%s",
       "javadoc.akka.http.base_url" -> {
         val v = if (isSnapshot.value) "current" else version.value