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

More complex custom rules: Scalafix Plugins/Bundles #304

Closed
olafurpg opened this issue Aug 21, 2017 · 4 comments
Closed

More complex custom rules: Scalafix Plugins/Bundles #304

olafurpg opened this issue Aug 21, 2017 · 4 comments

Comments

@olafurpg
Copy link
Contributor

olafurpg commented Aug 21, 2017

Opening this issue to discuss a potential feature that supersedes #280 and #111.

Problem: advanced custom rewrites are currently limited to either

Solution: support scalafix "plugins", which are a bundle of rewrites that can be invoked by their name instead of scala:fqn/file:path/Rewrite.scala. A scalafix plugin

  • is compiled/developed/released as any other library
  • installed by either
    • reflection: adding the library to the scalafix classpath and dynamically registered with some sys.prop or other means of configuration
    • static mixin: implement a new cli main entry point with the necessary plugins involved

The API to implement a plugin can be something like

trait ScalafixPlugin {
  def rules(index: SemanticdbIndex): Seq[Rule] = Nil
}
object MyPlugin extends ScalafixPlugin

The API to install rewrites can be something like this

class ScalafixMain(plugins: List[ScalafixPlugin]) {
  def main(args: String): Unit = ??? // regular main entrypoint.
  def nailMain(ngContext: NailContext): Unit = ?? // nailgun entrypoint
  // can generate tab completion for rewrites inside `plugins`.
}

// statically mix in plugins
object MyScalafix extends ScalafixMain(MyPlugin :: Nil)

// reflectively load plugins
$ scalafix -Dscalafix.plugins=com.foo.MyPlugin,com.bar.Plugin2

Example plugins that could be implemented:

  • dotty migration
  • sbt 1.0 migration
  • unsafe/best practices
  • your company's custom rewrites/linters
@gabro
Copy link
Collaborator

gabro commented Aug 22, 2017

I like this solution and it's indeed the occam's razor approach to the problem: libraries solve the problem of bundling multiple classes and deps after all.

Compared to what proposed in #280 it carries a bit more friction for authors, because it requires publishing (so PRs like mine to the cats repo would have required much more dedication by the maintainers). Can we maybe provide a dynamic way of loading the plugins if they don't carry external dependencies?

For libraries (e.g. the cats example) I like the approach of hosting the rewrite in the same repo of the code they are fixing, but requiring to publish a library may drive people in the opposite direction (an external repo), especially if the person authoring and maintaining the rewrites is not one of the core maintainers.

I wouldn't want to cause fragmentation between:

  • single-file rewrites: loaded dynamically, hosted on the repo
  • rewrite bundles: require publishing, likely to end up in a separate repo

Also, we must be careful in enforcing compatibility between the plugin's scalafix version and the "host's" scalafix version.

Overall a big 👍 , I just would challenge the UX for authors a bit more.

@olafurpg
Copy link
Contributor Author

Great point Gabro. I agree this proposal does increases friction for library maintainers like in the cats example.

I have a complementary idea how we could support multi-file rewrites: instead of expanding github:org/repo/v1 into a single file, we could

  1. clone the repository locally using jgit or similar
  2. compile all *.scala files under scalafix/rewrites/src/main/scala
  3. classload discovered rewrites under package fix.v1

If the /v1 part is omitted, then all discovered rewrites are classloaded. This would mean that rewrites can be split into multiple files but more complex build features like dependencies/buildinfo are not supported.

If that sounds like a good idea, then I can update the description of #280 and we keep both this issue and #280 open.

@olafurpg olafurpg changed the title Scalafix Plugins More complex custom rules: Scalafix Plugins Sep 12, 2017
@olafurpg olafurpg added this to the v0.5.1 milestone Sep 12, 2017
@olafurpg olafurpg changed the title More complex custom rules: Scalafix Plugins More complex custom rules: Scalafix Plugins/Bundles Sep 19, 2017
@olafurpg
Copy link
Contributor Author

After offline discussions with @gabro, we agreed "bundle" might be a better name than "plugin".

@olafurpg olafurpg removed this from the v0.5.4 milestone Nov 13, 2017
@olafurpg
Copy link
Contributor Author

This issue has now been fixed thanks to using JDK Service Loaders (https://docs.oracle.com/javase/7/docs/api/java/util/ServiceLoader.html). This means that as long as a custom rule is on the --tool-classpath of a scalafix rule (can be done with scalafixDependencies += "org" % "artifact" % "version" in sbt-scalafix) then it can be automatically referenced by name 😊 . All built-in rules have been migrated to use service loaders.

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

No branches or pull requests

2 participants