diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 479536fde..a92b7bd70 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,10 +66,10 @@ jobs: - run: sbt "scalafixAll --check" - run: ./bin/scalafmt --test mima: - name: MiMa + name: Version Policy runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - uses: olafurpg/setup-scala@v13 - run: git fetch --unshallow - - run: sbt +mimaReportBinaryIssues + - run: sbt +versionPolicyCheck diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 302d7da83..bcdb27cf3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,6 +11,9 @@ jobs: - uses: olafurpg/setup-scala@v13 - uses: olafurpg/setup-gpg@v3 - run: git fetch --unshallow + - name: Check that major or minor was bumped upon compatibility breakage + if: startsWith(github.ref, 'refs/tags/v') + run: sbt versionCheck - name: Publish ${{ github.ref }} run: sbt ci-release env: diff --git a/docs/developers/api.md b/docs/developers/api.md index ad25ec094..9c1f8a598 100644 --- a/docs/developers/api.md +++ b/docs/developers/api.md @@ -4,9 +4,26 @@ title: API Overview sidebar_label: Overview --- +## Compatibility considerations + +[`scalafix-core`](https://index.scala-lang.org/scalacenter/scalafix/scalafix-core) +makes the packages below available to built-in and external rules. + +Its X.Y.Z version scheme follows [Early SemVer](https://www.scala-lang.org/blog/2021/02/16/preventing-version-conflicts-with-versionscheme.html#early-semver-and-sbt-version-policy) +for binary compatiblity and aims at keeping source compatibility across +versions. +- Running a rule built against an older X or 0.Y version of Scalafix may cause + either runtime errors, or false positive/negative tree selection. In that + case, a warning is issued when fetching the rules(s) artifact(s). +- Running a rule built against a more recent X.Y or 0.Y.Z version of Scalafix + is not possible. In that case, a `ScalafixException` is raised when fetching + the rules(s) artifact(s). + +## Packages + The Scalafix public API documentation is composed of several packages. -## Scalafix v1 +### Scalafix v1 Latest Scaladoc: [v@VERSION@](https://static.javadoc.io/ch.epfl.scala/scalafix-core_2.12/@VERSION@/scalafix/v1/index.html) @@ -37,15 +54,7 @@ structures include: information such as trees/tokens and semantic information such as symbols/types/synthetics. -## Scalafix v0 - -Latest Scaladoc: -[v@VERSION@](https://static.javadoc.io/ch.epfl.scala/scalafix-core_2.12/@VERSION@/scalafix/v0/index.html) - -This is a legacy API that exists to smoothen the migration for existing Scalafix -v0.5 rules. If you are writing a new Scalafix rule, please use the v1 API. - -## Scalameta Trees +### Scalameta Trees Latest Scaladoc: [v@SCALAMETA@](https://static.javadoc.io/org.scalameta/trees_2.12/@SCALAMETA@/scala/meta/index.html) @@ -71,7 +80,7 @@ include: position". Statement position is the - `Defn`: the supertype for all definitions -## Scalameta Tokens +### Scalameta Tokens Latest Scaladoc: [v@SCALAMETA@](https://static.javadoc.io/org.scalameta/tokens_2.12/@SCALAMETA@/scala/meta/tokens/Token.html) diff --git a/project/ScalafixBuild.scala b/project/ScalafixBuild.scala index 54cea4967..b8e9f4f82 100644 --- a/project/ScalafixBuild.scala +++ b/project/ScalafixBuild.scala @@ -5,7 +5,8 @@ import sbt.nio.Keys._ import sbt.plugins.JvmPlugin import com.typesafe.tools.mima.plugin.MimaPlugin.autoImport._ import sbtbuildinfo.BuildInfoKey -import sbtbuildinfo.BuildInfoPlugin.autoImport.{BuildInfoKey, _} +import sbtbuildinfo.BuildInfoPlugin.autoImport._ +import sbtversionpolicy.SbtVersionPolicyPlugin.autoImport._ import com.typesafe.sbt.sbtghpages.GhpagesKeys import sbt.librarymanagement.ivy.IvyDependencyResolution import sbt.plugins.IvyPlugin @@ -179,6 +180,13 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys { private val PreviousScalaVersion: Map[String, String] = Map( ) + override def buildSettings: Seq[Setting[_]] = List( + versionPolicyIgnoredInternalDependencyVersions := + Some("^\\d+\\.\\d+\\.\\d+\\+\\d+".r), + versionScheme := Some("early-semver"), + versionPolicyIntention := Compatibility.None // TODO: harden after 0.10.0 + ) + override def projectSettings: Seq[Def.Setting[_]] = List( scalacOptions ++= compilerOptions.value, Compile / console / scalacOptions := diff --git a/project/plugins.sbt b/project/plugins.sbt index 7fcb2cd3d..304339ef0 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -1,7 +1,7 @@ addSbtPlugin("com.github.sbt" % "sbt-ci-release" % "1.5.10") addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.11.0") addSbtPlugin("com.typesafe.sbt" % "sbt-ghpages" % "0.6.3") -addSbtPlugin("com.typesafe" % "sbt-mima-plugin" % "1.0.1") +addSbtPlugin("ch.epfl.scala" % "sbt-version-policy" % "2.0.1") addSbtPlugin("org.scalameta" % "sbt-mdoc" % "2.3.1") addSbtPlugin("org.scoverage" % "sbt-scoverage" % "1.9.3") addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.9.34") diff --git a/scalafix-cli/src/main/scala/scalafix/internal/interfaces/ScalafixArgumentsImpl.scala b/scalafix-cli/src/main/scala/scalafix/internal/interfaces/ScalafixArgumentsImpl.scala index 82ae1ed8f..b0a288eb5 100644 --- a/scalafix-cli/src/main/scala/scalafix/internal/interfaces/ScalafixArgumentsImpl.scala +++ b/scalafix-cli/src/main/scala/scalafix/internal/interfaces/ScalafixArgumentsImpl.scala @@ -24,6 +24,10 @@ import scalafix.Versions import scalafix.cli.ExitStatus import scalafix.interfaces._ import scalafix.internal.config.ScalaVersion +import scalafix.internal.util.Compatibility +import scalafix.internal.util.Compatibility.Compatible +import scalafix.internal.util.Compatibility.Incompatible +import scalafix.internal.util.Compatibility.Unknown import scalafix.internal.v1.Args import scalafix.internal.v1.MainOps import scalafix.internal.v1.Rules @@ -76,7 +80,51 @@ final case class ScalafixArgumentsImpl(args: Args = Args.default) customDependenciesCoordinates, Versions.scalaVersion ) - val extraURLs = customURLs.asScala ++ customDependenciesJARs.asScala + + // External rules are built against `scalafix-core` to expose `scalafix.v1.Rule` implementations. The + // classloader loading `scalafix-cli` already contains `scalafix-core` to be able to discover them (which + // is why it must be the parent of the one loading the tool classpath), so effectively, the version/instance + // in the tool classpath will not be used. This adds a sanity check to warn or prevent the user in case of + // mismatch. + val scalafixCore = coursierapi.Module.parse( + "ch.epfl.scala::scalafix-core", + coursierapi.ScalaVersion.of(Versions.scalaVersion) + ) + customDependenciesJARs.getDependencies.asScala + .find(_.getModule == scalafixCore) + .foreach { dependency => + // We only check compatibility against THE scalafix-core returned by the coursier resolution, but + // this is not exhaustive as some old rules might coexist with recent ones, so the older versions + // get evicted. coursier-interface does not provide more granularity while the native coursier + // is stuck on a 2-years old version because it no longer cross-publishes for 2.11, so for now, + // the easiest option would be to run several resolutions in isolation, which seems like a massive + // cost for just issuing a warning. + Compatibility.earlySemver( + dependency.getVersion, + Versions.nightly + ) match { + case Incompatible => + throw new ScalafixException( + s"Scalafix version ${Versions.nightly} cannot load the registered external rules, " + + s"please upgrade to ${dependency.getVersion} or later" + ) + case Unknown => + args.out.println( + s"""INFO: loading external rule(s) built against an old version of scalafix (${dependency.getVersion}). + |This might not be a problem, but if you run into unexpected behavior, you should either: + |1. downgrade scalafix to ${dependency.getVersion} + |2. try a more recent version of the rules(s) if available; request the rule maintainer + | to build against scalafix ${Versions.nightly} or later if that does not help + """.stripMargin + ) + case Compatible => + } + } + + val extraURLs = customURLs.asScala ++ customDependenciesJARs + .getFiles() + .asScala + .map(_.toURI().toURL()) val classLoader = new URLClassLoader( extraURLs.toArray, getClass.getClassLoader diff --git a/scalafix-cli/src/main/scala/scalafix/internal/util/Compatibility.scala b/scalafix-cli/src/main/scala/scalafix/internal/util/Compatibility.scala new file mode 100644 index 000000000..2f79d0121 --- /dev/null +++ b/scalafix-cli/src/main/scala/scalafix/internal/util/Compatibility.scala @@ -0,0 +1,35 @@ +package scalafix.internal.util + +sealed trait Compatibility + +object Compatibility { + + case object Compatible extends Compatibility + case object Unknown extends Compatibility + case object Incompatible extends Compatibility + + private val Snapshot = """.*-SNAPSHOT""".r + private val XYZ = """^([0-9]+)\.([0-9]+)\.([0-9]+)""".r + + def earlySemver(builtAgainst: String, runWith: String): Compatibility = { + (builtAgainst, runWith) match { + case (Snapshot(), _) => Unknown + case (_, Snapshot()) => Unknown + case (XYZ(bX, _, _), XYZ(rX, _, _)) if bX.toInt > rX.toInt => + Incompatible + case (XYZ(bX, _, _), XYZ(rX, _, _)) if bX.toInt < rX.toInt => + Unknown + // --- X match given the cases above + case (XYZ(_, bY, _), XYZ(_, rY, _)) if bY.toInt > rY.toInt => + Incompatible + case (XYZ("0", bY, _), XYZ("0", rY, _)) if bY.toInt < rY.toInt => + Unknown + case (XYZ(_, bY, _), XYZ(_, rY, _)) if bY.toInt < rY.toInt => + Compatible + // --- X.Y match given the cases above + case (XYZ("0", _, bZ), XYZ("0", _, rZ)) if bZ.toInt > rZ.toInt => + Incompatible + case _ => Compatible + } + } +} diff --git a/scalafix-interfaces/src/main/java/scalafix/internal/interfaces/ScalafixCoursier.java b/scalafix-interfaces/src/main/java/scalafix/internal/interfaces/ScalafixCoursier.java index 83f96cca5..29ae8534c 100644 --- a/scalafix-interfaces/src/main/java/scalafix/internal/interfaces/ScalafixCoursier.java +++ b/scalafix-interfaces/src/main/java/scalafix/internal/interfaces/ScalafixCoursier.java @@ -15,26 +15,33 @@ public class ScalafixCoursier { - private static List fetch( + private static FetchResult fetch( List repositories, List dependencies, ResolutionParams resolutionParams ) throws ScalafixException { - List jars = new ArrayList<>(); try { - List files = Fetch.create() + return Fetch.create() .withRepositories(repositories.stream().toArray(Repository[]::new)) .withDependencies(dependencies.stream().toArray(Dependency[]::new)) .withResolutionParams(resolutionParams) - .fetch(); - for (File file : files) { + .fetchResult(); + } catch (CoursierError e) { + throw new ScalafixException("Failed to fetch " + dependencies + "from " + repositories, e); + } + } + + private static List toURLs(FetchResult result) throws ScalafixException { + List urls = new ArrayList<>(); + for (File file : result.getFiles()) { + try { URL url = file.toURI().toURL(); - jars.add(url); + urls.add(url); + } catch (MalformedURLException e) { + throw new ScalafixException("Failed to load dependency " + file, e); } - } catch (CoursierError | MalformedURLException e) { - throw new ScalafixException("Failed to fetch " + dependencies + "from " + repositories, e); } - return jars; + return urls; } public static List scalafixCliJars( @@ -58,22 +65,15 @@ public static List scalafixCliJars( ScalaVersion.of(scalaVersion) ).withConfiguration("runtime") ); - return fetch(repositories, dependencies, ResolutionParams.create()); + return toURLs(fetch(repositories, dependencies, ResolutionParams.create())); } - public static List toolClasspath( + public static FetchResult toolClasspath( List repositories, List extraDependenciesCoordinates, String scalaVersion ) throws ScalafixException { - // External rules are built against `scalafix-core` to expose `scalafix.v1.Rule` implementations. The - // classloader loading `scalafix-cli` already contains `scalafix-core` to be able to discover them (which - // is why it must be the parent of the one loading the tool classpath), so effectively, the version/instance - // in the tool classpath will not be used. This is OK, as `scalafix-core` should not break binary - // compatibility, as discussed in https://github.com/scalacenter/scalafix/issues/1108#issuecomment-639853899. - Module scalafixCore = Module.parse("ch.epfl.scala::scalafix-core", ScalaVersion.of(scalaVersion)); ResolutionParams excludeDepsInParentClassloader = ResolutionParams.create() - .addExclusion(scalafixCore.getOrganization(), scalafixCore.getName()) .addExclusion("org.scala-lang", "scala-library"); List dependencies = extraDependenciesCoordinates diff --git a/scalafix-tests/unit/src/test/scala/scalafix/tests/cli/CompatibilitySuite.scala b/scalafix-tests/unit/src/test/scala/scalafix/tests/cli/CompatibilitySuite.scala new file mode 100644 index 000000000..fea3a7fe4 --- /dev/null +++ b/scalafix-tests/unit/src/test/scala/scalafix/tests/cli/CompatibilitySuite.scala @@ -0,0 +1,104 @@ +package scalafix.tests.cli + +import org.scalatest.funsuite.AnyFunSuite +import scalafix.internal.util.Compatibility +import scalafix.internal.util.Compatibility._ + +class CompatibilitySuite extends AnyFunSuite { + + // to avoid struggles when testing nightlies + test("EarlySemver unknown if run or build is a snapshot") { + assert( + Compatibility.earlySemver( + builtAgainst = "0.9.34+52-a83785c4-SNAPSHOT", + runWith = "1.2.3" + ) == Unknown + ) + assert( + Compatibility.earlySemver( + builtAgainst = "0.9.34", + runWith = "1.2.3+1-bfe5ccd4-SNAPSHOT" + ) == Unknown + ) + } + + // backward compatibility within X.*.*, 0.Y.*, ... + test( + "EarlySemver compatible if run is equal or greater by minor (or patch in 0.)" + ) { + assert( + Compatibility.earlySemver( + builtAgainst = "1.3.27", + runWith = "1.3.28" + ) == Compatible + ) + assert( + Compatibility.earlySemver( + builtAgainst = "1.10.20", + runWith = "1.12.0" + ) == Compatible + ) + assert( + Compatibility.earlySemver( + builtAgainst = "0.6.12", + runWith = "0.6.12" + ) == Compatible + ) + assert( + Compatibility.earlySemver( + builtAgainst = "0.9.0", + runWith = "0.9.20" + ) == Compatible + ) + } + + // no forward compatibility: build might reference classes unknown to run + test("EarlySemver incompatible if run is lower by minor (or patch in 0.)") { + assert( + Compatibility.earlySemver( + builtAgainst = "0.10.8", + runWith = "0.9.16" + ) == Incompatible + ) + assert( + Compatibility.earlySemver( + builtAgainst = "0.10.17", + runWith = "0.10.4" + ) == Incompatible + ) + assert( + Compatibility.earlySemver( + builtAgainst = "2.0.0", + runWith = "1.1.1" + ) == Incompatible + ) + assert( + Compatibility.earlySemver( + builtAgainst = "1.4.7", + runWith = "1.2.8" + ) == Incompatible + ) + } + + // might be false positive/negative tree matches or link failures + test("EarlySemver unknown if run is greater by major (or minor in 0.)") { + assert( + Compatibility.earlySemver( + builtAgainst = "1.0.41", + runWith = "2.0.0" + ) == Unknown + ) + assert( + Compatibility.earlySemver( + builtAgainst = "0.9.38", + runWith = "0.10.2" + ) == Unknown + ) + assert( + Compatibility.earlySemver( + builtAgainst = "0.9.38", + runWith = "1.0.0" + ) == Unknown + ) + } +}