Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the 3 last versions of scala for each binary version for rules #1174

Merged
merged 3 commits into from
Jul 2, 2020

Conversation

mlachkar
Copy link
Collaborator

No description provided.

@mlachkar mlachkar force-pushed the scalaVersion branch 2 times, most recently from 0904637 to a51d230 Compare June 22, 2020 12:22
Copy link
Collaborator

@bjaglin bjaglin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be using ++ here - it should just be the test input that should be built with older versions of the scala compiler (and potentially the semanticdb plugin compiler). The rules themselves should run with the latest version, as we want to test the artifact being published.

Comment on lines 29 to 31
val supprtedVersions = compilerVersion
.map(int => Set(int - 2, int - 1, int).filter(_ >= 0))
.getOrElse(Set())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val supprtedVersions = compilerVersion
.map(int => Set(int - 2, int - 1, int).filter(_ >= 0))
.getOrElse(Set())
val supportedVersions = List.range(compilerVersion-2, compilerVersion).filter(_>=0)

@@ -194,15 +194,27 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
"unit/test" ::
"docs/run" ::
"interfaces/doc" ::
s"++$scala213V1" ::
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of hardcoding the previous versions, you can probably generate the list dynamically and loop over it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have more tests that fails when testing against more scala Version.
I was thinking about adding a specific test that will test only ExplicitResultType :
+unit/testOnly scalafix.tests.rule.RuleSuite -- -z explicitResult

@mlachkar
Copy link
Collaborator Author

If I set scalaVersion for both testsInput and testsOutput to another scalaVersion, it's supposed to verify what we want to test.
The test will look like :

set testsInput/scalaVersion := "2.12.8"
set testsOutput/scalaVersion := "2.12.8"
unit/testOnly scalafix.tests.rule.RuleSuite -- -z explicitResult

But I noticed that it's not failing if it's not within the 3 versions range. The version check uses as the scala version from TestkitProperties.version, which is not the correct one (?)

@bjaglin
Copy link
Collaborator

bjaglin commented Jun 22, 2020

The version check uses as the scala version from TestkitProperties.version, which is not the correct one (?)

That sounds a lot like the same problem as #1169 (comment), except that this time it's scalaVersion (and not scalacOptions) that was picked up from the test runner instead of the compiled sources.

@mlachkar
Copy link
Collaborator Author

I will check that.

