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

let scalafixScalaBV follow scalaBV to leverage 2.13 & deprecate it #419

Merged
merged 1 commit into from
May 1, 2024

Conversation

bjaglin
Copy link
Collaborator

@bjaglin bjaglin commented May 1, 2024

Rationale

Implementation

Instead of moving to 2.13 on all builds that do not have scalafixScalaBinaryVersion explicitly set (which would be a breaking change for users of ExplicitResultTypes on 2.12 sources), the default now honors the Scala binary version at the project level (leveraging #380), falling back to Scala 2.13 for Scala 3 projects until scalacenter/scalafix#1680 is sorted out.

Impact

Published community rules

We have been advocating for cross-publishing rules for 4 years (for new and existing projects), so all popular rules are now cross-published. As a result, running community rules with Scalafix in Scala 2.13 on projects with Scala 2.13 or Scala 3 should not cause artifact resolution issues.

Unpublished/source community rules

Running unpublished rules relies on on-the-fly compilation via scalafix-reflect, using the Scala compiler of the runtime Scala version. Even though some users were customizing the Scala version to use ExplicitResultTypes and therefore potentially exercising compilation of some unpublished rules with Scala 2.13, that was the minority.

This changes officializes the need for unpublished rules to support both Scala 2.12 and Scala 2.13 for a maximum reach. The biggest challenge is collection compatibility, but scalafix-core pulls scala-collection-compat since 0.9.7 (released 4,5 years ago), so rule authors have been able to rely on the Scala 2.13 collection framework syntax for as long as Scalafix is mature.

It's hard to tell what's the proportion of unpublished rules which can compile with Scala 2.12 but not with Scala 2.13. Users of those will be impacted in the following scenarios:

@bjaglin bjaglin changed the title improve performance by running scalafix with Scala 2.13 by default run scalafix with the same Scala version as the target May 1, 2024
@bjaglin bjaglin force-pushed the scala213 branch 2 times, most recently from 3574592 to 6236106 Compare May 1, 2024 12:30
@bjaglin bjaglin changed the title run scalafix with the same Scala version as the target run scalafix by default with 2.13 unless it is against 2.12 sources May 1, 2024
bjaglin added a commit to bjaglin/scalafix that referenced this pull request May 1, 2024
Leverage scalacenter/sbt-scalafix#419 to remove
sbt-projectmatrix workaround so that we run Scalafix on 2.12 sources.
@bjaglin bjaglin marked this pull request as ready for review May 1, 2024 13:23
@@ -0,0 +1 @@
rules = [OrganizeImports]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not detected as a rename, because I removed the unexercised DisableSyntax

@@ -0,0 +1,5 @@
import scala.Int
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not detected as a rename because I enabled removal of unused imports to make sure a rule was run (unrelated to the fix)

@bjaglin bjaglin changed the title run scalafix by default with 2.13 unless it is against 2.12 sources let scalafixScalaBinaryVersion follow scalaBinaryVersion and deprecate it May 1, 2024
@bjaglin bjaglin changed the title let scalafixScalaBinaryVersion follow scalaBinaryVersion and deprecate it let scalafixScalaBinaryV follow scalaBinaryV to leverage 2.13 and deprecate it May 1, 2024
@bjaglin bjaglin changed the title let scalafixScalaBinaryV follow scalaBinaryV to leverage 2.13 and deprecate it let scalafixScalaBV follow scalaBV to leverage 2.13 and deprecate it May 1, 2024
@bjaglin bjaglin force-pushed the scala213 branch 2 times, most recently from 9854976 to 00fa848 Compare May 1, 2024 15:53
Comment on lines -19 to -21
scalafixScalaBinaryVersion := {
if (scalaBinaryVersion.value == "3") scalafixScalaBinaryVersion.value
else scalaBinaryVersion.value
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a no-op, keeping the default of "2.12"

@bjaglin bjaglin changed the title let scalafixScalaBV follow scalaBV to leverage 2.13 and deprecate it let scalafixScalaBV follow scalaBV to leverage 2.13 & deprecate it May 1, 2024
@bjaglin bjaglin force-pushed the scala213 branch 3 times, most recently from 953b147 to d02d503 Compare May 1, 2024 17:30
val scalafixScalaBinaryVersion: SettingKey[String] =
settingKey[String](
"The Scala binary version used for scalafix execution. Can be set in ThisBuild or at project-level. " +
"Custom rules must be compiled against that binary version. Defaults to 2.12."
"The Scala binary version used for scalafix execution. Must be set at project-level. " +
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until recently, ThisBuild was the only place where scalafixScalaBinaryVersion could be set, so most of the clients must be setting it at that level currently. However, that should not be a problem because the only reason for them to set it was to follow scalaScalafixBinaryVersion, which is now the default (at project-level, thus the inability to override it at ThisBuild level).

That's why the key is being deprecated, and the change in where it effectively not a breaking change.

@bjaglin bjaglin merged commit 84c4cd5 into main May 1, 2024
6 checks passed
bjaglin added a commit to bjaglin/scalafix that referenced this pull request May 1, 2024
Leverage scalacenter/sbt-scalafix#419 to remove
sbt-projectmatrix workaround so that we run Scalafix on 2.12 sources.
bjaglin added a commit to bjaglin/scalafix that referenced this pull request May 1, 2024
Leverage scalacenter/sbt-scalafix#419 to remove
sbt-projectmatrix workaround so that we also run Scalafix on 2.12
sources.
bjaglin added a commit to bjaglin/scalafix that referenced this pull request May 1, 2024
Leverage scalacenter/sbt-scalafix#419 to remove
sbt-projectmatrix workaround so that we also run Scalafix on 2.12
sources.
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

Successfully merging this pull request may close these issues.

ExplicitResultTypes can't easily be used with projects using sbt-projectmatrix
1 participant