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

improvement: don't run compileAndLookForNewReferences #6429

Merged
merged 7 commits into from
Jun 4, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,20 @@ trait AdjustLspData {
new Location(location.getUri(), adjustRange(location.getRange()))

def adjustReferencesResult(
referencesResult: pc.ReferencesResult
referencesResult: pc.ReferencesResult,
additionalAdjust: AdjustRange,
text: String,
): ReferencesResult =
new ReferencesResult(
referencesResult.symbol,
referencesResult.locations().asScala.map(adjustLocation).toList,
referencesResult
.locations()
.asScala
.flatMap(loc =>
additionalAdjust(loc, text, referencesResult.symbol)
.map(adjustLocation)
)
.toList,
)

def adjustLocations(
Expand Down
23 changes: 21 additions & 2 deletions metals/src/main/scala/scala/meta/internal/metals/Compilers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ class Compilers(
def references(
params: ReferenceParams,
token: CancelToken,
additionalAdjust: AdjustRange,
): Future[List[ReferencesResult]] = {
withPCAndAdjustLsp(params) { case (pc, pos, adjust) =>
val requestParams = new internal.pc.PcReferencesRequest(
Expand All @@ -742,7 +743,17 @@ class Compilers(
)
pc.references(requestParams)
.asScala
.map(_.asScala.map(adjust.adjustReferencesResult).toList)
.map(
_.asScala
.map(
adjust.adjustReferencesResult(
_,
additionalAdjust,
requestParams.file.text(),
)
)
.toList
)
}
}.getOrElse(Future.successful(Nil))

Expand All @@ -751,6 +762,7 @@ class Compilers(
searchFiles: List[AbsolutePath],
includeDefinition: Boolean,
symbol: String,
additionalAdjust: AdjustRange,
): Future[List[ReferencesResult]] = {
// we filter only Scala files, since `references` for Java are not implemented
val filteredFiles = searchFiles.filter(_.isScala)
Expand All @@ -772,7 +784,14 @@ class Compilers(
compiler
.references(requestParams)
.asScala
.map(_.asScala.map(adjust.adjustReferencesResult).toList)
.map(
_.asScala
.map(
adjust
.adjustReferencesResult(_, additionalAdjust, input.text)
)
.toList
)
}
}
.getOrElse(Nil)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,13 @@ object IdentifierIndex {
id: BuildTargetIdentifier,
bloom: BloomFilter[CharSequence],
)

case class MaybeStaleIndexEntry(
id: BuildTargetIdentifier,
bloom: BloomFilter[CharSequence],
isStale: Boolean,
) {
def asStale: MaybeStaleIndexEntry =
MaybeStaleIndexEntry(id, bloom, isStale = true)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ import scala.meta.internal.parsing.ClassFinder
import scala.meta.internal.parsing.ClassFinderGranularity
import scala.meta.internal.parsing.DocumentSymbolProvider
import scala.meta.internal.parsing.FoldingRangeProvider
import scala.meta.internal.parsing.TokenEditDistance
import scala.meta.internal.parsing.Trees
import scala.meta.internal.rename.RenameProvider
import scala.meta.internal.search.SymbolHierarchyOps
Expand Down Expand Up @@ -870,6 +869,7 @@ abstract class MetalsLspService(
buffers.put(path, change.getText)
diagnostics.didChange(path)
compilers.didChange(path)
referencesProvider.didChange(path, change.getText)
parseTrees(path).asJava
}
}
Expand All @@ -894,7 +894,6 @@ abstract class MetalsLspService(
Future
.sequence(
List(
referencesProvider.indexIdentifiers(path, text),
renameProvider.runSave(),
parseTrees(path),
onChange(List(path)),
Expand Down Expand Up @@ -1112,64 +1111,21 @@ abstract class MetalsLspService(
params: ReferenceParams
): CompletableFuture[util.List[Location]] =
CancelTokens.future { _ =>
referencesResult(params).map(_.flatMap(_.locations).asJava)
referencesResult(params).map(getSortedLocations)
}

// Triggers a cascade compilation and tries to find new references to a given symbol.
// It's not possible to stream reference results so if we find new symbols we notify the
// user to run references again to see updated results.
private def compileAndLookForNewReferences(
params: ReferenceParams,
result: List[ReferencesResult],
): Unit = {
val path = params.getTextDocument.getUri.toAbsolutePath
val old = path.toInputFromBuffers(buffers)
compilations.cascadeCompileFiles(Seq(path)).foreach { _ =>
val newBuffer = path.toInputFromBuffers(buffers)
val newParams: Option[ReferenceParams] =
if (newBuffer.text == old.text) Some(params)
else {
val edit = TokenEditDistance(old, newBuffer, trees)
edit
.getOrElse(TokenEditDistance.NoMatch)
.toRevised(
params.getPosition.getLine,
params.getPosition.getCharacter,
)
.foldResult(
pos => {
params.getPosition.setLine(pos.startLine)
params.getPosition.setCharacter(pos.startColumn)
Some(params)
},
() => Some(params),
() => None,
)
}
newParams match {
case None =>
case Some(p) =>
referencesProvider.references(p).foreach { newResult =>
val diff = newResult
.flatMap(_.locations)
.length - result.flatMap(_.locations).length
val diffSyms: Set[String] =
newResult.map(_.symbol).toSet -- result.map(_.symbol).toSet
if (diffSyms.nonEmpty && diff > 0) {
import scala.meta.internal.semanticdb.Scala._
val names =
diffSyms
.map(sym => s"'${sym.desc.name.value}'")
.mkString(" and ")
val message =
s"Found new symbol references for $names, try running again."
scribe.info(message)
statusBar
.addMessage(clientConfig.icons.info + message)
}
}
private def getSortedLocations(referencesResult: List[ReferencesResult]) =
referencesResult
.flatMap(_.locations)
.groupBy(_.getUri())
.flatMap { case (_, locs) =>
locs.sortWith(sortByLocationPosition).distinct
}
}
.toSeq
.asJava

private def sortByLocationPosition(l1: Location, l2: Location): Boolean = {
l1.getRange.getStart.getLine < l2.getRange.getStart.getLine
}

def referencesResult(
Expand All @@ -1188,9 +1144,6 @@ abstract class MetalsLspService(
)
}
}
if (results.nonEmpty) {
compileAndLookForNewReferences(params, results)
}
results
}
}
Expand Down Expand Up @@ -1752,7 +1705,7 @@ abstract class MetalsLspService(
} else {
Future.successful(
DefinitionResult(
locations = results.flatMap(_.locations).asJava,
locations = getSortedLocations(results),
symbol = results.head.symbol,
definition = None,
semanticdb = None,
Expand Down
Loading