@mlachkar mlachkar changed the title [WIP] Add support for 3 last version of scala for each binary version Add support for the 3 last versions of scala for each binary version for explicitResultType rule Jun 30, 2020
@mlachkar mlachkar force-pushed the scalaVersion branch 3 times, most recently from 49e734c to 26303fe Compare June 30, 2020 16:00
@@ -19,10 +21,20 @@ final class ExplicitResultTypes(

def this() = this(ExplicitResultTypesConfig.default, LazyValue.now(None))

private def supportedScalaVersion = Properties.versionNumberString
private def supportedScalaVersions: List[String] = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not completely satisfied of duplicating this logic. Is there a way to retrieve supportedScalaVersionsForExplicitResultTypeRule from builldInfo

Copy link
Collaborator

@bjaglin bjaglin Jun 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely! you should define a specific buildInfo for the rules project in build.sbt - look for stuff with the buildInfo prefix there to see how it works (keys + plugin activation)

@mlachkar mlachkar requested a review from bjaglin June 30, 2020 16:02
Copy link
Collaborator

@bjaglin bjaglin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments/suggestions to make the testing more maintainable and the error messages (hopefully) more understandable.

List(
s"""set ThisBuild/scalaVersion := "$k"""",
s"""set testsInput/scalaVersion := "$v"""",
s"""set testsOutput/scalaVersion := "$v\"""",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not really required as only the source is used for assertion

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean set testsOutput/scalaVersion := "$v is not required.
The rest is here to verify that the testInput is compiled in a different version from the rule

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean set testsOutput/scalaVersion := "$v is not required.

yes, that specific line is not required, but the 2 others are indeed!

Comment on lines 46 to 53
private def supportedScalaVersions(scalaVersion: String): List[String] = {
val split = scalaVersion.split('.')
val binaryVersion = split.take(2).mkString(".")
val compilerVersion = Try(split.last.toInt).toOption
val supportedVersions =
compilerVersion.map(version => List.range(version - 2, version + 1).filter(_ >= 0)).getOrElse(Nil)
supportedVersions.map(v => s"$binaryVersion.$v")
}
Copy link
Collaborator

@bjaglin bjaglin Jun 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be called after what it does, not after what it is used for. What about something like this:

Suggested change
private def supportedScalaVersions(scalaVersion: String): List[String] = {
val split = scalaVersion.split('.')
val binaryVersion = split.take(2).mkString(".")
val compilerVersion = Try(split.last.toInt).toOption
val supportedVersions =
compilerVersion.map(version => List.range(version - 2, version + 1).filter(_ >= 0)).getOrElse(Nil)
supportedVersions.map(v => s"$binaryVersion.$v")
}
private def latestVersions(scalaVersion: String): List[String] = {
val split = scalaVersion.split('.')
val binaryVersion = split.take(2).mkString(".")
val compilerVersion = Try(split.last.toInt).toOption
val latestPatchVersions =
compilerVersion.map(version => List.range(version - 2, version + 1).filter(_ >= 0)).getOrElse(Nil)
latestPatchVersions.map(v => s"$binaryVersion.$v")
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

@@ -212,6 +213,21 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
s"unit/testOnly -- -l scalafix.internal.tests.utils.SkipWindows" ::
s
},
commands += Command.command("ci-explicit-result-types-rule") { s =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing a new command that loops through all binary versions, maybe we could patch existing ci-212 & ci-213 commands to run an extra unit/test scalafix.tests.rule.RuleSuite for the N-1 & N-2 versions (since N is already run as part of the unit/test)? I think CI failures there would be clearer to troubleshoot (and everything would be already compiled)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

if (config.scalacClasspath.nonEmpty && !supportedScalaVersions.contains(
config.scalaVersion
)) {
val shortScalaVersion =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val shortScalaVersion =
val inputBinaryScalaVersion =

)) {
val shortScalaVersion =
config.scalaVersion.split(".").take(2).mkString(".")
val shortRuleScalaVersion =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val shortRuleScalaVersion =
val runtimeBinaryScalaVersion =

Comment on lines 80 to 86
s"Your scalaVersion is ${config.scalaVersion} but the ExplicitResultTypes rule is compiled with ${shortRuleScalaVersion}" +
s"and only supports these Scala versions '$supportedScalaVersions'. " +
(if (shortScalaVersion == shortRuleScalaVersion)
s"To fix this problem, either remove `ExplicitResultTypes` from .scalafix.conf or change the Scala version to be within '$supportedScalaVersions'"
else
s"To fix this problem, either remove `ExplicitResultTypes` from .scalafix.conf or " +
s"specify a `scalafixScalaBinaryVersion` key in your build.sbt equal to $shortScalaVersion")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, I think this message could be different depending on whether the binary version matches or not.

inputBinaryScalaVersion == runtimeBinaryScalaVersion

The ExplicitResultTypes rule only supports the exact Scala versions '$supportedScalaVersion' for this binary version. To fix this problem, either remove ExplicitResultTypes from .scalafix.conf or change the Scala version of your build to match one of the supported versions.

($supportedScalaVersion should list the full versions supported for the binary version $inputBinaryScalaVersion)

inputBinaryScalaVersion != runtimeBinaryScalaVersion

The ExplicitResultTypes rule needs to run with the same Scala binary version as the one used to compile target sources ($inputBinaryScalaVersion). To fix this problem, either remove ExplicitResultTypes from .scalafix.conf or make sure the scalafixScalaBinaryVersion setting key matches $inputBinaryScalaVersion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@@ -67,6 +67,7 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
"scalameta" -> scalametaV,
scalaVersion,
"supportedScalaVersions" -> supportedScalaVersions,
"supportedScalaVersionsForExplicitResultTypeRule" -> supportedScalaVersionforExplicitResultRule,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the build info of core, I don't think it should know anything about built-in rules

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@@ -19,10 +21,20 @@ final class ExplicitResultTypes(

def this() = this(ExplicitResultTypesConfig.default, LazyValue.now(None))

private def supportedScalaVersion = Properties.versionNumberString
private def supportedScalaVersions: List[String] = {
Copy link
Collaborator

@bjaglin bjaglin Jun 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely! you should define a specific buildInfo for the rules project in build.sbt - look for stuff with the buildInfo prefix there to see how it works (keys + plugin activation)

@@ -11,6 +13,10 @@ object Dependencies {
def coursierV = "2.0.0-RC5-6"
def coursierInterfaceV = "0.0.22"
val currentScalaVersion = scala213
// we support 3 last binary versions of each scala Version, except for 2.11 because semanticdb-scalac_2.11.10 is missing.
val supportedScalaVersionforExplicitResultRule =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should not be specific to a single rule - we could test all of them (unit/testOnly scalafix.tests.rule.RuleSuite) against that set of versions (which will only be useful for those that use the presentation compiler, but at least we won't miss cross-testing it when a new one is introduced).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I agree.

@mlachkar mlachkar force-pushed the scalaVersion branch 3 times, most recently from 1abcfe3 to 16be6bd Compare July 1, 2020 13:02
@mlachkar
Copy link
Collaborator Author

mlachkar commented Jul 1, 2020

Thanks for your review and comments @bjaglin. I do find the new version way better.

Copy link
Collaborator

@bjaglin bjaglin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! A few more comments, but they are mostly cosmetic/implementation details that can be addressed later (or not).

override def description: String =
"Inserts type annotations for inferred public members. " +
s"Only compatible with Scala $supportedScalaVersion."
s"Only compatible with Scala $supportedScalaVersions."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the toString representation of that map reads well? maybe we should only show the values (and maybe mkstring them) ?

Suggested change
s"Only compatible with Scala $supportedScalaVersions."
s"Only compatible with Scala ${supportedScalaVersions.values.flatten}."

project/ScalafixBuild.scala Show resolved Hide resolved
.flatMap { v =>
List(
s"""set testsInput/scalaVersion := "$v"""",
s"""set testsOutput/scalaVersion := "$v"""",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed in the other thread, this does not need to be (re)-compiled, as only sources are used for assertions

Suggested change
s"""set testsOutput/scalaVersion := "$v"""",

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right .. forgot to remove it.

project/ScalafixBuild.scala Show resolved Hide resolved
project/ScalafixBuild.scala Show resolved Hide resolved
build.sbt Outdated
@@ -71,13 +71,17 @@ lazy val rules = project
.in(file("scalafix-rules"))
.settings(
moduleName := "scalafix-rules",
buildInfoKeys ++= Seq[BuildInfoKey](
"supportedScalaVersionforRules" -> supportedScalaVersionforRules
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is local to the rules module, I don't think the suffix is required for the key

Suggested change
"supportedScalaVersionforRules" -> supportedScalaVersionforRules
"supportedScalaVersions" -> supportedScalaVersionforRules

build.sbt Show resolved Hide resolved
Copy link
Collaborator

@bjaglin bjaglin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (atomic) set of suggestions is a thought/proposal I came up with after our discussion around the filter - maybe it's better to have deal with a list of previously supported versions for the current binary version, explicitly named so?

WDYT? I can probably open a separate PR after this is merged to start the discussion... Also, I forgot to update ExplicitResultTypes.scala in that suggestion set, to reflect the fact that RulesBuildInfo.supportedScalaVersions would be a list rather than a map.

build.sbt Outdated
Comment on lines 74 to 78
buildInfoKeys ++= Seq[BuildInfoKey](
"supportedScalaVersions" -> supportedScalaVersionforRules
),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
buildInfoKeys ++= Seq[BuildInfoKey](
"supportedScalaVersions" -> supportedScalaVersionforRules
),
buildInfoKeys ++= Seq[BuildInfoKey](
"supportedScalaVersions" -> scalaVersion.value :: testedPreviousScalaVersions(scalaBinaryVersion.value)
),

Comment on lines 17 to 19
val supportedScalaVersionforRules =
List(scala213, scala212).map(version => version -> latestVersions(version)).toMap ++
Map(scala211 -> List(scala211))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val supportedScalaVersionforRules =
List(scala213, scala212).map(version => version -> latestVersions(version)).toMap ++
Map(scala211 -> List(scala211))
val testedPreviousScalaVersions =
List(scala213, scala212).map(version => version -> previousVersions(version)).toMap

Comment on lines 47 to 53
private def latestVersions(scalaVersion: String): List[String] = {
val split = scalaVersion.split('.')
val binaryVersion = split.take(2).mkString(".")
val compilerVersion = Try(split.last.toInt).toOption
val latestPatchVersions =
compilerVersion.map(version => List.range(version - 2, version + 1).filter(_ >= 0)).getOrElse(Nil)
latestPatchVersions.map(v => s"$binaryVersion.$v")
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private def latestVersions(scalaVersion: String): List[String] = {
val split = scalaVersion.split('.')
val binaryVersion = split.take(2).mkString(".")
val compilerVersion = Try(split.last.toInt).toOption
val latestPatchVersions =
compilerVersion.map(version => List.range(version - 2, version + 1).filter(_ >= 0)).getOrElse(Nil)
latestPatchVersions.map(v => s"$binaryVersion.$v")
}
private def previousVersions(scalaVersion: String): List[String] = {
val split = scalaVersion.split('.')
val binaryVersion = split.take(2).mkString(".")
val compilerVersion = Try(split.last.toInt).toOption
val previousPatchVersions =
compilerVersion.map(version => List.range(version - 2, version).filter(_ >= 0)).getOrElse(Nil)
previousPatchVersions.map(v => s"$binaryVersion.$v")
}

Comment on lines 345 to 351
private def testRulesFor(scalaVersion: String, state: State): State = {
supportedScalaVersionforRules(scalaVersion)
.filter(_ != scalaVersion)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private def testRulesFor(scalaVersion: String, state: State): State = {
supportedScalaVersionforRules(scalaVersion)
.filter(_ != scalaVersion)
private def testRulesAgainstPreviousScalaVersions(scalaVersion: String, state: State): State = {
testedPreviousScalaVersions(scalaVersion)

"unit/test" ::
"docs/run" ::
"interfaces/doc" ::
s
testRulesFor(scala213, s)
Copy link
Collaborator

@bjaglin bjaglin Jul 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testRulesFor(scala213, s)
testRulesAgainstPreviousScalaVersions(scala213, s)

"unit/test" ::
s
testRulesFor(scala212, s)
Copy link
Collaborator

@bjaglin bjaglin Jul 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testRulesFor(scala212, s)
testRulesAgainstPreviousScalaVersions(scala212, s)

"unit/test" ::
s
testRulesFor(scala211, s)
Copy link
Collaborator

@bjaglin bjaglin Jul 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testRulesFor(scala211, s)
testRulesAgainstPreviousScalaVersions(scala211, s)

@@ -18,11 +18,14 @@ final class ExplicitResultTypes(
) extends SemanticRule("ExplicitResultTypes") {

def this() = this(ExplicitResultTypesConfig.default, LazyValue.now(None))
val supportedScalaVersions = RulesBuildInfo.supportedScalaVersions
val runtimeScalaVersion = RulesBuildInfo.scalaVersion
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I remember, I suggested the name runtimeScalaVersion because you were using scala.util.Properties.versionNumberString in an earlier commit of this PR. Now that it's using RulesBuildInfo, I think the right naming would be

Suggested change
val runtimeScalaVersion = RulesBuildInfo.scalaVersion
val compilerScalaVersion = RulesBuildInfo.scalaVersion

In some rare cases, I guess the runtime can differ from that. But from my understanding anyway, this is actually closer to what we want: compare the presentation compiler version brought by scalameta (using the exact same scalaVersion as rules) against the version of scalac used for the input.

Am I getting this right @olafurpg ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, this is just naming so it can be tweaked later (maybe in the same PR as the previous set of cosmetic suggestions I had), I just wanted to make sure we were doing/testing the right thing here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

against the version of scalac used for the input

JFYI, there's also scala.tools.nsc.Properties.versionNumberString which is the compiler's version, but it's highly unlikely to vary compared to the standard library version.

Copy link
Collaborator

@bjaglin bjaglin Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's highly unlikely to vary compared to the standard library version.

I guess that can only happen if the scala version of scalafix-core & scalameta drift, which is rarely the case (https://github.com/scalacenter/scalafix/pull/1179/files bumps both for example) is impossible 😄

$ cs resolve -t ch.epfl.scala::scalafix-rules:0.9.17
  Result:
└─ ch.epfl.scala:scalafix-rules_2.13:0.9.17
   ├─ ch.epfl.scala:scalafix-core_2.13:0.9.17
   │  ├─ com.geirsson:metaconfig-typesafe-config_2.13:0.9.10
   │  │  ├─ com.geirsson:metaconfig-core_2.13:0.9.10
   │  │  │  ├─ com.lihaoyi:pprint_2.13:0.5.9
   │  │  │  │  ├─ com.lihaoyi:fansi_2.13:0.2.9
   │  │  │  │  │  └─ com.lihaoyi:sourcecode_2.13:0.2.1
   │  │  │  │  └─ com.lihaoyi:sourcecode_2.13:0.2.1
   │  │  │  ├─ org.scala-lang:scala-library:2.13.1 -> 2.13.2
   │  │  │  ├─ org.scala-lang.modules:scala-collection-compat_2.13:2.1.2 -> 2.1.6
   │  │  │  │  └─ org.scala-lang:scala-library:2.13.1 -> 2.13.2
   │  │  │  └─ org.typelevel:paiges-core_2.13:0.3.0
   │  │  │     └─ org.scala-lang:scala-library:2.13.0 -> 2.13.2
   │  │  ├─ com.typesafe:config:1.2.1
   │  │  └─ org.scala-lang:scala-library:2.13.1 -> 2.13.2
   │  ├─ com.googlecode.java-diff-utils:diffutils:1.3.0
   │  ├─ org.scala-lang:scala-library:2.13.2
   │  ├─ org.scala-lang.modules:scala-collection-compat_2.13:2.1.6
   │  │  └─ org.scala-lang:scala-library:2.13.1 -> 2.13.2
   │  └─ org.scalameta:scalameta_2.13:4.3.14
   │     ├─ org.scala-lang:scala-library:2.13.2
   │     ├─ org.scala-lang:scalap:2.13.2
   │     │  └─ org.scala-lang:scala-compiler:2.13.2
   │     │     ├─ net.java.dev.jna:jna:5.3.1
   │     │     ├─ org.jline:jline:3.14.1
   │     │     ├─ org.scala-lang:scala-library:2.13.2
   │     │     └─ org.scala-lang:scala-reflect:2.13.2
   │     │        └─ org.scala-lang:scala-library:2.13.2
   │     └─ org.scalameta:parsers_2.13:4.3.14
   │        ├─ org.scala-lang:scala-library:2.13.2
   │        └─ org.scalameta:trees_2.13:4.3.14
   │           ├─ com.thesamet.scalapb:scalapb-runtime_2.13:0.10.3
   │           │  ├─ com.google.protobuf:protobuf-java:3.11.4
   │           │  ├─ com.lihaoyi:fastparse_2.13:2.3.0
   │           │  │  ├─ com.lihaoyi:geny_2.13:0.6.0
   │           │  │  └─ com.lihaoyi:sourcecode_2.13:0.2.1
   │           │  ├─ com.thesamet.scalapb:lenses_2.13:0.10.3
   │           │  │  ├─ org.scala-lang:scala-library:2.13.1 -> 2.13.2
   │           │  │  └─ org.scala-lang.modules:scala-collection-compat_2.13:2.1.6
   │           │  │     └─ org.scala-lang:scala-library:2.13.1 -> 2.13.2
   │           │  ├─ org.scala-lang:scala-library:2.13.1 -> 2.13.2
   │           │  └─ org.scala-lang.modules:scala-collection-compat_2.13:2.1.6
   │           │     └─ org.scala-lang:scala-library:2.13.1 -> 2.13.2
   │           ├─ org.scala-lang:scala-library:2.13.2
   │           ├─ org.scalameta:common_2.13:4.3.14
   │           │  ├─ com.lihaoyi:sourcecode_2.13:0.2.1
   │           │  └─ org.scala-lang:scala-library:2.13.2
   │           └─ org.scalameta:fastparse_2.13:1.0.1
   │              ├─ com.lihaoyi:sourcecode_2.13:0.1.7 -> 0.2.1 (possible incompatibility)
   │              ├─ org.scala-lang:scala-library:2.13.0 -> 2.13.2
   │              └─ org.scalameta:fastparse-utils_2.13:1.0.1
   │                 ├─ com.lihaoyi:sourcecode_2.13:0.1.7 -> 0.2.1 (possible incompatibility)
   │                 └─ org.scala-lang:scala-library:2.13.0 -> 2.13.2
   ├─ org.scala-lang:scala-library:2.13.2

Copy link
Contributor

@dwijnand dwijnand Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's impossible in sbt without disabling things as it has this OverrideScalaMediator (sbt/sbt#2634) which keeps scala-compiler/scala-library/scala-reflect in sync. And then there are also, in the compiler, various sanity checks on the classes in the library, so it crashes early if you mix major versions for example.

@mlachkar mlachkar changed the title Add support for the 3 last versions of scala for each binary version for explicitResultType rule Add support for the 3 last versions of scala for each binary version for rules Jul 2, 2020
project/ScalafixBuild.scala Outdated Show resolved Hide resolved
project/Dependencies.scala Outdated Show resolved Hide resolved
…project rules

We were failing if the scalaversion is not exactly the same than the one used to compile
explicitResultType rule. Now it can work with the 3 last versions, and we have tests to verify that it works as expected
Co-authored-by: Brice Jaglin <bjaglin@gmail.com>
Co-authored-by: Brice Jaglin <bjaglin@gmail.com>
@mlachkar mlachkar merged commit f7adc0e into scalacenter:master Jul 2, 2020
@mlachkar mlachkar deleted the scalaVersion branch July 2, 2020 13:05
mlachkar added a commit to mlachkar/scalafix that referenced this pull request Jul 2, 2020
…for rules (scalacenter#1174)

* Add support for the 3 last versions of scala212 and scala213 for the project rules

We were failing if the scalaversion is not exactly the same than the one used to compile
explicitResultType rule. Now it can work with the 3 last versions, and we have tests to verify that it works as expected

* Update project/ScalafixBuild.scala

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>

* Update project/Dependencies.scala

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>
mlachkar added a commit to mlachkar/scalafix that referenced this pull request Jul 2, 2020
…for rules (scalacenter#1174)

* Add support for the 3 last versions of scala212 and scala213 for the project rules

We were failing if the scalaversion is not exactly the same than the one used to compile
explicitResultType rule. Now it can work with the 3 last versions, and we have tests to verify that it works as expected

* Update project/ScalafixBuild.scala

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>

* Update project/Dependencies.scala

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>
mlachkar added a commit to mlachkar/scalafix that referenced this pull request Jul 2, 2020
…for rules (scalacenter#1174)

* Add support for the 3 last versions of scala212 and scala213 for the project rules

We were failing if the scalaversion is not exactly the same than the one used to compile
explicitResultType rule. Now it can work with the 3 last versions, and we have tests to verify that it works as expected

* Update project/ScalafixBuild.scala

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>

* Update project/Dependencies.scala

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>
mlachkar added a commit to mlachkar/scalafix that referenced this pull request Jul 2, 2020
mlachkar added a commit to mlachkar/scalafix that referenced this pull request Jul 2, 2020
mlachkar added a commit to mlachkar/scalafix that referenced this pull request Jul 2, 2020
…for rules (scalacenter#1174)

* Add support for the 3 last versions of scala212 and scala213 for the project rules

We were failing if the scalaversion is not exactly the same than the one used to compile
explicitResultType rule. Now it can work with the 3 last versions, and we have tests to verify that it works as expected

* Update project/ScalafixBuild.scala

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>

* Update project/Dependencies.scala

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>

Co-authored-by: Brice Jaglin <bjaglin@gmail.com>
mlachkar added a commit to mlachkar/scalafix that referenced this pull request Jul 2, 2020
mlachkar added a commit to mlachkar/scalafix that referenced this pull request Jul 2, 2020
bjaglin pushed a commit to bjaglin/scalafix that referenced this pull request Jul 3, 2020
bjaglin pushed a commit to bjaglin/scalafix that referenced this pull request Jul 3, 2020
@bjaglin bjaglin mentioned this pull request Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants