Skip to content

Commit

Permalink
compile on each build target change
Browse files Browse the repository at this point in the history
  • Loading branch information
kasiaMarek authored and tgodzik committed Jan 30, 2024
1 parent 12a5d99 commit 0ea507c
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,12 @@ final class Compilations(

private val isCompiling = TrieMap.empty[b.BuildTargetIdentifier, Boolean]
private var lastCompile: collection.Set[b.BuildTargetIdentifier] = Set.empty
private val wasCompiled = TrieMap.empty[b.BuildTargetIdentifier, Boolean]

def wasEverCompiled(buildTarget: b.BuildTargetIdentifier): Boolean =
buildTargets.isInverseDependency(buildTarget, wasCompiled.keySet.toList)

def currentlyCompiling: Iterable[b.BuildTargetIdentifier] = isCompiling.keys
def isCurrentlyCompiling(buildTarget: b.BuildTargetIdentifier): Boolean =
isCompiling.contains(buildTarget)

def previouslyCompiled: Iterable[b.BuildTargetIdentifier] = lastCompile
def wasPreviouslyCompiled(buildTarget: b.BuildTargetIdentifier): Boolean =
lastCompile.contains(buildTarget)

def compilationFinished(targets: Seq[BuildTargetIdentifier]): Future[Unit] =
if (currentlyCompiling.isEmpty) {
Expand Down Expand Up @@ -261,7 +255,6 @@ final class Compilations(
}
targets.foreach { target =>
isCompiling(target) = true
wasCompiled(target) = true
}
val compilation = connection.compile(params, timeout)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,11 @@ class MetalsLspService(
// In some cases like peeking definition didOpen might be followed up by close
// and we would lose the notion of the focused document
recentlyOpenedFiles.add(path)
val prevBuildTarget = focusedDocumentBuildTarget.getAndUpdate { current =>
buildTargets
.inverseSources(path)
.getOrElse(current)
}

// Update md5 fingerprint from file contents on disk
fingerprints.add(path, FileIO.slurp(path, charset))
Expand Down Expand Up @@ -1089,7 +1094,7 @@ class MetalsLspService(
Future
.sequence(
List(
maybeCompileOnDidFocus(path),
maybeCompileOnDidFocus(path, prevBuildTarget),
compilers.load(List(path)),
publishSynthetics0,
)
Expand Down Expand Up @@ -1128,9 +1133,11 @@ class MetalsLspService(
uri: String
): CompletableFuture[DidFocusResult.Value] = {
val path = uri.toAbsolutePath
buildTargets
.inverseSources(path)
.foreach(focusedDocumentBuildTarget.set)
val prevBuildTarget = focusedDocumentBuildTarget.getAndUpdate { current =>
buildTargets
.inverseSources(path)
.getOrElse(current)
}
// Don't trigger compilation on didFocus events under cascade compilation
// because save events already trigger compile in inverse dependencies.
if (path.isDependencySource(folder)) {
Expand All @@ -1141,45 +1148,28 @@ class MetalsLspService(
} else {
publishSynthetics(path)
worksheetProvider.onDidFocus(path)
maybeCompileOnDidFocus(path).asJava
maybeCompileOnDidFocus(path, prevBuildTarget).asJava
}
}

private def maybeCompileOnDidFocus(path: AbsolutePath) = {
private def maybeCompileOnDidFocus(
path: AbsolutePath,
prevBuildTarget: b.BuildTargetIdentifier,
) =
buildTargets.inverseSources(path) match {
case Some(target) =>
val wasNeverCompiled = !compilations.wasEverCompiled(target)
def isAffectedByCurrentCompilation =
!compilations.isCurrentlyCompiling(target) && path.isWorksheet ||
buildTargets.isInverseDependency(
target,
compilations.currentlyCompiling.toList,
)

def isAffectedByLastCompilation: Boolean =
!compilations.wasPreviouslyCompiled(target) &&
buildTargets.isInverseDependency(
target,
compilations.previouslyCompiled.toList,
)

val needsCompile =
wasNeverCompiled || isAffectedByCurrentCompilation || isAffectedByLastCompilation
if (needsCompile) {
compilations
.compileFile(path)
.map(_ => DidFocusResult.Compiled)
} else {
Future.successful(DidFocusResult.AlreadyCompiled)
}
case None if path.isWorksheet =>
case Some(target) if prevBuildTarget != target =>
compilations
.compileFile(path)
.map(_ => DidFocusResult.Compiled)
case _ if path.isWorksheet =>
compilations
.compileFile(path)
.map(_ => DidFocusResult.Compiled)
case Some(_) =>
Future.successful(DidFocusResult.AlreadyCompiled)
case None =>
Future.successful(DidFocusResult.NoBuildTarget)
}
}

def pause(): Unit = pauseables.pause()

Expand Down
20 changes: 5 additions & 15 deletions tests/unit/src/test/scala/tests/DidFocusLspSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ class DidFocusLspSuite extends BaseLspSuite("did-focus") {
_ = assertNoDiagnostics()
_ = fakeTime.elapseSeconds(10)
didCompile <- server.didFocus("a/src/main/scala/a/A2.scala")
_ = assert(didCompile == AlreadyCompiled)
_ = assert(didCompile == Compiled)
didCompile <- server.didFocus("b/src/main/scala/b/B.scala")
_ = assert(didCompile == AlreadyCompiled)
_ = assert(didCompile == Compiled)
didCompile <- server.didFocus("c/src/main/scala/c/C.scala")
// fake delete the diagnostic to see that `c` won't get recompiled
_ = client.diagnostics(server.toPath("c/src/main/scala/c/C.scala")) =
Expand All @@ -64,7 +64,7 @@ class DidFocusLspSuite extends BaseLspSuite("did-focus") {
_ = fakeTime.elapseSeconds(10)
_ = assertNoDiagnostics()
didCompile <- server.didFocus("a/src/main/scala/a/A2.scala")
_ = assert(didCompile == AlreadyCompiled)
_ = assert(didCompile == Compiled)
didCompile <- server.didFocus("b/src/main/scala/b/B.scala")
_ = assert(didCompile == Compiled)
_ = assertNoDiff(
Expand Down Expand Up @@ -148,7 +148,7 @@ class DidFocusWhileCompilingLspSuite
}

test(
"Trigger compilation by didFocus when current compile may affect focused buffer"
"Trigger compilation when current compile may affect the focused buffer"
) {
cleanWorkspace()
for {
Expand Down Expand Up @@ -199,19 +199,9 @@ class DidFocusWhileCompilingLspSuite
client.workspaceDiagnostics,
xMismatch,
)
didSaveA = server.didSave("a/src/main/scala/a/A.scala")(
_ <- server.didSave("a/src/main/scala/a/A.scala")(
_.replace("Int", "String")
)
// Wait until compilation against project a is started (before we invoke didFocus on project b)
_ <- server.waitFor(compileDelayMillis / 2)
// Focus before compilation of A.scala is complete.
// And make sure didFocus during the compilation causes compilation against project b.
didCompile <- server.didFocus("b/src/main/scala/b/B.scala")
_ <- didSaveA
_ = assert(
didCompile == Compiled,
s"expect 'Compiled', actual: ${didCompile}",
)
_ = assertNoDiff(
client.workspaceDiagnostics,
"""|b/src/main/scala/b/B.scala:3:16: error: type mismatch;
Expand Down

0 comments on commit 0ea507c

Please sign in to comment.