define & apply binary compatibility strategy

.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

.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:

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)

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 :=

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")

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

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
+    }
+  }
+}

scalafix-interfaces/src/main/java/scalafix/internal/interfaces/ScalafixCoursier.java

@@ -15,26 +15,33 @@
 
 public class ScalafixCoursier {
 
-    private static List<URL> fetch(
+    private static FetchResult fetch(
             List<Repository> repositories,
             List<Dependency> dependencies,
             ResolutionParams resolutionParams
     ) throws ScalafixException {
-        List<URL> jars = new ArrayList<>();
         try {
-            List<File> 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<URL> toURLs(FetchResult result) throws ScalafixException {
+        List<URL> 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<URL> scalafixCliJars(
@@ -58,22 +65,15 @@ public static List<URL> scalafixCliJars(
                 ScalaVersion.of(scalaVersion)
             ).withConfiguration("runtime")
         );
-        return fetch(repositories, dependencies, ResolutionParams.create());
+        return toURLs(fetch(repositories, dependencies, ResolutionParams.create()));
     }
 
-    public static List<URL> toolClasspath(
+    public static FetchResult toolClasspath(
             List<Repository> repositories,
             List<String> 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<Dependency> dependencies = extraDependenciesCoordinates

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
+    )
+  }
+}