Skip to content

Commit

Permalink
define & apply binary compatibility strategy
Browse files Browse the repository at this point in the history
  • Loading branch information
bjaglin committed Mar 19, 2022
1 parent aa9e409 commit 5a4c18d
Show file tree
Hide file tree
Showing 6 changed files with 217 additions and 30 deletions.
1 change: 1 addition & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ lazy val core = project
.in(file("scalafix-core"))
.settings(
moduleName := "scalafix-core",
versionScheme := Some("pvp"),
buildInfoSettingsForCore,
libraryDependencies ++= List(
scalameta,
Expand Down
31 changes: 20 additions & 11 deletions docs/developers/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 A.B.C version scheme follows [PVP](https://pvp.haskell.org/) for binary compatiblity and aims at
keeping source compatibility across versions.
- A.B (the major version) is increased whenever a binary incompatibility is introduced. Running a rule
built against an older major 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).
- C (the minor version) is increased for bugfixes or to add new features. Running a rule built against
a more recent 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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -76,7 +80,48 @@ 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.pvp(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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
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 PVP = """^([0-9]+)\.([0-9]+)\.([0-9]+)""".r // A.B.C

def pvp(builtAgainst: String, runWith: String): Compatibility = {
(builtAgainst, runWith) match {
case (Snapshot(), _) => Unknown
case (_, Snapshot()) => Unknown
case (PVP(buildA, _, _), PVP(runA, _, _)) if buildA.toInt > runA.toInt =>
Incompatible
case (PVP(buildA, _, _), PVP(runA, _, _)) if buildA.toInt < runA.toInt =>
Unknown
// --- A match given the cases above
case (PVP(_, buildB, _), PVP(_, runB, _)) if buildB.toInt > runB.toInt =>
Incompatible
case (PVP(_, buildB, _), PVP(_, runB, _)) if buildB.toInt < runB.toInt =>
Unknown
// --- A.B match given the cases above
case (PVP(_, _, buildC), PVP(_, _, runC)) if buildC.toInt > runC.toInt =>
Incompatible
case _ => Compatible
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package scalafix.tests.cli

import org.scalatest.funsuite.AnyFunSuite
import scalafix.internal.util.Compatibility
import scalafix.internal.util.Compatibility._

class CompatibilitySuite extends AnyFunSuite {

test("PVP compatible if full match") {
assert(
Compatibility.pvp(
builtAgainst = "0.9.34",
runWith = "0.9.34"
) == Compatible
)
assert(
Compatibility.pvp(
builtAgainst = "1.2.3",
runWith = "1.2.3"
) == Compatible
)
}

// to avoid struggles when testing nightlies
test("PVP unknown if run or build is a snapshot") {
assert(
Compatibility.pvp(
builtAgainst = "0.9.34+52-a83785c4-SNAPSHOT",
runWith = "1.2.3"
) == Unknown
)
assert(
Compatibility.pvp(
builtAgainst = "0.9.34",
runWith = "1.2.3+1-bfe5ccd4-SNAPSHOT"
) == Unknown
)
}

// no binary breaking change in 0.9.x, 0.10.x, ...
test("PVP compatible if run is greater by minor") {
assert(
Compatibility.pvp(
builtAgainst = "0.10.0",
runWith = "0.10.20"
) == Compatible
)
}

// might be false positive/negative tree matches (AST updates)
test("PVP unknown if run is greater by major") {
assert(
Compatibility.pvp(
builtAgainst = "0.9.38",
runWith = "0.10.2"
) == Unknown
)
assert(
Compatibility.pvp(
builtAgainst = "0.9.38",
runWith = "1.0.0"
) == Unknown
)
}

// build might reference classes unknown to run
test("PVP incompatible if build is greater") {
assert(
Compatibility.pvp(
builtAgainst = "2.0.0",
runWith = "1.1.1"
) == Incompatible
)
assert(
Compatibility.pvp(
builtAgainst = "1.4.7",
runWith = "1.4.6"
) == Incompatible
)
assert(
Compatibility.pvp(
builtAgainst = "1.4.7",
runWith = "1.2.8"
) == Incompatible
)
assert(
Compatibility.pvp(
builtAgainst = "0.10.8",
runWith = "0.9.16"
) == Incompatible
)
assert(
Compatibility.pvp(
builtAgainst = "0.10.17",
runWith = "0.10.4"
) == Incompatible
)
}
}

0 comments on commit 5a4c18d

Please sign in to comment.