diff --git a/build.sbt b/build.sbt index 56a0886e0..2f5d8ed2f 100644 --- a/build.sbt +++ b/build.sbt @@ -63,6 +63,7 @@ lazy val core = project .in(file("scalafix-core")) .settings( moduleName := "scalafix-core", + versionScheme := Some("early-semver"), buildInfoSettingsForCore, libraryDependencies ++= List( scalameta, 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/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 + ) + } +}