Skip to content
This repository has been archived by the owner on Jul 14, 2023. It is now read-only.

Test Scala 3 support using sbt-projectmatrix #179

Merged
merged 5 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ jobs:
- uses: olafurpg/setup-scala@v11
- uses: coursier/cache-action@v6
- name: Lint
run: sbt scalafmtCheckAll "rules/scalafix --check"
run: sbt scalafmtCheckAll "rules2_12/scalafix --check"
Copy link
Contributor Author

@bjaglin bjaglin May 20, 2021

Choose a reason for hiding this comment

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

this is a no-op against master as rules/scalafix is running with the default scalaVersion there - namely 2.12

- name: Test
run: sbt coverage +tests/test +rules/coverageReport
run: sbt coverage tests/test coverageAggregate
Copy link
Contributor Author

@bjaglin bjaglin May 20, 2021

Choose a reason for hiding this comment

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

  1. The cross-building + is no longer required since we don't need to mutate the state to loop through all scala versions. It won't hurt to leave it though.
  2. I am not sure why https://github.com/liancheng/scalafix-organize-imports/pull/179/checks?check_run_id=2628246932 fails as this should collect reports for all rules* projects

- uses: codecov/codecov-action@v1
with:
token: ${{ secrets.CODECOV_TOKEN }}
Expand Down
28 changes: 26 additions & 2 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,18 @@ Assuming that the first two commands run successfully, the last `scalafixAll --c

However, you should make sure that the source-formatting tools you use do not rewrite import statements in ways that conflict with `OrganizeImports`. For example, when using Scalafmt together with `OrganizeImports`, the `ExpandImportSelectors`, `SortImports`, and `AsciiSortImports` rewriting rules should not be used.

=== Scala 3

Running the rule on source files compiled with Scala 3 is still experimental.

Known limitations:

. You must use Scalafix 0.9.28 or later
. The <<removeUnused, `removeUnused`>> option must be explicitly set to `false`
. Source files using new syntax introduced in Scala 3 such as https://docs.scala-lang.org/scala3/book/ca-given-imports.html[`given` imports] may not work properly
. Usage of http://dotty.epfl.ch/docs/reference/dropped-features/package-objects.html[deprecated package objects] may result in incorrect imports
. The <<groupExplicitlyImportedImplicitsSeparately, groupExplicitlyImportedImplicitsSeparately>> option has no effect

== Configuration

=== Default Configuration values
Expand Down Expand Up @@ -444,6 +456,13 @@ Unfortunately, Scalafix is not able to surgically identify conflicting implicit

CAUTION: In general, order-sensitive imports are fragile, and can easily be broken by either human collaborators or tools (e.g., the IntelliJ IDEA Scala import optimizer does not handle this case correctly). They should be eliminated whenever possible. This option is mostly useful when you are dealing with a large trunk of legacy codebase, and you want to minimize manual intervention and guarantee correctness in all cases.


[IMPORTANT]
====
The `groupExplicitlyImportedImplicitsSeparately` option has currently no effect on source files compiled with Scala 3, as the https://github.com/lampepfl/dotty/issues/12766[compiler does not expose full signature information], preventing the rule to identify imported implicits.
====


==== Value type

Boolean
Expand Down Expand Up @@ -472,7 +491,7 @@ Configuration:
----
OrganizeImports {
groups = ["scala.", "*"]
groupExplicitlyImportedImplicitsSeparately = true
groupExplicitlyImportedImplicitsSeparately = true // not supported in Scala 3
}
----

Expand Down Expand Up @@ -1319,6 +1338,11 @@ Remove unused imports.
As mentioned in <<remove-unused-warning, a previous section>>, the `removeUnused` option doesn't play perfectly with the `expandRelative` option. Setting `expandRelative` to `true` might introduce new unused imports (see <<expandRelative, `expandRelative`>>). These newly introduced unused imports cannot be removed by setting `removeUnused` to `true`. This is because unused imports are identified using Scala compilation diagnostics information, and the compilation phase happens before Scalafix rules get applied.
====

[IMPORTANT]
====
The `removeUnused` option is currently not supported for source files compiled with Scala 3, as the https://docs.scala-lang.org/scala3/guides/migration/options-lookup.html#warning-settings[compiler cannot issue warnings for unused imports yet]. As a result, you must set `removeUnused` to `false` when running the rule on source files compiled with Scala 3.
====

==== Value type

Boolean
Expand All @@ -1335,7 +1359,7 @@ Configuration:
----
OrganizeImports {
groups = ["javax?\\.", "scala.", "*"]
removeUnused = true
removeUnused = true // not supported in Scala 3
}
----

Expand Down
152 changes: 110 additions & 42 deletions build.sbt
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
lazy val v = _root_.scalafix.sbt.BuildInfo

lazy val rulesCrossVersions = Seq(v.scala213, v.scala212, v.scala211)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FTR, we might cross-build scalafix & rules to Scala3 in the future, but for the moment it's not a priority as the vast majority of the rules don't need to match the sources they are fixing - see scalacenter/scalafix#1316 & scalacenter/sbt-scalafix#214.

lazy val scala3Version = "3.0.0"

inThisBuild(
List(
organization := "com.github.liancheng",
Expand All @@ -13,81 +16,146 @@ inThisBuild(
url("https://github.com/liancheng")
)
),
scalaVersion := v.scala212,
crossScalaVersions := List(v.scala211, v.scala212, v.scala213),
scalacOptions ++= List(
"-deprecation",
"-Yrangepos",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"-P:semanticdb:synthetics:on"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this wasn't useful (nor documented)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess scalac-semanticdb does not honor the documented default of false as synthestics are present without this on Scala 2.

~/git/projects/scalafix-organize-imports$ metap ./input/target/scala-2.12/classes/META-INF/semanticdb/input/src/main/scala/fix/ExplicitlyImportedImplicits.scala.semanticdb | tail -n3
Synthetics:
[14:2..14:6) => *(intImplicit)
[15:2..15:6) => *(stringImplicit)

),
conflictManager := ConflictManager.strict,
dependencyOverrides ++= List(
"org.scala-lang.modules" %% "scala-xml" % "1.2.0",
"org.slf4j" % "slf4j-api" % "1.7.25",
"com.lihaoyi" %% "sourcecode" % "0.2.1",
"org.scala-lang.modules" %% "scala-collection-compat" % "2.1.6"
"-deprecation"
),
addCompilerPlugin(scalafixSemanticdb),
semanticdbEnabled := true,
// semanticdbTargetRoot makes it hard to have several input modules
semanticdbIncludeInJar := true,
semanticdbVersion := scalafixSemanticdb.revision,
scalafixDependencies += "com.github.liancheng" %% "organize-imports" % "0.5.0",
// Super shell output often messes up Scalafix test output.
useSuperShell := false
)
)

skip in publish := true

lazy val rules = project
lazy val `scalafix-organize-imports` = project
.in(file("."))
.aggregate(
rules.projectRefs ++
input.projectRefs ++
output.projectRefs ++
tests.projectRefs: _*
)
Comment on lines +34 to +39
Copy link
Contributor Author

Choose a reason for hiding this comment

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

required with https://github.com/sbt/sbt-projectmatrix to keep aggregation working (for scalafmtAll for example)

.settings(
publish / skip := true
)
lazy val rules = projectMatrix
.settings(
moduleName := "organize-imports",
dependencyOverrides += "com.lihaoyi" %% "sourcecode" % "0.2.1",
conflictManager := ConflictManager.strict,
dependencyOverrides ++= List(
"org.scala-lang.modules" %% "scala-collection-compat" % "2.1.6",
"com.lihaoyi" %% "sourcecode" % "0.2.1"
),
libraryDependencies += "ch.epfl.scala" %% "scalafix-core" % v.scalafixVersion,
scalacOptions ++= List("-Ywarn-unused"),
scalafixOnCompile := true
)
.defaultAxes(VirtualAxis.jvm)
.jvmPlatform(rulesCrossVersions)

lazy val shared = project.settings(skip in publish := true)
lazy val shared = projectMatrix
.settings(
publish / skip := true,
coverageEnabled := false
)
.defaultAxes(VirtualAxis.jvm)
.jvmPlatform(scalaVersions = rulesCrossVersions :+ scala3Version)

lazy val input = project
lazy val input = projectMatrix
.dependsOn(shared)
.settings(skip in publish := true)
.settings(
publish / skip := true,
coverageEnabled := false
)
.defaultAxes(VirtualAxis.jvm)
.jvmPlatform(scalaVersions = rulesCrossVersions :+ scala3Version)

lazy val output = project
lazy val output = projectMatrix
.dependsOn(shared)
.settings(skip in publish := true)
.settings(
publish / skip := true,
coverageEnabled := false
)
.defaultAxes(VirtualAxis.jvm)
Copy link

Choose a reason for hiding this comment

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

It's pity that the default it's not VirtualAxis.jvm: so we wouldn't need to specify it everytime

.jvmPlatform(scalaVersions = rulesCrossVersions :+ scala3Version)

lazy val inputUnusedImports = project
lazy val inputUnusedImports = projectMatrix
.dependsOn(shared)
.settings(
skip in publish := true,
publish / skip := true,
coverageEnabled := false,
scalacOptions += {
if (scalaVersion.value.startsWith("2.11."))
"-Ywarn-unused-import"
else
"-Ywarn-unused"
}
)
.defaultAxes(VirtualAxis.jvm)
.jvmPlatform(scalaVersions = rulesCrossVersions :+ scala3Version)

lazy val tests = project
lazy val testsAggregate = Project("tests", file("target/testsAggregate"))
.aggregate(tests.projectRefs: _*)
Comment on lines +99 to +100
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aggregation dummy project to maintain backward compatibility on CI statements and for convenience for sbt ~tests/test


lazy val tests = projectMatrix
.dependsOn(rules)
.enablePlugins(ScalafixTestkitPlugin)
.settings(
skip in publish := true,
scalacOptions ++= List("-Ywarn-unused"),
publish / skip := true,
coverageEnabled := false,
libraryDependencies +=
"ch.epfl.scala" % "scalafix-testkit" % v.scalafixVersion % Test cross CrossVersion.full,
(compile in Compile) := (compile in Compile)
.dependsOn(
compile in (input, Compile),
compile in (inputUnusedImports, Compile)
)
.value,
scalafixTestkitOutputSourceDirectories := sourceDirectories.in(output, Compile).value,
scalafixTestkitInputSourceDirectories := (
sourceDirectories.in(input, Compile).value ++
sourceDirectories.in(inputUnusedImports, Compile).value
),
scalafixTestkitInputClasspath := (
fullClasspath.in(input, Compile).value ++
fullClasspath.in(inputUnusedImports, Compile).value
).distinct
scalafixTestkitOutputSourceDirectories :=
TargetAxis.resolve(output, Compile / unmanagedSourceDirectories).value,
scalafixTestkitInputSourceDirectories := {
val inputSrc = TargetAxis.resolve(
input,
Compile / unmanagedSourceDirectories
).value
val inputUnusedImportsSrc = TargetAxis.resolve(
inputUnusedImports,
Compile / unmanagedSourceDirectories
).value
inputSrc ++ inputUnusedImportsSrc
},
scalafixTestkitInputClasspath := {
val inputClasspath = TargetAxis.resolve(
input,
Compile / fullClasspath
).value
val inputUnusedImportsClasspath = TargetAxis.resolve(
inputUnusedImports,
Compile / fullClasspath
).value
inputClasspath ++ inputUnusedImportsClasspath
},
scalafixTestkitInputScalacOptions :=
TargetAxis.resolve(inputUnusedImports, Compile / scalacOptions).value,
scalafixTestkitInputScalaVersion :=
TargetAxis.resolve(inputUnusedImports, Compile / scalaVersion).value
)
.defaultAxes(
rulesCrossVersions.map(VirtualAxis.scalaABIVersion) :+ VirtualAxis.jvm: _*
)
.customRow(
scalaVersions = Seq(v.scala212),
axisValues = Seq(TargetAxis(scala3Version), VirtualAxis.jvm),
settings = Seq()
)
.customRow(
scalaVersions = Seq(v.scala213),
axisValues = Seq(TargetAxis(v.scala213), VirtualAxis.jvm),
settings = Seq()
)
.customRow(
scalaVersions = Seq(v.scala212),
axisValues = Seq(TargetAxis(v.scala212), VirtualAxis.jvm),
settings = Seq()
)
.customRow(
scalaVersions = Seq(v.scala211),
axisValues = Seq(TargetAxis(v.scala211), VirtualAxis.jvm),
settings = Seq()
)
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
OrganizeImports.expandRelative = true
*/
package fix
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
OrganizeImports.groupExplicitlyImportedImplicitsSeparately = true
*/
package fix
Expand Down
1 change: 1 addition & 0 deletions input/src/main/scala/ExpandRelativeRootPackage.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ rules = [OrganizeImports]
OrganizeImports {
expandRelative = true
groupedImports = Explode
removeUnused = false
}
*/
import P._
Expand Down
1 change: 1 addition & 0 deletions input/src/main/scala/OrganizeImportsRootPackage.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
OrganizeImports.groups = ["re:javax?\\.", "*", "scala."]
*/
import java.time.Clock
Expand Down
1 change: 1 addition & 0 deletions input/src/main/scala/fix/AlreadyOrganized.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
OrganizeImports.groupedImports = Merge
*/
package fix
Expand Down
2 changes: 2 additions & 0 deletions input/src/main/scala/fix/BlankLinesManual.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]

OrganizeImports {
blankLines = Manual
groups = [
Expand All @@ -8,6 +9,7 @@ OrganizeImports {
"---"
"*"
]
removeUnused = false
}
*/
package fix
Expand Down
1 change: 1 addition & 0 deletions input/src/main/scala/fix/CoalesceImportees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ rules = [OrganizeImports]
OrganizeImports {
groupedImports = Keep
coalesceToWildcardImportThreshold = 3
removeUnused = false
}
*/
package fix
Expand Down
5 changes: 4 additions & 1 deletion input/src/main/scala/fix/CurlyBracedSingleImportee.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
/* rules = [OrganizeImports] */
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
*/
package fix

import scala.collection.{Map}
Expand Down
5 changes: 4 additions & 1 deletion input/src/main/scala/fix/DeduplicateImportees.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
/* rules = [OrganizeImports] */
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
*/
package fix

import scala.collection.immutable.Vector
Expand Down
1 change: 1 addition & 0 deletions input/src/main/scala/fix/ExpandRelative.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
OrganizeImports.expandRelative = true
*/
package fix
Expand Down
1 change: 1 addition & 0 deletions input/src/main/scala/fix/ExpandRelativeMultiGroups.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
OrganizeImports.expandRelative = true
*/
package fix
Expand Down
1 change: 1 addition & 0 deletions input/src/main/scala/fix/ExpandRelativeQuotedIdent.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
OrganizeImports.expandRelative = true
*/
package fix
Expand Down
1 change: 1 addition & 0 deletions input/src/main/scala/fix/ExpandRelativeRootPackage.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
OrganizeImports.expandRelative = true
*/
package fix
Expand Down
5 changes: 4 additions & 1 deletion input/src/main/scala/fix/GlobalImportsOnly.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
/* rules = [OrganizeImports] */
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
*/
package fix

import scala.collection.mutable
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
rules = [OrganizeImports]
OrganizeImports.removeUnused = false
OrganizeImports.groupedImports = AggressiveMerge
*/
package fix
Expand Down
Loading