bazelbuild / rules_scala

Scala rules for Bazel
Apache License 2.0
363 stars 275 forks source link

document or directly support running scalafix #582

Open johnynek opened 6 years ago

johnynek commented 6 years ago

https://github.com/scalacenter/scalafix

Scalafix is being mentioned in almost every discussion about scala 3, and often about scala 2.14 (and perhaps scala 2.13).

Scala 3 is going to allow a ton of changes which will probably be impossible to migrate by hand large code bases, or you must use scalafix. To get out in front of this, we should show how to do that.

@ianoc has run scalafix with rules_scala at Stripe, and it seems to be just a compiler plugin, so it may be pretty easy, but we should document an example and consider special support to make it easier.

One problem is that it does not totally fit the bazel model since it rewrites the source. Normally baze,l contemplates rules as functions on source to outputs, not updaters to source. Code formatting is another example of this. It could be possible bazel needs to have a new kind of rule: source rewrite, which expects sources to be input and output.

johnynek commented 6 years ago

http://eed3si9n.com/stricter-scala-with-xlint-xfatal-warnings-and-scalafix

@ianoc can you add some comments to what you did? Did you just set up the plugin and some scalac options? Any gotchas? Any particularly great use cases?

virusdave commented 4 years ago

Boy this would be nice to have. I don't suppose anyone is going to work on it?

johnynek commented 4 years ago

@ianoc has done this. He might explain how. Scalafix is a plug-in and we have scalac plugin support.

kmate-ct commented 7 months ago

The source I could find is pretty old at this point: https://github.com/ianoc/bazel-scalafix

If I understand it right, it uses a wrapper script to run scalafix, that is:

  1. Collecting targets to be rewritten with bazel query
  2. For each target, collects all dependencies and creates a Scala binary without a valid main class, just to collect all deps into a single JAR
  3. Uses that dep JAR to run scalafix with the rest of the params extracted from the Bazel query

This approach seems to be a bit fragile in some places e.g. the detection of source files etc. applies some heuristics.

I don't know if this gets easier now that the SemanticDB support is in place, but it might be. We'd need to look into it a bit more closely.

kmate-ct commented 7 months ago

After taking a quick look at the cli args of scalafix, it seems to me that collecting the classpath is still being required besides having SemanticDB.

kmate-ct commented 7 months ago

I'm thinking if putting a Bazel aspect on _scalac or some other attribute would be a good idea here, then using that aspect to build a minimal scalafix wrapper script. Then, most of the implementation would go into the aspect, which would just collect all params for scalafix then invoke it. I think the most complex part is the classpath, but we have code in this repo to put that together for scalac; hence it could be reused here as well I guess.

johnynek commented 7 months ago

https://eed3si9n.com/automate-refactoring-with-bazel-and-scalafix/

This could be helpful.

kmate-ct commented 7 months ago

Yeah thanks, I was also thinking about having a custom shim to forward args in a file (there will be a "few" if I add all jars, etc). But, if I could, I'd try to avoid having a custom rule, and a separate target for each lib to fix.

So far I managed to write an aspect that fetches all dependent jars, source files to fix and the semantic db target root. I think I could puzzle the rest together in a few days.

kmate-ct commented 7 months ago

I have a version working in our own repo, but I'd like to contribute this back to rules_scala if there's interest. However, there are a few things to be cleared before I could do so (rough edges):

  1. My current solution executes scalafix through a wrapper from bazel (ctx.actions.run). This has implications, like a fake "output", as scalafix rewrites the original sources. It uses command-line aspects to do it, so it needs an output group etc. to function. Would it be better to print out the args instead to a file directly then executing scalafix from a wrapper script rather than from within bazel?
  2. Also I'm having some hacks around path handling. I need to resolve the "source root" dir (the source paths are relative to this) and the "bazel output root" (the semanticdb paths are relative to this). I'm having some hacks in place now to get these as the current working directory is a "worker sandbox dir" now. This might be a result of 1), actually.
kmate-ct commented 7 months ago

Also I noted when I turn on semanticdb on the toolchain I get tons of output on each bazel build like:

DT:bazel-out/darwin_arm64-fastbuild/bin/external/io_bazel_rules_scala_scala_library/io_bazel_rules_scala_scala_library.stamp/scala-library-2.12.18-stamped.jar