diff --git a/README.md b/README.md index fccfb64fa..4223c59e6 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,34 @@ going back to how it used to `extend {Name}Service[Future]`.) This Scalafix rule should be idempotent, so it can be rerun many times. +### `AdaptHigherKindedThriftCode` + +Because the `AddCatsTaglessInstances` rewrite rule couldn't easily move the new `{Name}Service` trait up +to the same level as the `{Name}Service` object, the new traits must be addressed differently. In other +words, instead of finding the trait at `com.example.ThriftService`, it will now be +at `com.example.ThriftService.ThriftService`. + +The `AdaptHigherKindedThriftCode` rule exists to adapt existing code to the new location. It will +find references to traits that extend `com.twitter.finagle.thrift.ThriftService` and have a type +parameter of the correct shape, and add the object name before the trait name (i.e., rewriting +`ThriftService` to `ThriftService.ThriftService` or `com.example.ThriftService` to +`com.example.ThriftService.ThriftService`). + +This rule is not idempotent, but it will typically only be executed once per codebase. + +The order in which the rule is executed matters. Follow these steps: + +1. Add Scalafix to your project by following steps 1 and 2 under "Scalafix Rule" above. +2. Look at your project's sbt project graph. Because the rule is a semantic rule, it depends + on the compiler being able to compile the code it will modify. This means the leaves of + the project graph need to be updated before the nodes that depend on each leaf. + + For example, run `Test/scalafix AdaptHigherKindedThriftCode` before + running `Compile/scalafix AdaptHigherKindedThriftCode`. +3. Only after running the `AdaptHigherKindedThriftCode` rule should you update the Scrooge + and Finagle version being used in the project. Once this is updated, you can run the + `AddCatsTaglessInstances` rule on the updated generated code. + ## Artifacts The Group ID for each artifact is `"com.dwolla"`. All artifacts are published to Maven Central. diff --git a/build.sbt b/build.sbt index 875c0ed92..bd04cac9c 100644 --- a/build.sbt +++ b/build.sbt @@ -252,6 +252,7 @@ lazy val `scalafix-input` = (project in file("scalafix/input")) semanticdbEnabled := true, semanticdbVersion := scalafixSemanticdb.revision, ) + .dependsOn(`scalafix-input-dependency`) .disablePlugins(ScalafixPlugin) lazy val `scalafix-output` = (project in file("scalafix/output")) @@ -268,6 +269,7 @@ lazy val `scalafix-output` = (project in file("scalafix/output")) scalacOptions += "-nowarn", scalacOptions ~= { _.filterNot(_ == "-Xfatal-warnings") }, ) + .dependsOn(`scalafix-output-dependency`) .disablePlugins(ScalafixPlugin) lazy val `scalafix-tests` = (projectMatrix in file("scalafix/tests")) @@ -284,6 +286,32 @@ lazy val `scalafix-tests` = (projectMatrix in file("scalafix/tests")) .dependsOn(`scalafix-rules`) .enablePlugins(ScalafixTestkitPlugin) +lazy val `scalafix-input-dependency` = (project in file("scalafix/input-dependency")) + .settings( + publish / skip := true, + scalaVersion := Scala2Versions.head, + libraryDependencies ++= { + Seq( + "com.twitter" %% "finagle-thrift" % TwitterUtilsLatestV, + "org.typelevel" %% "cats-tagless-core" % CatsTaglessV, + "org.typelevel" %% "cats-tagless-macros" % CatsTaglessV, + ) + }, + ) + +lazy val `scalafix-output-dependency` = (project in file("scalafix/output-dependency")) + .settings( + publish / skip := true, + scalaVersion := Scala2Versions.head, + libraryDependencies ++= { + Seq( + "com.twitter" %% "util-core" % TwitterUtilsLatestV, + "org.typelevel" %% "cats-tagless-core" % CatsTaglessV, + "org.typelevel" %% "cats-tagless-macros" % CatsTaglessV, + ) + }, + ) + lazy val `async-utils-root` = (project in file(".")) .aggregate( Seq( diff --git a/scalafix/input-dependency/src/main/scala/example/foo/FooService.scala b/scalafix/input-dependency/src/main/scala/example/foo/FooService.scala new file mode 100644 index 000000000..70bcb341f --- /dev/null +++ b/scalafix/input-dependency/src/main/scala/example/foo/FooService.scala @@ -0,0 +1,30 @@ +/* + * this is a pared-down version of the code that used to be generated + * by scrooge for a hypothetical "Foo" Thrift interface, containing + * a single service named "Foo", with a method named "bar". + * + * It exists because we need the input for the AdaptHigherKindedThriftCode + * rule to compile against the old structure. It's in a separate submodule + * because we don't want any of our Scalafix rules to modify it, and we + * don't want it to be available in this form when compiling the Scalafix + * output module. + */ + +package example.foo + +import com.twitter.util.Future + +@javax.annotation.Generated(value = Array("com.twitter.scrooge.Compiler")) +trait FooService[+MM[_]] extends _root_.com.twitter.finagle.thrift.ThriftService { + def bar: MM[Unit] +} + +object FooService { + trait MethodPerEndpoint extends FooService[Future] + + implicit def FooServiceInReaderT[F[_]]: FooService[({type Λ[β0] = _root_.cats.data.ReaderT[F, FooService[F], β0]})#Λ] = + _root_.cats.tagless.Derive.readerT[FooService, F] + + implicit val FooServiceFunctorK: _root_.cats.tagless.FunctorK[FooService] = _root_.cats.tagless.Derive.functorK[FooService] + +} diff --git a/scalafix/input/src/main/scala/example/dwolla/UsesFooService.scala b/scalafix/input/src/main/scala/example/dwolla/UsesFooService.scala new file mode 100644 index 000000000..4ab60f253 --- /dev/null +++ b/scalafix/input/src/main/scala/example/dwolla/UsesFooService.scala @@ -0,0 +1,9 @@ +/*rule = AdaptHigherKindedThriftCode*/ +package example.dwolla + +import cats.tagless.FunctorK +import example.foo.FooService + +class UsesFooService[F[_]](val fooService: example.foo.FooService[F]) { + implicitly[FunctorK[FooService]] +} diff --git a/scalafix/output-dependency/src/main/scala/example/foo/FooService.scala b/scalafix/output-dependency/src/main/scala/example/foo/FooService.scala new file mode 100644 index 000000000..fed2e842c --- /dev/null +++ b/scalafix/output-dependency/src/main/scala/example/foo/FooService.scala @@ -0,0 +1,26 @@ +/* + * this is a pared-down version of the code that will be generated + * by scrooge, and then modified by our AddCatsTaglessInstances rule, + * for a hypothetical "Foo" Thrift interface, containing a single + * service named "Foo", with a method named "bar". + */ + +package example.foo + +import com.twitter.util.Future + +object FooService { + trait FooService[F[_]] { + def bar: F[Unit] + } + + object FooService { + implicit def FooServiceInReaderT[F[_]]: FooService[({type Λ[β0] = _root_.cats.data.ReaderT[F, FooService[F], β0]})#Λ] = + _root_.cats.tagless.Derive.readerT[FooService, F] + + implicit val FooServiceFunctorK: _root_.cats.tagless.FunctorK[FooService] = _root_.cats.tagless.Derive.functorK[FooService] + + } + + trait MethodPerEndpoint extends FooService[Future] +} diff --git a/scalafix/output/src/main/scala/example/dwolla/UsesFooService.scala b/scalafix/output/src/main/scala/example/dwolla/UsesFooService.scala new file mode 100644 index 000000000..8fa15faa3 --- /dev/null +++ b/scalafix/output/src/main/scala/example/dwolla/UsesFooService.scala @@ -0,0 +1,8 @@ +package example.dwolla + +import cats.tagless.FunctorK +import example.foo.FooService + +class UsesFooService[F[_]](val fooService: example.foo.FooService.FooService[F]) { + implicitly[FunctorK[FooService.FooService]] +} diff --git a/scalafix/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/scalafix/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index c494d686c..28c6a3418 100644 --- a/scalafix/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/scalafix/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -1 +1,2 @@ com.dwolla.scrooge.scalafix.AddCatsTaglessInstances +com.dwolla.scrooge.scalafix.AdaptHigherKindedThriftCode diff --git a/scalafix/rules/src/main/scala/com/dwolla/scrooge/scalafix/AdaptHigherKindedThriftCode.scala b/scalafix/rules/src/main/scala/com/dwolla/scrooge/scalafix/AdaptHigherKindedThriftCode.scala new file mode 100644 index 000000000..64cb0eabf --- /dev/null +++ b/scalafix/rules/src/main/scala/com/dwolla/scrooge/scalafix/AdaptHigherKindedThriftCode.scala @@ -0,0 +1,49 @@ +package com.dwolla.scrooge.scalafix + +import com.dwolla.scrooge.scalafix.AdaptHigherKindedThriftCode._ +import scalafix.v1 +import scalafix.v1._ + +import scala.meta._ + +class AdaptHigherKindedThriftCode extends SemanticRule("AdaptHigherKindedThriftCode") { + override def fix(implicit doc: SemanticDocument): Patch = + addObjectQualifierForThriftServiceTrait(doc.tree) +} + +object AdaptHigherKindedThriftCode { + private def isGeneratedThriftService(t: Type.Name) + (implicit doc: SemanticDocument): Boolean = { + val info = t.symbol.info + + val annotatedWithGenerated = info.exists(_.annotations.exists(_.tpe.toString() == "Generated")) + + val anyRef = v1.Symbol("scala/AnyRef#") + val thriftService = Symbol("com/twitter/finagle/thrift/ThriftService#") + + val extendsThriftServiceAndIsHigherKinded = + info + .map(_.signature) + .collect { + + // class that extends AnyRef and com.twitter.finagle.thrift.ThriftService, and has a single type parameter + // e.g. FooService[F[_]] extends com.twitter.finagle.thrift.ThriftService + case ClassSignature(List(singleTypeParameter), List(TypeRef(NoType, `anyRef`, Nil), TypeRef(NoType, `thriftService`, Nil)), _, _) => singleTypeParameter.signature + } + .collect { + // type parameter that has a single hole, e.g. F[_] + case TypeSignature(List(_), _, _) => true + } + .getOrElse(false) + + annotatedWithGenerated && extendsThriftServiceAndIsHigherKinded + } + + def addObjectQualifierForThriftServiceTrait(tree: Tree) + (implicit doc: SemanticDocument): Patch = + tree.collect { + case t@Type.Name(name) if isGeneratedThriftService(t) => + Patch.addLeft(t, s"$name.") + } + .fold(Patch.empty)(_ + _) +}