Skip to content

Commit

Permalink
feature: Add support for Scala 3's Best Effort compilation (#5219)
Browse files Browse the repository at this point in the history
* feat: Add Best Effort compilation support for Scala 3

This PR is meant to add support for Best Effort compilation in metals,
with a few additional considerations:
* Some unnecessary options are replaced from bloop and set for PC
* Previously, any project could be run from metals if semanticdb was
able to be generated. This caused a somewhat weird UX for best effort.
Now the build directory is explicitly checked for the existence of non
META-INF files.
* Previously, in metals doctor, compilation would be set as correct if
a compilation did not produce any errors. However, with best effort
compilation, the compiler may not find any additional errors, beyond
the ones from its dependencies (meaning that a project that cannot
actually be compiled, could show up as on that was compiling). A note
about that was added in the doctor.

* Setup previous outline tests so they are alos run for best effort

* Address review comments

Now we check diagnostics of all transitive dependencies for errors,
instead of checking the existence of non meta-inf files.

Also the keep-after-error test now does not check for remaining code
lens after saving.
  • Loading branch information
jchyb authored Jul 18, 2024
1 parent 0ee74e0 commit ab9fd79
Show file tree
Hide file tree
Showing 15 changed files with 805 additions and 356 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,7 @@ object BuildServerConnection {
javaSemanticdbVersion: String,
semanticdbVersion: String,
supportedScalaVersions: java.util.List[String],
enableBestEffortMode: Boolean,
)

