diff --git a/app/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java b/app/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java index 64bc5de9fa..449386aebd 100755 --- a/app/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java +++ b/app/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java @@ -44,23 +44,22 @@ import com.itsaky.androidide.models.SearchResult; import com.itsaky.androidide.tasks.TaskExecutor; import com.itsaky.androidide.ui.CodeEditorView; -import com.itsaky.androidide.utils.FileIOUtils; import com.itsaky.androidide.utils.FileUtils; import com.itsaky.androidide.utils.FlashbarActivityUtilsKt; import com.itsaky.androidide.utils.FlashbarUtilsKt; import com.itsaky.androidide.utils.LSPUtils; import io.github.rosemoe.sora.lang.diagnostic.DiagnosticsContainer; -import io.github.rosemoe.sora.text.Content; import java.io.File; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import kotlin.Unit; import org.slf4j.Logger; @@ -107,6 +106,9 @@ public static void shutdown() { private final Map> diagnostics = new HashMap<>(); + /** Identifies the most recent {@link #showLocations(List)} request; older ones must not publish. */ + private final AtomicInteger showLocationsRequest = new AtomicInteger(); + protected EditorHandlerActivity activity; private IDELanguageClientImpl(EditorHandlerActivity provider) { @@ -271,56 +273,74 @@ public void showLocations(List locations) { return; } - boolean error = locations == null || locations.isEmpty(); - activity.handleSearchResultVisibility(error); + // Claims the panel for this request. The publish below is asynchronous, so without this a slow + // request that started first would land last and overwrite the newer search the user is looking at. + final int request = showLocationsRequest.incrementAndGet(); + boolean error = locations == null || locations.isEmpty(); if (error) { + activity.handleSearchResultVisibility(true); activity .setSearchResultAdapter( new SearchListAdapter(Collections.emptyMap(), this::noOp, this::noOp)); return; } - final Map> results = new HashMap<>(); - for (int i = 0; i < locations.size(); i++) { - try { - final Location loc = locations.get(i); - if (loc == null) { - continue; - } + // Group by file first. Reads then cost one pass per file instead of one full read per hit, which + // is what this used to do - and it did it on this thread. See SearchResultGrouping. + final Map> byFile = new LinkedHashMap<>(); + for (final Location loc : locations) { + if (loc == null) { + continue; + } + byFile.computeIfAbsent(loc.getFile().toFile(), f -> new ArrayList<>()).add(loc); + } - final File file = loc.getFile().toFile(); - if (!file.exists() || !file.isFile()) { - continue; + // A file with an open editor is resolved here, on the UI thread: its Content is live UI state + // that a background thread must not touch, and pulling a few lines out of it is substring work + // with no I/O. Everything else is read off this thread below. + final Map> fromEditors = new HashMap<>(); + final Map> onDisk = new LinkedHashMap<>(); + for (final Map.Entry> entry : byFile.entrySet()) { + final var frag = findEditorByFile(entry.getKey()); + if (frag != null && frag.getEditor() != null) { + final List rows = SearchResultGrouping.INSTANCE.resultsFor( + entry.getKey(), entry.getValue(), frag.getEditor().getText()); + if (!rows.isEmpty()) { + fromEditors.put(entry.getKey(), rows); } - var frag = findEditorByFile(file); - Content content; - if (frag != null && frag.getEditor() != null) { - content = frag.getEditor().getText(); - } else { - content = new Content(FileIOUtils.readFile2String(file)); - } - final List matches = results.containsKey(file) ? results.get(file) : new ArrayList<>(); - Objects.requireNonNull(matches) - .add( - new SearchResult( - loc.getRange(), - file, - content.getLineString(loc.getRange().getStart().getLine()), - content - .subContent( - loc.getRange().getStart().getLine(), - loc.getRange().getStart().getColumn(), - loc.getRange().getEnd().getLine(), - loc.getRange().getEnd().getColumn()) - .toString())); - results.put(file, matches); - } catch (Throwable th) { - LOG.error("Failed to show file location", th); + } else { + onDisk.put(entry.getKey(), entry.getValue()); } } - activity.handleSearchResults(results); + if (onDisk.isEmpty()) { + publishLocations(fromEditors); + return; + } + + // Some other search may publish (and bump the generation) while the read is in flight; capture it + // here so this request does not overwrite whatever replaced it. + final int generation = activity.getEditorViewModel().getCurrentSearchGeneration(); + + TaskExecutor.executeAsyncProvideError( + () -> SearchResultGrouping.INSTANCE.readFromDisk(onDisk), + (result, throwable) -> { + if (!canUseActivity() + || request != showLocationsRequest.get() + || generation != activity.getEditorViewModel().getCurrentSearchGeneration()) { + // Superseded, or the activity went away. Leave the panel to whoever owns it now: this + // request's results would be an answer to a question no longer on screen. + return; + } + final Map> merged = new HashMap<>(fromEditors); + if (result != null) { + merged.putAll(result); + } else { + LOG.error("Failed to read search result files", throwable); + } + publishLocations(merged); + }); } private Boolean applyActionEdits(@Nullable final IDEEditor editor, final CodeActionItem action) { @@ -476,4 +496,14 @@ private List mapAsGroup(Map> map) { private Unit noOp(final Object obj) { return Unit.INSTANCE; } + + /** + * Shows {@code results} in the search panel. + * + * Visibility and rows are committed together: a publish that never happens - superseded, or the activity recreated mid-read - must not leave the panel open with the "no results" placeholder hidden over the previous query's rows. + */ + private void publishLocations(final Map> results) { + activity.handleSearchResultVisibility(results.isEmpty()); + activity.handleSearchResults(results); + } } diff --git a/app/src/main/java/com/itsaky/androidide/lsp/SearchResultGrouping.kt b/app/src/main/java/com/itsaky/androidide/lsp/SearchResultGrouping.kt new file mode 100644 index 0000000000..797113bf90 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/lsp/SearchResultGrouping.kt @@ -0,0 +1,146 @@ +package com.itsaky.androidide.lsp + +import com.itsaky.androidide.models.Location +import com.itsaky.androidide.models.Range +import com.itsaky.androidide.models.SearchResult +import io.github.rosemoe.sora.text.Content +import org.slf4j.LoggerFactory +import java.io.BufferedReader +import java.io.File + +/** + * Builds the search-results panel's rows for a set of [Location]s. + * + * Exists because the panel used to read every result file **in full, once per hit, on the main + * thread**: a file with twelve usages was read and materialised twelve times. Find usages made that a + * real cost rather than a latent one. + * + * A row needs only two short strings per hit - the hit's line, and the matched text - so nothing here + * retains a file's contents. Reads are one sequential pass per file, and peak memory is one line rather + * than one file. A per-file content cache would fix the repeated reads but hold every result file's text + * at once, which is the wrong trade on a phone. + */ +internal object SearchResultGrouping { + private val logger = LoggerFactory.getLogger(SearchResultGrouping::class.java) + + /** + * Rows for [locations] in [file], built from already-available [lines] (0-based line number to text). + * + * A location whose lines are not all present is dropped: a stale location can point past the end of + * a file that has since been edited, and a row referring to a line that no longer exists is worse + * than no row. + */ + fun resultsFor( + file: File, + locations: List, + lines: Map, + ): List = + locations.mapNotNull { location -> + val range = location.range + val lineText = lines[range.start.line] ?: return@mapNotNull null + val match = matchedText(lineText, range, lines) + if (match == null) { + logger.debug("Dropping stale search result in {}", file.name) + return@mapNotNull null + } + SearchResult(range, file, lineText, match) + } + + /** Rows for [locations] in [file], read from the live editor buffer [content]. */ + fun resultsFor( + file: File, + locations: List, + content: Content, + ): List { + val lines = + linesNeededBy(locations) + .filter { it >= 0 && it < content.lineCount } + .associateWith { content.getLineString(it) } + + return resultsFor(file, locations, lines) + } + + /** Rows for every file in [byFile], reading each file exactly once. */ + fun readFromDisk(byFile: Map>): Map> = + byFile + .mapValues { (file, locations) -> resultsFor(file, locations, readLines(file, linesNeededBy(locations))) } + .filterValues { it.isNotEmpty() } + + /** Every 0-based line number whose text [locations] need. */ + fun linesNeededBy(locations: List): Set = + locations + .flatMapTo(mutableSetOf()) { location -> + location.range.start.line..location.range.end.line + } + + /** + * The text of just the [wanted] lines of [file], in one sequential pass. + * + * Stops as soon as the last wanted line has been seen, and never holds more than the current line, + * so a hit near the top of a large file does not read the rest of it. Missing lines - a file shorter + * than the location claims, or an unreadable file - are simply absent from the result. + */ + fun readLines( + file: File, + wanted: Set, + ): Map { + if (wanted.isEmpty()) { + return emptyMap() + } + + val last = wanted.max() + val lines = HashMap(wanted.size) + return try { + file.bufferedReader().use { reader -> + reader.collectLines(wanted, last, lines) + } + lines + } catch (e: Exception) { + // A result file that has been deleted or is unreadable drops its rows, which is what the + // previous implementation did too by way of an exists() check per hit. + logger.debug("Could not read search result file {}", file, e) + lines + } + } + + private fun BufferedReader.collectLines( + wanted: Set, + last: Int, + into: MutableMap, + ) { + var number = 0 + while (number <= last) { + val line = readLine() ?: return + if (number in wanted) { + into[number] = line + } + number++ + } + } + + /** + * The text [range] covers, given [firstLine] (the text of the line it starts on) and [lines] for the + * rest. Null when any line it spans is missing. + */ + private fun matchedText( + firstLine: String, + range: Range, + lines: Map, + ): String? { + val start = range.start + val end = range.end + if (start.line == end.line) { + val from = start.column.coerceIn(0, firstLine.length) + return firstLine.substring(from, end.column.coerceIn(from, firstLine.length)) + } + + return buildString { + append(firstLine.substring(start.column.coerceIn(0, firstLine.length))) + for (line in (start.line + 1) until end.line) { + append('\n').append(lines[line] ?: return null) + } + val lastLine = lines[end.line] ?: return null + append('\n').append(lastLine.substring(0, end.column.coerceIn(0, lastLine.length))) + } + } +} diff --git a/app/src/test/java/com/itsaky/androidide/lsp/SearchResultGroupingTest.kt b/app/src/test/java/com/itsaky/androidide/lsp/SearchResultGroupingTest.kt new file mode 100644 index 0000000000..d047e7c876 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/lsp/SearchResultGroupingTest.kt @@ -0,0 +1,154 @@ +package com.itsaky.androidide.lsp + +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.models.Location +import com.itsaky.androidide.models.Position +import com.itsaky.androidide.models.Range +import io.github.rosemoe.sora.text.Content +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File + +/** + * The panel used to read each result file in full, once per hit, on the main thread. These pin the + * replacement: one pass per file, only the lines a hit needs, and stale hits dropped rather than + * throwing. + */ +class SearchResultGroupingTest { + @get:Rule + val folder = TemporaryFolder() + + private fun location( + file: File, + startLine: Int, + startColumn: Int, + endLine: Int = startLine, + endColumn: Int = startColumn, + ) = Location( + file.toPath(), + Range(Position(startLine, startColumn, 0), Position(endLine, endColumn, 0)), + ) + + @Test + fun `a single-line hit carries its line and the matched text`() { + val file = File("Example.kt") + val lines = mapOf(1 to "fun caller() { target() }") + + val results = SearchResultGrouping.resultsFor(file, listOf(location(file, 1, 15, 1, 21)), lines) + + assertThat(results).hasSize(1) + assertThat(results[0].line).isEqualTo("fun caller() { target() }") + assertThat(results[0].match).isEqualTo("target") + assertThat(results[0].file).isEqualTo(file) + } + + @Test + fun `a multi-line hit joins the lines it spans`() { + val file = File("Multi.kt") + val lines = mapOf(0 to "first line", 1 to "middle", 2 to "last line") + + val results = SearchResultGrouping.resultsFor(file, listOf(location(file, 0, 6, 2, 4)), lines) + + assertThat(results).hasSize(1) + assertThat(results[0].match).isEqualTo("line\nmiddle\nlast") + // The row's line text is the line the hit starts on. + assertThat(results[0].line).isEqualTo("first line") + } + + @Test + fun `a hit on a line that no longer exists is dropped`() { + val file = File("Stale.kt") + + val results = SearchResultGrouping.resultsFor(file, listOf(location(file, 9, 0, 9, 3)), mapOf(0 to "only line")) + + assertThat(results).isEmpty() + } + + @Test + fun `a column past the end of its line is clamped rather than throwing`() { + val file = File("Clamped.kt") + + val results = SearchResultGrouping.resultsFor(file, listOf(location(file, 0, 2, 0, 99)), mapOf(0 to "short")) + + assertThat(results).hasSize(1) + assertThat(results[0].match).isEqualTo("ort") + } + + @Test + fun `an open file's rows come from its buffer, not its saved bytes`() { + val file = folder.newFile("Buffered.kt") + file.writeText("saved text\n") + + val results = + SearchResultGrouping.resultsFor(file, listOf(location(file, 0, 4, 0, 10)), Content("fun target() {}")) + + assertThat(results).hasSize(1) + assertThat(results[0].line).isEqualTo("fun target() {}") + assertThat(results[0].match).isEqualTo("target") + } + + @Test + fun `a hit past the end of the buffer is dropped`() { + // The Content overload filters out-of-range lines itself, before the shared row builder sees them. + val file = File("StaleBuffer.kt") + + val results = SearchResultGrouping.resultsFor(file, listOf(location(file, 1, 0, 1, 3)), Content("only")) + + assertThat(results).isEmpty() + } + + @Test + fun `only the lines a hit needs are collected`() { + val file = folder.newFile("Wanted.kt") + file.writeText("zero\none\ntwo\nthree\nfour\n") + + assertThat(SearchResultGrouping.readLines(file, setOf(1, 3))) + .isEqualTo(mapOf(1 to "one", 3 to "three")) + } + + @Test + fun `lines past the end of the file are absent rather than failing`() { + val file = folder.newFile("Short.kt") + file.writeText("only\n") + + assertThat(SearchResultGrouping.readLines(file, setOf(0, 7))).isEqualTo(mapOf(0 to "only")) + } + + @Test + fun `an unreadable file yields no lines rather than throwing`() { + val missing = File(folder.root, "Absent.kt") + + assertThat(SearchResultGrouping.readLines(missing, setOf(0))).isEmpty() + } + + @Test + fun `every hit in a file is built from one read`() { + val file = folder.newFile("Several.kt") + file.writeText("fun a() { target() }\nfun b() { target() }\n") + + val results = + SearchResultGrouping.readFromDisk( + mapOf(file to listOf(location(file, 0, 10, 0, 16), location(file, 1, 10, 1, 16))), + ) + + assertThat(results.keys).containsExactly(file) + assertThat(results.getValue(file).map { it.match }).containsExactly("target", "target") + } + + @Test + fun `a file whose every hit is stale is omitted entirely`() { + val file = folder.newFile("AllStale.kt") + file.writeText("one line\n") + + assertThat(SearchResultGrouping.readFromDisk(mapOf(file to listOf(location(file, 40, 0, 40, 2))))).isEmpty() + } + + @Test + fun `linesNeededBy covers every line a hit spans`() { + val file = File("Spans.kt") + + assertThat(SearchResultGrouping.linesNeededBy(listOf(location(file, 2, 0, 4, 1), location(file, 9, 0)))) + .containsExactly(2, 3, 4, 9) + } +} diff --git a/docs/adr/0010-navigation-resolves-via-analysis-api.md b/docs/adr/0010-navigation-resolves-via-analysis-api.md index c72d3fad1d..78c8bcbafd 100644 --- a/docs/adr/0010-navigation-resolves-via-analysis-api.md +++ b/docs/adr/0010-navigation-resolves-via-analysis-api.md @@ -43,4 +43,6 @@ It cannot. The index stores names, kinds, visibility, and containing-class metad ## Related - [docs/features/kotlin-goto-definition.md](../features/kotlin-goto-definition.md) - the first feature built on this decision +- [docs/features/kotlin-find-usages.md](../features/kotlin-find-usages.md) - the second, which additionally has no reference-search infrastructure to fall back on: the bundled Analysis API ships no `ReferencesSearch`, no `PsiSearchHelper` and no word index +- [ADR 0011](0011-command-analysis-priority.md) - the analysis priority those features run at - [ADR 0001](0001-prefer-room-for-persistence.md) - persistence choices for the indexes this ADR declines to use diff --git a/docs/adr/0011-command-analysis-priority.md b/docs/adr/0011-command-analysis-priority.md new file mode 100644 index 0000000000..5db2e7ed62 --- /dev/null +++ b/docs/adr/0011-command-analysis-priority.md @@ -0,0 +1,65 @@ +# 0011. User-invoked commands get their own analysis priority + +- **Status:** Proposed +- **Date:** 2026-08-03 +- **Deciders:** Code On The Go team + +## Context + +Analysis in the K2 Kotlin LSP is serialised behind one priority lock (`AnalysisScheduler`). Until now it had three tiers: + +| Priority | `supersedesSamePriority` | Preempted work | +|---|---|---| +| `INDEXING` | false | re-queued | +| `DIAGNOSTICS` | false | re-queued | +| `INTERACTIVE` | **true** | **discarded** | + +`INTERACTIVE`'s defining property is *"a newer request of the same priority makes me stale, so discard my work"*. That is exactly right for completion and signature help: they fire on keystrokes, and an in-flight result for text the user has already moved past is worthless. + +It is wrong for a command the user invoked from the code-actions menu. The user tapped a menu item and is watching a progress flashbar; the request is not stale, and discarding it silently produces a wrong answer rather than no answer. Yet three commands sat on `INTERACTIVE`: + +- `GoToDefinitionAction` - discovered the problem and worked around it with a one-shot retry (ADFA-4823). +- `OrganizeImportsAction` - no retry. A completion request discards it and it silently does nothing. +- `ImplementMembersAction` - same. + +Find usages (ADFA-4824) makes this acute. It is user-invoked, runs one analysis session per candidate file, and can take seconds across a workspace. On `INTERACTIVE` a single keystroke anywhere would discard an in-flight file's work, and two concurrent searches would discard each other. + +## Decision + +**Add a fourth priority, `COMMAND`, for user-invoked commands, ordered between `DIAGNOSTICS` and `INTERACTIVE`, with `supersedesSamePriority = false`.** + +```text +INDEXING < DIAGNOSTICS < COMMAND < INTERACTIVE +``` + +- Every user-invoked command runs at `COMMAND`: find usages, go-to-definition, organize imports, implement members. +- `supersedesSamePriority = false`, so **two commands never discard each other**; the second waits for the lock. +- Keystroke-driven features (completion, signature help) stay on `INTERACTIVE` and therefore still win against a command. +- A command preempted by `INTERACTIVE` retries. Long-running commands take their session **per unit of work** - for find usages, per candidate file - so a preemption costs one file, not the whole request. + +## Consequences + +**Positive** + +- The silent-failure bug in organize-imports and implement-members is fixed, not just in the one action that happened to notice it. +- Commands stop competing destructively with each other, which is what makes a multi-file search viable at all. +- Typing responsiveness is untouched. On a phone, completion is part of how text gets entered; starving it is the one regression a user would feel immediately. +- The priority now says what it means. `INTERACTIVE` is "stale on newer input"; `COMMAND` is "explicitly requested, must finish or be cancelled". + +**Negative / costs** + +- Commands still need a retry policy, because `INTERACTIVE` outranks them. The retry is one line at each call site and already proven in `findDefinitionAt`, but it is a rule every future command has to remember. +- Four tiers instead of three is more scheduler surface to reason about. +- Background diagnostics now lose to any command, so a long search delays diagnostics for its duration. Acceptable: diagnostics are re-queued, never discarded. + +## Alternatives considered + +- **`COMMAND` above `INTERACTIVE`** - rejected, though tempting. Nothing could preempt a command, so retries would disappear everywhere and the two buggy actions would be fixed for free. But a multi-second search would then starve the completion popup for its whole duration, and releasing the lock between files would not help - the command wins it straight back. Fixing *that* means teaching the scheduler to yield to waiting requesters between chunks, which is new machinery for a case only find usages hits. +- **Keep commands on `INTERACTIVE` and add a retry to each** - rejected: it leaves `supersedesSamePriority = true` applying to requests that are never stale, so two commands still discard each other, and every command pays for a property none of them want. +- **Flip `INTERACTIVE.supersedesSamePriority` to false** - rejected: completion genuinely needs discard-on-newer. Rapid typing would otherwise queue a chain of results for text the user has already left. + +## Related + +- [ADR 0010](0010-navigation-resolves-via-analysis-api.md) - Kotlin navigation resolves via the Analysis API, not the symbol index +- [docs/features/kotlin-find-usages.md](../features/kotlin-find-usages.md) - the feature that forced the distinction +- `lsp/kotlin/.../compiler/modules/AnalysisScheduler.kt` - the scheduler and priority enum diff --git a/docs/adr/README.md b/docs/adr/README.md index 9bb6db0c4a..7139d240d5 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -24,3 +24,4 @@ Format is lightweight **MADR / Nygard**: Context → Decision → Consequences | [0008](0008-retain-androidide-namespace.md) | Retain the `com.itsaky.androidide` namespace after rebrand | Proposed | | [0009](0009-jetpack-compose-for-new-ui.md) | Build new UI in Jetpack Compose, not XML Views | Proposed | | [0010](0010-navigation-resolves-via-analysis-api.md) | Kotlin navigation resolves via the Analysis API, not the symbol index | Proposed | +| [0011](0011-command-analysis-priority.md) | User-invoked commands get their own analysis priority | Proposed | diff --git a/docs/features/kotlin-find-usages.md b/docs/features/kotlin-find-usages.md new file mode 100644 index 0000000000..92cbdc4200 --- /dev/null +++ b/docs/features/kotlin-find-usages.md @@ -0,0 +1,278 @@ +# Kotlin find usages (K2 LSP) + +- **Ticket:** ADFA-4824 (subtask of ADFA-3317; split out of the closed ADFA-3321 "Navigation") +- **Status:** Implemented in `lsp/kotlin/navigation/`, pending on-device QA +- **Module:** `lsp/kotlin` + +From a Kotlin declaration - or from a reference to one - list every place in the workspace that uses it, across three scopes: same file, another file in the same module, another module in the workspace. + +`KotlinLanguageServer.findReferences` already exists as a stub that answers empty; this feature fills it in. Everything downstream of it (`ReferenceResult`, `IDEEditor.onFindReferencesResult`, the search-results panel) already existed for the Java server. + +The sibling feature [go-to-definition](kotlin-goto-definition.md) answers the *opposite* question and shares this feature's caret handling, symbol-to-location conversion, and test fixture. Read its Language section first; the terms below extend it rather than replace it. + +## Language + +**Usage**: +A reference that resolves into the match set. This is the unit the feature reports. +_Avoid_: reference (that is the PSI element, per go-to-definition's glossary), occurrence, hit, match. + +**Target**: +The declaration whose usages are being searched for. Derived from the caret either directly (the caret is on the declaration's own name) or by resolving the reference under the caret. +_Avoid_: symbol, subject, source, declaration (reserve that for the PSI element a reference resolves to). + +**Match set**: +The target plus every declaration a call to the target may legitimately have been written against: its **workspace-source** supers, and - when the target is a classifier - its constructors. A reference is a usage if and only if it resolves into this set. +_Avoid_: hierarchy, family, candidates (go-to-definition uses "candidate" for a resolved declaration). + +**Search scope**: +The set of modules a usage could possibly live in, derived from the target's visibility. Distinct from go-to-definition's **resolution scope** (same-file / inter-file / inter-module), which describes coverage rather than a bound. These two are easy to conflate and are deliberately named apart. +_Avoid_: scope (unqualified), visibility scope, module scope. + +**Candidate file**: +A file that survived the text prefilter and is therefore worth parsing and resolving. Most candidate files contain no usage at all - the prefilter is a cheap over-approximation. +_Avoid_: match, result, hit. + +**Workspace boundary**: +The line between declarations with source PSI in a source module and everything else (the stdlib, the framework, library jars). The match set stops at it, and so does the reportable result set. +_Avoid_: project boundary, library edge. + +## Scope + +### In scope + +Any reference, in any of the three resolution scopes, that resolves into the match set - where both the reference and the target's declaration are workspace sources. + +The **target** may be a Java-source declaration. A caret on a Kotlin reference to a workspace `.java` class or method resolves to it (go-to-definition's AC5 already covers that direction), and its Kotlin usages are found like any other target's. + +**Convention references are valid entry points.** A caret on `a + b`, on `by`, on `[`, on a `for` loop's `in`, or on a destructuring entry resolves through to `plus` / `getValue` / `get` / `iterator` / `componentN`, and the feature then searches for *named* usages of that function. This costs nothing beyond what go-to-definition already does. + +### Out of scope + +- **Implicit call sites as results.** A usage search on `operator fun plus` finds explicit `a.plus(b)` calls, not `a + b`. Discovering implicit sites would mean resolving every operator, index, call, delegate and loop expression in every file in scope, because the text of `a + b` contains no name to prefilter on. Java's find-references reports no implicit usages either. +- **`.java` files as search targets.** Kotlin declarations *are* visible to Java PSI as light classes here (`symbol-light-classes.xml` registers `KotlinAsJavaSupport`, and `JavaElementFinder` is registered), but nothing in the repo exercises Java PSI *resolution*, and the Java server has its own find-references. Kotlin call sites of a Java declaration work; Java call sites of a Kotlin declaration are not searched. +- **Usages reachable only through a subclass.** See R3 and Non-goals. +- **Binary symbols.** As with go-to-definition: no decompiler, and `showLocations` can only open a real file. A search from a reference to `listOf` finds nothing. +- **Test sources.** Not a choice made here - `AndroidModule.getSourceDirectories()` returns `mainSourceSet` only, so `src/test/**` and `src/androidTest/**` are not content roots for *any* Kotlin LSP feature. + +## Requirements + +**R1 - Trigger.** A "Find references" item appears in the Kotlin code-actions menu, mirroring Java's. `FindReferencesAction` extends `BaseKotlinCodeAction`, id `ide.editor.lsp.kt.findReferences`, reuses `R.string.action_find_references`, and delegates to `ILspEditor.findReferences()`. Registered in `KotlinCodeActionsMenu` immediately after `GoToDefinitionAction`, matching Java's ordering. + +It carries its own tooltip tag, `EDITOR_CODE_ACTIONS_KT_FIND_REFS = "editor.codeactions.kotlin.findrefs"`, not Java's `EDITOR_CODE_ACTIONS_FIND_REFS` - the same split go-to-definition made, so Kotlin and Java can carry different tooltip text. The tooltips database is not in this repo, so the tag shows no text until a row exists for it; that row is a hand-off item, not code. + +The item is **always visible** for `.kt`/`.kts` and never conditioned on what the caret is sitting on: deciding "is there a target here" needs PSI and the project lock, and `prepare()` runs on the UI thread. A caret on whitespace therefore flashes "No references found". A `.kts` file shows the item and it does nothing, because a script has no `CompilationEnvironment` - identical to go-to-definition. + +**R2 - Target at caret.** The caret maps to a target declaration by trying, in order: + +1. **The caret is on a declaration's own name** - the leaf is the `nameIdentifier` of a `KtNamedDeclaration`. That declaration is the target. +2. **The caret is on a reference** - delegate to go-to-definition's `referenceAtCaret`, then resolve it to its declaration, which becomes the target. + +Order matters, and it makes the two features answer differently from one identical caret. For `val (x, y) = p` with the caret on `x`, go-to-definition navigates to `component1`; find usages targets the local `x`. That is deliberate: `x` is both a declaration and a convention reference, and each feature wants the reading that is useful to it. + +`referenceAtCaret` cannot be reused for step 1. It is built so that a caret on a declaration's own name resolves nothing - go-to-definition's no-self-jump rule - which is precisely the caret position find usages is normally invoked from. Step 1 is therefore a new, separate check; the token accept-list and the `offset - 1` retry are shared. + +**R3 - Match set.** Assembled once, in the caret's analysis session: + +- The target symbol, normalised through `fakeOverrideOriginal`. A call `derived.foo()` where `Derived` does not redeclare `foo` resolves to a substituted fake override, not to `Base.foo`, so both sides of every comparison are normalised. +- Its supers, via `allOverriddenSymbols`, **stopping at the workspace boundary**. So a call dispatched through a workspace `Base.foo` counts as a usage of `Derived.foo`, wherever `Base` lives - which is why R4's scope unions the supers' modules too. Library supers are excluded: including them would make a usage search on an overridden `toString` match every `.toString()` call in the workspace, and a library super can never yield a reportable result anyway. +- When the target is a classifier, its **constructors**. Otherwise `Foo()` - which resolves to the constructor, not the class (go-to-definition's R4) - would not count as a usage of `class Foo`, and the feature would miss every instantiation. The reverse expansion is not applied: a target that *is* a specific constructor stays that constructor, because asking for usages of one overload is a deliberate act. + +The walk goes **up** only. Usages reachable solely through a subclass (`Base.foo` searched, `derived.foo()` written) are not found - that needs a workspace inheritor search, and `DirectInheritorsProvider.computeIndex()` rebuilds its entire index on every call. + +**Import directives count as usages.** `import a.b.Foo` resolves to `Foo`, so it is one by construction. The panel has no categories to separate them into, and the noise is bounded at one hit per importing file. + +**R4 - Search scope.** Derived from the target's visibility, which is an exact bound rather than a heuristic: + +| Target | Scope | +|---|---| +| local val/var, parameter, local fun, local class, loop variable | containing file | +| `private` top-level declaration | containing file (Kotlin private top-level is file-private) | +| `private` class/object member | containing file | +| `internal` | the target's module | +| `protected`, `public`, default | every match-set member's module + its transitive dependents (`KotlinModuleDependentsProvider.getTransitiveDependents`) | + +The ticket's three resolution scopes fall out of this one code path rather than being three implementations. Cheap cases stay cheap: a search on a local variable never leaves the open file. + +The last row unions **every match-set member's** module, not just the target's, because R3's up-walk and R4's dependents pull in opposite directions. With `Base` in `lib` and `Derived` in `app`, a `base.paint()` call written in `lib` is a usage of `Derived.paint` - but `lib` is a *dependency* of `app`, not a dependent, so the target's own module and dependents would never look at it. Library supers are already out of the match set, so the union cannot escape the workspace. The first three rows need no union: `private` cannot override, and this project model has no friend modules, so `internal` cannot be overridden across one. + +The first three rows need the declaration's path. The file the user is editing is a live `KtFile` built from the editor buffer, whose `virtualFile` is a non-physical `LightVirtualFile`, so the path comes from `backingFilePath` first and the VFS only as a fallback - go-to-definition's derivation. A target that still has no path cannot be confined to one file, but it is still unreferenceable outside its own module, so it falls back to the `internal` row rather than to the last one. + +`internal` needs no widening for test sources. There is no test module to widen to - `collectKtModules` builds one `KtSourceModule` per Gradle module from `mainSourceSet` only, and `directFriendDependencies` is empty everywhere. + +**R5 - Candidate discovery.** Two tiers, because find usages is run *while* editing and unsaved text must not be invisible: + +| File | Prefilter text | PSI | +|---|---|---| +| open in the editor | the live buffer | `ktSymbolIndex.getCurrentKtFile(path).await()`, awaited **outside** `project.read` | +| everything else | disk | `ktSymbolIndex.getKtFile(path)` | + +A module's files include `.java`, which is a non-goal to search, so candidates are filtered on the extension *before* the read - otherwise a Java-heavy workspace spends most of the prefilter reading files whose result is already known. + +The prefilter is `mentionsName`, word-boundary exact on the target's simple name, reading line by line through `FileManager.getReader(path)` - which returns the live document when the file is open and the file itself otherwise, so the two tiers above need no branch of their own. Its errors are one-directional: a file that mentions the name but contains no usage is parsed and discarded (wasted work, correct result), while a file that does not mention the name cannot contain a named usage. An unreadable file drops out of the scan with a log rather than failing the search. + +Open documents are tab-count many, so the live tier is free. Without it, a usage the user just typed would be missed entirely - the prefilter would never select the file, so it would never be parsed. + +Deliberately **not** `StringSearch.containsWord`, the equivalent helper the Java server prefilters with. It reads only the first 1 MB of a file, so a usage below the mark would be silently dropped; it reads through one process-global `ByteBuffer` that the Java server mutates concurrently from its own threads; and it rethrows an unreadable file as a `RuntimeException`, which here would abort the whole search. A name cannot span a line break, so matching per line loses nothing. + +Only `KtSimpleNameExpression`s are examined. That is what makes the name filter cheap - it runs on PSI alone, so it runs *before the analysis session is opened*, and a text-prefilter hit whose only mention is a comment or a string literal never costs an analysis-lock acquisition, a FIR session or a match-set restore. It is also what implements "convention references are not results": `a + b` contains no `plus` token, so it is never a candidate. The cost is that a **KDoc `[link]`** to the target is not reported, even though go-to-definition navigates from one; a documented gap rather than a decision worth its own machinery in v1. + +**R6 - Identity.** A reference is a usage if its resolved symbol is in the match set. Deciding that across files needs care, because `KaSymbol` is session-scoped and the same declaration exists as two PSI instances - the on-disk `KtFile` cached in the index, and the dangling `KtFile` built from the editor buffer for an open file. + +Matching therefore uses `KaSymbolPointer`: `createPointer()` for each match-set member in the caret's session, then `restoreSymbol(session)` **once per candidate session**, then `==` against each resolved candidate symbol inside that session. This is the platform's cross-session identity mechanism, with structural implementations per symbol kind, and it is the direct analogue of the Java server re-deriving its target `Element` inside each compile task. + +Locals pay almost none of it: R4 confines them to one file, so there is a single candidate session and the pointers restore once. + +A pointer that fails to restore drops that session's candidates, with a log. That under-reports rather than reporting something false, which is the safe direction, and it is tested. + +Neither a PSI identity check nor a (file, offset) key works here. Both break exactly when the target's own file has unsaved edits: the live PSI and the on-disk PSI disagree about offsets, so every cross-file usage would be silently missed - and editing-then-searching is the common case. + +**R7 - Results.** Each usage becomes a `Location` whose range covers the reference's **name identifier** (`foo` in `a.b.foo()`, `Foo` in `Foo()`), matching go-to-definition's R6. Deduplicated by file plus range, ordered by file path then start offset. + +`includeDeclaration` is **ignored**, and the target's own declaration is never emitted. Java's provider ignores it too. Honouring it would also create a trap: a declaration with no usages would return exactly one location in the current file, which `onFindReferencesResult` turns into a silent `setSelection` on the declaration the caret is already on - indistinguishable from a broken no-op. Returning empty flashes "No references found", which is true. + +There is **no result cap**. See R10 for why one is not needed. + +**R8 - Result handling.** The server returns `ReferenceResult(locations)`; `IDEEditor.onFindReferencesResult` applies unchanged: + +- empty -> flash `msg_no_references` +- one location in the current file -> `setSelection` +- otherwise -> `languageClient.showLocations`, the grouped search-results panel + +**R9 - Scheduling.** The request runs at the new `AnalysisPriority.COMMAND` ([ADR 0011](../adr/0011-command-analysis-priority.md)), behind the editor's existing cancellable progress flashbar (`msg_finding_references`). + +Granularity is per candidate file, and it is load-bearing: + +- **One analysis session per candidate file.** A preemption by completion costs one file's work, which is retried once - `findDefinitionAt`'s pattern. The target-resolution phase is retried twice over, because losing *it* loses the whole search rather than one file (R12). One session for the whole search would let a single keystroke discard a whole-workspace scan. A file preempted *twice* is dropped like any other failed candidate (R12), not rethrown: keystroke-driven work winning the lock must not turn a search with plenty of hits into "no references". +- **`project.read` per candidate file, never once for the search.** A whole-workspace search holding the read lock start to finish would block every `project.write`, which is what index refresh needs. +- **The live-document await stays outside `project.read`.** The refresh it waits on needs `project.write`; awaiting it under the read lock deadlocks. Go-to-definition's R10 records the same constraint. +- `params.cancelChecker` is honoured per prefiltered file, between candidate files **and** between references within a file. The prefilter checks it per file rather than once for the pass: a whole-workspace scan is seconds of I/O, and cancelling has to stop it rather than let it finish and discard the result. + +The prefilter pass runs first, before any analysis, and takes no *analysis* lock at all (`computeFiles` takes `project.read` per file to resolve one path to a `VirtualFile`, but nothing is held across the pass). No progress count is shown - `launchCancellableAsyncWithProgress` takes a fixed `@StringRes`, and threading a live count through it would change a shared editor API for a cosmetic gain. No timeout and no file budget: the search finishes or the user cancels. + +**R10 - Panel cost.** `IDELanguageClientImpl.showLocations` used to read each result file **in full, once per hit, on the main thread** (`FileIOUtils.readFile2String` inside the per-location loop, plus an `exists()` stat per hit). That is a main-thread I/O violation and O(hits) file reads; Java's find-references had it too and simply rarely produced enough hits to hurt. + +Rewritten to: group locations by file, then one sequential `BufferedReader` pass per file pulling only the lines its ranges touch, retaining nothing before moving on. The disk pass runs **off** the main thread through `TaskExecutor`, which posts its callback back to the UI thread. + +A file with an **open editor** is still resolved **on** the UI thread. Its `Content` is live UI state that a background thread must not touch, and pulling a few lines out of it is substring work with no I/O. That is also what keeps unsaved edits reflected in the panel. + +Reads drop from O(hits) to O(files), peak memory is one line rather than one file (deliberately *not* a per-file content cache - holding every result file's text at once is the wrong trade on a phone), and the main thread does no I/O. This removes the need for a result cap, which would otherwise silently truncate. + +Because the publish is now asynchronous, it is also guarded: `showLocations` claims the panel with a request counter and captures `EditorViewModel.currentSearchGeneration`, and the callback publishes only if both still hold. Otherwise a slow request that started first would land last and overwrite the newer search the user is looking at. Panel visibility is committed *with* the rows for the same reason - a publish that never happens (superseded, or the activity recreated mid-read) must not leave the panel open with the "no results" placeholder hidden over the previous query's rows. + +Two behaviour changes, both improvements: a hit whose line no longer exists is dropped rather than yielding whatever `Content` returned, and a file whose every hit is stale is omitted rather than contributing an empty group. The grouping and line extraction live in `SearchResultGrouping` so they can be unit-tested; the activity call is a thin shell. + +**R11 - Not ready.** No `CompilationEnvironment` for the file (a script, a file outside the content roots), or no analysis session yet, answers empty and logs. There is no "still indexing" signal; that gap is cross-cutting across every LSP feature and is not solved here. + +**R12 - Failure isolation.** A resolution failure on one candidate file drops that file and continues - one unparseable file must not lose the whole result. So does an unreadable one in the prefilter pass, and one preempted past `retryingOnPreemption`'s single retry. A failure in the target-resolution phase returns empty. + +Preemption is not cancellation, and the two must not share a handler. Both unwind as a `CancellationException`, but a preempted request is still *wanted* - the user is watching the flashbar - so reporting empty for it is a wrong answer, while reporting empty for a cancelled one is invisible (the cancelled coroutine never reaches `onFindReferencesResult`). Hence a candidate file preempted past `retryingOnPreemption`'s single retry drops like any other failed candidate, and the target-resolution phase runs `planAt` twice - up to four underlying attempts - before giving up with a warning. Genuine cancellation short-circuits to empty at any depth. Nothing propagates an exception to the editor or leaves the progress flashbar up. + +## Non-goals + +- **Rename / safe-delete**, or anything that edits the usages found. +- **Usages via subclasses** (the down-walk). Blocked on `DirectInheritorsProvider.computeIndex()` being cached; filed separately. +- **Searching `.java` files** for usages of a Kotlin declaration. Filed separately. +- **Usages in test source sets.** Filed separately, as an LSP-wide content-root gap. +- **Implicit call sites as results** (see Scope). +- **KDoc `[link]`s as results.** Only `KtSimpleNameExpression`s are examined (R5). Go-to-definition navigates *from* a KDoc link, so this is an asymmetry, but a bounded one. +- **Library-source usages**, via decompilation or `-sources.jar`. +- **Categorising results** (imports vs calls vs type references) - the panel has no grouping beyond file. +- **A partiality signal.** `ReferenceResult` is shared with the Java and XML servers and has no field for it, and `showLocations` has no header slot; the same caveat already applies silently to test sources. +- **A gesture trigger.** Editor-wide UX change that would apply to Java too. + +## Acceptance criteria + +1. "Find references" appears in a Kotlin file's code-actions menu and is absent in a non-Kotlin file. +2. Same-file: a local function's call sites are listed. +3. Inter-file: usages of a class in a sibling file of the same module are listed. +4. Inter-module: usages in a dependent module are listed. +5. Invoked from a **reference** rather than a declaration, the result is the same set. +6. A `private` top-level declaration reports no usages from another file, even when that file contains a same-named unrelated declaration. +7. An `internal` declaration reports usages within its module only. +8. A local variable's usages are confined to its file. +9. `Foo()` is reported as a usage of `class Foo`. +10. An `import` of the target is reported as a usage. +11. A call dispatched via a workspace `Base.foo` is reported as a usage of `Derived.foo`, including when `Base` lives in a module `Derived`'s depends on. +12. A usage search on an override of `toString` does **not** report unrelated `.toString()` calls. +13. A usage typed into an open, unsaved file is reported. +14. A target with no usages flashes "No references found". +15. The target's own declaration never appears in the results. +16. Cancelling the progress flashbar mid-search leaves the editor responsive and unchanged. +17. Typing during a search does not discard it. +18. A search from a reference to a stdlib or framework symbol flashes "No references found". +19. A caret on whitespace, in a comment, or on a non-navigable keyword produces no search. +20. Invoking before the project finishes loading flashes "No references found" and does not crash or hang. +21. A result set spanning many files opens the panel without a main-thread stall. + +## Design + +Resolution goes through the Analysis API and PSI only; the symbol indexes are never consulted - see [ADR 0010](../adr/0010-navigation-resolves-via-analysis-api.md). That decision is load-bearing here for a second reason: there is no reference-search infrastructure to fall back on. `analysis-api-standalone-embeddable-for-ide` ships no `ReferencesSearch`, no `PsiSearchHelper` and no word index, and `KtFileMetadata` records declarations only. The search is built here. + +```text +FindReferencesAction.execAction lsp/kotlin/actions + -> ILspEditor.findReferences() editor (unchanged: progress flashbar + cancel checker) + -> KotlinLanguageServer.findReferences(params) + guards: settings.referencesEnabled(), DocumentUtils.isKotlinFile + compilationEnvironmentFor(params.file) ?: empty [R11] + -> context(env) { findUsagesAt(params) } navigation/FindUsages.kt + planWithRetry -> planAt(params): (twice, each retrying a preemption once) [R12] + ktFile = env.ktSymbolIndex.getCurrentKtFile(file).await() ?: empty [R5, R11] + env.project.read { + target = targetAtCaret(ktFile, offset) navigation/TargetAtCaret.kt [R2] + analyzeMaybeDangling(ktFile, COMMAND, cancelChecker) { + planFor(target) -> simpleName, matchSet pointers, scope [R3, R4, R6] + } + } + candidateFiles(plan, cancelChecker) [R5] + per candidate file: (retried once if preempted) [R9] + await live PSI if open (outside project.read) + env.project.read { + walk name references (PSI only); no hit -> skip the file [R5] + analyzeMaybeDangling(file, COMMAND, cancelChecker) { + restore pointers once, compare [R6] + } + } -> locations [R7] + <- ReferenceResult(locations) [R8] +``` + +New components: + +- **`navigation/TargetAtCaret.kt`** - `targetAtCaret(file: KtFile, offset: Int): CaretTarget?`, returning either a `Declaration` or a `Reference` so the resolution step does not re-derive which case it is looking at. Pure PSI, no analysis session, so R2's caret rules are testable without one. Shares `ReferenceAtCaret.kt`'s token accept-list, which becomes `internal`. It checks the leaf at the offset **and** the one before it, because `referenceAtCaret`'s single retry is not enough here: a caret just past `fun target` lands on `(`, which is navigable in its own right, so checking only that leaf made a caret one character past a declaration's name find nothing. +- **`navigation/FindUsages.kt`** - `planAt` (target, match set, scope) and the per-file resolve loop, reusing go-to-definition's `symbolsAt` and range helper. `planAt`, `SearchPlan` and `candidateFiles` are `internal` rather than private so the visibility ladder is directly assertable: it is *not* observable from a result set, since symbol matching means a same-named decoy can never be a false positive whatever the scope. +- **`SearchResultGrouping`** (in `app/`) - R10's grouping and line extraction. + +An **ambiguous** reference at the caret (overloads, broken code) searches for its first resolved candidate and logs. The alternative is a chooser the panel cannot host, and refusing to search would be worse. + +Touched existing components: + +- **`KotlinLanguageServer.findReferences`** - the stub's guards stay; it now delegates inside the file's `CompilationEnvironment`, matching how `findDefinition` and `signatureHelp` dispatch. +- **`navigation/ReferenceAtCaret.kt`** - visibility loosened for reuse. Behaviour unchanged, and its existing tests are kept as the proof of that. +- **`AnalysisPriority` / `AnalysisScheduler`** - the new `COMMAND` tier, plus `retryingOnPreemption`, which holds the two invariants every command's retry depends on: a fresh `ScheduledCancelChecker` per attempt (`preempt()` latches), and re-fetching the `KtFile` inside the attempt ([ADR 0011](../adr/0011-command-analysis-priority.md)). +- **`GoToDefinitionAction`, `OrganizeImportsAction`, `ImplementMembersAction`** - migrated to `COMMAND`; the latter two gain the retry they never had, and take the delegate `ICancelChecker` rather than a pre-wrapped one since wrapping is now per attempt. +- **`GoToDefinition.symbolsAt`** - `internal`, so the reference-at-caret resolution is shared rather than duplicated. +- **`services/ModuleDependentsProvider`** - its direct- and refinement-dependents maps are now accumulated across all modules instead of built per module and merged with `Map + Map`, which *replaced* a shared dependency's dependent set. R4's last row reads that map, so a module used by more than one other silently lost every dependent but the last. +- **`IDELanguageClientImpl.showLocations`** - R10's grouped streaming rewrite, plus its staleness guard. +- **`TooltipTag`** - one new constant (R1). + +Unchanged: `ReferenceParams`/`ReferenceResult`, `ILanguageServer`, `IDEEditor`, and every string resource. + +## Verification + +`flox activate -d flox/local -- ./gradlew :lsp:kotlin:testV7DebugUnitTest` and `:app:testV7DebugUnitTest`, split to match the helpers: + +- **`TargetAtCaretTest`** (13) - PSI only, no session. Caret on a function's / class's / property's / parameter's own name; caret on a reference rather than the enclosing declaration; one past a declaration's name; a local declaration inside a lambda; a destructuring entry targeting the local rather than `componentN`; an operator; whitespace / comment / non-navigable keyword. One case asserts the contrast directly: the same caret that `referenceAtCaret` rejects still yields a target. +- **`ReferenceAtCaretTest`** - kept as-is, as the regression proof that loosening visibility changed no behaviour. +- **`FindUsagesTest`** (20) - the `lib` + `app(dependsOn = lib)` fixture from ADFA-4823: the three resolution scopes; each row of R4's visibility ladder, asserted on the plan's scope rather than the result set; R3's super-walk, workspace-boundary cutoff and constructor expansion; imports; a Java-source target; a same-named decoy in another package; ordering; property reads and writes; a stdlib reference; a caret that names nothing; and a pre-cancelled request. +- **`FindUsagesLiveDocumentTest`** (2) - R5's live tier, which needs `enableParserEventSystem`: a usage that exists only in an unsaved buffer is found, and one deleted in the buffer but still on disk is not. +- **`AnalysisSerializationTest`** (+5) - `COMMAND`'s three ordering properties, plus `retryingOnPreemption`'s one-retry-with-a-fresh-checker contract and its refusal to loop. +- **`KotlinCodeActionTooltipTagTest`** - the new tag row. +- **`SearchResultGroupingTest`** (10, in `:app`) - single-line and multi-line hits, a hit on a line that no longer exists, a column past its line's end, only-the-wanted-lines collection, a short file, an unreadable file, and several hits in one file from one read. + +Not unit-testable, so covered by on-device QA via the "Steps to QA" field on ADFA-4824: the menu item and its tooltip tag, the panel with a large result set, cancelling mid-search, and typing during a search without losing it. + +## Related + +- [docs/features/kotlin-goto-definition.md](kotlin-goto-definition.md) - the sibling feature whose helpers and fixture this reuses +- [ADR 0010](../adr/0010-navigation-resolves-via-analysis-api.md) - navigation resolves via the Analysis API, not the symbol index +- [ADR 0011](../adr/0011-command-analysis-priority.md) - user-invoked commands get their own analysis priority +- [ARCHITECTURE.md](../../ARCHITECTURE.md) diff --git a/docs/features/kotlin-goto-definition.md b/docs/features/kotlin-goto-definition.md index 30cf72a7ba..8bd6d0c872 100644 --- a/docs/features/kotlin-goto-definition.md +++ b/docs/features/kotlin-goto-definition.md @@ -111,7 +111,7 @@ Both rules are enforced by construction rather than by filtering afterwards: an ## Non-goals -- **Find usages** - ADFA-4824, the sibling subtask. It will share the reference-at-caret resolution helper. +- **Find usages** - ADFA-4824, the sibling subtask; see [kotlin-find-usages.md](kotlin-find-usages.md). - **Go-to-implementation.** A call through an interface or abstract member resolves to the declaring member only. Walking down to overriding implementations needs an inheritance search over the workspace. - **Go-to-super.** - **Library-source navigation**, via decompilation, generated stubs, or `-sources.jar` extraction. @@ -161,7 +161,7 @@ The dispatch mirrors `signatureHelp` line for line, which is what buys R3 and R1 Touched components: - **`KotlinLanguageServer.findDefinition`** - guards stay (`definitionsEnabled()`, `isKotlinFile`), then delegates inside the file's `CompilationEnvironment`, matching how `signatureHelp` and `analyze` already dispatch. A `.kts` has no environment, so the lookup returns null there and the request answers empty. -- **`navigation/ReferenceAtCaret.kt`** - `referenceAtCaret(file: KtFile, offset: Int): KtElement?`. Pure PSI, no analysis session: the caret-token accept-list, the `offset - 1` retry, and the two-level climb (R2). ADFA-4824 imports this verbatim; it needs the reference element, not the declarations. +- **`navigation/ReferenceAtCaret.kt`** - `referenceAtCaret(file: KtFile, offset: Int): KtElement?`. Pure PSI, no analysis session: the caret-token accept-list, the `offset - 1` retry, and the two-level climb (R2). ADFA-4824 reuses its accept-list and retry, but not the function: this deliberately resolves nothing when the caret is on a declaration's own name, which is exactly where find usages is invoked from. See [kotlin-find-usages.md](kotlin-find-usages.md) R2. - **`navigation/GoToDefinition.kt`** - `findDefinitionAt(params)` under `context(env: CompilationEnvironment)`. The two symbol paths (R4), then symbol -> source PSI -> name-identifier range -> `Location`, with dedup, ordering, cancellation and failure isolation (R5, R6, R10, R11). - **`GoToDefinitionAction` in `lsp/kotlin/actions`** extending `BaseKotlinCodeAction`, id `ide.editor.lsp.kt.gotoDefinition` (the prefix every other Kotlin action uses), `requiresUIThread = true` like Java's, registered in `KotlinCodeActionsMenu` after the comment actions - the same slot Java uses. - **`TooltipTag.EDITOR_CODE_ACTIONS_KT_GOTO_DEF`** - one new constant (R1). diff --git a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt index d8e71fd5dd..02a0571d1f 100644 --- a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt +++ b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt @@ -92,6 +92,7 @@ object TooltipTag { const val EDITOR_CODE_ACTIONS_KT_NULL_SAFETY_FIX = "editor.codeactions.kotlin.nullsafetyfix" const val EDITOR_CODE_ACTIONS_KT_SURROUND_TRY_CATCH = "editor.codeactions.kotlin.trycatch" const val EDITOR_CODE_ACTIONS_KT_GOTO_DEF = "editor.codeactions.kotlin.gotodef" + const val EDITOR_CODE_ACTIONS_KT_FIND_REFS = "editor.codeactions.kotlin.findrefs" const val EXIT_TO_MAIN = "exit.to.main" diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionsMenu.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionsMenu.kt index 7530a6fe9b..1188a15022 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionsMenu.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionsMenu.kt @@ -6,6 +6,7 @@ import com.itsaky.androidide.lsp.actions.CommentLineAction import com.itsaky.androidide.lsp.actions.IActionsMenuProvider import com.itsaky.androidide.lsp.actions.UncommentLineAction import com.itsaky.androidide.lsp.kotlin.actions.AddImportAction +import com.itsaky.androidide.lsp.kotlin.actions.FindReferencesAction import com.itsaky.androidide.lsp.kotlin.actions.GoToDefinitionAction import com.itsaky.androidide.lsp.kotlin.actions.ImplementMembersAction import com.itsaky.androidide.lsp.kotlin.actions.NullSafetyAction @@ -32,6 +33,7 @@ object KotlinCodeActionsMenu : IActionsMenuProvider { TooltipTag.EDITOR_CODE_ACTIONS_KT_UNCOMMENT, ), GoToDefinitionAction(), + FindReferencesAction(), AddImportAction(), OrganizeImportsAction(), SurroundWithTryCatchAction(), diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt index ae9f0d903c..958928e292 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt @@ -38,6 +38,7 @@ import com.itsaky.androidide.lsp.kotlin.compiler.index.KT_SOURCE_FILE_META_INDEX import com.itsaky.androidide.lsp.kotlin.completion.codeComplete import com.itsaky.androidide.lsp.kotlin.diagnostic.collectDiagnosticsFor import com.itsaky.androidide.lsp.kotlin.navigation.findDefinitionAt +import com.itsaky.androidide.lsp.kotlin.navigation.findUsagesAt import com.itsaky.androidide.lsp.kotlin.signaturehelp.doSignatureHelp import com.itsaky.androidide.lsp.models.CompletionParams import com.itsaky.androidide.lsp.models.CompletionResult @@ -233,7 +234,11 @@ class KotlinLanguageServer : ILanguageServer { return ReferenceResult.empty() } - return ReferenceResult.empty() + logger.debug("findReferences(position={}, file={})", params.position, params.file) + return compiler + ?.compilationEnvironmentFor(params.file) + ?.let { context(it) { findUsagesAt(params) } } + ?: ReferenceResult.empty() } override suspend fun findDefinition(params: DefinitionParams): DefinitionResult { diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/FindReferencesAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/FindReferencesAction.kt new file mode 100644 index 0000000000..ab40a307da --- /dev/null +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/FindReferencesAction.kt @@ -0,0 +1,50 @@ +package com.itsaky.androidide.lsp.kotlin.actions + +import com.itsaky.androidide.actions.ActionData +import com.itsaky.androidide.actions.hasRequiredData +import com.itsaky.androidide.actions.markInvisible +import com.itsaky.androidide.editor.api.ILspEditor +import com.itsaky.androidide.idetooltips.TooltipTag +import com.itsaky.androidide.resources.R +import io.github.rosemoe.sora.widget.CodeEditor + +/** + * Lists every usage of the declaration at the caret, or of whatever the reference at the caret names. + * + * Mirrors the Java action: the real work is the editor's own cancellable request, so this only has to + * start it. + */ +class FindReferencesAction : BaseKotlinCodeAction() { + override var titleTextRes: Int = R.string.action_find_references + override val id: String = ID + override var label: String = "" + override var tooltipTag: String = TooltipTag.EDITOR_CODE_ACTIONS_KT_FIND_REFS + + // execAction only starts the editor's own background request, so it must not be moved off the UI + // thread. Nothing here or in prepare() touches the project lock, the index, or an analysis session - + // but super.prepare() -> BaseKotlinCodeAction.prepare -> isKotlinFile() does stat the file + // (Files.exists + Files.isDirectory) on the UI thread. Pre-existing, shared by every Kotlin/Java + // code action, and out of scope here. + override var requiresUIThread: Boolean = true + + override fun prepare(data: ActionData) { + super.prepare(data) + + // Deliberately not conditioned on what the caret sits on: answering that needs PSI and the + // project read lock, and prepare() runs on the UI thread. A caret that names nothing therefore + // shows the item and flashes "no references", exactly as go-to-definition does. + if (!visible || !data.hasRequiredData(CodeEditor::class.java)) { + markInvisible() + return + } + } + + override suspend fun execAction(data: ActionData): Any { + val editor = data[CodeEditor::class.java] ?: return false + return (editor as? ILspEditor)?.findReferences() ?: false + } + + companion object { + const val ID = "ide.editor.lsp.kt.findReferences" + } +} diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt index 69a5673035..6c103772bf 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt @@ -8,8 +8,9 @@ import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.lsp.kotlin.KotlinLanguageServer import com.itsaky.androidide.lsp.kotlin.compiler.AbstractCompilationEnvironment import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority -import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling +import com.itsaky.androidide.lsp.kotlin.compiler.modules.isAnalysisCancellation +import com.itsaky.androidide.lsp.kotlin.compiler.modules.retryingOnPreemption import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.utils.membersToImplement import com.itsaky.androidide.lsp.kotlin.utils.renderOverrideStub @@ -20,6 +21,7 @@ import com.itsaky.androidide.lsp.models.Command import com.itsaky.androidide.lsp.models.DocumentChange import com.itsaky.androidide.lsp.models.TextEdit import com.itsaky.androidide.models.Range +import com.itsaky.androidide.progress.ICancelChecker import com.itsaky.androidide.resources.R import com.itsaky.androidide.tasks.createJobCancelChecker import org.jetbrains.kotlin.analysis.api.symbols.KaClassKind @@ -52,7 +54,7 @@ class ImplementMembersAction : BaseKotlinCodeAction() { val offset = data.requireEditor().cursor.left val env = server.compilationEnvironmentFor(nioPath) ?: return emptyList() // Ties the analysis to this action's coroutine: cancelling the action aborts the queued analysis. - return computeImplementMembersEdit(env, nioPath, offset, ScheduledCancelChecker(createJobCancelChecker())) + return computeImplementMembersEdit(env, nioPath, offset, createJobCancelChecker()) } /** @@ -70,27 +72,39 @@ class ImplementMembersAction : BaseKotlinCodeAction() { env: AbstractCompilationEnvironment, nioPath: Path, offset: Int, - cancelChecker: ScheduledCancelChecker, + cancelChecker: ICancelChecker, ): List = runCatching { - val ktFile = env.ktSymbolIndex.getCurrentKtFile(nioPath).get() ?: return emptyList() - env.project.read { - val classOrObject = findEnclosingClassOrObject(ktFile, offset) ?: return@read emptyList() - analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, cancelChecker) { - val classSymbol = classOrObject.symbol as? KaClassSymbol ?: return@analyzeMaybeDangling emptyList() - if (!isImplementable(classSymbol)) return@analyzeMaybeDangling emptyList() - - val classIndent = classIndentOf(ktFile, classOrObject) - val unit = detectIndentUnit(ktFile.text) - val memberIndent = memberIndentOf(ktFile, classOrObject, classIndent, unit) - val stubs = membersToImplement(classSymbol).mapNotNull { renderOverrideStub(it, memberIndent, unit) } - if (stubs.isEmpty()) return@analyzeMaybeDangling emptyList() - - buildInsertionEdit(ktFile, classOrObject, stubs, classIndent) + // A user-invoked command: AnalysisPriority.COMMAND, retried once if keystroke-driven work + // preempts it (ADR 0011). Without the retry a preemption fell into the getOrElse below and the + // action silently inserted nothing. The file is re-fetched per attempt because the preemptor + // also refreshed the live PSI. + retryingOnPreemption(cancelChecker, "Implement members for $nioPath") { checker -> + val ktFile = env.ktSymbolIndex.getCurrentKtFile(nioPath).get() ?: return@retryingOnPreemption emptyList() + env.project.read { + val classOrObject = findEnclosingClassOrObject(ktFile, offset) ?: return@read emptyList() + analyzeMaybeDangling(ktFile, AnalysisPriority.COMMAND, checker) { + val classSymbol = classOrObject.symbol as? KaClassSymbol ?: return@analyzeMaybeDangling emptyList() + if (!isImplementable(classSymbol)) return@analyzeMaybeDangling emptyList() + + val classIndent = classIndentOf(ktFile, classOrObject) + val unit = detectIndentUnit(ktFile.text) + val memberIndent = memberIndentOf(ktFile, classOrObject, classIndent, unit) + val stubs = membersToImplement(classSymbol).mapNotNull { renderOverrideStub(it, memberIndent, unit) } + if (stubs.isEmpty()) return@analyzeMaybeDangling emptyList() + + buildInsertionEdit(ktFile, classOrObject, stubs, classIndent) + } } } }.getOrElse { e -> - logger.warn("Failed to compute implement-members edit", e) + if (e.isAnalysisCancellation()) { + // Cancelled, or preempted past the retry above: not a failure, and warn-logging it would + // bury the ones that are. + logger.debug("Implement-members edit for {} was cancelled", nioPath, e) + } else { + logger.warn("Failed to compute implement-members edit", e) + } emptyList() } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt index 1294b3259d..4f2012bef2 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt @@ -7,8 +7,9 @@ import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.lsp.kotlin.KotlinLanguageServer import com.itsaky.androidide.lsp.kotlin.compiler.AbstractCompilationEnvironment import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority -import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling +import com.itsaky.androidide.lsp.kotlin.compiler.modules.isAnalysisCancellation +import com.itsaky.androidide.lsp.kotlin.compiler.modules.retryingOnPreemption import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.utils.collectImportUsage import com.itsaky.androidide.lsp.kotlin.utils.organizedImportBlock @@ -19,6 +20,7 @@ import com.itsaky.androidide.lsp.models.Command import com.itsaky.androidide.lsp.models.DocumentChange import com.itsaky.androidide.lsp.models.TextEdit import com.itsaky.androidide.models.Range +import com.itsaky.androidide.progress.ICancelChecker import com.itsaky.androidide.resources.R import com.itsaky.androidide.tasks.createJobCancelChecker import org.slf4j.LoggerFactory @@ -41,7 +43,7 @@ class OrganizeImportsAction : BaseKotlinCodeAction() { val nioPath = data.requireFile().toPath() val env = server.compilationEnvironmentFor(nioPath) ?: return emptyList() // Ties the analysis to this action's coroutine: cancelling the action aborts the queued analysis. - return computeOrganizeEdit(env, nioPath, ScheduledCancelChecker(createJobCancelChecker())) + return computeOrganizeEdit(env, nioPath, createJobCancelChecker()) } /** @@ -57,20 +59,32 @@ class OrganizeImportsAction : BaseKotlinCodeAction() { internal fun computeOrganizeEdit( env: AbstractCompilationEnvironment, nioPath: Path, - cancelChecker: ScheduledCancelChecker, + cancelChecker: ICancelChecker, ): List = runCatching { - val ktFile = env.ktSymbolIndex.getCurrentKtFile(nioPath).get() ?: return emptyList() - if (ktFile.importDirectives.isEmpty()) return emptyList() - env.project.read { - val usage = analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, cancelChecker) { collectImportUsage(ktFile) } - val newText = organizedImportBlock(ktFile, usage) ?: return@read emptyList() - val range = ktFile.importList?.textRange?.toRange(ktFile) ?: return@read emptyList() - if (range == Range.NONE) return@read emptyList() - listOf(TextEdit(range, newText)) + // A user-invoked command: AnalysisPriority.COMMAND, retried once if keystroke-driven work + // preempts it (ADR 0011). Without the retry a preemption fell into the getOrElse below and + // organize-imports silently did nothing. The file is re-fetched per attempt because the + // preemptor also refreshed the live PSI. + retryingOnPreemption(cancelChecker, "Organize imports for $nioPath") { checker -> + val ktFile = env.ktSymbolIndex.getCurrentKtFile(nioPath).get() ?: return@retryingOnPreemption emptyList() + if (ktFile.importDirectives.isEmpty()) return@retryingOnPreemption emptyList() + env.project.read { + val usage = analyzeMaybeDangling(ktFile, AnalysisPriority.COMMAND, checker) { collectImportUsage(ktFile) } + val newText = organizedImportBlock(ktFile, usage) ?: return@read emptyList() + val range = ktFile.importList?.textRange?.toRange(ktFile) ?: return@read emptyList() + if (range == Range.NONE) return@read emptyList() + listOf(TextEdit(range, newText)) + } } }.getOrElse { e -> - logger.warn("Failed to organize imports", e) + if (e.isAnalysisCancellation()) { + // Cancelled, or preempted past the retry above: not a failure, and warn-logging it would + // bury the ones that are. + logger.debug("Organize imports for {} was cancelled", nioPath, e) + } else { + logger.warn("Failed to organize imports", e) + } emptyList() } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt index a4f3afef95..c4874af24d 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisScheduler.kt @@ -1,6 +1,8 @@ package com.itsaky.androidide.lsp.kotlin.compiler.modules import com.itsaky.androidide.progress.ICancelChecker +import org.slf4j.Logger +import org.slf4j.LoggerFactory import java.util.concurrent.CancellationException import java.util.concurrent.CopyOnWriteArrayList import java.util.concurrent.TimeUnit @@ -12,20 +14,37 @@ import kotlin.concurrent.withLock * lower-priority analysis that is currently running, and is served before any lower-priority request * that is merely waiting. * - * Order: [INDEXING] < [DIAGNOSTICS] < [INTERACTIVE] — interactive requests (completion, signature - * help) beat background diagnostics, which beats bulk indexing. + * Order: [INDEXING] < [DIAGNOSTICS] < [COMMAND] < [INTERACTIVE] — keystroke-driven requests + * (completion, signature help) beat user-invoked commands, which beat background diagnostics, which + * beat bulk indexing. * * [supersedesSamePriority] additionally lets a *newer* request preempt an in-flight one of the * **same** priority. On for [INTERACTIVE] only: rapid typing makes the in-flight request stale, so * the newer one cancels it and the superseded work is *discarded* (nothing reschedules it). Off for - * [DIAGNOSTICS]/[INDEXING], whose preempted work is re-queued — there same-priority preemption would - * livelock, two contenders endlessly re-queuing and re-preempting each other. + * the rest, whose preempted work is re-queued — there same-priority preemption would livelock, two + * contenders endlessly re-queuing and re-preempting each other. */ internal enum class AnalysisPriority( val supersedesSamePriority: Boolean, ) { INDEXING(supersedesSamePriority = false), DIAGNOSTICS(supersedesSamePriority = false), + + /** + * A command the user invoked from the code-actions menu: find usages, go-to-definition, organize + * imports, implement members. Distinct from [INTERACTIVE] because such a request is never *stale* — + * the user tapped a menu item and is watching a progress flashbar, so discarding the work produces + * a wrong answer rather than no answer. Hence [supersedesSamePriority] is off: two commands must + * not discard each other. + * + * Ordered below [INTERACTIVE] so a long command never starves the completion popup, which on a + * phone is part of how text gets entered. The cost is that a command *can* be preempted, so its + * call site must retry — and a long-running one should take the lock per unit of work (find usages + * takes it per candidate file) so a preemption costs one unit rather than the whole request. + * + * See ADR 0011 (docs/adr/0011-command-analysis-priority.md). + */ + COMMAND(supersedesSamePriority = false), INTERACTIVE(supersedesSamePriority = true), } @@ -96,6 +115,37 @@ internal class ScheduledCancelChecker( } } +/** + * Runs [attempt] and, if it was preempted, runs it exactly once more. + * + * The retry policy every [AnalysisPriority.COMMAND] call site needs. A command is preempted by + * keystroke-driven work ([AnalysisPriority.INTERACTIVE]), which - unlike a genuine cancellation - + * leaves the user's own request alive, so reporting the empty/failed result would be a lie: "no + * references" for a symbol that has plenty, or a silently skipped organize-imports. + * + * Two details this centralises: + * - **A fresh [ScheduledCancelChecker] per attempt.** [ScheduledCancelChecker.preempt] latches, so + * reusing the checker would make the retry abort at its first checkpoint. + * - **The whole pipeline is retried, not just the `analyze` block.** Whatever preempted the first + * attempt also refreshed the live PSI, unregistering the `KtFile` that attempt held; re-analyzing + * that stale file fails. So [attempt] must re-fetch the file too. + * + * A second preemption propagates - this is one retry, not a loop. + */ +internal inline fun retryingOnPreemption( + delegate: ICancelChecker, + label: String, + attempt: (ScheduledCancelChecker) -> R, +): R = + try { + attempt(ScheduledCancelChecker(delegate)) + } catch (e: AnalysisPreemptedException) { + schedulerLogger.debug("{} preempted; retrying once", label) + attempt(ScheduledCancelChecker(delegate)) + } + +internal val schedulerLogger: Logger = LoggerFactory.getLogger("AnalysisScheduler") + /** * A process-global, priority-aware, preemptive lock that serializes all Kotlin Analysis API access. * diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/services/ModuleDependentsProvider.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/services/ModuleDependentsProvider.kt index 5b057064c8..d334483f9c 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/services/ModuleDependentsProvider.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/services/ModuleDependentsProvider.kt @@ -9,59 +9,58 @@ import org.jetbrains.kotlin.cli.jvm.index.JavaRoot import org.jetbrains.kotlin.com.intellij.mock.MockProject import org.jetbrains.kotlin.com.intellij.util.containers.ContainerUtil.createConcurrentSoftMap -internal class ModuleDependentsProvider : KtLspService, KotlinModuleDependentsProviderBase() { - +internal class ModuleDependentsProvider : + KotlinModuleDependentsProviderBase(), + KtLspService { private lateinit var modules: List override fun setupWith( project: MockProject, index: KtSymbolIndex, modules: List, - libraryRoots: List + libraryRoots: List, ) { this.modules = modules } private val directDependentsByKtModule by lazy { - modules.asSequence() - .map { module -> - buildDependentsMap(module, module.allDirectDependencies()) - } - .reduce { acc, value -> acc + value } + buildDependentsMap(modules) { it.allDirectDependencies() } } private val transitiveDependentsByKtModule = createConcurrentSoftMap>() private val refinementDependentsByKtModule by lazy { - modules - .asSequence() - .map { buildDependentsMap(it, it.transitiveDependsOnDependencies.asSequence()) } - .reduce { acc, map -> acc + map } + buildDependentsMap(modules) { it.transitiveDependsOnDependencies.asSequence() } } - override fun getDirectDependents(module: KaModule): Set { - return directDependentsByKtModule[module].orEmpty() - } + override fun getDirectDependents(module: KaModule): Set = directDependentsByKtModule[module].orEmpty() - override fun getRefinementDependents(module: KaModule): Set { - return refinementDependentsByKtModule[module].orEmpty() - } + override fun getRefinementDependents(module: KaModule): Set = refinementDependentsByKtModule[module].orEmpty() - override fun getTransitiveDependents(module: KaModule): Set { - return transitiveDependentsByKtModule.computeIfAbsent(module) { key -> + override fun getTransitiveDependents(module: KaModule): Set = + transitiveDependentsByKtModule.computeIfAbsent(module) { key -> computeTransitiveDependents( - key + key, ) } - } } +/** + * Inverts every module's dependency edges into one dependency -> dependents map. + * + * Accumulated across all of [modules] rather than built per module and merged: `Map + Map` *replaces* a + * shared dependency's dependent set, so a module used by more than one other kept only the last of them + * and find usages then missed every call site in the rest. + */ private fun buildDependentsMap( - module: KaModule, - dependencies: Sequence, -): Map> = buildMap { - dependencies.forEach { dependency -> - if (dependency == module) return@forEach - val dependents = computeIfAbsent(dependency) { mutableSetOf() } - dependents.add(module) + modules: List, + dependenciesOf: (KtModule) -> Sequence, +): Map> = + buildMap> { + modules.forEach { module -> + dependenciesOf(module).forEach { dependency -> + if (dependency != module) { + getOrPut(dependency) { mutableSetOf() }.add(module) + } + } + } } -} diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/FindUsages.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/FindUsages.kt new file mode 100644 index 0000000000..afadb2c4ad --- /dev/null +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/FindUsages.kt @@ -0,0 +1,604 @@ +package com.itsaky.androidide.lsp.kotlin.navigation + +import com.itsaky.androidide.lsp.kotlin.compiler.AbstractCompilationEnvironment +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPreemptedException +import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority +import com.itsaky.androidide.lsp.kotlin.compiler.modules.KtModule +import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker +import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling +import com.itsaky.androidide.lsp.kotlin.compiler.modules.asFlatSequence +import com.itsaky.androidide.lsp.kotlin.compiler.modules.backingFilePath +import com.itsaky.androidide.lsp.kotlin.compiler.modules.isAnalysisCancellation +import com.itsaky.androidide.lsp.kotlin.compiler.modules.isSourceModule +import com.itsaky.androidide.lsp.kotlin.compiler.modules.retryingOnPreemption +import com.itsaky.androidide.lsp.kotlin.compiler.read +import com.itsaky.androidide.lsp.kotlin.compiler.services.ProjectStructureProvider +import com.itsaky.androidide.lsp.kotlin.utils.rangeOf +import com.itsaky.androidide.lsp.models.ReferenceParams +import com.itsaky.androidide.lsp.models.ReferenceResult +import com.itsaky.androidide.models.Location +import com.itsaky.androidide.models.Range +import com.itsaky.androidide.progress.ICancelChecker +import com.itsaky.androidide.projects.FileManager +import kotlinx.coroutines.future.await +import org.jetbrains.kotlin.analysis.api.KaSession +import org.jetbrains.kotlin.analysis.api.platform.projectStructure.KotlinModuleDependentsProvider +import org.jetbrains.kotlin.analysis.api.symbols.KaCallableSymbol +import org.jetbrains.kotlin.analysis.api.symbols.KaClassSymbol +import org.jetbrains.kotlin.analysis.api.symbols.KaConstructorSymbol +import org.jetbrains.kotlin.analysis.api.symbols.KaDeclarationSymbol +import org.jetbrains.kotlin.analysis.api.symbols.KaSymbol +import org.jetbrains.kotlin.analysis.api.symbols.KaSymbolLocation +import org.jetbrains.kotlin.analysis.api.symbols.KaSymbolVisibility +import org.jetbrains.kotlin.analysis.api.symbols.markers.KaNamedSymbol +import org.jetbrains.kotlin.analysis.api.symbols.pointers.KaSymbolPointer +import org.jetbrains.kotlin.analysis.api.symbols.sourcePsiSafe +import org.jetbrains.kotlin.analysis.low.level.api.fir.util.originalKtFile +import org.jetbrains.kotlin.com.intellij.psi.PsiElement +import org.jetbrains.kotlin.com.intellij.psi.PsiRecursiveElementWalkingVisitor +import org.jetbrains.kotlin.idea.references.mainReference +import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.psi.KtSimpleNameExpression +import org.slf4j.LoggerFactory +import java.io.IOException +import java.nio.file.Path + +private val logger = LoggerFactory.getLogger("FindUsages") + +/** How many times [planWithRetry] runs [planAt], each of which retries a preemption once itself. */ +private const val PLAN_ATTEMPTS = 2 + +/** + * Where a usage could possibly be written, derived from the target's visibility (R4). + * + * Kotlin's visibility rules are an exact bound, not a heuristic: a `private` declaration cannot be + * referenced from another file, and a `public` one cannot be referenced from a module that does not + * depend on its own. Narrowing here is what keeps the common cases cheap - a search on a local + * variable never leaves the open file - and it is also what makes the ticket's three resolution + * scopes fall out of one code path. + */ +internal sealed interface UsageSearchScope { + data class SingleFile( + val path: Path, + ) : UsageSearchScope + + data class Modules( + val modules: List, + ) : UsageSearchScope +} + +/** + * Everything the per-file search loop needs, computed once in the caret's analysis session. + * + * [matchSet] holds pointers rather than symbols because a [KaSymbol] cannot cross a session boundary, + * and each candidate file may be analyzed in a different one (R6). + */ +internal class SearchPlan( + val simpleName: String, + val matchSet: List>, + val scope: UsageSearchScope, +) + +/** + * Computes the usage result for [params]. + * + * Structured so that no lock spans the whole search (R9): the target is resolved under one short + * `project.read`, candidate selection holds nothing across the pass (`computeFiles` takes `project.read` + * per file, for one path lookup), and each candidate then takes its own read lock and analysis session. + * A whole-workspace search holding either for its full duration would block index refresh (which needs + * `project.write`) and would lose all its work to a single keystroke. + */ +context(env: AbstractCompilationEnvironment) +internal suspend fun findUsagesAt(params: ReferenceParams): ReferenceResult { + logger.debug("findUsagesAt requested for file={} position={}", params.file, params.position) + + if (params.cancelChecker.isCancelled()) { + logger.debug("References request for {} was cancelled before processing", params.file) + return ReferenceResult.empty() + } + + return try { + val plan = planWithRetry(params) ?: return ReferenceResult.empty() + val candidates = candidateFiles(plan, params.cancelChecker) + logger.debug("Usage search for '{}': {} candidate file(s)", plan.simpleName, candidates.size) + + val locations = + candidates + .flatMap { candidate -> + params.cancelChecker.abortIfCancelled() + usagesIn(candidate, plan, params.cancelChecker) + }.distinctBy { it.file to it.range } + .sortedWith(compareBy({ it.file.toString() }, { it.range.start.index })) + + logger.debug("Usage result for {}: {} location(s)", params.file, locations.size) + ReferenceResult(locations) + } catch (e: Throwable) { + if (e.isAnalysisCancellation()) { + logger.debug("Usage search for {} cancelled", params.file) + return ReferenceResult.empty() + } + logger.warn("Usage search failed for {}", params.file, e) + ReferenceResult.empty() + } +} + +/** + * [planAt], retried on a preemption that outlived its own single retry. + * + * Without this a *second* preemption escapes as an [AnalysisPreemptedException], which is a + * [java.util.concurrent.CancellationException], so [findUsagesAt]'s cancellation branch turns it into + * an empty result and the editor flashes "No references found" for a symbol with plenty - the wrong + * answer ADR 0011 exists to prevent. [usagesIn] draws the same distinction per candidate file. + * + * Retrying is cheap here: the plan phase is one file and one short session. Genuine cancellation is + * not caught - the delegate throws a plain [java.util.concurrent.CancellationException], not this + * subtype. + */ +context(env: AbstractCompilationEnvironment) +private suspend fun planWithRetry(params: ReferenceParams): SearchPlan? { + repeat(PLAN_ATTEMPTS) { + try { + return planAt(params) + } catch (e: AnalysisPreemptedException) { + logger.debug("Usage search plan for {} was preempted twice; retrying the plan", params.file) + } + } + + logger.warn("Usage search for {} abandoned: target resolution kept being preempted", params.file) + return null +} + +/** + * The search plan for [params]' caret, or null when it names nothing searchable. + * + * Its own short-lived read lock and analysis session, released before any candidate file is touched. + */ +context(env: AbstractCompilationEnvironment) +internal suspend fun planAt(params: ReferenceParams): SearchPlan? { + val offset = params.position.requireIndex() + + return retryingOnPreemption(params.cancelChecker, "Usage search target for ${params.file}") { cancelChecker -> + // Awaited per attempt and outside project.read, exactly as in findDefinitionAt: the refresh this + // waits on needs project.write, and a preemption invalidates the KtFile it returned. + val ktFile = env.ktSymbolIndex.getCurrentKtFile(params.file).await() + if (ktFile == null) { + logger.warn("File {} cannot be loaded for usage search", params.file) + null + } else { + cancelChecker.abortIfCancelled() + env.project.read { + val target = targetAtCaret(ktFile, offset) ?: return@read null + analyzeMaybeDangling(ktFile, AnalysisPriority.COMMAND, cancelChecker) { + planFor(target) + } + } + } + } +} + +/** The search plan for [target], or null when it names nothing searchable. */ +context(env: AbstractCompilationEnvironment) +private fun KaSession.planFor(target: CaretTarget): SearchPlan? { + val symbol = targetSymbol(target) ?: return null + val declaration = symbol.sourcePsiSafe() + if (declaration == null) { + // Not a workspace source: the stdlib, the framework, a library jar. Its usages are unreachable + // for the same reason go-to-definition cannot navigate to it. + logger.debug("Usage search target is not a workspace source; nothing to search") + return null + } + + val simpleName = prefilterName(symbol) ?: return null + + val matchSet = matchSet(symbol) + + return SearchPlan( + simpleName = simpleName, + matchSet = matchSet.map { it.createPointer() }, + scope = scopeOf(symbol, declaration, pathOf(declaration), matchSet), + ) +} + +/** + * The on-disk path of [declaration]'s file, or null when it has none. + * + * [backingFilePath] is tried before the VFS, exactly as in go-to-definition: the file the user is + * editing is a live [KtFile] built from the editor buffer, whose `virtualFile` is a non-physical + * `LightVirtualFile`. Reading the VFS alone would leave the common case pathless, and a pathless local + * or `private` target loses its single-file scope (R4) and widens to the whole module graph. + */ +private fun pathOf(declaration: PsiElement): Path? { + val psiFile = declaration.containingFile ?: return null + val ktFile = psiFile as? KtFile + + return (ktFile?.backingFilePath ?: ktFile?.originalKtFile?.backingFilePath) + ?: psiFile.virtualFile + ?.takeIf { it.fileSystem.protocol == "file" } + ?.let { runCatching { it.toNioPath() }.getOrNull() } +} + +/** + * The declaration [target] names. + * + * A [CaretTarget.Declaration] already *is* the declaration, so it answers through its own symbol; a + * [CaretTarget.Reference] answers through the same two resolution paths go-to-definition uses. + */ +private fun KaSession.targetSymbol(target: CaretTarget): KaDeclarationSymbol? = + when (target) { + is CaretTarget.Declaration -> { + runCatching { target.declaration.symbol }.getOrNull() + } + + is CaretTarget.Reference -> { + symbolsAt(target.element) + .also { + if (it.size > 1) { + // An ambiguous reference (overloads, broken code). Searching for the first candidate + // beats refusing to search; the alternative is a chooser UI the panel cannot host. + logger.debug("Reference at caret resolved to {} symbols; searching the first", it.size) + } + }.firstOrNull() as? KaDeclarationSymbol + } + }?.let { symbol -> + // A call through a subtype that does not redeclare the member resolves to a substituted fake + // override rather than to the declaration the user wrote. Normalise both sides of every + // comparison, starting here. + (symbol as? KaCallableSymbol)?.fakeOverrideOriginal ?: symbol + } + +/** + * The declarations a reference may resolve to and still count as a usage of [symbol] (R3). + * + * Two edges are added to the target itself: + * - **Workspace-source supers.** A call dispatched through `Base.foo` may reach `Derived.foo`, so it + * counts as a usage of it. The walk stops at the workspace boundary: `Any.toString` in the match set + * would make a usage search on an overridden `toString` report every `.toString()` call in the + * workspace, and a library super can never contribute a reportable result anyway. + * - **A classifier's constructors.** `Foo()` resolves to a constructor, not to the class, so without + * this a search on `class Foo` would miss every instantiation. Not applied in reverse: a target that + * *is* one constructor stays that constructor, because asking for usages of one overload is a + * deliberate act. + */ +private fun KaSession.matchSet(symbol: KaDeclarationSymbol): List = + buildList { + add(symbol) + + if (symbol is KaCallableSymbol) { + addAll( + symbol.allOverriddenSymbols + .map { it.fakeOverrideOriginal } + .filter { it.sourcePsiSafe() != null }, + ) + } + + if (symbol is KaClassSymbol) { + addAll(symbol.declaredMemberScope.constructors) + } + } + +/** + * The simple name to prefilter candidate files on, or null when there is none to search by. + * + * A constructor is written as its class's name, never as its own, so prefiltering on the symbol's own + * name would match nothing. + */ +private fun KaSession.prefilterName(symbol: KaDeclarationSymbol): String? { + val named = + if (symbol is KaConstructorSymbol) { + symbol.containingDeclaration as? KaNamedSymbol + } else { + symbol as? KaNamedSymbol + } + + return named?.name?.asString()?.takeUnless { it.isEmpty() } +} + +/** + * [symbol]'s search scope, per R4's visibility ladder. + * + * [matchSet] widens the module case: see the dependents comment below. + */ +context(env: AbstractCompilationEnvironment) +private fun KaSession.scopeOf( + symbol: KaDeclarationSymbol, + declaration: PsiElement, + declarationPath: Path?, + matchSet: List, +): UsageSearchScope { + val fileOnly = declarationPath?.let(UsageSearchScope::SingleFile) + + // A local is confined to its declaring block, and a private declaration to its file: Kotlin's + // private top-level is file-private, and a private member cannot escape the class body it is + // written in. Both are the cheap, exact cases. + val fileConfined = + symbol.location == KaSymbolLocation.LOCAL || symbol.visibility == KaSymbolVisibility.PRIVATE + if (fileOnly != null && fileConfined) { + return fileOnly + } + + val module = moduleOf(declaration) ?: return fileOnly ?: UsageSearchScope.Modules(sourceModules()) + + // internal is module-wide, and there is no associated test module to widen to: this project model + // builds one module per Gradle module from the main source set only. A file-confined target with no + // derivable path lands here too - it cannot be narrowed to one file, but it is still unreferenceable + // outside its own module, so it must not fall through to the dependents below. + if (fileConfined || symbol.visibility == KaSymbolVisibility.INTERNAL) { + return UsageSearchScope.Modules(listOf(module)) + } + + // Anything more visible can be referenced from any module that depends on this one. Dependents, + // not all modules: a module that cannot see the declaration cannot reference it. + // + // Every match-set member contributes its own module and dependents, not just the target's. A call + // written against a workspace `Base.foo` declared in a *dependency* module is a usage of the + // override (R3), and that module is not a dependent of the override's own - so scoping to the + // target's dependents alone would never look at it. + val provider = KotlinModuleDependentsProvider.getInstance(env.project) + val roots = LinkedHashSet() + roots.add(module) + for (member in matchSet) { + val memberDeclaration = member.sourcePsiSafe() ?: continue + moduleOf(memberDeclaration)?.let(roots::add) + } + + val searched = LinkedHashSet() + for (root in roots) { + searched.add(root) + provider.getTransitiveDependents(root).filterIsInstanceTo(searched) + } + + return UsageSearchScope.Modules(searched.toList()) +} + +context(env: AbstractCompilationEnvironment) +private fun moduleOf(declaration: PsiElement): KtModule? = + runCatching { + ProjectStructureProvider.getInstance(env.project).getModule(declaration, useSiteModule = null) as? KtModule + }.getOrNull() + +context(env: AbstractCompilationEnvironment) +private fun sourceModules(): List = + env.modules + .asFlatSequence() + .filter { it.isSourceModule } + .toList() + +/** + * The files worth parsing and resolving for [plan]. + * + * The prefilter is a one-directional over-approximation: a file that mentions the name but contains no + * usage is parsed and discarded, while a file that does not mention it cannot contain a named usage. + * [mentionsName] reads an open file's live editor buffer rather than its saved bytes, so a usage typed + * but not yet saved is still found - which matters here, because find usages is run *while* editing. + */ +context(env: AbstractCompilationEnvironment) +internal fun candidateFiles( + plan: SearchPlan, + cancelChecker: ICancelChecker, +): List = + when (val scope = plan.scope) { + // The declaration's own file always contains its name, so there is nothing to filter. + is UsageSearchScope.SingleFile -> { + listOf(scope.path) + } + + is UsageSearchScope.Modules -> { + scope.modules + .asSequence() + .filter { it.isSourceModule } + .flatMap { it.computeFiles(extended = true) } + // A source module's files are .kt *and* .java, and `ktFileFor` rejects a non-Kotlin path + // anyway (searching .java is a non-goal). Dropping them here, on the extension alone, + // stops a Java-heavy workspace spending most of the prefilter's I/O - the part the user + // waits on - reading files whose result is already known to be nothing. The extensions + // mirror `DocumentUtils.isKotlinFile`, which is what decides it downstream. + .filter { it.extension == "kt" || it.extension == "kts" } + .mapNotNull { runCatching { it.toNioPath() }.getOrNull() } + .distinct() + .filter { + // Checked per file: a whole-workspace scan is seconds of I/O, and cancelling must stop it + // rather than let it run to completion and then discard the result. + cancelChecker.abortIfCancelled() + mentionsName(it, plan.simpleName) + }.toList() + } + } + +/** + * Whether the file at [path] writes [name] as a whole word. + * + * Read line by line through [FileManager] rather than through `StringSearch.containsWord`: that helper + * scans only a file's first megabyte, so a usage below the mark is silently dropped, it does so through + * one process-global `ByteBuffer` the Java LSP mutates concurrently from its own threads, and it rethrows + * an unreadable file as a `RuntimeException` - which here would abort the whole search rather than skip + * one file. [FileManager] keeps the property that matters: an open file is matched against its live + * editor buffer. A name cannot span a line break, so matching per line is exact. + */ +private fun mentionsName( + path: Path, + name: String, +): Boolean = + try { + FileManager.getReader(path).use { reader -> + reader.lineSequence().any { it.containsWord(name) } + } + } catch (e: IOException) { + // One unreadable file must not lose the whole result. + logger.debug("Usage search could not prefilter candidate {}", path, e) + false + } + +/** Whether this line contains [name] bounded by non-identifier characters on both sides. */ +private fun String.containsWord(name: String): Boolean { + var at = indexOf(name) + while (at >= 0) { + val before = at - 1 + val after = at + name.length + if ((before < 0 || !this[before].isIdentifierChar()) && + (after >= length || !this[after].isIdentifierChar()) + ) { + return true + } + at = indexOf(name, at + 1) + } + return false +} + +private fun Char.isIdentifierChar(): Boolean = isLetterOrDigit() || this == '_' || this == '$' + +/** + * Every usage of [plan]'s target in the file at [path]. + * + * One analysis session per file, so a preemption costs this file rather than the whole search, and the + * live-PSI await stays outside `project.read` (R9). + */ +context(env: AbstractCompilationEnvironment) +private suspend fun usagesIn( + path: Path, + plan: SearchPlan, + delegate: ICancelChecker, +): List = + try { + retryingOnPreemption(delegate, "Usage search in $path") { cancelChecker -> + val ktFile = ktFileFor(path) + if (ktFile == null) { + logger.debug("Skipping candidate {}: no PSI", path) + emptyList() + } else { + env.project.read { + // The name filter is pure PSI, so it runs before the analysis session opens. A text + // prefilter hit whose only mention is a comment or a string literal must not cost an + // analysis-lock acquisition, a FIR session and a match-set restore to rule out - and on a + // short, common name most candidates are exactly that. + val named = namedReferences(ktFile, plan.simpleName, cancelChecker) + if (named.isEmpty()) { + emptyList() + } else { + analyzeMaybeDangling(ktFile, AnalysisPriority.COMMAND, cancelChecker) { + matchingReferences(named, plan, ktFile, path, cancelChecker) + } + } + } + } + } + } catch (e: AnalysisPreemptedException) { + // A preemption that outlived retryingOnPreemption's single retry is keystroke-driven work winning + // the lock, not the user cancelling. Rethrowing it would discard every location collected so far + // and report "no references" for a symbol with plenty, so it costs this file like any other + // failure. Genuine cancellation still propagates below (R12). + logger.debug("Usage search gave up on candidate {}: preempted twice", path) + emptyList() + } catch (e: Throwable) { + if (e.isAnalysisCancellation()) throw e + // One unresolvable file must not lose the whole result. + logger.debug("Usage search skipped candidate {}", path, e) + emptyList() + } + +/** + * PSI for a candidate file: refreshed to the live editor buffer when the file is open, the indexed + * on-disk instance otherwise. + * + * The open case must be awaited here, outside `project.read`, because the refresh it waits on needs + * `project.write`. `getKtFile` cannot do it - it runs under `project.read` inside Analysis API + * services, so it only ever peeks the live cache. + */ +context(env: AbstractCompilationEnvironment) +private suspend fun ktFileFor(path: Path): KtFile? = + if (FileManager.isActive(path)) { + env.ktSymbolIndex.getCurrentKtFile(path).await() + } else { + env.ktSymbolIndex.getKtFile(path) + } + +/** + * The simple-name references in [ktFile] written as [simpleName]. + * + * PSI alone, so it can rule a candidate file out before any analysis session is opened. It is also what + * implements "convention references are not discovered": `a + b` contains no `plus` token, so it is never + * a candidate. + * + * Filters during the walk rather than collecting every [KtSimpleNameExpression] and filtering after: on + * the case the text prefilter is worst at - a short, common name in a large file - the intermediate list + * is the bulk of the allocation, and the walk is long enough to need a cancellation checkpoint of its own. + */ +private fun namedReferences( + ktFile: KtFile, + simpleName: String, + cancelChecker: ICancelChecker, +): List { + val found = mutableListOf() + + ktFile.accept( + object : PsiRecursiveElementWalkingVisitor() { + override fun visitElement(element: PsiElement) { + cancelChecker.abortIfCancelled() + if (element is KtSimpleNameExpression && element.getReferencedName() == simpleName) { + found.add(element) + } + super.visitElement(element) + } + }, + ) + + return found +} + +/** + * The [references] that resolve into [plan]'s match set. + * + * Match-set pointers are restored **once** for this session; [KaSymbol] equality within a single + * session compares the underlying FIR symbol, so it is the right comparison once both sides come from + * the same session (R6). + */ +private fun KaSession.matchingReferences( + references: List, + plan: SearchPlan, + ktFile: KtFile, + path: Path, + cancelChecker: ICancelChecker, +): List { + val targets = plan.matchSet.mapNotNull { it.restoreSymbol() } + if (targets.isEmpty()) { + // Under-reporting beats reporting something false, so a pointer that will not restore drops this + // file rather than falling back to a looser comparison. + logger.debug("No match-set symbol restored in {}; skipping", path) + return emptyList() + } + + return references.mapNotNull { reference -> + cancelChecker.abortIfCancelled() + if (resolvesInto(reference, targets)) locationOf(reference, ktFile, path) else null + } +} + +/** Whether [reference] resolves to one of [targets]. */ +private fun KaSession.resolvesInto( + reference: KtSimpleNameExpression, + targets: List, +): Boolean = + runCatching { + reference.mainReference + .resolveToSymbols() + .asSequence() + .map { (it as? KaCallableSymbol)?.fakeOverrideOriginal ?: it } + .any { resolved -> targets.any { it == resolved } } + }.getOrElse { + if (it.isAnalysisCancellation()) throw it + logger.debug("Could not resolve '{}'", reference.text, it) + false + } + +/** [reference]'s name range as an editor [Location], or null when the file has no document. */ +private fun locationOf( + reference: KtSimpleNameExpression, + ktFile: KtFile, + path: Path, +): Location? { + val range = rangeOf(reference.getReferencedNameElement(), ktFile) + if (range == Range.NONE) { + logger.debug("No document for {}; dropping usage", path) + return null + } + return Location(path, range) +} diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/GoToDefinition.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/GoToDefinition.kt index 9380913215..da360c5536 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/GoToDefinition.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/GoToDefinition.kt @@ -3,10 +3,10 @@ package com.itsaky.androidide.lsp.kotlin.navigation import com.itsaky.androidide.lsp.kotlin.compiler.AbstractCompilationEnvironment import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPreemptedException import com.itsaky.androidide.lsp.kotlin.compiler.modules.AnalysisPriority -import com.itsaky.androidide.lsp.kotlin.compiler.modules.ScheduledCancelChecker import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling import com.itsaky.androidide.lsp.kotlin.compiler.modules.backingFilePath import com.itsaky.androidide.lsp.kotlin.compiler.modules.isAnalysisCancellation +import com.itsaky.androidide.lsp.kotlin.compiler.modules.retryingOnPreemption import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.utils.rangeOf import com.itsaky.androidide.lsp.kotlin.utils.toRange @@ -78,8 +78,11 @@ private fun KaSession.resolvedLocations( * * Resolution over broken code throws, and a throw must read as "not found" rather than crash the * request, so both paths are guarded. + * + * Shared with find usages, which resolves the reference under the caret the same way before searching + * for what it names. */ -private fun KaSession.symbolsAt(element: KtElement): List = +internal fun KaSession.symbolsAt(element: KtElement): List = runCatching { element.mainReference ?.resolveToSymbols() @@ -197,47 +200,35 @@ internal suspend fun findDefinitionAt(params: DefinitionParams): DefinitionResul return try { val offset = params.position.requireIndex() - // Navigation is user-initiated: run at INTERACTIVE priority so it preempts background - // diagnostics/indexing and is discarded when a newer interactive request wins. - // params.cancelChecker is request-scoped (CancellableRequestParams), so wrap it directly. - // - // INTERACTIVE.supersedesSamePriority is true, so a concurrent completion/signature-help - // request can preempt this lookup even though the user's own request is still alive - unlike - // a genuine cancellation, that coroutine survives, so surfacing an empty result would be a lie - // ("Definition not found" for a reference that resolves fine). One retry, with a fresh - // checker, covers it without turning this into a retry loop. - suspend fun attempt(): List { - // Awaited per attempt, not once: whatever preempted the first attempt also refreshed the - // live PSI, unregistering the KtFile that attempt held, and analyzing it again would fail. - // - // Safe to await a (possibly blocking) refresh here: this runs outside any project.read/write - // block, so it can't deadlock against the refresh's project.write. Refreshed to the open - // document's current version, so the caret offset and the PSI it indexes into come from the - // same text - a stale snapshot points at the wrong element. (params.position is fixed by the - // request, so a retry after the user typed can still be one edit behind; that resolves to - // the wrong element or to nothing, never to a crash.) - val ktFile = env.ktSymbolIndex.getCurrentKtFile(params.file).await() - if (ktFile == null) { - logger.warn("File {} cannot be loaded for definition lookup", params.file) - return emptyList() - } - - val cancelChecker = ScheduledCancelChecker(params.cancelChecker) - cancelChecker.abortIfCancelled() - return env.project.read { - val element = referenceAtCaret(ktFile, offset) ?: return@read emptyList() - analyzeMaybeDangling(ktFile, AnalysisPriority.INTERACTIVE, cancelChecker) { - definitionLocations(element, cancelChecker) - } - } - } - + // Navigation is a user-invoked command: AnalysisPriority.COMMAND preempts background + // diagnostics/indexing but yields to keystroke-driven completion, and is never discarded by + // another command. It can still be preempted by INTERACTIVE, so it retries once (see + // retryingOnPreemption, and ADR 0011). params.cancelChecker is request-scoped + // (CancellableRequestParams), so it is the delegate the per-attempt checker wraps. val locations = - try { - attempt() - } catch (e: AnalysisPreemptedException) { - logger.debug("Definition lookup for {} preempted; retrying once", params.file) - attempt() + retryingOnPreemption(params.cancelChecker, "Definition lookup for ${params.file}") { cancelChecker -> + // Awaited per attempt, not once: whatever preempted the first attempt also refreshed the + // live PSI, unregistering the KtFile that attempt held, and analyzing it again would fail. + // + // Safe to await a (possibly blocking) refresh here: this runs outside any project.read/write + // block, so it can't deadlock against the refresh's project.write. Refreshed to the open + // document's current version, so the caret offset and the PSI it indexes into come from the + // same text - a stale snapshot points at the wrong element. (params.position is fixed by the + // request, so a retry after the user typed can still be one edit behind; that resolves to + // the wrong element or to nothing, never to a crash.) + val ktFile = env.ktSymbolIndex.getCurrentKtFile(params.file).await() + if (ktFile == null) { + logger.warn("File {} cannot be loaded for definition lookup", params.file) + emptyList() + } else { + cancelChecker.abortIfCancelled() + env.project.read { + val element = referenceAtCaret(ktFile, offset) ?: return@read emptyList() + analyzeMaybeDangling(ktFile, AnalysisPriority.COMMAND, cancelChecker) { + definitionLocations(element, cancelChecker) + } + } + } } logger.debug("Definition result for {}: {} location(s)", params.file, locations.size) DefinitionResult(locations) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/ReferenceAtCaret.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/ReferenceAtCaret.kt index b954a8d72f..755352688a 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/ReferenceAtCaret.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/ReferenceAtCaret.kt @@ -75,7 +75,12 @@ internal fun referenceAtCaret( return null } -private fun navigableLeafAt( +/** + * The leaf token at [offset] if a caret there could name something, else null. Shared with + * [targetAtCaret], which applies the same accept-list before asking whether the leaf is a + * declaration's own name. + */ +internal fun navigableLeafAt( file: KtFile, offset: Int, ): PsiElement? { diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/TargetAtCaret.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/TargetAtCaret.kt new file mode 100644 index 0000000000..3d7e2e2179 --- /dev/null +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/TargetAtCaret.kt @@ -0,0 +1,86 @@ +package com.itsaky.androidide.lsp.kotlin.navigation + +import org.jetbrains.kotlin.com.intellij.psi.PsiElement +import org.jetbrains.kotlin.com.intellij.psi.util.PsiTreeUtil +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.psi.KtNamedDeclaration +import org.slf4j.LoggerFactory + +private val logger = LoggerFactory.getLogger("TargetAtCaret") + +/** + * What a caret names, for a feature that starts *from* a declaration rather than navigating to one. + * + * Find usages can be invoked from either end - on the declaration itself, or on any reference to it - + * and the two need different resolution, so the distinction is made once here rather than re-derived + * by a type test later. + */ +internal sealed interface CaretTarget { + /** The caret sits on [declaration]'s own name identifier. Its symbol is the search target. */ + data class Declaration( + val declaration: KtNamedDeclaration, + ) : CaretTarget + + /** The caret sits on a reference. Resolving [element] yields the search target. */ + data class Reference( + val element: KtElement, + ) : CaretTarget +} + +/** + * What the caret at [offset] in [file] names, or null when it names nothing. + * + * Declaration-first: a caret on a declaration's own name targets *that declaration*, and only a caret + * that names nothing declarable is interpreted as a reference. The order is observable for a + * destructuring entry, which is both at once - `x` in `val (x, y) = p` targets the local `x` here, + * while go-to-definition navigates from the same caret to `component1`. + * + * Callers must hold the project read lock. Pure PSI: no analysis session is needed or used. + */ +internal fun targetAtCaret( + file: KtFile, + offset: Int, +): CaretTarget? { + declarationAtCaret(file, offset)?.let { return CaretTarget.Declaration(it) } + + // Not a declaration's name, so fall back to go-to-definition's reference lookup, which repeats the + // leaf lookup above. One extra findElementAt is worth leaving that helper's contract untouched: + // it must keep returning null for a declaration's own name, which is the caret we just handled. + return referenceAtCaret(file, offset)?.let(CaretTarget::Reference)?.also { + logger.debug("Caret at {} in {} names a reference", offset, file.name) + } +} + +/** + * The declaration whose own name the caret at [offset] sits on, or null. + * + * Both candidate leaves are tried, not just the first navigable one. `referenceAtCaret` can stop at + * the first, because it retries only when the primary leaf names nothing at all; here the primary + * leaf can be navigable in its own right and still not be a name - a caret just past `fun target` + * lands on `(`, which is navigable for the invoke convention. Checking only that leaf would make a + * caret one character past a declaration's name find nothing. + */ +private fun declarationAtCaret( + file: KtFile, + offset: Int, +): KtNamedDeclaration? = + ( + declarationNamedBy(navigableLeafAt(file, offset)) + ?: declarationNamedBy(navigableLeafAt(file, (offset - 1).coerceAtLeast(0))) + )?.also { + logger.debug("Caret at {} in {} names declaration '{}'", offset, file.name, it.name) + } + +/** + * The declaration [leaf] is the name identifier of, or null. + * + * The identity check is what makes this precise: every caret has some enclosing declaration - a call + * site's nearest one is the function containing it - so proximity alone would target the container + * for every reference in the file. + */ +private fun declarationNamedBy(leaf: PsiElement?): KtNamedDeclaration? { + leaf ?: return null + val declaration = PsiTreeUtil.getParentOfType(leaf, KtNamedDeclaration::class.java) ?: return null + return declaration.takeIf { it.nameIdentifier === leaf } +} diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.kt index 117ce7ef9e..352e81eaec 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.kt @@ -5,6 +5,7 @@ import com.itsaky.androidide.lsp.actions.CommentLineAction import com.itsaky.androidide.lsp.actions.UncommentLineAction import com.itsaky.androidide.lsp.kotlin.KotlinCodeActionsMenu.KT_LANG import com.itsaky.androidide.lsp.kotlin.actions.AddImportAction +import com.itsaky.androidide.lsp.kotlin.actions.FindReferencesAction import com.itsaky.androidide.lsp.kotlin.actions.GoToDefinitionAction import com.itsaky.androidide.lsp.kotlin.actions.ImplementMembersAction import com.itsaky.androidide.lsp.kotlin.actions.NullSafetyAction @@ -34,6 +35,7 @@ class KotlinCodeActionTooltipTagTest { CommentLineAction.idFor(KT_LANG) to TooltipTag.EDITOR_CODE_ACTIONS_KT_COMMENT, UncommentLineAction.idFor(KT_LANG) to TooltipTag.EDITOR_CODE_ACTIONS_KT_UNCOMMENT, GoToDefinitionAction.ID to TooltipTag.EDITOR_CODE_ACTIONS_KT_GOTO_DEF, + FindReferencesAction.ID to TooltipTag.EDITOR_CODE_ACTIONS_KT_FIND_REFS, AddImportAction.ID to TooltipTag.EDITOR_CODE_ACTIONS_KT_IMPORT_CLASS, OrganizeImportsAction.ID to TooltipTag.EDITOR_CODE_ACTIONS_KT_ORGANIZE_IMPORTS, NullSafetyAction.ID to TooltipTag.EDITOR_CODE_ACTIONS_KT_NULL_SAFETY_FIX, diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt index 72d55ce4d2..319917428d 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt @@ -418,6 +418,177 @@ class AnalysisSerializationTest : KtLspTest() { assertThat(newerRan.get()).isTrue() } + /** + * ADR 0011's central property. Two user-invoked commands must not discard each other - before + * [AnalysisPriority.COMMAND] existed they both ran at [AnalysisPriority.INTERACTIVE], where the + * newer one superseded the older and the older silently produced nothing. + * + * The holder polls its own checker while waiting. Preemption is cooperative, so a holder that only + * blocks would keep the lock even when wrongly flagged, the second command could not enter before + * the release either way, and the entry assertions alone would pass with + * [AnalysisPriority.supersedesSamePriority] set on [AnalysisPriority.COMMAND]. + */ + @Test(timeout = 10_000) + fun `a command does not supersede an in-flight command`() { + val holderChecker = ScheduledCancelChecker(ICancelChecker.NOOP) + val holding = CountDownLatch(1) + val release = CountDownLatch(1) + val firstPreempted = AtomicBoolean(false) + val secondEntered = AtomicBoolean(false) + + val first = + Thread { + try { + withAnalysisLock(AnalysisPriority.COMMAND, holderChecker) { + holding.countDown() + while (!release.await(10, TimeUnit.MILLISECONDS)) { + holderChecker.abortIfCancelled() + } + } + } catch (e: AnalysisPreemptedException) { + firstPreempted.set(true) + } + } + first.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + val second = + Thread { + withAnalysisLock(AnalysisPriority.COMMAND, ScheduledCancelChecker(ICancelChecker.NOOP)) { + secondEntered.set(true) + } + } + second.start() + + // Give the second command time to (incorrectly) barge in. + Thread.sleep(300) + val enteredWhileHeld = secondEntered.get() + + release.countDown() + first.join(5_000) + second.join(5_000) + + assertThat(firstPreempted.get()).isFalse() + assertThat(enteredWhileHeld).isFalse() + assertThat(secondEntered.get()).isTrue() + } + + @Test(timeout = 10_000) + fun `a command preempts an in-flight diagnostics`() { + val holderChecker = ScheduledCancelChecker(ICancelChecker.NOOP) + val holding = CountDownLatch(1) + val preempted = AtomicBoolean(false) + val commandRan = AtomicBoolean(false) + + val diagnostics = + Thread { + try { + withAnalysisLock(AnalysisPriority.DIAGNOSTICS, holderChecker) { + holding.countDown() + repeat(2_000) { + holderChecker.abortIfCancelled() + Thread.sleep(5) + } + } + } catch (e: AnalysisPreemptedException) { + preempted.set(true) + } + } + diagnostics.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + val command = + Thread { + withAnalysisLock(AnalysisPriority.COMMAND, ScheduledCancelChecker(ICancelChecker.NOOP)) { + commandRan.set(true) + } + } + command.start() + command.join(5_000) + diagnostics.join(5_000) + + assertThat(preempted.get()).isTrue() + assertThat(commandRan.get()).isTrue() + } + + /** + * The cost ADR 0011 accepts in exchange for typing responsiveness: a command *is* preemptable, so + * every command call site retries (see [retryingOnPreemption]). + */ + @Test(timeout = 10_000) + fun `keystroke-driven work preempts an in-flight command`() { + val holderChecker = ScheduledCancelChecker(ICancelChecker.NOOP) + val holding = CountDownLatch(1) + val preempted = AtomicBoolean(false) + val completionRan = AtomicBoolean(false) + + val command = + Thread { + try { + withAnalysisLock(AnalysisPriority.COMMAND, holderChecker) { + holding.countDown() + repeat(2_000) { + holderChecker.abortIfCancelled() + Thread.sleep(5) + } + } + } catch (e: AnalysisPreemptedException) { + preempted.set(true) + } + } + command.start() + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue() + + val completion = + Thread { + withAnalysisLock(AnalysisPriority.INTERACTIVE, ScheduledCancelChecker(ICancelChecker.NOOP)) { + completionRan.set(true) + } + } + completion.start() + completion.join(5_000) + command.join(5_000) + + assertThat(preempted.get()).isTrue() + assertThat(completionRan.get()).isTrue() + } + + @Test(timeout = 10_000) + fun `retryingOnPreemption runs a preempted attempt exactly once more with a fresh checker`() { + val attempts = AtomicInteger(0) + + val result = + retryingOnPreemption(ICancelChecker.NOOP, "test") { checker -> + // A latched checker would abort the retry immediately, so each attempt must get its own. + assertThat(checker.isCancelled()).isFalse() + if (attempts.incrementAndGet() == 1) { + checker.preempt() + checker.abortIfCancelled() + } + "done" + } + + assertThat(attempts.get()).isEqualTo(2) + assertThat(result).isEqualTo("done") + } + + @Test(timeout = 10_000) + fun `retryingOnPreemption propagates a second preemption rather than looping`() { + val attempts = AtomicInteger(0) + + val thrown = + runCatching { + retryingOnPreemption(ICancelChecker.NOOP, "test") { checker -> + attempts.incrementAndGet() + checker.preempt() + checker.abortIfCancelled() + } + }.exceptionOrNull() + + assertThat(attempts.get()).isEqualTo(2) + assertThat(thrown).isInstanceOf(AnalysisPreemptedException::class.java) + } + @Test(timeout = 10_000) fun `same priority diagnostics does not preempt an in-flight diagnostics`() { val holding = CountDownLatch(1) diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/FindUsagesLiveDocumentTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/FindUsagesLiveDocumentTest.kt new file mode 100644 index 0000000000..6e3c18539c --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/FindUsagesLiveDocumentTest.kt @@ -0,0 +1,93 @@ +package com.itsaky.androidide.lsp.kotlin.navigation + +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.eventbus.events.editor.DocumentCloseEvent +import com.itsaky.androidide.eventbus.events.editor.DocumentOpenEvent +import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest +import com.itsaky.androidide.lsp.models.ReferenceParams +import com.itsaky.androidide.models.Position +import com.itsaky.androidide.progress.ICancelChecker +import com.itsaky.androidide.projects.FileManager +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Test +import java.nio.file.Path + +/** + * R5's live-buffer tier: a usage that exists only in an unsaved editor buffer must still be found. + * + * Separate from [FindUsagesTest] because it needs `enableParserEventSystem`, so that the `KtFile` built + * from the buffer is physical the way production's is (see `KtLspTestEnvironment`). + * + * This is the case find usages is most often run in - you search *while* editing - and the one a + * disk-only prefilter silently gets wrong: the file would never be selected as a candidate, so it would + * never be parsed and the usage would simply not appear. + */ +class FindUsagesLiveDocumentTest : KtLspTest() { + override val enableParserEventSystem = true + + private val openedPaths = mutableListOf() + + @After + fun closeDocs() { + openedPaths.forEach { FileManager.onDocumentClose(DocumentCloseEvent(it)) } + openedPaths.clear() + } + + private fun openDocument( + path: Path, + content: String, + ) { + FileManager.onDocumentOpen(DocumentOpenEvent(path, content, 1)) + openedPaths.add(path) + } + + @Test + fun `a usage typed into an unsaved buffer is found`() { + val declarationText = "fun target() {}" + val declaration = createSourceFile("Declaration.kt", declarationText) + val declarationPath = Path.of(declaration.virtualFile.path) + + // On disk this file contains no usage at all, so a prefilter reading saved bytes would skip it. + val usage = createSourceFile("Usage.kt", "fun caller() { }") + val usagePath = Path.of(usage.virtualFile.path) + val editedText = "fun caller() { target() }" + openDocument(usagePath, editedText) + + val params = + ReferenceParams( + declarationPath, + Position(0, 0, declarationText.indexOf("target")), + true, + ICancelChecker.NOOP, + ) + val locations = runBlocking { context(env) { findUsagesAt(params) } }.locations + + assertThat(locations).hasSize(1) + assertThat(locations[0].file).isEqualTo(usagePath) + assertThat(locations[0].range.start.index).isEqualTo(editedText.indexOf("target()")) + } + + @Test + fun `a usage deleted in an unsaved buffer is not reported`() { + val declarationText = "fun target() {}" + val declaration = createSourceFile("GoneDeclaration.kt", declarationText) + val declarationPath = Path.of(declaration.virtualFile.path) + + // The saved bytes still mention the name, so this file is still a candidate; it is resolution, + // not the prefilter, that must reject it. + val usage = createSourceFile("GoneUsage.kt", "fun caller() { target() }") + val usagePath = Path.of(usage.virtualFile.path) + openDocument(usagePath, "fun caller() { }") + + val params = + ReferenceParams( + declarationPath, + Position(0, 0, declarationText.indexOf("target")), + true, + ICancelChecker.NOOP, + ) + + assertThat(runBlocking { context(env) { findUsagesAt(params) } }.locations).isEmpty() + } +} diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/FindUsagesTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/FindUsagesTest.kt new file mode 100644 index 0000000000..ae894b71f1 --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/FindUsagesTest.kt @@ -0,0 +1,326 @@ +package com.itsaky.androidide.lsp.kotlin.navigation + +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest +import com.itsaky.androidide.lsp.kotlin.fixtures.TestSourceModuleSpec +import com.itsaky.androidide.lsp.models.ReferenceParams +import com.itsaky.androidide.models.Position +import com.itsaky.androidide.progress.ICancelChecker +import kotlinx.coroutines.runBlocking +import org.junit.Test +import java.nio.file.Path + +/** + * The search itself: match set, visibility-derived scope, candidate selection and matching. + * + * Driven through `findUsagesAt`/`planAt` rather than the individual helpers, so each case exercises + * the real request path. + */ +class FindUsagesTest : KtLspTest() { + override val moduleSpecs = + listOf( + TestSourceModuleSpec("lib"), + TestSourceModuleSpec("app", dependsOn = listOf("lib")), + ) + + private class Source( + val path: Path, + val text: String, + ) + + private fun source( + module: String, + name: String, + text: String, + ): Source = Source(Path.of(createSourceFile(module, name, text).virtualFile.path), text) + + private fun paramsAt( + source: Source, + marker: String, + delta: Int = 0, + cancelChecker: ICancelChecker = ICancelChecker.NOOP, + ): ReferenceParams { + val offset = + source.text.indexOf(marker).also { check(it >= 0) { "marker '$marker' not in source" } } + delta + return ReferenceParams(source.path, Position(0, 0, offset), true, cancelChecker) + } + + /** Usages for a caret at `marker + delta` in [source], as `fileName:startOffset` pairs. */ + private fun usagesAt( + source: Source, + marker: String, + delta: Int = 0, + cancelChecker: ICancelChecker = ICancelChecker.NOOP, + ): List = + runBlocking { + context(env) { findUsagesAt(paramsAt(source, marker, delta, cancelChecker)) } + .locations + .map { "${it.file.fileName}:${it.range.start.index}" } + } + + private fun scopeAt( + source: Source, + marker: String, + delta: Int = 0, + ): UsageSearchScope? = + runBlocking { + context(env) { planAt(paramsAt(source, marker, delta))?.scope } + } + + private fun expected( + source: Source, + vararg markers: String, + ): List = + markers.map { marker -> + val index = source.text.indexOf(marker).also { check(it >= 0) { "marker '$marker' not in source" } } + "${source.path.fileName}:$index" + } + + @Test + fun `a same-file call is a usage`() { + val file = source("app", "SameFile.kt", "fun target() {}\nfun caller() { target() }") + + assertThat(usagesAt(file, "fun target", delta = 5)).isEqualTo(expected(file, "target() }")) + } + + @Test + fun `every call in the file is reported, ordered by offset`() { + val text = "fun target() {}\nfun a() { target() }\nfun b() { target() }" + val file = source("app", "Many.kt", text) + + val usages = usagesAt(file, "fun target", delta = 5) + + assertThat(usages).hasSize(2) + assertThat(usages).isEqualTo( + listOf( + "Many.kt:${text.indexOf("target() }")}", + "Many.kt:${text.lastIndexOf("target() }")}", + ), + ) + } + + @Test + fun `the declaration itself is never reported`() { + // includeDeclaration is ignored (R7): a target with no usages must come back empty so the editor + // flashes "no references" rather than silently selecting the declaration the caret is already on. + val file = source("app", "Unused.kt", "fun unused() {}") + + assertThat(usagesAt(file, "fun unused", delta = 5)).isEmpty() + } + + @Test + fun `an inter-file call in the same module is a usage`() { + val declaration = source("app", "Decl.kt", "fun shared() {}") + val usage = source("app", "Use.kt", "fun caller() { shared() }") + + assertThat(usagesAt(declaration, "fun shared", delta = 5)).isEqualTo(expected(usage, "shared()")) + } + + @Test + fun `an inter-module call is a usage`() { + val declaration = source("lib", "LibApi.kt", "fun libFun() {}") + val usage = source("app", "AppUse.kt", "fun caller() { libFun() }") + + assertThat(usagesAt(declaration, "fun libFun", delta = 5)).isEqualTo(expected(usage, "libFun()")) + } + + @Test + fun `searching from a reference finds the same usages as from the declaration`() { + val declaration = source("app", "FromRefDecl.kt", "fun shared() {}") + val usage = source("app", "FromRefUse.kt", "fun caller() { shared() }") + + val fromDeclaration = usagesAt(declaration, "fun shared", delta = 5) + val fromReference = usagesAt(usage, "shared()", delta = 1) + + assertThat(fromReference).isEqualTo(fromDeclaration) + assertThat(fromReference).isNotEmpty() + } + + @Test + fun `a constructor call is a usage of the class`() { + val declaration = source("app", "Widget.kt", "class Widget") + val usage = source("app", "WidgetUse.kt", "fun caller() { Widget() }") + + assertThat(usagesAt(declaration, "class Widget", delta = 7)).isEqualTo(expected(usage, "Widget()")) + } + + @Test + fun `an import is a usage`() { + val declaration = source("lib", "Imported.kt", "package lib\n\nclass Imported") + val usage = source("app", "ImportUse.kt", "package app\n\nimport lib.Imported\n\nfun caller(p: Imported) {}") + + assertThat(usagesAt(declaration, "class Imported", delta = 7)) + .isEqualTo(expected(usage, "Imported\n", "Imported) {}")) + } + + @Test + fun `a same-named declaration elsewhere is not a usage`() { + // Matching is by symbol, not by name: the decoy shares the name and nothing else. Separate + // packages are load-bearing - two top-level `fun ambiguous()` in one package is a redeclaration, + // and the decoy's call then legitimately binds to whichever the resolver picks first. + val declaration = source("app", "Real.kt", "package real\n\nfun ambiguous() {}") + source("app", "Decoy.kt", "package decoy\n\nfun ambiguous() {}\nfun decoyCaller() { ambiguous() }") + + assertThat(usagesAt(declaration, "fun ambiguous", delta = 5)).isEmpty() + } + + @Test + fun `a call dispatched through a workspace supertype is a usage of the override`() { + val declaration = + source( + "app", + "Hierarchy.kt", + """ + interface Base { + fun render() + } + + class Impl : Base { + override fun render() {} + } + """.trimIndent(), + ) + val usage = source("app", "HierarchyUse.kt", "fun caller(b: Base) { b.render() }") + + // The call statically resolves to Base.render, but may dispatch to Impl.render at runtime. + assertThat(usagesAt(declaration, "override fun render", delta = 14)) + .isEqualTo(expected(usage, "render() }")) + } + + @Test + fun `a call dispatched through a supertype in a dependency module is a usage of the override`() { + source("lib", "DepBase.kt", "package lib\n\nopen class DepBase {\n\topen fun paint() {}\n}") + val call = source("lib", "DepBaseUse.kt", "package lib\n\nfun caller(b: DepBase) { b.paint() }") + val override = + source( + "app", + "DepDerived.kt", + "package app\n\nimport lib.DepBase\n\nclass DepDerived : DepBase() {\n\toverride fun paint() {}\n}", + ) + + // The call is written in lib, a *dependency* of app rather than a dependent of it, so scoping to + // the override's own module and its dependents would never look at it. + assertThat(usagesAt(override, "override fun paint", delta = 14)) + .isEqualTo(expected(call, "paint() }")) + } + + @Test + fun `an override is scoped to its supertype's module as well as its own`() { + source("lib", "ScopeBase.kt", "package lib\n\nopen class ScopeBase {\n\topen fun tick() {}\n}") + val override = + source( + "app", + "ScopeDerived.kt", + "package app\n\nimport lib.ScopeBase\n\nclass ScopeDerived : ScopeBase() {\n\toverride fun tick() {}\n}", + ) + + val scope = scopeAt(override, "override fun tick", delta = 14) + + assertThat(scope).isInstanceOf(UsageSearchScope.Modules::class.java) + // app, the override's own module, plus lib, its supertype's. lib's dependents re-add app. + assertThat((scope as UsageSearchScope.Modules).modules.map { it.id }).containsExactly("app", "lib") + } + + @Test + fun `an override of a library member does not match unrelated calls to it`() { + // The up-walk stops at the workspace boundary: with Any.toString in the match set this would + // report every .toString() call in the workspace. + val declaration = + source( + "app", + "Renderer.kt", + "class Renderer {\n\toverride fun toString(): String = \"r\"\n}", + ) + source("app", "OtherToString.kt", "fun caller(value: Int) = value.toString()") + + assertThat(usagesAt(declaration, "override fun toString", delta = 14)).isEmpty() + } + + @Test + fun `a local declaration is scoped to its own file`() { + val file = source("app", "LocalScope.kt", "fun caller() {\n\tval count = 1\n\tprintln(count)\n}") + + assertThat(scopeAt(file, "val count", delta = 4)) + .isEqualTo(UsageSearchScope.SingleFile(file.path)) + assertThat(usagesAt(file, "val count", delta = 4)).isEqualTo(expected(file, "count)")) + } + + @Test + fun `a private top-level declaration is scoped to its own file`() { + val file = source("app", "PrivateScope.kt", "private fun hidden() {}\nfun caller() { hidden() }") + + assertThat(scopeAt(file, "fun hidden", delta = 5)) + .isEqualTo(UsageSearchScope.SingleFile(file.path)) + } + + @Test + fun `an internal declaration is scoped to its own module`() { + val file = source("lib", "InternalScope.kt", "internal fun shared() {}") + + val scope = scopeAt(file, "fun shared", delta = 5) + + assertThat(scope).isInstanceOf(UsageSearchScope.Modules::class.java) + assertThat((scope as UsageSearchScope.Modules).modules.map { it.id }).hasSize(1) + } + + @Test + fun `a public declaration is scoped to its module and dependents`() { + val file = source("lib", "PublicScope.kt", "fun exported() {}") + + val scope = scopeAt(file, "fun exported", delta = 5) + + assertThat(scope).isInstanceOf(UsageSearchScope.Modules::class.java) + // lib plus app, which depends on it. + assertThat((scope as UsageSearchScope.Modules).modules.map { it.id }).hasSize(2) + } + + /** + * Direction 1 of the cross-language split: a Java-source *target* is in scope, because resolving a + * Kotlin reference to it already works. Searching `.java` files for usages is not: a source module's + * files include them, so `candidateFiles` drops them on the extension before reading anything, and + * `getKtFile` would reject one anyway. + */ + @Test + fun `a workspace Java declaration is a valid target`() { + env.createFile("lib", "lib/JavaGreeter.java", "package lib;\npublic class JavaGreeter {}") + val usage = + source( + "app", + "app/JavaUse.kt", + "package app\n\nimport lib.JavaGreeter\n\nfun make(): JavaGreeter? = null", + ) + + // The caret is on the Kotlin reference; the target it resolves to is the Java class. + assertThat(usagesAt(usage, ": JavaGreeter", delta = 2)) + .isEqualTo(expected(usage, "JavaGreeter\n", "JavaGreeter? = null")) + } + + @Test + fun `a reference to a stdlib symbol yields no usages`() { + val file = source("app", "Stdlib.kt", "fun caller() { listOf(1) }") + + assertThat(usagesAt(file, "listOf", delta = 1)).isEmpty() + } + + @Test + fun `a caret that names nothing yields no usages`() { + val file = source("app", "Nothing.kt", "fun caller() { }") + + assertThat(usagesAt(file, "{ }", delta = 2)).isEmpty() + } + + @Test + fun `a cancelled request yields no usages rather than throwing`() { + val file = source("app", "Cancelled.kt", "fun target() {}\nfun caller() { target() }") + + assertThat(usagesAt(file, "fun target", delta = 5, cancelChecker = ICancelChecker.CANCELLED)).isEmpty() + } + + @Test + fun `a property read and write are both usages`() { + val text = "var counter = 0\nfun caller() {\n\tcounter = 1\n\tprintln(counter)\n}" + val file = source("app", "Property.kt", text) + + assertThat(usagesAt(file, "var counter", delta = 5)).hasSize(2) + } +} diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/TargetAtCaretTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/TargetAtCaretTest.kt new file mode 100644 index 0000000000..94673eb9d5 --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/TargetAtCaretTest.kt @@ -0,0 +1,167 @@ +package com.itsaky.androidide.lsp.kotlin.navigation + +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.lsp.kotlin.compiler.read +import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest +import org.jetbrains.kotlin.psi.KtClass +import org.jetbrains.kotlin.psi.KtDestructuringDeclarationEntry +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtOperationReferenceExpression +import org.jetbrains.kotlin.psi.KtParameter +import org.jetbrains.kotlin.psi.KtProperty +import org.junit.Test + +/** + * R2's caret rules for find usages. Pure PSI, no analysis session. + * + * The interesting cases are the ones where this must answer *differently* from + * [ReferenceAtCaretTest]: a caret on a declaration's own name is nothing to navigate to, but it is + * the normal place to search for usages from. + */ +class TargetAtCaretTest : KtLspTest() { + /** The target for a caret at `text.indexOf(marker) + delta` in a file containing [text]. */ + private fun targetAt( + name: String, + text: String, + marker: String, + delta: Int = 0, + ): CaretTarget? { + val file = createSourceFile(name, text) + val offset = + text.indexOf(marker).also { check(it >= 0) { "marker '$marker' not in source" } } + delta + return env.project.read { targetAtCaret(file, offset) } + } + + private fun assertDeclaration( + target: CaretTarget?, + name: String, + ): CaretTarget.Declaration { + assertThat(target).isInstanceOf(CaretTarget.Declaration::class.java) + val declaration = (target as CaretTarget.Declaration) + assertThat(declaration.declaration.name).isEqualTo(name) + return declaration + } + + @Test + fun `caret on a function's own name targets that function`() { + val target = targetAt("A.kt", "fun target() {}", "target", delta = 1) + assertDeclaration(target, "target") + assertThat((target as CaretTarget.Declaration).declaration).isInstanceOf(KtNamedFunction::class.java) + } + + @Test + fun `caret on a class's own name targets that class`() { + val target = targetAt("B.kt", "class Widget", "Widget", delta = 2) + assertDeclaration(target, "Widget") + assertThat((target as CaretTarget.Declaration).declaration).isInstanceOf(KtClass::class.java) + } + + @Test + fun `caret on a property's own name targets that property`() { + val target = targetAt("C.kt", "fun caller() {\n\tval count = 1\n}", "count", delta = 1) + assertDeclaration(target, "count") + assertThat((target as CaretTarget.Declaration).declaration).isInstanceOf(KtProperty::class.java) + } + + @Test + fun `caret on a parameter's own name targets that parameter`() { + val target = targetAt("D.kt", "fun caller(value: Int) = value", "value", delta = 1) + assertDeclaration(target, "value") + assertThat((target as CaretTarget.Declaration).declaration).isInstanceOf(KtParameter::class.java) + } + + /** + * The contrast that makes this file necessary: `referenceAtCaret` returns null here, because a + * declaration's own name is not something go-to-definition can navigate to. + */ + @Test + fun `a caret that go-to-definition rejects still yields a target`() { + val text = "fun target() {}" + val file = createSourceFile("E.kt", text) + val offset = text.indexOf("target") + 1 + + env.project.read { + assertThat(referenceAtCaret(file, offset)).isNull() + assertThat(targetAtCaret(file, offset)).isInstanceOf(CaretTarget.Declaration::class.java) + } + } + + @Test + fun `caret on a call targets the reference, not the enclosing declaration`() { + // The nearest enclosing KtNamedDeclaration is `caller`, so this only works because the + // declaration check requires the caret's leaf to *be* that declaration's name identifier. + val target = targetAt("F.kt", "fun target() {}\nfun caller() { target() }", "{ target()", delta = 3) + assertThat(target).isInstanceOf(CaretTarget.Reference::class.java) + } + + @Test + fun `caret one past a declaration's name targets that declaration`() { + // The character after `target` is '(', which is navigable in its own right (the invoke + // convention), so this asserts the declaration check runs on the primary leaf before any + // reference interpretation of it. + val target = targetAt("G.kt", "fun target() {}", "target", delta = 6) + assertDeclaration(target, "target") + } + + @Test + fun `caret on a local declaration inside a lambda targets that declaration`() { + // ReferenceAtCaretTest asserts this same caret navigates nowhere. Searching for usages of a + // local function is legitimate, so it must not inherit that null. + val target = + targetAt( + "H.kt", + "fun run(block: () -> Unit) {}\nfun caller() { run { fun inner() {} } }", + "inner", + delta = 1, + ) + assertDeclaration(target, "inner") + } + + /** + * Q15c / R2: a destructuring entry is simultaneously a declaration and a convention reference to + * `componentN`. Go-to-definition reads it as the reference; find usages reads it as the + * declaration, so a search from here finds usages of `x` rather than of `component1`. + */ + @Test + fun `caret on a destructuring entry targets the entry as a declaration`() { + val target = + targetAt( + "I.kt", + "data class P(val x: Int, val y: Int)\nfun caller(p: P) { val (x, y) = p }", + "(x, y)", + delta = 1, + ) + assertDeclaration(target, "x") + assertThat((target as CaretTarget.Declaration).declaration) + .isInstanceOf(KtDestructuringDeclarationEntry::class.java) + } + + @Test + fun `caret on an operator targets the operation reference`() { + val target = + targetAt( + "J.kt", + "class P { operator fun plus(other: P): P = this }\nfun caller(a: P, b: P) { a + b }", + "a + b", + delta = 2, + ) + assertThat(target).isInstanceOf(CaretTarget.Reference::class.java) + assertThat((target as CaretTarget.Reference).element) + .isInstanceOf(KtOperationReferenceExpression::class.java) + } + + @Test + fun `caret on whitespace yields no target`() { + assertThat(targetAt("K.kt", "fun caller() { }", " ", delta = 1)).isNull() + } + + @Test + fun `caret in a comment yields no target`() { + assertThat(targetAt("L.kt", "// target here\nfun target() {}", "target here", delta = 1)).isNull() + } + + @Test + fun `caret on a non-navigable keyword yields no target`() { + assertThat(targetAt("M.kt", "fun target() {}", "fun", delta = 1)).isNull() + } +} diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImplementMembersEndToEndTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImplementMembersEndToEndTest.kt index f3d017163f..fe0ebae8f0 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImplementMembersEndToEndTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImplementMembersEndToEndTest.kt @@ -3,6 +3,7 @@ package com.itsaky.androidide.lsp.kotlin.utils import com.itsaky.androidide.lsp.kotlin.actions.ImplementMembersAction import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest import com.itsaky.androidide.lsp.models.TextEdit +import com.itsaky.androidide.progress.ICancelChecker import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test @@ -15,7 +16,7 @@ class ImplementMembersEndToEndTest : KtLspTest() { ): List { createSourceFile("Main.kt", content) val mainPath = env.sourceRoots.first().resolve("Main.kt") - return ImplementMembersAction().computeImplementMembersEdit(env, mainPath, caret, noopCancelChecker()) + return ImplementMembersAction().computeImplementMembersEdit(env, mainPath, caret, ICancelChecker.NOOP) } /** Applies a single edit's newText over its [TextEdit.range] index span, returning the resulting text. */ diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt index 535a4ae7b9..5fd9ea2ac2 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt @@ -4,6 +4,7 @@ import com.itsaky.androidide.lsp.kotlin.actions.OrganizeImportsAction import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest import com.itsaky.androidide.models.Position import com.itsaky.androidide.models.Range +import com.itsaky.androidide.progress.ICancelChecker import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test @@ -31,7 +32,7 @@ class OrganizeImportsEndToEndTest : KtLspTest() { val mainPath = env.sourceRoots.first().resolve("Main.kt") // Drive the action's real plumbing: fetch-before-read ordering + full guard chain. - val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, noopCancelChecker()) + val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, ICancelChecker.NOOP) assertEquals(1, edits.size) assertEquals("import lib.Used", edits.single().newText) @@ -63,7 +64,7 @@ class OrganizeImportsEndToEndTest : KtLspTest() { """.trimIndent(), ) val mainPath = env.sourceRoots.first().resolve("Main.kt") - val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, noopCancelChecker()) + val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, ICancelChecker.NOOP) // Already organized -> no edit. A dropped import would produce a rewrite that removes it. assertTrue("constructor-only import must survive", edits.isEmpty()) } @@ -86,7 +87,7 @@ class OrganizeImportsEndToEndTest : KtLspTest() { """.trimIndent(), ) val mainPath = env.sourceRoots.first().resolve("Main.kt") - val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, noopCancelChecker()) + val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, ICancelChecker.NOOP) assertTrue("annotation-only import must survive", edits.isEmpty()) } @@ -109,7 +110,7 @@ class OrganizeImportsEndToEndTest : KtLspTest() { """.trimIndent(), ) val mainPath = env.sourceRoots.first().resolve("Main.kt") - val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, noopCancelChecker()) + val edits = OrganizeImportsAction().computeOrganizeEdit(env, mainPath, ICancelChecker.NOOP) assertTrue("typealias-only import used as constructor must survive", edits.isEmpty()) } }