-
Notifications
You must be signed in to change notification settings - Fork 191
Issue 1747 docs cross compilation #2323
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
base: main
Are you sure you want to change the base?
Issue 1747 docs cross compilation #2323
Conversation
|
CI is no really needed, since this is just a doc, but it might take a while before we review in detail though. |
|
okay got it |
bjaglin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to write that down, docs are very important! Have a look at my suggestions, I think the doc could be a bit more concise.
Also, I think the amount of people using the CLI is very low, so I wonder if we couldn't just make a paragraph explaining the sbt setup in the main page (which is sbt-centric)?
docs/users/cross-compilation.md
Outdated
| Scala 3 compilation unit is processed, and vice versa. Until Scalafix grows | ||
| conditional configuration, the safest way to avoid those failures is to split |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no plan to do conditional configuration so the "Until" can be misleading. IMHO, documentation should be about what's available.
docs/users/cross-compilation.md
Outdated
| * Scala 2 only flags such as `-Ywarn-unused-import` or SemanticDB options using | ||
| `-P:semanticdb` are rejected by Scala 3. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is that impacting the configuration file? That looks like a build tool concern to me.
docs/users/cross-compilation.md
Outdated
| * Isolate risky scalac options (`-Wunused`, `-Ywarn-unused-import`, `-P:semanticdb`) | ||
| inside each version-specific file or sbt setting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above - scalac-options are not part of the config so I don't understand that part
docs/users/cross-compilation.md
Outdated
| ### Looking ahead | ||
|
|
||
| Issue [#1747](https://github.com/scalacenter/scalafix/issues/1747) tracks better | ||
| ergonomics for cross compilation. Potential improvements include: | ||
|
|
||
| * Conditional configuration blocks directly inside `.scalafix.conf` (for example | ||
| `if scalaVersion.startsWith("3.")`). | ||
| * First-class support for including multiple files via CLI flags. | ||
| * Allowing rule selection based on the detected input Scala dialect. | ||
|
|
||
| Until those land, the include-based layout above is the recommended, battle-tested | ||
| approach. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should advertize potential improvements in the docs, as it's unclear if there is a need for it, especially as 2.x is on its way out and scalafix is kept updated but no-one is actively adding features to it. As a matter of fact, I was hoping to close #1747 with this doc update.
| ``` | ||
| ### Multiple values for CLI flags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is merged, you can rebase against main
docs/users/cross-compilation.md
Outdated
|
|
||
| rules += RemoveUnused | ||
|
|
||
| // Scala 2 only compilers flags or rule settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned elsewhere, we should focus on compatibility (it's not necessarily about compiler flags but it can be new language features - although that's mostly for community rules, that you don't mention)
| // Scala 2 only compilers flags or rule settings | |
| // Scala 2 only rules or rule settings |
docs/users/cross-compilation.md
Outdated
| ├── project/ | ||
| │ └── build config… |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Irrelevant? also, build.sbt is at the root
|
@arnavsharma990 did you forget to push? |
- Rename file to cross-building.md for URL consistency - Replace 'cross-compilation' with 'cross-building' throughout - Remove speculative/future-looking sections - Add Limitations section explaining no conditional config support - Clarify that scalac/semanticdb flags belong in build tool - Update CLI examples to use defaults from config file - Add sidebar entry under Usage section - Update translations in en.json
bf4a709 to
42fe994
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive documentation for using Scalafix in cross-compiled Scala 2 and Scala 3 projects, addressing issue #1747. It introduces best practices for managing version-specific configurations.
- Explains why separate configuration files are needed for different Scala versions
- Provides concrete examples of file layouts using HOCON's
includepattern - Shows how to wire version-specific configs in sbt, CLI, and CI environments
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| website/sidebars.json | Adds "cross-building" entry to the Usage section navigation |
| website/i18n/en.json | Adds internationalization title entry for the new cross-building page |
| docs/users/cross-building.md | New comprehensive guide covering configuration splitting, file organization, sbt setup, CLI usage, and recommendations for cross-compiled projects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@bjaglin Sorry, I somehow forgot to push earlier , I’ve pushed the updates now. PTAL |
| ## Recommendations | ||
|
|
||
| * Keep rules that truly work on both versions inside `common.conf`. | ||
| * Manage scalac and SemanticDB flags inside the build tool per target; Scalafix | ||
| configs focus solely on rules and rule-specific settings. | ||
| * Document in `README.md` or `CONTRIBUTING.md` which rules run on which Scala | ||
| version to reduce confusion for new contributors. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the first item is already mentioned above and the 2 last are not related to scalafix, so I would simply drop that
| ## Recommendations | |
| * Keep rules that truly work on both versions inside `common.conf`. | |
| * Manage scalac and SemanticDB flags inside the build tool per target; Scalafix | |
| configs focus solely on rules and rule-specific settings. | |
| * Document in `README.md` or `CONTRIBUTING.md` which rules run on which Scala | |
| version to reduce confusion for new contributors. |
| HOCON syntax supports nested includes, so feel free to create the hierarchy that | ||
| matches your team conventions. | ||
|
|
||
| ## Selecting the right config in sbt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## Selecting the right config in sbt | |
| ## Selecting the right configuration file | |
| ### sbt |
| the HOCON `include` syntax. One convenient layout is: | ||
|
|
||
| ``` | ||
| . | ||
| └── scalafix/ | ||
| ├── common.conf | ||
| ├── scala2.conf | ||
| └── scala3.conf | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In most OSS projects I have seen, files are at the root, where .scalafix.conf is looked up by default - I think adding an extra directory complicates discovery.
https://github.com/search?q=.scalafix-scala3.conf&type=code
What about flatteing the layour here and across the doc?
- .scalafix-common.conf
- .scalafix-scala2.conf
- .scalafix-scala3.conf
| Cross-building the same codebase with Scala 2.13 and Scala 3.3+ is common, but | ||
| it collides with how Scalafix resolves rules and compiler options. A rule or | ||
| build-tool flag that only works on Scala 2 can crash Scalafix when the Scala 3 | ||
| compilation unit is processed, and vice versa. Split your configuration per | ||
| Scala version and let the build tool wire the right file. | ||
|
|
||
| ## Why separate configs are required today | ||
|
|
||
| * Scala 2 only scalac flags such as `-Ywarn-unused-import` or SemanticDB options | ||
| using `-P:semanticdb` belong in the build tool. Those settings differ per | ||
| target and cannot be inferred by Scalafix. | ||
| * Some built-in or community rules rely on compiler symbols present only in a | ||
| single Scala major version. | ||
| * The CLI currently ingests a single `.scalafix.conf`; there is no per-target | ||
| override like sbt’s `CrossVersion`. | ||
|
|
||
| ## Limitations | ||
|
|
||
| Scalafix cannot conditionally toggle rules or scalac options based on the input | ||
| Scala version. Provide a separate config file per Scala version and have sbt, | ||
| mill, or your CLI invocation choose the right file along with the correct | ||
| compiler flags. See [issue #1747](https://github.com/scalacenter/scalafix/issues/1747) | ||
| for additional background. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still find this quite hard to follow since it mixes context/rationale, problem and solution space. Here is an attempt at doing this with one paragraph for each:
In a codebase cross-building against several binary versions of Scala, it is advised to run Scalafix several times, for each Scala version. That allows to cover version-specific files and to benefit from compiler diagnostics from several Scala versions for common files.
However, since rules can be specific or only support a subset of features for a given Scala version, a different set of rules or rule features might have to be selected for each run, in order to avoid failing the entire run and preventing relevant rules to run.
Since nothing in the current API allows rules to advertize their limitations programmatically to Scalafix, this page explains how to do it manually.
| `scala2.conf` | ||
| ```scala | ||
| include "common.conf" | ||
|
|
||
| rules += RemoveUnused | ||
|
|
||
| // Scala 2 only rule settings | ||
| RemoveUnused { | ||
| imports = true | ||
| } | ||
| ``` | ||
|
|
||
| `scala3.conf` | ||
| ```scala | ||
| include "common.conf" | ||
|
|
||
| rules += LeakingImplicitClassVal | ||
|
|
||
| // Scala 3 specific tweaks go here | ||
| OrganizeImports { | ||
| groupedImports = Keep | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these configuration files solving real limitations as described in the intro? AFAIK RemoveUnused.imports is fine with Scala 3 and LeakingImplicitClassVal is not related to Scala 3 (more to Scala 3 actually).


Fixes #1747
This PR adds a new documentation page explaining best practices for using
Scalafix in cross-compiled Scala 2 and Scala 3 projects.
It covers:
base.confplus per-version overridesThis is a documentation-only update.