/**
Expand All @@ -709,6 +710,7 @@ object BuildServerConnection {
BuildInfo.javaSemanticdbVersion,
BuildInfo.scalametaVersion,
BuildInfo.supportedScala2Versions.asJava,
true,
)

val capabilities = new BuildClientCapabilities(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ final class Compilations(
) {
params.setArguments(List("--verbose").asJava)
}
params.setArguments(List("--best-effort").asJava)
targets.foreach { target =>
isCompiling(target) = true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,34 @@ class CompilerConfiguration(
protected def newCompiler(classpath: Seq[Path]): PresentationCompiler = {
val name = scalaTarget.scalac.getTarget().getUri
val options = enrichWithReleaseOption(scalaTarget)
// Best Effort option `-Ybest-effort` is useless for PC,
// and it may unnecesarily dump semanticdb and tasty files
val bestEffortOpt = "-Ybest-effort"
val withBetastyOpt = "-Ywith-best-effort-tasty"
val nonBestEffortOptions =
if (scalaTarget.isBestEffort)
options
.filter(_ != bestEffortOpt)
.filter(_ != withBetastyOpt) :+ withBetastyOpt
else options

val bestEffortDirs = scalaTarget.info
.getDependencies()
.asScala
.flatMap { buildId =>
if (scalaTarget.isBestEffort)
buildTargets.scalaTarget(buildId).map(_.bestEffortPath)
else None
}
.toSeq
val selfBestEffortDir =
if (scalaTarget.isBestEffort) Seq(scalaTarget.bestEffortPath)
else Seq.empty

fromMtags(
mtags,
options,
classpath ++ additionalClasspath,
nonBestEffortOptions,
classpath ++ additionalClasspath ++ bestEffortDirs ++ selfBestEffortDir,
name,
search,
referenceCounter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,11 @@ class Compilers(

def didCompile(report: CompileReport): Unit = {
val isSuccessful = report.getErrors == 0
val isBestEffortCompilation =
buildTargets
.scalaTarget(report.getTarget)
.map(_.isBestEffort)
.getOrElse(false)
buildTargetPCFromCache(report.getTarget).foreach { pc =>
if (
outlineFilesProvider.shouldRestartPc(
Expand All @@ -276,10 +281,9 @@ class Compilers(

outlineFilesProvider.onDidCompile(report.getTarget(), isSuccessful)

if (isSuccessful) {
if (isSuccessful || isBestEffortCompilation) {
// Restart PC for all build targets that depend on this target since the classfiles
// may have changed.

for {
target <- buildTargets.allInverseDependencies(report.getTarget)
if target != report.getTarget
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ object Directories {
RelativePath(".metals").resolve("metals.log")
def semanticdb: RelativePath =
RelativePath("META-INF").resolve("semanticdb")
def bestEffort: RelativePath =
RelativePath("META-INF").resolve("best-effort")
def pc: RelativePath =
RelativePath(".metals").resolve("pc.log")
def workspaceSymbol: RelativePath =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ abstract class MetalsLspService(
() => userConfig,
trees,
folder,
diagnostics,
)
val goSuperLensProvider = new SuperMethodCodeLens(
buffers,
Expand Down
17 changes: 17 additions & 0 deletions metals/src/main/scala/scala/meta/internal/metals/ScalaTarget.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package scala.meta.internal.metals

import scala.util.Success
import scala.util.Try

import scala.meta.Dialect
import scala.meta.dialects._
import scala.meta.internal.builds.BazelBuildTool
Expand Down Expand Up @@ -111,6 +114,20 @@ case class ScalaTarget(
else
Some(scalac.getClasspath().asScala.toList)

def bestEffortPath: java.nio.file.Path =
targetroot.resolve(Directories.bestEffort).toNIO

def isBestEffort: Boolean = {
val minVersion = SemVer.Version.fromString("3.5.0")
Try(SemVer.Version.fromString(scalaVersion)) match {
case Success(version) =>
// we compare only major and minor, as we still want RCs and nightlys to work as well
version.major >= minVersion.major &&
version.minor >= minVersion.minor
case _ => false
}
}

def classDirectory: String = scalac.getClassDirectory()

def scalaVersion: String = scalaInfo.getScalaVersion()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import scala.meta.internal.metals.BuildTargets
import scala.meta.internal.metals.ClientCommands.StartDebugSession
import scala.meta.internal.metals.ClientCommands.StartRunSession
import scala.meta.internal.metals.ClientConfiguration
import scala.meta.internal.metals.Diagnostics
import scala.meta.internal.metals.JavaBinary
import scala.meta.internal.metals.JsonParser._
import scala.meta.internal.metals.MetalsEnrichments._
Expand Down Expand Up @@ -54,6 +55,7 @@ final class RunTestCodeLens(
userConfig: () => UserConfiguration,
trees: Trees,
workspace: AbsolutePath,
diagnostics: Diagnostics,
)(implicit val ec: ExecutionContext)
extends CodeLens {

Expand Down Expand Up @@ -84,6 +86,7 @@ final class RunTestCodeLens(
val distance = buffers.tokenEditDistance(path, textDocument.text, trees)
val lenses = for {
buildTargetId <- buildTargets.inverseSources(path)
if canActuallyCompile(buildTargetId, diagnostics)
buildTarget <- buildTargets.info(buildTargetId)
isJVM = buildTarget.asScalaBuildTarget.forall(
_.getPlatform == b.ScalaPlatform.JVM
Expand Down Expand Up @@ -112,12 +115,30 @@ final class RunTestCodeLens(
path,
isJVM,
)

}

lenses.getOrElse(Future.successful(Nil))
}

/**
* Checks if there exist any files outside of the META-INF directory of build target classpath.
*
* When using Scala 3 Best Effort compilation, the compilation may fail,
* but semanticDB and betasty (and thus the META_INF directory) may still be produced.
* We want to avoid creating run/test/debug code lens in those situations.
*/
private def canActuallyCompile(
buildTargetId: BuildTargetIdentifier,
diagnostics: Diagnostics,
): Boolean = {
val allBuildTargetsIds: Seq[BuildTargetIdentifier] =
buildTargetId +: (buildTargets
.buildTargetTransitiveDependencies(buildTargetId)
.toSeq)
allBuildTargetsIds.forall { id =>
!diagnostics.hasCompilationErrors(id)
}
}

/**
* Java main method: public static void main(String[] args)
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,8 +544,23 @@ final class Doctor(
): DoctorStatus = {
if (diagnostics.hasCompilationErrors(targetId))
DoctorStatus.error
else
DoctorStatus.check
else {
val scalaTargetMaybe = buildTargets.scalaTarget(targetId)

val dependenciesCompile =
scalaTargetMaybe.flatMap {
_.info.getDependencies().asScala.collectFirst { buildId =>
diagnostics.hasCompilationErrors(buildId)
}
}.isEmpty

if (
scalaTargetMaybe
.map(_.isBestEffort)
.getOrElse(false) && !dependenciesCompile
) DoctorStatus.alert
else DoctorStatus.check
}
}

private def extractJavaInfo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ object DoctorExplanation {
val correctMessage: String =
s"${Icons.unicode.check} - code is compiling"
val incorrectMessage: String =
s"${Icons.unicode.error} - code isn't compiling, open problems tab to see detailed compilation errors"
s"""|${Icons.unicode.error} - code isn't compiling, open problems tab to see detailed compilation errors
|${Icons.unicode.alert} - code compiles in the "best effort" mode while depending on errored projects""".stripMargin

def show(allTargetsInfo: Seq[DoctorTargetInfo]): Boolean =
allTargetsInfo.exists(_.compilationStatus.isCorrect == false)
Expand Down
Loading

0 comments on commit ab9fd79

Please sign in to comment.