Scalafix rewrites via dependency when target="build"

[russellremple]

All our Scalafix migrations are in private repos. By using the "dependency:" prefix in rewrite rule definitions, Scala Steward leverages Coursier to resolve these from our private artifactory. But this only seems to work when the target="sources" (default).

I'm attempting to create a new rewrite for SBT files by using target="build" and I am unable to fetch these rules from our artifactory. It appears the issue is that, where "sources" uses the Scalafix SBT plugin, "build" uses the Scalafix CLI, and the CLI does not support the "dependency:" prefix for rules. The Scalafix docs indicate that "Users of the Scalafix command-line interface can use the --tool-classpath flag" to use published artifacts.

If there is a better way to achieve this through configuration, environment variables, etc., that would be amazing. But I haven't been able to work out how to do that, and without it, I'm thinking a change like this might solve the problem. Looking for feedback and guidance on this approach. Basically it would keep existing target="build" rules using https, github, file, etc. unchanged, but would reinterpret rules specified in the form dependency:ruleName@repo::artifact:version to be ruleName --tool-classpath $(cs fetch repo::artifact:version -p), consistent with the Scalafix documentation linked above.

If I'm on the right track here, I'd be happy to update documentation and write a couple tests.

[codecov[bot]]

Codecov Report

Merging #2668 (c28bb20) into main (f55c990) will increase coverage by 0.00%. The diff coverage is 100.00%.

:exclamation: Current head c28bb20 differs from pull request most recent head 0cf51c1. Consider uploading reports for the commit 0cf51c1 to get more accurate results

@@           Coverage Diff           @@
##             main    #2668   +/-   ##
=======================================
  Coverage   81.32%   81.33%           
=======================================
  Files         146      146           
  Lines        2592     2593    +1     
  Branches       43       46    +3     
=======================================
+ Hits         2108     2109    +1     
  Misses        484      484           

Impacted Files	Coverage Δ
.../scalasteward/core/edit/scalafix/ScalafixCli.scala	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f55c990...0cf51c1. Read the comment docs.

[fthomas]

The approach look good to me or at least I don't have a better idea how to use published rules with the Scalafix CLI. But I don't think that the proposed solution would work:

          s"--rules=$ruleName --tool-classpath $$(cs fetch ${depParts.mkString(":")} -p)"

Scala Steward will pass this argument verbatim to java.lang.ProcessBuilder and not a shell so that $(cs fetch ... -p) will not be expanded unless Scalafix will do the expansion (which I doubt). A solution that could work in this case is to add a new toolClasspathCommand (or toolClasspathCommands if Scalafix allows multiple --tool-classpaths arguments) to ScalafixMigration which is(are) executed by Scala Steward and whose output(s) is(are) then used as value(s) of --tool-classpath argument(s).

WDYT?

[fthomas]

A solution that could work in this case is to add a new toolClasspathCommand

We could of course keep the proposed syntax and let Scala Steward execute the cs fetch ... command but adding a new field to ScalafixMigration is IMO more general, easier to implement and easier to maintain.

[russellremple]

Thanks for the thoughtful input. You are definitely correct, that this will not work as it is. We tried simulating the change locally, and the $(cs fetch ... -p) part does not expanded. I've tested the Scalafix CLI locally and it does syntactically allow for multiple --tool-classpath arguments s on the same line (e.g., --rules=a --tool-classpath pathsForA- -rules=b --tool-classpath pathsForB), but I don't know if it actually processes them all. I really like the idea of a new toolClasspathCommands. I'll struggle with that for a bit and update here if/when I get stuck. (Putting this in draft for now.) Thanks again!

[russellremple]

Not sure when I'll get a chance to get back to this.