Skip to content

Commit

Permalink
define & document binary compatibility strategy
Browse files Browse the repository at this point in the history
  • Loading branch information
bjaglin committed Mar 12, 2022
1 parent a83785c commit 81158f3
Show file tree
Hide file tree
Showing 5 changed files with 214 additions and 30 deletions.
29 changes: 18 additions & 11 deletions docs/developers/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,24 @@ 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 scheme follows [PVP](https://pvp.haskell.org/) for syntactic and semantic binary compatiblity
and aims at keeping source compatibility across versions.
- A or B (the major version) is increased whenever running rules built against the upcoming version
with an environment frozen at the current version may cause either runtime errors, or false positive
or negative tree selection. A warning is issued at runtime in that case.
- C (the minor version) is increased for bugfixes or to add new features. Rules built against recent
versions cannot be run on older versions of Scalafix.

## 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 +52,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 +78,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.scalaVersion} cannot load the registered external rules, " +
s"please upgrade to ${dependency.getVersion} or later"
)
case Unknown =>
args.out.println(
s"""INFO: loaded external rule(s) built against an old major version of scalafix (${dependency.getVersion}).
|If you encounter unexpected behavior or for correctness, you should either:
|1. downgrade scalafix to ${dependency.getVersion}
|2. check if a more recent version is available and contact the rule maintainer to build
| against ${Versions.scalaVersion} 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 81158f3

Please sign in to comment.