-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,137 @@ | ||||||||||||||||
| --- | ||||||||||||||||
| id: cross-building | ||||||||||||||||
| title: Cross-building setups | ||||||||||||||||
| --- | ||||||||||||||||
|
|
||||||||||||||||
| 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. | ||||||||||||||||
|
Comment on lines
+6
to
+28
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
|
||||||||||||||||
|
|
||||||||||||||||
| ## File layout suggestion | ||||||||||||||||
|
|
||||||||||||||||
| Keep shared defaults in one file and let version-specific files `include` it via | ||||||||||||||||
| the HOCON `include` syntax. One convenient layout is: | ||||||||||||||||
|
|
||||||||||||||||
| ``` | ||||||||||||||||
| . | ||||||||||||||||
| └── scalafix/ | ||||||||||||||||
| ├── common.conf | ||||||||||||||||
| ├── scala2.conf | ||||||||||||||||
| └── scala3.conf | ||||||||||||||||
| ``` | ||||||||||||||||
|
Comment on lines
+33
to
+41
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 https://github.com/search?q=.scalafix-scala3.conf&type=code What about flatteing the layour here and across the doc?
|
||||||||||||||||
|
|
||||||||||||||||
| `common.conf` | ||||||||||||||||
| ```scala | ||||||||||||||||
| rules = [ | ||||||||||||||||
| DisableSyntax, | ||||||||||||||||
| OrganizeImports | ||||||||||||||||
| ] | ||||||||||||||||
|
|
||||||||||||||||
| DisableSyntax { | ||||||||||||||||
| noFinalize = true | ||||||||||||||||
| } | ||||||||||||||||
| ``` | ||||||||||||||||
|
|
||||||||||||||||
| `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 | ||||||||||||||||
| } | ||||||||||||||||
| ``` | ||||||||||||||||
|
Comment on lines
+55
to
+77
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). |
||||||||||||||||
|
|
||||||||||||||||
| ### Multiple include files | ||||||||||||||||
|
|
||||||||||||||||
| You may split out even more granular snippets (for example `linting.conf`, | ||||||||||||||||
| `rewrites.conf`) and include them from both `scala2.conf` and `scala3.conf`. The | ||||||||||||||||
| HOCON syntax supports nested includes, so feel free to create the hierarchy that | ||||||||||||||||
| matches your team conventions. | ||||||||||||||||
|
|
||||||||||||||||
| ## Selecting the right config in sbt | ||||||||||||||||
bjaglin marked this conversation as resolved.
Show resolved
Hide resolved
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| Point `scalafixConfig` at the version-specific file, typically inside a helper | ||||||||||||||||
| setting applied to all cross-built projects: | ||||||||||||||||
|
|
||||||||||||||||
| ```scala | ||||||||||||||||
| import scalafix.sbt.ScalafixPlugin.autoImport._ | ||||||||||||||||
|
|
||||||||||||||||
| lazy val commonSettings = Seq( | ||||||||||||||||
| scalafixConfig := { | ||||||||||||||||
| val base = (ThisBuild / baseDirectory).value / "scalafix" | ||||||||||||||||
| val file = | ||||||||||||||||
| CrossVersion.partialVersion(scalaVersion.value) match { | ||||||||||||||||
| case Some((3, _)) => base / "scala3.conf" | ||||||||||||||||
| case _ => base / "scala2.conf" | ||||||||||||||||
| } | ||||||||||||||||
| Some(file) | ||||||||||||||||
| } | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| lazy val core = project | ||||||||||||||||
| .settings(commonSettings) | ||||||||||||||||
| .settings( | ||||||||||||||||
| scalaVersion := "3.3.3", | ||||||||||||||||
| crossScalaVersions := Seq("2.13.14", "3.3.3") | ||||||||||||||||
| ) | ||||||||||||||||
| ``` | ||||||||||||||||
|
|
||||||||||||||||
| For builds that already differentiate per configuration (`Compile`, `Test`, | ||||||||||||||||
| `IntegrationTest`), you can set `Compile / scalafixConfig` and | ||||||||||||||||
| `Test / scalafixConfig` separately if the inputs diverge. | ||||||||||||||||
|
|
||||||||||||||||
| ## Command-line usage | ||||||||||||||||
|
|
||||||||||||||||
| When invoking the CLI directly, pass the desired config with `--config`: | ||||||||||||||||
|
|
||||||||||||||||
| ``` | ||||||||||||||||
| scalafix --config scalafix/scala2.conf | ||||||||||||||||
| scalafix --config scalafix/scala3.conf | ||||||||||||||||
| ``` | ||||||||||||||||
|
|
||||||||||||||||
| Your CI job can loop over each target Scala version, selecting the matching | ||||||||||||||||
| config before running `scalafix --check`. | ||||||||||||||||
|
|
||||||||||||||||
| ## 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. | ||||||||||||||||
|
|
||||||||||||||||
|
Comment on lines
+130
to
+137
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
|
||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.