Skip to content

Commit

Permalink
[Scalafix] Report problems with invalid path and fix flaky behaviour
Browse files Browse the repository at this point in the history
Two things are done in this PR:
- make sure that if the custom configuration file doesn't exist that we will properly report it to the user
- make sure that Scalafix will always pick up the semanticdb file from the targetroot

The second problem popped up in cases that semanticdb started being written to a different directory (`/target/scala-2.13/meta/`), which meant that Scalafix could pick up the old stale files from .bloop directory.

The change most likely came from sbt, which might have started adding it as the default, but I am not entirely sure why.
  • Loading branch information
tgodzik committed Apr 30, 2021
1 parent 5c96ad2 commit 318d523
Showing 1 changed file with 37 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,8 @@ case class ScalafixProvider(
val tmp = AbsolutePath(Files.createTempFile("metals", ".scala"))
tmp.writeText("object Main{}\n")
for (target <- targets)
scalafixEvaluate(
tmp,
target.scalaVersion,
target.scalaBinaryVersion,
target.fullClasspath
)
scalafixEvaluate(tmp, target)

tmp.delete()
} catch {
case e: Throwable =>
Expand All @@ -79,13 +75,7 @@ case class ScalafixProvider(
Future.successful(Nil)
} else {
compilations.compilationFinished(file).flatMap { _ =>
val scalaBinaryVersion = scalaTarget.scalaBinaryVersion
val scalafixEvaluation = scalafixEvaluate(
file,
scalaTarget.scalaVersion,
scalaBinaryVersion,
scalaTarget.fullClasspath
)
val scalafixEvaluation = scalafixEvaluate(file, scalaTarget)

scalafixEvaluation match {
case Failure(exception) =>
Expand Down Expand Up @@ -132,18 +122,44 @@ case class ScalafixProvider(
}

private def scalafixConf: Option[Path] = {
val scalafixConfPath = userConfig().scalafixConfigPath
.getOrElse(workspace.resolve(".scalafix.conf"))
if (scalafixConfPath.isFile) Some(scalafixConfPath.toNIO)
else None
val defaultLocation = workspace.resolve(".scalafix.conf")
userConfig().scalafixConfigPath match {
case Some(path) if !path.isFile && defaultLocation.isFile =>
languageClient.showMessage(
MessageType.Warning,
s"No configuration at $path, using default at $defaultLocation."
)
Some(defaultLocation.toNIO)
case Some(path) if !path.isFile =>
languageClient.showMessage(
MessageType.Warning,
s"No configuration at $path, using Scalafix defaults."
)
None
case Some(path) => Some(path.toNIO)
case None if defaultLocation.isFile =>
Some(defaultLocation.toNIO)
case _ => None
}
}

private def scalafixEvaluate(
file: AbsolutePath,
scalaVersion: String,
scalaBinaryVersion: String,
fullClasspath: java.util.List[Path]
scalaTarget: ScalaTarget
): Try[ScalafixEvaluation] = {
val scalaBinaryVersion = scalaTarget.scalaBinaryVersion

val targetRoot =
buildTargets.scalacOptions(scalaTarget.info.getId()).map {
scalacOptions =>
scalacOptions.targetroot(scalaTarget.scalaVersion).toNIO
}
val scalaVersion = scalaTarget.scalaVersion
// It seems that Scalafix ignores the targetroot parameter and searches the classpath
// Prepend targetroot to make sure that it's picked up first always
val classpath =
(targetRoot.toList ++ scalaTarget.fullClasspath.asScala).asJava

for {
api <- getScalafix(scalaBinaryVersion)
urlClassLoaderWithExternalRule <- getRuleClassLoader(
Expand All @@ -158,7 +174,7 @@ case class ScalafixProvider(
api
.newArguments()
.withScalaVersion(scalaVersion)
.withClasspath(fullClasspath)
.withClasspath(classpath)
.withToolClasspath(urlClassLoaderWithExternalRule)
.withConfig(scalafixConf.asJava)
.withRules(List(organizeImportRuleName).asJava)
Expand Down

0 comments on commit 318d523

Please sign in to comment.