Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update Finagle and Scalafix rule to rewrite Scrooge-generated code to work with this project #42

Merged
merged 5 commits into from
Apr 14, 2022

Conversation

bpholt
Copy link
Member

@bpholt bpholt commented Apr 6, 2022

Unfortunately, several months ago Twitter's Scrooge project modified the way it generates code from Thrift IDL files, in a way that is no longer compatible with this project.

It used to generate code like this (with quite a lot more boilerplate—this is the distilled version):

trait FooService[+MM[_]] extends ThriftService {
  def foo: MM[Unit]
}
object FooService {
  trait MethodPerEndpoint extends FooService[Future]
}

Scrooge removed the generation of the higher-kinded FooService[+MM[_]], and now generates code like this:

object FooService {
  trait MethodPerEndpoint extends ThriftService {
    def foo: Future[Unit]
  }
}

async-utils relied on the higher-kinded trait to do its magic, so we need to reintroduce it. The rewrite rule will modify the generated code to look like this:

object FooService {
  trait FooService[F[_]] extends ThriftService {
    def foo: F[Unit]
  }

  trait MethodPerEndpoint extends FooService[Future]
}

I couldn't find a straightforward way to do those rewrites and also move the trait FooService[F[_]]… up to the same level as object FooService. As a consequence, code that used to refer to the higher-kinded trait needs to refer to the new trait inside the object. I plan to follow this PR with another that will add another rewrite rule to automate those changes as well.

@bpholt
Copy link
Member Author

bpholt commented Apr 12, 2022

I rebased the commits in this PR to hopefully make it a little easier to review. The real changes are in 76cefd8; the other commits simply get dependencies updated and prepare the project to make the changes in 76cefd8.

(The main Scalafix rule tests operate on input and output files that are committed to the repo. The input files are the source files before the Scalafix rules are applied, and the output files are what we expect the files to look like after the rules run. Since Scrooge changed what the generated code looks like, the input and output files had to change accordingly. Those changes make up the bulk of the changes in the commits other than 76cefd8.)

@bpholt bpholt merged commit 3e5201e into main Apr 14, 2022
@bpholt bpholt deleted the finagle branch April 14, 2022 16:32
bpholt pushed a commit that referenced this pull request May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants