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

initial support for running rules on Scala 3 sources #210

Merged
merged 2 commits into from
May 12, 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
3 changes: 2 additions & 1 deletion .scalafmt.conf
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
version = "2.7.5"
version = "3.0.0-RC2"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

bumped in a preliminary commit to support the sources with significant indentation in the scripted tests

project.git=true
align.preset=none
assumeStandardLibraryStripMargin = true
runner.dialect = scala3
onTestFailure = "To fix this, run `./bin/scalafmt` from the project base directory"
1 change: 1 addition & 0 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import sbt._

object Dependencies {
val x = List(1) // scalafix:ok
// when bumping remove dep on SNAPSHOT in sbt-1.5 scripted tests
def scalafixVersion: String = "0.9.27"
val all = List(
"org.eclipse.jgit" % "org.eclipse.jgit" % "5.11.0.202103091610-r",
Expand Down
5 changes: 3 additions & 2 deletions src/main/scala/scalafix/internal/sbt/BlockingCache.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ class BlockingCache[K, V] {
// Number of keys is expected to be very small so the global lock should not be a bottleneck
private val underlying = ju.Collections.synchronizedMap(new ju.HashMap[K, V])

/** @param value By-name parameter evaluated when the key if missing. Value computation is guaranteed
* to be called only once per key across all invocations.
/** @param value
* By-name parameter evaluated when the key if missing. Value computation
* is guaranteed to be called only once per key across all invocations.
*/
def getOrElseUpdate(key: K, value: => V): V =
underlying.computeIfAbsent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import java.io.{ByteArrayOutputStream, OutputStream}

import sbt.{Level, Logger}

/** Split an OutputStream into messages and feed them to a given logger at a specified level. Not thread-safe. */
/** Split an OutputStream into messages and feed them to a given logger at a
* specified level. Not thread-safe.
*/
class LoggingOutputStream(
logger: Logger,
level: Level.Value,
Expand Down
23 changes: 11 additions & 12 deletions src/main/scala/scalafix/internal/sbt/SemanticRuleValidator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,23 @@ package scalafix.internal.sbt

import java.nio.file.Path

import sbt.{CrossVersion, ModuleID}
import sbt.ModuleID

import scala.collection.mutable.ListBuffer

class SemanticRuleValidator(ifNotFound: SemanticdbNotFound) {
def findErrors(
files: Seq[Path],
dependencies: Seq[ModuleID],
scalacOpts: Seq[String],
interface: ScalafixInterface
): Seq[String] = {
if (files.isEmpty) Nil
else {
val errors = ListBuffer.empty[String]
val hasSemanticdb =
dependencies.exists(_.name.startsWith("semanticdb-scalac"))
dependencies.exists(_.name.startsWith("semanticdb-scalac")) ||
scalacOpts.contains("-Xsemanticdb")
Copy link
Collaborator Author

@bjaglin bjaglin May 10, 2021

Choose a reason for hiding this comment

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

this will bring a false positive error with early dotty & 3.0.0-M* compilers, but I guess it's fine

Copy link
Contributor

Choose a reason for hiding this comment

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

The goal is to support the final release of scala 3, so I agree it's fine

if (!hasSemanticdb)
errors += ifNotFound.message
val invalidArguments = interface.validate()
Expand All @@ -30,27 +32,24 @@ class SemanticRuleValidator(ifNotFound: SemanticdbNotFound) {

class SemanticdbNotFound(
ruleNames: Seq[String],
scalaVersion: String,
sbtVersion: String
scalaVersion: String
) {
def message: String = {
val names = ruleNames.mkString(", ")

val recommendedSetting = CrossVersion.partialVersion(sbtVersion) match {
case Some((1, n)) if n < 3 => atMostSbt12
case Some((0, _)) => atMostSbt12
case _ => atLeastSbt13(scalaVersion)
}
val recommendedSetting =
if (SemanticdbPlugin.available) semanticdbPluginSetup(scalaVersion)
else manualSetup

s"""|The semanticdb-scalac compiler plugin is required to run semantic rules like $names.
s"""|The scalac compiler should produce semanticdb files to run semantic rules like $names.
|To fix this problem for this sbt shell session, run `scalafixEnable` and try again.
|To fix this problem permanently for your build, add the following settings to build.sbt:
|
|$recommendedSetting
|""".stripMargin
}

private def atLeastSbt13(scalaVersion: String) =
private def semanticdbPluginSetup(scalaVersion: String) =
s"""inThisBuild(
| List(
| scalaVersion := "$scalaVersion",
Expand All @@ -60,7 +59,7 @@ class SemanticdbNotFound(
|)
|""".stripMargin

private val atMostSbt12 =
private val manualSetup =
s"""addCompilerPlugin(scalafixSemanticdb)
|scalacOptions += "-Yrangepos"
|""".stripMargin
Expand Down
19 changes: 19 additions & 0 deletions src/main/scala/scalafix/internal/sbt/SemanticdbPlugin.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package scalafix.internal.sbt

import sbt._

import scala.util.Try

/** Helper to use sbt 1.3+ SemanticdbPlugin features when available */
object SemanticdbPlugin {

// Copied from https://github.com/sbt/sbt/blob/v1.3.0/main/src/main/scala/sbt/Keys.scala#L190-L195
val semanticdbEnabled = settingKey[Boolean]("")
val semanticdbVersion = settingKey[String]("")

lazy val available = Try {
Class.forName("sbt.plugins.SemanticdbPlugin")
true
}.getOrElse(false)

}
87 changes: 69 additions & 18 deletions src/main/scala/scalafix/sbt/ScalafixEnable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package scalafix.sbt
import sbt._
import sbt.Keys._
import sbt.internal.sbtscalafix.Compat
import scalafix.internal.sbt.SemanticdbPlugin

/** Command to automatically enable semanticdb-scalac for shell session */
/** Command to automatically enable semanticdb compiler output for shell session
Copy link
Collaborator Author

@bjaglin bjaglin May 11, 2021

Choose a reason for hiding this comment

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

This class is getting quite complex as it currently handles:

  • different sbt/Zinc versions
    • 1.x with x >= 3 SemanticdbPlugin available
    • 1.x with x < 3 SemanticdbPlugin unavailable
    • 0.13.x SemanticdbPlugin unavailable & forcing usage of Compat
  • different scalac versions and how they may support semanticdb
    • 3.x built-in
    • 2.x with x >= 11 supported by semanticdb-scalac
    • 2.x with x < 11 unsupported by semanticdb-scalac

*/
object ScalafixEnable {

/** sbt 1.0 and 0.13 compatible implementation of partialVersion */
Expand All @@ -13,24 +15,73 @@ object ScalafixEnable {
(a.toLong, b.toLong)
}

lazy val partialToFullScalaVersion: Map[(Long, Long), String] = (for {
/** If the provided Scala binary version is supported, return the full version
* required for running semanticdb-scalac.
*/
private lazy val semanticdbScalacFullScalaVersion
: PartialFunction[(Long, Long), String] = (for {
v <- BuildInfo.supportedScalaVersions
p <- partialVersion(v).toList
} yield p -> v).toMap

def projectsWithMatchingScalaVersion(
state: State
): Seq[(ProjectRef, String)] = {
val extracted = Project.extract(state)
for {
p <- extracted.structure.allProjectRefs
version <- scalaVersion.in(p).get(extracted.structure.data).toList
partialVersion <- partialVersion(version).toList
fullVersion <- partialToFullScalaVersion.get(partialVersion).toList
} yield p -> fullVersion
/** If the provided Scala binary version is supported, return the full version
* required for running semanticdb-scalac or None if support is built-in in
* the compiler and the full version does not need to be adjusted.
*/
private lazy val maybeSemanticdbScalacFullScalaVersion
: PartialFunction[(Long, Long), Option[String]] =
semanticdbScalacFullScalaVersion.andThen(Some.apply).orElse {
// semanticdb is built-in in the Scala 3 compiler
case (major, _) if major == 3 => None
}

/** Collect projects across the entire build, using the partial function
* accepting a Scala binary version
*/
private def collectProjects[U](
extracted: Extracted,
pf: PartialFunction[(Long, Long), U]
): Seq[(ProjectRef, U)] = for {
p <- extracted.structure.allProjectRefs
version <- scalaVersion.in(p).get(extracted.structure.data).toList
partialVersion <- partialVersion(version).toList
res <- pf.lift(partialVersion).toList
} yield p -> res

lazy val command =
if (SemanticdbPlugin.available) withSemanticdbPlugin
else withSemanticdbScalac

private lazy val withSemanticdbPlugin = Command.command(
"scalafixEnable",
briefHelp = "Configure SemanticdbPlugin for scalafix.",
detail = """1. set semanticdbEnabled where supported
|2. conditionally sets semanticdbVersion & scalaVersion when support is not built-in in the compiler""".stripMargin
) { s =>
val extracted = Project.extract(s)
val scalacOptionsSettings = Seq(Compile, Test).flatMap(
inConfig(_)(ScalafixPlugin.relaxScalacOptionsConfigSettings)
)
val settings = for {
(p, maybeFullVersion) <- collectProjects(
extracted,
maybeSemanticdbScalacFullScalaVersion
)
enableSemanticdbPlugin <- maybeFullVersion.toList.flatMap { fullVersion =>
List(
scalaVersion := fullVersion,
SemanticdbPlugin.semanticdbVersion := BuildInfo.scalametaVersion
)
} :+ (SemanticdbPlugin.semanticdbEnabled := true)
settings <-
inScope(ThisScope.in(p))(
scalacOptionsSettings ++ enableSemanticdbPlugin
)
} yield settings
Compat.append(extracted, settings, s)
}

lazy val command = Command.command(
private lazy val withSemanticdbScalac = Command.command(
"scalafixEnable",
briefHelp =
"Configure libraryDependencies, scalaVersion and scalacOptions for scalafix.",
Expand All @@ -46,7 +97,10 @@ object ScalafixEnable {
)
)
val settings: Seq[Setting[_]] = for {
(p, fullVersion) <- projectsWithMatchingScalaVersion(s)
(p, fullVersion) <- collectProjects(
extracted,
semanticdbScalacFullScalaVersion
)
isSemanticdbEnabled =
libraryDependencies
.in(p)
Expand All @@ -62,10 +116,7 @@ object ScalafixEnable {
inScope(ThisScope.in(p))(scalacOptionsSettings) ++
(if (!isSemanticdbEnabled) addSemanticdbCompilerPlugin else List())
} yield settings

val scalafixReady = Compat.append(extracted, settings, s)

scalafixReady
Compat.append(extracted, settings, s)
}

private val semanticdbConfigSettings: Seq[Def.Setting[_]] =
Expand Down
11 changes: 5 additions & 6 deletions src/main/scala/scalafix/sbt/ScalafixPlugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ object ScalafixPlugin extends AutoPlugin {
Arg.PrintStream(errorLogger),
Arg.Config(scalafixConf),
Arg.Rules(shell.rules),
Arg.ScalaVersion(scalaVersion.value),
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 verified that this change is required for the syntactic-only invocation in the scala-3 scripted to successfully parse a source with significant indentation.

Arg.ParsedArgs(shell.extra)
)
val rulesThatWillRun = mainInterface.rulesThatWillRun()
Expand Down Expand Up @@ -429,13 +430,11 @@ object ScalafixPlugin extends AutoPlugin {
Def.taskDyn {
val dependencies = allDependencies.in(config).value
val files = filesToFix(shellArgs, config).value
val withScalaInterface = mainArgs.withArgs(
Arg.ScalaVersion(scalaVersion.value),
Arg.ScalacOptions(scalacOptions.in(config, compile).value)
)
val scalacOpts = scalacOptions.in(config, compile).value
val withScalaInterface = mainArgs.withArgs(Arg.ScalacOptions(scalacOpts))
val errors = new SemanticRuleValidator(
new SemanticdbNotFound(ruleNames, scalaVersion.value, sbtVersion.value)
).findErrors(files, dependencies, withScalaInterface)
new SemanticdbNotFound(ruleNames, scalaVersion.value)
).findErrors(files, dependencies, scalacOpts, withScalaInterface)
if (errors.isEmpty) {
val task = Def.task {
// don't use fullClasspath as it results in a cyclic dependency via compile when scalafixOnCompile := true
Expand Down
1 change: 1 addition & 0 deletions src/sbt-test/sbt-1.5/scala-3/build.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
scalaVersion := "3.0.0-RC3"
1 change: 1 addition & 0 deletions src/sbt-test/sbt-1.5/scala-3/project/build.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sbt.version=1.5.2
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The way scripted are tested against different versions of sbt is quite messy at the moment, but I would rather wait for scalacenter/scalafix#1333 to clean it up.

6 changes: 6 additions & 0 deletions src/sbt-test/sbt-1.5/scala-3/project/plugins.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
resolvers += Resolver.sonatypeRepo("public")
addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % sys.props("plugin.version"))

//FIXME: remove when scalafixVersion is bumped to 0.9.28
resolvers += Resolver.sonatypeRepo("snapshots")
dependencyOverrides += "ch.epfl.scala" % "scalafix-interfaces" % "0.9.27+52-6c9eeec9-SNAPSHOT"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object SignificantIndentation:
implicit class XtensionVal(val str: String) extends AnyVal:
def doubled: String = str + str
4 changes: 4 additions & 0 deletions src/sbt-test/sbt-1.5/scala-3/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# check that we can run a syntactic rule against a Scala 3 dialect source file
-> scalafix --check LeakingImplicitClassVal
> scalafix LeakingImplicitClassVal
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 found a built-in syntactic rewriting rule that works on a scala3 source with significant indentation!

[info] [info] Running scalafix on 1 Scala sources
[info] [error] --- /tmp/sbt_704f6c79/src/main/scala/SignificantIndentation.scala
[info] [error] +++ <expected fix>
[info] [error] @@ -1,3 +1,3 @@
[info] [error]  object SignificantIndentation:
[info] [error] -  implicit class XtensionVal(val str: String) extends AnyVal:
[info] [error] +  implicit class XtensionVal(private val str: String) extends AnyVal:
[info] [error]      def doubled: String = str + str

🎉

> scalafix --check LeakingImplicitClassVal
19 changes: 19 additions & 0 deletions src/sbt-test/sbt-1.5/scalafixEnable/build.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
ThisBuild / scalafixDependencies += "ch.epfl.scala" %% "example-scalafix-rule" % "1.6.0"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This contains a very simple semantic rule so it's good for an initial scripted until built-in rules pick up actual support. The only downside is that the rule does not even evaluate the semantic document, so I am not sure it really tests the generation of semanticdb compiler output... Maybe we could tag a 1.6.1 with https://github.com/scalacenter/sbt-scalafix/pull/125/files#diff-38f3b339536ccc14af957f820b7bfda9be5e1226210bb2d8938d7fab17003911R7 to lift this doubt?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe!


lazy val scala210 = project
.in(file("scala210"))
.settings(
scalaVersion := "2.10.7" // unsupported by semanticdb-scalac
)

lazy val scala211 = project
.in(file("scala211"))
.settings(
scalaVersion := "2.11.12" // supported by semanticdb-scalac
)

lazy val scala3 = project
.in(file("scala3"))
.settings(
scalaVersion := "3.0.0-RC3" // built-in support for semanticdb
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sbt.version=1.5.2
6 changes: 6 additions & 0 deletions src/sbt-test/sbt-1.5/scalafixEnable/project/plugins.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
resolvers += Resolver.sonatypeRepo("public")
addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % sys.props("plugin.version"))

//FIXME: remove when scalafixVersion is bumped to 0.9.28
resolvers += Resolver.sonatypeRepo("snapshots")
dependencyOverrides += "ch.epfl.scala" % "scalafix-interfaces" % "0.9.27+52-6c9eeec9-SNAPSHOT"
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
object Main {
def foo(a: (Int, String)) = a
foo(1, "str")
def main(args: Array[String]) {
println(1)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
object Main {
def foo(a: (Int, String)) = a
foo(1, "str")
def main(args: Array[String]) {
println(1)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
object SignificantIndentation:
val hello = "world"
15 changes: 15 additions & 0 deletions src/sbt-test/sbt-1.5/scalafixEnable/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# enable semanticdb output where supported
> scalafixEnable

# check that projects not supported by semanticdb-scalac can still compile
> scala210/compile

# check that we can run a semantic rule against a Scala 2.11 dialect source file
-> scala211/scalafix --check SemanticRule
> scala211/scalafix SemanticRule
> scala211/scalafix --check SemanticRule

# check that we can run a semantic rule against a Scala 3 dialect source file
-> scala3/scalafix --check SemanticRule
> scala3/scalafix SemanticRule
> scala3/scalafix --check SemanticRule