diff --git a/build.gradle.kts b/build.gradle.kts index e05942b176..97bda1ab3b 100755 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -67,6 +67,16 @@ buildscript { } } +// `jacocoAggregateReport` dependsOn every subproject's `testV8DebugUnitTest` and `sonarqube` +// dependsOn that, so a hard test failure skips both - even under `--continue` - and the analysis +// run uploads an empty coverage artifact with no Sonar analysis at all. That, not any individual +// module's broken suite, is the only reason `ignoreFailures` exists here: keep test failures +// non-fatal when the analysis chain is what was asked for, and let every ordinary build gate on them. +val analysisRun = + gradle.startParameter.taskNames.any { + it.substringAfterLast(':') in setOf("sonar", "sonarqube", "jacocoAggregateReport") + } + subprojects { plugins.apply("jacoco") @@ -78,14 +88,28 @@ subprojects { FDroidConfig.load(project) tasks.withType { - // Continue even if tests fail, so coverage data is written - ignoreFailures = true + ignoreFailures = analysisRun + + // Gradle's default test-worker heap is 512m, too small for the Robolectric + + // Kotlin Analysis API suites (:lsp:kotlin peaks near 240m and keeps growing). + // Keep it explicit so the suites fail on a real regression, not on the default. + maxHeapSize = "1g" // Backstop: kill any individual Test task that runs longer than 10 minutes. // Prevents a single hung test JVM (e.g. the Tooling API child) from burning // the entire CI job budget. timeout.set(Duration.ofMinutes(10)) + // A test worker's default working dir is the module directory, so an unpathed + // -XX:+HeapDumpOnOutOfMemoryError drops a heap dump of up to maxHeapSize into the source + // tree, untracked and not gitignored. Keep dumps under build/ instead, one per task. + val heapDumpFile = + layout.buildDirectory + .file("test-heapdumps/$name.hprof") + .get() + .asFile + doFirst { heapDumpFile.parentFile.mkdirs() } + // JPMS opens required by the unit-test stack on JDK 17+: // - jdk.unsupported/sun.misc: HiddenApiBypass. reflectively // resolves sun.misc.Unsafe; without this the IDEApplication @@ -105,6 +129,14 @@ subprojects { "--add-opens=java.base/java.util=ALL-UNNAMED", "--add-opens=java.base/java.util.concurrent=ALL-UNNAMED", "--add-opens=jdk.unsupported/sun.misc=ALL-UNNAMED", + // An OutOfMemoryError inside a test deadlocks Gradle's TestWorker: the + // OOM unwinds the runQueue.take() loop, whose finally needs the TestWorker + // monitor that the in-flight stop() already holds while blocked on + // runQueue.put() (capacity 1). The worker then never exits and the build + // hangs. Exiting on the first OOM turns that hang into a task failure. + "-XX:+ExitOnOutOfMemoryError", + "-XX:+HeapDumpOnOutOfMemoryError", + "-XX:HeapDumpPath=${heapDumpFile.absolutePath}", ) // Attach jacoco agent diff --git a/docs/adr/0010-navigation-resolves-via-analysis-api.md b/docs/adr/0010-navigation-resolves-via-analysis-api.md new file mode 100644 index 0000000000..c72d3fad1d --- /dev/null +++ b/docs/adr/0010-navigation-resolves-via-analysis-api.md @@ -0,0 +1,46 @@ +# 0010. Kotlin navigation resolves via the Analysis API, not the symbol index + +- **Status:** Proposed +- **Date:** 2026-07-27 +- **Deciders:** Code On The Go team + +## Context + +The K2 Kotlin LSP carries a substantial symbol-indexing stack: `JvmSymbolIndex` over library jars, `KtFileMetadataIndex` over source files, and `KtSymbolIndex` tying them together, all SQLite-backed and refreshed by background workers. Completion and add-import lean on it heavily, and it is the cheap way to answer "what symbols named X exist in this workspace". + +Navigation features - go-to-definition (ADFA-4823) and find usages (ADFA-4824) - look superficially similar: given a name, find where it lives. A reader who has just read the completion code will reasonably expect navigation to query the same index. + +It cannot. The index stores names, kinds, visibility, and containing-class metadata, but **no source offsets** - `JvmSymbol`/`JvmSymbolInfo` have nowhere to put a declaration's position, and `KtFileMetadata` records only a file path plus its symbol keys. An index hit narrows the answer to a file at best; something still has to parse that file to find where in it the declaration sits. Worse, the index answers by *name*, while navigation must answer by *resolution* - which of the seven overloads of `foo`, through this module's dependency graph and content scopes, does this call site actually bind to. + +## Decision + +**Kotlin navigation resolves through the Analysis API and PSI only. The symbol indexes are not consulted.** + +- Given a caret offset, find the reference, then `analyze(ktFile) { reference.mainReference.resolveToSymbols() }`, falling back to `resolveToCall()` for convention references (`a + b`, `a[i]`, `by lazy`, destructuring, `for` loops) that have no name reference to resolve. +- Convert each resolved symbol to its PSI declaration, and the declaration to a file plus a name-identifier range. +- Correctness of scoping - module dependencies, content scopes, visibility - is delegated to the analysis session rather than reimplemented over index rows. + +## Consequences + +**Positive** + +- Results are *resolved*, not name-matched: the right overload, the right receiver, the right module. +- Module dependency graphs and content scopes are respected by construction. No second, divergent notion of "which module can see what" to keep in sync with `ProjectStructureProvider`. +- One code path for all three resolution scopes (same-file, inter-file, inter-module), so the test matrix covers behaviour rather than plumbing. + +**Negative / costs** + +- **Navigation requires a live analysis session.** Before one exists, go-to-definition returns nothing; it cannot degrade to an index-only answer. The user-facing gap - no way to say "still indexing" rather than "not found" - is cross-cutting across every LSP feature and remains unsolved. +- Resolving a cross-module target means building PSI for the target file, which is more work than an index row lookup. Acceptable for a user-initiated, cancellable, one-at-a-time request; it would not be acceptable for a per-keystroke feature. +- Symbols with no source PSI - stdlib, framework, any library jar - are simply unreachable. Library navigation would need decompilation or source-jar extraction, neither of which exists in the tree. + +## Alternatives considered + +- **Index-first, analysis fallback** - rejected: the index has no declaration offsets, so it can only narrow to a file and a second pass must parse it anyway. Extra machinery, no saved work, and a name-matched shortlist that can disagree with what the call site actually binds to. +- **Index-only for cross-module targets** - rejected: two divergent code paths for the same user action, and cross-module results would be position-less, so they could not be selected in the editor. +- **Extend the index to store declaration offsets** - rejected for now: offsets go stale on every edit, so the index would need write-through invalidation on document change to stay trustworthy for navigation, and it still would not answer overload resolution. Revisit only if navigation latency becomes a measured problem. + +## Related + +- [docs/features/kotlin-goto-definition.md](../features/kotlin-goto-definition.md) - the first feature built on this decision +- [ADR 0001](0001-prefer-room-for-persistence.md) - persistence choices for the indexes this ADR declines to use diff --git a/docs/adr/README.md b/docs/adr/README.md index 4f5fac8179..9bb6db0c4a 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -23,3 +23,4 @@ Format is lightweight **MADR / Nygard**: Context → Decision → Consequences | [0007](0007-strictmode-whitelist-engine.md) | Enforce StrictMode via a custom whitelist engine | Proposed | | [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 | diff --git a/docs/features/kotlin-goto-definition.md b/docs/features/kotlin-goto-definition.md new file mode 100644 index 0000000000..30cf72a7ba --- /dev/null +++ b/docs/features/kotlin-goto-definition.md @@ -0,0 +1,186 @@ +# Kotlin go-to-definition (K2 LSP) + +- **Ticket:** ADFA-4823 (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` + +Jump from a Kotlin symbol reference to the declaration it resolves to, across three scopes: same file, another file in the same module, another module in the workspace. + +`KotlinLanguageServer.findDefinition` dispatches into `navigation/`; everything downstream of it (the editor's multi-result panel, `DefinitionResult`, `IDEEditor`) already existed. + +## Language + +**Reference**: +A Kotlin PSI element that names something declared elsewhere - an identifier in a call, a type position, an import, an annotation, a named argument. +_Avoid_: usage, symbol reference, occurrence. + +**Declaration**: +The PSI element a reference resolves to. This is what the feature navigates to, even though the feature is called "go-to-definition". +_Avoid_: definition (reserve that for the user-facing verb), target, decl. + +**Candidate**: +One declaration a reference resolved to. A reference normally has exactly one; ambiguous overloads and broken code can yield several. +_Avoid_: match, result, hit. + +**Location**: +The existing `com.itsaky.androidide.models.Location` (file + range) that a candidate is converted into for transport to the editor. A candidate has a location only if its declaration lives in a workspace source. +_Avoid_: position, target, site. + +**Workspace source**: +A `.kt` or `.java` file inside a source module's content roots, including generated sources under `build/generated/**`. Contrast with a **binary symbol**, which lives in a jar or the JDK and has no source file on device. +_Avoid_: project file, local file, user code. + +**Resolution scope**: +Where the declaration lives relative to the reference: **same-file**, **inter-file** (same module), or **inter-module**. These are the three scopes the ticket enumerates; they are a way to talk about coverage, not three code paths. + +**Convention reference**: +A reference with no name to point at, where the compiler picks a declaration by convention: `a + b` -> `plus`, `a[i]` -> `get`, `val (x, y) = p` -> `component1`, `by lazy` -> `getValue`, a `for` loop -> `iterator`. It resolves through a resolved-call lookup rather than a name reference, but navigates to a declaration like any other. +_Avoid_: implicit call, synthetic reference. + +## Scope + +### In scope + +Any reference whose declaration is a workspace source, in all three resolution scopes. Kotlin references into workspace `.java` sources count - Java files are part of a Kotlin source module's content scope (`AbstractSourceModule.computeBaseContentScope`), so a Kotlin call into a Java class in the same project navigates. + +Convention references are in scope too. They need a second resolution path on top of `mainReference` - a resolved-call lookup - but that path is a single expression (`resolveToCall()?.successfulFunctionCallOrNull()?.symbol`) that `utils/ImportUsageCollector.kt` already uses, and everything after it (symbol -> PSI -> `Location`) is shared with the name-reference path. KDoc `[links]` need no second path at all: `KtElement.mainReference` already has a `KDocName` branch. + +### Out of scope + +Binary symbols - the Kotlin stdlib, the Android framework, and every library jar. There is no decompiler and no source-jar handling anywhere in the repo, and `IDELanguageClientImpl.showDocument` only opens a real, existing, UTF-8 file on disk. A jump onto `listOf` or `Activity` reports "Definition not found", the same as a genuine resolution failure. Distinguishing the two would mean a new field on `DefinitionResult`, which is shared with the Java and XML servers. + +## Requirements + +**R1 - Trigger.** A "Go to definition" item appears in the editor code-actions menu for `.kt`/`.kts` files, mirroring Java's. It reuses `R.string.action_goto_definition` and delegates to `ILspEditor.findDefinition()`. It is invisible for non-Kotlin files. + +It carries its **own** tooltip tag, `EDITOR_CODE_ACTIONS_KT_GOTO_DEF = "editor.codeactions.kotlin.gotodef"` - a new constant in `TooltipTag.kt`, not Java's `EDITOR_CODE_ACTIONS_GOTO_DEF` - so Kotlin and Java go-to-definition can carry different tooltip text. This follows the existing split for fix-imports (`editor.codeactions.kotlin.fiximports`). Tooltip *content* is keyed by tag in the tooltips database, which is not in this repo; `ToolTipManager.getTooltip` logs and returns null on a miss, so the new tag shows no tooltip text until a row exists for it. That row is a hand-off item, not code. + +**R2 - Caret mapping.** Resolution starts from the reference at the caret offset. If there is no reference there, retry at `offset - 1`, so a caret resting just past an identifier still works - touch caret placement is imprecise. + +A caret position is navigable only if its token is one of: an identifier; `this` or `super`; `in` (for-loop convention) or `by` (property delegate); `(` (`invoke`) or `[` (`get`/`set`); an operator token; a KDoc name. A caret on whitespace, a comment, a string body, a brace, a literal, or any other keyword yields no candidates. + +A caret on a declaration's **own** name also yields no candidates; there is no self-jump, which would read as a broken no-op. The one exception is a destructuring entry - `x` in `val (x, y) = p` is both a declaration and a convention reference, and it navigates to `component1`. + +Both rules are enforced by construction rather than by filtering afterwards: an accept-list of caret tokens, and a walk from that token up **at most two PSI levels** to find the reference. The cap is what prevents a caret on the name of a local `fun foo` declared inside `run { ... }` from climbing out of the declaration into the enclosing call and navigating to `run`. + +**R3 - Live offsets.** The caret offset is interpreted against the live document contents, not an async-lagged PSI snapshot - an offset resolved against stale text points at the wrong element. In practice that means `KtSymbolIndex.getCurrentKtFile(path)`, which refreshes PSI to the open document's current version, rather than a cached or on-disk `KtFile`. + +**R4 - Coverage.** Two resolution paths, tried in order: `KtElement.mainReference.resolveToSymbols()`, then - when there is no `mainReference` or it yields nothing - `resolveToCall()?.successfulFunctionCallOrNull()?.symbol`. Everything after that point is shared, so the paths differ only in how the symbols are obtained. + +| Reference | Declaration navigated to | +|---|---| +| local variable, parameter | its declaration | +| property read/write | the property (or its Java field) | +| function call, extension call | the function | +| infix / operator-named call written as a call | the function | +| class, object, interface, enum entry | the classifier | +| type reference, generic argument | the classifier | +| constructor call | the invoked constructor; the class when there is no explicit one | +| import directive | the imported declaration | +| package reference | nothing (no candidates) | +| annotation | the annotation class | +| typealias reference | the typealias declaration, not its expansion | +| companion / object reference | the companion or object | +| named argument | the corresponding parameter | +| `super`, super constructor delegation | the supertype's member or constructor | +| label (`return@foo`) | the labelled expression | +| KDoc `[link]` | the linked declaration | +| operator (`a + b`, `a[i]`, `f()`) | `plus`, `get`/`set`, `invoke` | +| destructuring entry (`val (x, y) = p`) | that entry's `componentN` | +| property delegate (caret on `by`) | `getValue`/`setValue` | +| for-loop (caret on `in`) | `iterator`, `hasNext`, `next` - three candidates, so the multi-result panel | + +**R5 - Candidates.** All resolved symbols are considered, not just the first. Candidates whose declaration has no workspace source PSI are dropped - the test is symbol **origin** (`sourcePsiSafe()`, non-null only for `SOURCE` and `JAVA_SOURCE`), not a null check on the PSI, because a library symbol has a non-null PSI pointing into a class file. Survivors are deduplicated by file plus range and ordered by file path then start offset. + +**R6 - Location range.** A candidate's range covers the declaration's **name identifier** (`PsiNameIdentifierOwner.nameIdentifier`), so the editor highlights just the name. When there is no name token - an anonymous object, an `init` block, an implicit primary constructor - the range collapses to the declaration's start offset. A property accessor resolves to its enclosing property; a constructor with no PSI of its own resolves to its class, which is what makes R4's "the class when there is no explicit constructor" fall out. + +**R7 - Result handling.** The server returns `DefinitionResult(locations)`; the editor's existing logic in `IDEEditor.onFindDefinitionResult` applies unchanged: + +- empty -> flash `msg_no_definition` +- one location in the current file -> `setSelection` +- one location elsewhere -> `showDocument`, which opens the file and selects the range +- more than one -> `languageClient.showLocations`, the search-results panel + +**R8 - Generated sources.** Declarations under `build/generated/**` are valid targets; they are already in a module's content roots (`AndroidModule.getSourceDirectories`). Two documented caveats: the target reflects the last build, and `R` normally comes from `R.jar`, so `R.string.foo` reports "Definition not found". The generated file opens read-write, which is pre-existing IDE-wide behaviour. + +**R9 - Not ready.** When there is no analysis session yet, or the file maps to no module (a script, a file outside the content roots), the server logs and returns an empty result. There is no dedicated "still indexing" signal; that gap is cross-cutting across every LSP feature and is not solved here. + +**R10 - Responsiveness.** The request runs off the main thread through the editor's existing cancellable progress flashbar (`msg_finding_definition`). No `project.read` on the UI thread - the project lock is a plain `ReentrantReadWriteLock` and a UI-thread read can block behind a background index write. The action's `prepare()`, which *does* run on the UI thread, adds no lock, index or analysis work of its own; it reads only `ActionData`. It is **not** filesystem-free, because `BaseKotlinCodeAction.prepare` calls `DocumentUtils.isKotlinFile`, which stats the file (`Files.exists` plus `Files.isDirectory`). That is pre-existing and shared by every Java and Kotlin code action, so it is not fixed here, but it does mean the menu still does two stat calls per action on the UI thread - worth a separate ticket against `DocumentUtils`. The live-document await (R3) happens **outside** `project.read`, because the refresh it waits on needs `project.write` and awaiting it under the read lock would deadlock (`KotlinSignatureHelp.kt` records the same constraint). `params.cancelChecker` is honoured between candidate resolutions and before returning. No numeric latency budget: the repo has no LSP benchmark harness, so a number would be unverifiable. + +**R11 - Failure isolation.** A resolution failure returns an empty result and logs; it never propagates an exception to the editor and never leaves the progress flashbar up. + +## Non-goals + +- **Find usages** - ADFA-4824, the sibling subtask. It will share the reference-at-caret resolution helper. +- **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. +- **A gesture trigger** (long-press or double-tap to navigate). That is an editor-wide UX change that would apply to Java too. +- **Read-only buffers for generated files.** + +## Acceptance criteria + +1. Go to definition appears in the code-actions menu of a Kotlin file and is absent in a non-Kotlin file. +2. Same-file: a call to a top-level function declared above it selects that function's name. +3. Inter-file: a reference to a class declared in a sibling file opens that file with the class name selected. +4. Inter-module: a reference to a class in a dependency module opens that module's file. +5. A reference to a workspace Java class from Kotlin navigates. +6. A reference to a stdlib or framework symbol flashes "Definition not found". +7. An ambiguous reference lists its candidates in the search-results panel. +8. The caret placed immediately after an identifier resolves the same as inside it. +9. The caret on a declaration's own name produces no navigation. +10. Cancelling the progress flashbar mid-request leaves the editor responsive and unchanged. +11. Invoking before the project finishes loading flashes "Definition not found" and does not crash or hang. +12. The caret on an operator between two workspace types navigates to that type's operator function. +13. The caret on `by` in a delegated property whose delegate is a workspace class navigates to its `getValue`. +14. The caret on a destructuring entry navigates to the matching `componentN`. +15. The caret on a KDoc `[link]` navigates to the linked declaration. +16. The caret on whitespace, inside a comment, or on a non-navigable keyword produces no navigation. + +## 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). + +``` +GoToDefinitionAction.execAction lsp/kotlin/actions + -> ILspEditor.findDefinition() editor (unchanged: progress flashbar + cancel checker) + -> KotlinLanguageServer.findDefinition(params) + guards: settings.definitionsEnabled(), DocumentUtils.isKotlinFile + compilationEnvironmentFor(params.file) ?: empty [R9] + -> context(env) { findDefinitionAt(params) } navigation/GoToDefinition.kt + ktFile = env.ktSymbolIndex.getCurrentKtFile(file).await() ?: empty [R3, R9] + env.project.read { [R10] + element = referenceAtCaret(ktFile, offset) navigation/ReferenceAtCaret.kt [R2] + analyzeMaybeDangling(ktFile) { symbolsAt(element) } [R4] + } -> locations [R5, R6] + <- DefinitionResult(locations) [R7] +``` + +The dispatch mirrors `signatureHelp` line for line, which is what buys R3 and R10 - `getCurrentKtFile(...).await()` returns PSI refreshed to the live document's version, and awaiting it outside `project.read` avoids deadlocking against the refresh's `project.write`. + +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/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). +- **`KtLspTestEnvironment` / `KtLspTestRule`** - accept a `List` (`name`, `dirName`, `dependsOn`), defaulting to today's single `src` module so no existing test changes. Today every test source module depends only on the JDK, stdlib and extra jars, so inter-module resolution cannot be exercised at all. + +Splitting caret-from-resolution is the one structural decision here: it makes the R2 caret rules testable with no analysis session (as `CallAtCursorFinderTest` already does for signature help), and gives ADFA-4824 a helper it can use unchanged. A single `resolveDeclarationsAt(file, offset)` entry point would force both features through a full session; a shared `ResolvedReference` value type cannot work, because `KaSymbol` is session-scoped and must not escape the `analyze` block. + +Unchanged: `DefinitionParams`/`DefinitionResult`, `ILanguageServer`, `IDEEditor`, `IDELanguageClientImpl`, and every string resource. R8 needs no code - `build/generated/**` is already inside module content roots. + +## Verification + +Unit tests in `:lsp:kotlin` (`flox activate -d flox/local -- ./gradlew :lsp:kotlin:testV7DebugUnitTest`), split to match the two helpers: + +- **`ReferenceAtCaretTest`** - no analysis session, PSI only. The R2 rules: inside vs one-past an identifier, whitespace/comment/non-navigable keyword, a declaration's own name, the climb cap (`run { fun foo() {} }` must not navigate to `run`), and each navigable convention token. +- **`GoToDefinitionTest`** - one case per R4 row, all three resolution scopes (inter-module via a two-spec fixture: `lib`, and `app` with `dependsOn = listOf("lib")`), a workspace Java target, a stdlib reference yielding nothing, multi-candidate dedup and ordering (R5), the range rules (R6), and a pre-cancelled `cancelChecker` returning empty without resolving (R10). + +The action's `prepare()`/`ActionData` path is not unit-testable, consistent with the other Kotlin code actions. It is covered by on-device QA, along with the multi-result panel, cancellation, and the new tooltip tag, via the "Steps to QA" field on ADFA-4823. + +## Related + +- [ADR 0010](../adr/0010-navigation-resolves-via-analysis-api.md) - navigation resolves via the Analysis API, not the symbol index +- [ARCHITECTURE.md](../../ARCHITECTURE.md) 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 a94cc33edf..c4865af7ab 100644 --- a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt +++ b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt @@ -90,6 +90,7 @@ object TooltipTag { const val EDITOR_CODE_ACTIONS_KT_IMPLEMENT_MEMBERS = "editor.codeactions.kotlin.implementmembers" 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 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 7acaabf9a1..7530a6fe9b 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.GoToDefinitionAction import com.itsaky.androidide.lsp.kotlin.actions.ImplementMembersAction import com.itsaky.androidide.lsp.kotlin.actions.NullSafetyAction import com.itsaky.androidide.lsp.kotlin.actions.OrganizeImportsAction @@ -30,6 +31,7 @@ object KotlinCodeActionsMenu : IActionsMenuProvider { KT_LINE_COMMENT_TOKEN, TooltipTag.EDITOR_CODE_ACTIONS_KT_UNCOMMENT, ), + GoToDefinitionAction(), 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 3f41f63179..ae9f0d903c 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 @@ -37,6 +37,7 @@ import com.itsaky.androidide.lsp.kotlin.compiler.index.KT_SOURCE_FILE_INDEX_KEY import com.itsaky.androidide.lsp.kotlin.compiler.index.KT_SOURCE_FILE_META_INDEX_KEY 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.signaturehelp.doSignatureHelp import com.itsaky.androidide.lsp.models.CompletionParams import com.itsaky.androidide.lsp.models.CompletionResult @@ -244,7 +245,11 @@ class KotlinLanguageServer : ILanguageServer { return DefinitionResult.empty() } - return DefinitionResult.empty() + logger.debug("findDefinition(position={}, file={})", params.position, params.file) + return compiler + ?.compilationEnvironmentFor(params.file) + ?.let { context(it) { findDefinitionAt(params) } } + ?: DefinitionResult.empty() } override suspend fun expandSelection(params: ExpandSelectionParams): Range = params.selection diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/GoToDefinitionAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/GoToDefinitionAction.kt new file mode 100644 index 0000000000..8b4b27aac0 --- /dev/null +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/GoToDefinitionAction.kt @@ -0,0 +1,47 @@ +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 + +/** + * Navigates from the reference at the caret to the declaration it resolves to. + * + * Mirrors the Java action: the real work is the editor's own cancellable request, so this only has + * to start it. + */ +class GoToDefinitionAction : BaseKotlinCodeAction() { + override var titleTextRes: Int = R.string.action_goto_definition + override val id: String = ID + override var label: String = "" + override var tooltipTag: String = TooltipTag.EDITOR_CODE_ACTIONS_KT_GOTO_DEF + + // 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) + + 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)?.findDefinition() ?: false + } + + companion object { + const val ID = "ide.editor.lsp.kt.gotoDefinition" + } +} diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/Compiler.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/Compiler.kt index e8c5d41fed..3ebca7fc5c 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/Compiler.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/Compiler.kt @@ -31,17 +31,18 @@ internal class Compiler( get() = defaultCompilationEnv.parser init { - defaultCompilationEnv = CompilationEnvironment( - name = "default", - kind = CompilationKind.Default, - workspace = workspace, - ktProject = projectModel, - intellijPluginRoot = intellijPluginRoot, - jdkHome = jdkHome, - jdkRelease = jdkRelease, - languageVersion = languageVersion, - enableParserEventSystem = true, - ) + defaultCompilationEnv = + CompilationEnvironment( + name = "default", + kind = CompilationKind.Default, + workspace = workspace, + ktProject = projectModel, + intellijPluginRoot = intellijPluginRoot, + jdkHome = jdkHome, + jdkRelease = jdkRelease, + languageVersion = languageVersion, + enableParserEventSystem = true, + ) // must be initialized AFTER the compilation env has been initialized fileSystem = @@ -63,10 +64,18 @@ internal class Compiler( throw IllegalStateException("Cannot get compilation kind for file: ${file.pathString}. It may not be supported.") } + /** + * The environment for [file], or null when there is none. A `.kts` has no environment yet; + * this returns null for it rather than letting the kind-based overload's + * [UnsupportedOperationException] escape into an LSP request on an open build script. + */ fun compilationEnvironmentFor(file: Path): CompilationEnvironment? { if (!DocumentUtils.isKotlinFile(file)) return null - return compilationEnvironmentFor(compilationKindFor(file)) + val kind = compilationKindFor(file) + if (kind == CompilationKind.Script) return null + + return compilationEnvironmentFor(kind) } fun compilationEnvironmentFor(compilationKind: CompilationKind): CompilationEnvironment = @@ -78,4 +87,4 @@ internal class Compiler( override fun close() { defaultCompilationEnv.close() } -} \ No newline at end of file +} diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtModule.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtModule.kt index b12b7d31bb..92a7c684a3 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtModule.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtModule.kt @@ -2,12 +2,12 @@ package com.itsaky.androidide.lsp.kotlin.compiler.modules import org.jetbrains.kotlin.analysis.api.KaPlatformInterface import org.jetbrains.kotlin.analysis.api.projectStructure.KaModule +import org.jetbrains.kotlin.analysis.api.projectStructure.KaSourceModule import org.jetbrains.kotlin.com.intellij.openapi.vfs.VirtualFile import java.nio.file.Path @OptIn(KaPlatformInterface::class) internal interface KtModule : KaModule { - val id: String val contentRoots: Set @@ -19,21 +19,34 @@ internal interface KtModule : KaModule { fun computeFiles(extended: Boolean): Sequence } +/** + * Whether this module holds sources rather than binaries. + * + * Asks the Analysis API's own question ([KaSourceModule]) rather than naming one concrete class. + * `KtSourceModule` is the only production `KaSourceModule`, so a nominal `is KtSourceModule` check + * agreed with this one in production; it only disagreed for `TestKtSourceModule`, which it + * misclassified as a library. This fixes that test fixture's classification and hardens the + * predicate against future source-module implementations, rather than fixing a shipped regression. + */ internal val KtModule.isSourceModule: Boolean - get() = this is KtSourceModule + get() = this is KaSourceModule internal fun List.asFlatSequence(): Sequence { val processedModules = mutableSetOf() return this.asSequence().flatMap { getModuleFlatSequence(it, processedModules) } } -private fun getModuleFlatSequence(ktModule: KtModule, processed: MutableSet): Sequence = sequence { - if (processed.contains(ktModule.id)) return@sequence +private fun getModuleFlatSequence( + ktModule: KtModule, + processed: MutableSet, +): Sequence = + sequence { + if (processed.contains(ktModule.id)) return@sequence - yield(ktModule) - processed.add(ktModule.id) + yield(ktModule) + processed.add(ktModule.id) - ktModule.directRegularDependencies.forEach { dependency -> - yieldAll(getModuleFlatSequence(dependency, processed)) + ktModule.directRegularDependencies.forEach { dependency -> + yieldAll(getModuleFlatSequence(dependency, processed)) + } } -} \ No newline at end of file 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 new file mode 100644 index 0000000000..9380913215 --- /dev/null +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/GoToDefinition.kt @@ -0,0 +1,256 @@ +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.read +import com.itsaky.androidide.lsp.kotlin.utils.rangeOf +import com.itsaky.androidide.lsp.kotlin.utils.toRange +import com.itsaky.androidide.lsp.models.DefinitionParams +import com.itsaky.androidide.lsp.models.DefinitionResult +import com.itsaky.androidide.models.Location +import com.itsaky.androidide.models.Range +import com.itsaky.androidide.progress.ICancelChecker +import kotlinx.coroutines.future.await +import org.jetbrains.kotlin.analysis.api.KaSession +import org.jetbrains.kotlin.analysis.api.components.containingDeclaration +import org.jetbrains.kotlin.analysis.api.resolution.successfulFunctionCallOrNull +import org.jetbrains.kotlin.analysis.api.resolution.symbol +import org.jetbrains.kotlin.analysis.api.symbols.KaSymbol +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.openapi.util.TextRange +import org.jetbrains.kotlin.com.intellij.psi.PsiElement +import org.jetbrains.kotlin.com.intellij.psi.PsiNameIdentifierOwner +import org.jetbrains.kotlin.idea.references.mainReference +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.psi.KtPropertyAccessor +import org.slf4j.LoggerFactory + +private val logger = LoggerFactory.getLogger("GoToDefinition") + +/** + * Every workspace-source declaration [element] resolves to, as editor [Location]s. + * + * Deduplicated by file plus range and ordered by file path then start offset, so a multi-candidate + * result is stable. Candidates whose declaration is not a workspace source - the stdlib, the + * framework, any library jar - are dropped: there is no decompiler on device and the editor can + * only open a real file. + * + * Must be called inside an analysis session, while holding the project read lock. + */ +internal fun KaSession.definitionLocations( + element: KtElement, + cancelChecker: ICancelChecker, +): List = + resolvedLocations(element, cancelChecker) + .distinctBy { it.file to it.range } + .sortedWith(compareBy({ it.file.toString() }, { it.range.start.index })) + +private fun KaSession.resolvedLocations( + element: KtElement, + cancelChecker: ICancelChecker, +): List { + val symbols = symbolsAt(element) + if (symbols.isNotEmpty()) { + return symbols.mapNotNull { + cancelChecker.abortIfCancelled() + locationOf(it) + } + } + + // mainReference resolved but named nothing with a KaSymbol: a break/continue/return label names + // the labelled loop or lambda itself, which the Analysis API does not model as a symbol. Fall + // back to the reference's raw PSI resolution and build the location from that PSI directly. + cancelChecker.abortIfCancelled() + return listOfNotNull(rawReferenceLocation(element)) +} + +/** + * The symbols [element] resolves to. Name references answer through [mainReference]; convention + * references (`a + b`, `a[i]`, `by lazy`, destructuring, `for` loops) have no name to resolve and + * answer through the resolved call instead. + * + * Resolution over broken code throws, and a throw must read as "not found" rather than crash the + * request, so both paths are guarded. + */ +private fun KaSession.symbolsAt(element: KtElement): List = + runCatching { + element.mainReference + ?.resolveToSymbols() + ?.toList() + // A destructuring entry (`val (first, second) = p`) is simultaneously a declaration and + // a convention reference: resolveToSymbols() legitimately returns both the entry's own + // local variable symbol and the componentN function it calls. Drop the self-symbol so + // this reads as R2's "no self-jump" rule rather than a stray extra candidate. + ?.filterNot { it.sourcePsiSafe() === element } + ?.ifEmpty { null } + ?: listOfNotNull(element.resolveToCall()?.successfulFunctionCallOrNull()?.symbol) + }.getOrElse { + // A cancellation is not a resolution failure. CancelCheckerProgressIndicator throws + // ProcessCanceledException from inside FIR resolution so analysis can abort mid-analyze; + // swallowing it here would turn a preemption into "definition not found" and leave the + // coarse cancelChecker calls as the only place cancellation can take effect. + if (it.isAnalysisCancellation()) throw it + logger.debug("Resolution failed for '{}'", element.text, it) + emptyList() + } + +/** [symbol]'s declaration as a [Location], or null when it is not a workspace source. */ +private fun KaSession.locationOf(symbol: KaSymbol): Location? { + // sourcePsiSafe() is non-null only for SOURCE and JAVA_SOURCE origins. A library symbol has a + // non-null psi pointing into a class file, so origin - not nullability - is the test here. + val declaration = + symbol.sourcePsiSafe() + // A symbol the compiler generated rather than the user writing it (an implicit primary + // constructor, a data class member) has no PSI of its own; its container does. + ?: symbol.containingDeclaration?.sourcePsiSafe() + ?: return null + + return locationOfPsi(declaration) +} + +/** + * [element]'s reference resolved directly through PSI rather than a [KaSymbol]. This fallback runs + * whenever [symbolsAt] came back empty - a label (which names the labelled loop or lambda itself, not + * a declaration with a symbol) is the common case, but any other empty resolution takes the same path, + * including one that resolves into a binary. No separate workspace-source filter is needed here: unlike + * [locationOf], which tests symbol origin, this relies on [locationOfPsi] itself rejecting anything + * whose containing file has neither a live-document `backingFilePath` (only ever set for a workspace + * source open in the editor) nor a file-protocol virtual file - a jar-backed target fails both. + */ +private fun rawReferenceLocation(element: KtElement): Location? = + runCatching { element.mainReference?.resolve() } + .getOrElse { + // See the matching comment in symbolsAt: a cancellation must propagate, not be reported + // as "nothing resolved". + if (it.isAnalysisCancellation()) throw it + logger.debug("Raw resolution failed for '{}'", element.text, it) + null + }?.let(::locationOfPsi) + +/** + * [declaration]'s [Location], or null when it is not a workspace source. + * + * The open-document case is the common one: the file the user is looking at is a live [KtFile] built + * by `KtSymbolIndex.refreshToCurrent` from the editor buffer, whose `virtualFile` is a non-physical + * `LightVirtualFile` (protocol `"mock"` in production, null under Robolectric) rather than the on-disk + * file - so it must be tried through [backingFilePath] first, falling back to the VFS only for + * declarations reached without going through the live-document cache. + */ +private fun locationOfPsi(declaration: PsiElement): Location? { + val target = (declaration as? KtPropertyAccessor)?.property ?: declaration + val psiFile = target.containingFile ?: return null + val ktFile = psiFile as? KtFile + val path = + (ktFile?.backingFilePath ?: ktFile?.originalKtFile?.backingFilePath) + ?: psiFile.virtualFile + ?.takeIf { it.fileSystem.protocol == "file" } + ?.let { runCatching { it.toNioPath() }.getOrNull() } + ?: return null + + val nameIdentifier = (target as? PsiNameIdentifierOwner)?.nameIdentifier + val range = + if (nameIdentifier != null) { + rangeOf(nameIdentifier, psiFile) + } else { + // No name token: an anonymous object, an init block, a primary constructor. Collapse to + // the declaration's start so the editor puts the caret there without selecting a body. + val start = target.textRange.startOffset + TextRange(start, start).toRange(psiFile) + } + + if (range == Range.NONE) { + logger.debug("No document for {}; dropping candidate", path) + return null + } + + return Location(path, range) +} + +/** + * Computes the definition result for [params]. + * + * Mirrors `doSignatureHelp`: the live-PSI await happens outside `project.read`, because the refresh + * it waits on needs `project.write` and awaiting it under the read lock would deadlock. Every + * failure short of cancellation collapses to an empty result, which the editor renders as + * "Definition not found". + * + * The context is [AbstractCompilationEnvironment] rather than the concrete `CompilationEnvironment` + * so the test environment, which subclasses [AbstractCompilationEnvironment] directly, can drive this + * entry point. + */ +context(env: AbstractCompilationEnvironment) +internal suspend fun findDefinitionAt(params: DefinitionParams): DefinitionResult { + logger.debug("findDefinitionAt requested for file={} position={}", params.file, params.position) + + if (params.cancelChecker.isCancelled()) { + logger.debug("Definition request for {} was cancelled before processing", params.file) + return DefinitionResult.empty() + } + + 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) + } + } + } + + val locations = + try { + attempt() + } catch (e: AnalysisPreemptedException) { + logger.debug("Definition lookup for {} preempted; retrying once", params.file) + attempt() + } + logger.debug("Definition result for {}: {} location(s)", params.file, locations.size) + DefinitionResult(locations) + } catch (e: Throwable) { + if (e.isAnalysisCancellation()) { + logger.debug( + "Definition lookup for {} cancelled (preempted={})", + params.file, + e is AnalysisPreemptedException, + ) + return DefinitionResult.empty() + } + logger.warn("Definition lookup failed for {}", params.file, e) + DefinitionResult.empty() + } +} 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 new file mode 100644 index 0000000000..b954a8d72f --- /dev/null +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/navigation/ReferenceAtCaret.kt @@ -0,0 +1,108 @@ +package com.itsaky.androidide.lsp.kotlin.navigation + +import org.jetbrains.kotlin.com.intellij.psi.PsiElement +import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType +import org.jetbrains.kotlin.idea.references.mainReference +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.KtArrayAccessExpression +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.psi.KtForExpression +import org.jetbrains.kotlin.psi.KtOperationReferenceExpression +import org.jetbrains.kotlin.psi.KtPropertyDelegate +import org.slf4j.LoggerFactory + +private val logger = LoggerFactory.getLogger("ReferenceAtCaret") + +/** + * Tokens a caret may sit on and still name something. An accept-list rather than a reject-list, so + * whitespace, comments, string bodies, braces, literals and every other keyword navigate nowhere. + * IDENTIFIER also covers KDoc link names, which are lexed as identifiers inside a KDocName. + */ +private val NAVIGABLE_TOKENS: Set = + setOf( + KtTokens.IDENTIFIER, + KtTokens.THIS_KEYWORD, + KtTokens.SUPER_KEYWORD, + KtTokens.IN_KEYWORD, // for-loop iterator/hasNext/next + KtTokens.BY_KEYWORD, // property delegate getValue/setValue + KtTokens.LPAR, // invoke + KtTokens.LBRACKET, // get/set + ) + +/** + * Levels walked from the caret's leaf token up to the element that resolves. Two is what the + * reference kinds actually need: a name's reference sits on its parent, and a convention host at + * most one above that (the call-paren case needs both levels). The cap just bounds the walk so it + * cannot wander arbitrarily far into an unrelated enclosing element; it is not what stops a caret on + * a declaration's own name from resolving to an enclosing call - in `run { fun inner() {} }`, a + * caret on `inner` finds nothing because none of the intervening ancestors are resolvable, several + * levels before the cap would even matter. + */ +private const val MAX_CLIMB = 2 + +/** + * The element to resolve for a caret at [offset] in [file], or null when the caret names nothing. + * + * Callers must hold the project read lock. Pure PSI: no analysis session is needed or used. + */ +internal fun referenceAtCaret( + file: KtFile, + offset: Int, +): KtElement? { + // Touch caret placement is imprecise, so a caret resting just past an identifier must behave + // like a caret inside it. + val leaf = + navigableLeafAt(file, offset) + ?: navigableLeafAt(file, (offset - 1).coerceAtLeast(0)) + ?: run { + logger.debug("No navigable token at offset {} in {}", offset, file.name) + return null + } + + var node: PsiElement? = leaf.parent + var climbed = 0 + while (node != null && node !is KtFile && climbed < MAX_CLIMB) { + if (node is KtElement && node.isResolvable()) { + return node + } + node = node.parent + climbed++ + } + + logger.debug("Token '{}' at offset {} in {} resolves nothing", leaf.text, offset, file.name) + return null +} + +private fun navigableLeafAt( + file: KtFile, + offset: Int, +): PsiElement? { + val leaf = file.findElementAt(offset) ?: return null + val type = leaf.node?.elementType + if (type != null && type in NAVIGABLE_TOKENS) { + return leaf + } + // Operator tokens are too many to enumerate, but the reference element wrapping them is not. + if (leaf.parent is KtOperationReferenceExpression) { + return leaf + } + return null +} + +/** + * Either the element is a convention host whose declaration is found through a resolved-call lookup, + * or it carries a name reference. + * + * The convention hosts are tested first because [mainReference] is not as null-safe as its type says: + * on a `KtReferenceExpression` it is `references.firstIsInstance()`, which throws when no + * `KtReference` is contributed. Reading it last, guarded, keeps a throw from aborting the whole climb + * and keeps the null-returning contract [referenceAtCaret] documents. + */ +private fun KtElement.isResolvable(): Boolean = + this is KtCallExpression || + this is KtArrayAccessExpression || + this is KtPropertyDelegate || + this is KtForExpression || + runCatching { mainReference != null }.getOrDefault(false) 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 7111eb0103..117ce7ef9e 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.GoToDefinitionAction import com.itsaky.androidide.lsp.kotlin.actions.ImplementMembersAction import com.itsaky.androidide.lsp.kotlin.actions.NullSafetyAction import com.itsaky.androidide.lsp.kotlin.actions.OrganizeImportsAction @@ -32,6 +33,7 @@ class KotlinCodeActionTooltipTagTest { mapOf( 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, 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/fixtures/InterModuleResolutionTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/InterModuleResolutionTest.kt new file mode 100644 index 0000000000..75f2743907 --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/InterModuleResolutionTest.kt @@ -0,0 +1,52 @@ +package com.itsaky.androidide.lsp.kotlin.fixtures + +import com.google.common.truth.Truth.assertThat +import org.jetbrains.kotlin.analysis.api.symbols.KaClassLikeSymbol +import org.jetbrains.kotlin.idea.references.mainReference +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType +import org.junit.Test + +/** + * The fixture must be able to express "module app depends on module lib". Without it, no + * inter-module navigation test is possible: every test source module would see only the JDK, + * the stdlib and extra jars. + */ +class InterModuleResolutionTest : KtLspTest() { + override val moduleSpecs = + listOf( + TestSourceModuleSpec("lib"), + TestSourceModuleSpec("app", dependsOn = listOf("lib")), + ) + + @Test + fun `a reference in app resolves to a class declared in lib`() { + createSourceFile("lib", "lib/Greeter.kt", "package lib\n\nclass Greeter") + val appFile = + createSourceFile( + "app", + "app/Main.kt", + "package app\n\nimport lib.Greeter\n\nfun make(): Greeter? = null", + ) + + // Return a String, not the symbol: a KaSymbol must not escape the analyze block. + val resolvedClassId = + analyze(appFile) { + appFile + .collectDescendantsOfType() + .last { it.getReferencedName() == "Greeter" } + .mainReference + .resolveToSymbols() + .firstNotNullOfOrNull { (it as? KaClassLikeSymbol)?.classId?.asString() } + } + + assertThat(resolvedClassId).isEqualTo("lib/Greeter") + } + + @Test + fun `each module gets its own source root`() { + assertThat(env.sourceRoots).hasSize(2) + assertThat(env.sourceRoots[0].fileName.toString()).isEqualTo("lib") + assertThat(env.sourceRoots[1].fileName.toString()).isEqualTo("app") + } +} diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt index d9935d9c77..673504cbd1 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.kt @@ -24,9 +24,20 @@ import org.robolectric.RobolectricTestRunner */ @RunWith(RobolectricTestRunner::class) abstract class KtLspTest { + /** Source modules for this test class. Override to exercise inter-module resolution. */ + internal open val moduleSpecs: List = + listOf(TestSourceModuleSpec("src")) + + /** + * See [KtLspTestEnvironment]'s parameter of the same name. Override to `true` only for tests that + * open a document and drive resolution/ranges through the live KtFile it produces - the default + * `false` makes such files non-physical, which production never hits. + */ + internal open val enableParserEventSystem: Boolean = false + @get:Rule @PublishedApi - internal val lspTestRule = KtLspTestRule() + internal val lspTestRule = KtLspTestRule({ moduleSpecs }, { enableParserEventSystem }) internal val env: KtLspTestEnvironment get() = lspTestRule.env @@ -34,8 +45,14 @@ abstract class KtLspTest { protected fun createSourceFile( relativePath: String, content: String, + ): KtFile = createSourceFile(moduleSpecs.first().name, relativePath, content) + + protected fun createSourceFile( + moduleName: String, + relativePath: String, + content: String, ): KtFile { - val file = env.createSourceFile(relativePath, content) + val file = env.createSourceFile(moduleName, relativePath, content) // See the comment in `analyzeMaybeDanglingForTest` below: freshly-created files are invisible to // unqualified name resolution until they're registered with the symbol index's file metadata, // which in production happens via the background indexer. Do that synchronously here so every diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestEnvironment.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestEnvironment.kt index 54e2337745..06c220199f 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestEnvironment.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestEnvironment.kt @@ -26,11 +26,14 @@ import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreApplicationEnvironment import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreApplicationEnvironmentMode import org.jetbrains.kotlin.cli.jvm.index.JavaRoot import org.jetbrains.kotlin.com.intellij.mock.MockProject +import org.jetbrains.kotlin.com.intellij.openapi.application.ApplicationManager import org.jetbrains.kotlin.com.intellij.openapi.vfs.local.CoreLocalFileSystem +import org.jetbrains.kotlin.com.intellij.psi.PsiFile import org.jetbrains.kotlin.com.intellij.psi.PsiManager import org.jetbrains.kotlin.config.LanguageVersion import org.jetbrains.kotlin.psi.KtFile import java.nio.file.Path +import kotlin.io.path.createDirectories import kotlin.io.path.name import kotlin.io.path.pathString import kotlin.io.path.writeText @@ -39,47 +42,73 @@ import org.jetbrains.kotlin.analysis.api.analyze as ktAnalyze /** * A self-contained Kotlin Analysis API environment for use in plain JVM unit tests. * - * @param sourceRoots Directories containing Kotlin/Java source files for the test. + * @param baseDir Directory the per-module source roots are created under. + * @param moduleSpecs Source modules to create, including any dependencies between them. * @param extraLibraryJars Additional JARs to add as library modules. * @param languageVersion Kotlin language version; defaults to [DEFAULT_LANGUAGE_VERSION]. * @param jdkRelease JDK release version; defaults to the host JVM's feature version. */ @OptIn(K1Deprecation::class, KaImplementationDetail::class) internal class KtLspTestEnvironment( - val sourceRoots: List, + baseDir: Path, + val moduleSpecs: List = listOf(TestSourceModuleSpec("src")), private val extraLibraryJars: List = emptyList(), languageVersion: LanguageVersion = DEFAULT_LANGUAGE_VERSION, jdkRelease: Int = checkNotNull(System.getProperty("java.specification.version")).toInt(), + // false (the historical default here) makes KtPsiFactory produce non-physical files: virtualFile + // is null and PsiDocumentManager can't find a Document for them, unlike production + // (CompilationEnvironment leaves this at the AbstractCompilationEnvironment default of true). + // Tests that open a document and drive resolution/ranges through it (the live-document path) + // need true to be representative; everything else keeps the historical false. + enableParserEventSystem: Boolean = false, ) : AbstractCompilationEnvironment( - name = "test", - kind = CompilationKind.Default, - intellijPluginRoot = findIntellijPluginRoot(), - jdkHome = Path.of(System.getProperty("java.home")), - jdkRelease = jdkRelease, - languageVersion = languageVersion, - applicationEnvironmentMode = KotlinCoreApplicationEnvironmentMode.UnitTest, - enableParserEventSystem = false, -) { + name = "test", + kind = CompilationKind.Default, + intellijPluginRoot = findIntellijPluginRoot(), + jdkHome = Path.of(System.getProperty("java.home")), + jdkRelease = jdkRelease, + languageVersion = languageVersion, + applicationEnvironmentMode = KotlinCoreApplicationEnvironmentMode.UnitTest, + enableParserEventSystem = enableParserEventSystem, + ) { + /** + * One root per spec, in spec order. Declared before the `init` block below: `initialize()` + * builds the modules and would otherwise read these before they are assigned. + */ + val sourceRoots: List = + moduleSpecs.map { spec -> baseDir.resolve(spec.dirName).createDirectories() } + + private val rootByModule: Map = + moduleSpecs.map { it.name }.zip(sourceRoots).toMap() + private lateinit var localFileSystem: CoreLocalFileSystem init { - initialize(::buildModules, ::buildKtSymbolIndex) + try { + initialize(::buildModules, ::buildKtSymbolIndex) + } catch (failure: Throwable) { + // A throwing constructor never hands the instance to KtLspTestRule, so its finally has + // nothing to close and the refcounted, process-wide application environment leaks for the + // rest of the suite - one bad TestSourceModuleSpec would reproduce the suite-wide OOM. + // Release it here, where the half-built instance is still reachable. + runCatching { closeInWriteAction() }.exceptionOrNull()?.let(failure::addSuppressed) + throw failure + } } - override fun createServiceRegistrars(): List { - return listOf( + override fun createServiceRegistrars(): List = + listOf( LspAnalysisApiServiceRegistrar( - provider = AnalysisApiServiceProviders.Production - .toBuilder() - .apply { - appService(KotlinAnalysisPermissionOptions::class, replace = true) { - AnalysisPermissionOptions(defaultIsAnalysisAllowedOnEdt = true) - } - } - .build() - ) + provider = + AnalysisApiServiceProviders.Production + .toBuilder() + .apply { + appService(KotlinAnalysisPermissionOptions::class, replace = true) { + AnalysisPermissionOptions(defaultIsAnalysisAllowedOnEdt = true) + } + }.build(), + ), ) - } override fun postInit(libraryRoots: List) { super.postInit(libraryRoots) @@ -90,44 +119,40 @@ internal class KtLspTestEnvironment( project: MockProject, applicationEnv: KotlinCoreApplicationEnvironment, ): List { - val jdkModule = buildKtLibraryModule(project, applicationEnv) { - id = "jdk" - isSdk = true - addContentRoot(jdkHome) - } - - val stdlibModule = findKotlinStdlibJar()?.let { jar -> + val jdkModule = buildKtLibraryModule(project, applicationEnv) { - id = jar.pathString - addContentRoot(jar) - addDependency(jdkModule) + id = "jdk" + isSdk = true + addContentRoot(jdkHome) } - } - val extraLibModules = extraLibraryJars.map { jar -> - buildKtLibraryModule(project, applicationEnv) { - id = jar.pathString - addContentRoot(jar) - addDependency(jdkModule) - stdlibModule?.let { addDependency(it) } + val stdlibModule = + findKotlinStdlibJar()?.let { jar -> + buildKtLibraryModule(project, applicationEnv) { + id = jar.pathString + addContentRoot(jar) + addDependency(jdkModule) + } } - } - val sourceDeps: List = buildList { - add(jdkModule) - stdlibModule?.let { add(it) } - addAll(extraLibModules) - } + val extraLibModules = + extraLibraryJars.map { jar -> + buildKtLibraryModule(project, applicationEnv) { + id = jar.pathString + addContentRoot(jar) + addDependency(jdkModule) + stdlibModule?.let { addDependency(it) } + } + } - val sourceModules = sourceRoots.mapIndexed { i, root -> - TestKtSourceModule( - project = project, - name = "test-source-$i", - roots = setOf(root), - dependencies = sourceDeps, - languageVersion = languageVersion, - ) - } + val sourceDeps: List = + buildList { + add(jdkModule) + stdlibModule?.let { add(it) } + addAll(extraLibModules) + } + + val sourceModules = buildSourceModules(project, sourceDeps) return buildList { addAll(sourceModules) @@ -137,15 +162,55 @@ internal class KtLspTestEnvironment( } } + /** + * Builds one [TestKtSourceModule] per spec, in dependency order - a module's dependencies are + * constructor arguments, so they must exist first. + */ + private fun buildSourceModules( + project: MockProject, + libraryDeps: List, + ): List { + val specByName = moduleSpecs.associateBy { it.name } + require(specByName.size == moduleSpecs.size) { + "Duplicate module names in $moduleSpecs" + } + + val built = LinkedHashMap() + + fun build( + name: String, + path: List, + ) { + if (built.containsKey(name)) return + check(name !in path) { + "Cyclic test module dependency: ${(path + name).joinToString(" -> ")}" + } + val spec = specByName[name] ?: error("Unknown test source module '$name'") + spec.dependsOn.forEach { build(it, path + name) } + built[name] = + TestKtSourceModule( + project = project, + name = spec.name, + roots = setOf(checkNotNull(rootByModule[name])), + dependencies = libraryDeps + spec.dependsOn.map { checkNotNull(built[it]) }, + languageVersion = languageVersion, + ) + } + + moduleSpecs.forEach { build(it.name, emptyList()) } + return built.values.toList() + } + private fun buildKtSymbolIndex( modules: List, libraryRoots: List, ): KtSymbolIndex { val inMemoryJvmBackingIndex = InMemoryIndex(JvmSymbolDescriptor) - val inMemoryJvmSymbolIndex = object : JvmSymbolIndex(inMemoryJvmBackingIndex, BackgroundIndexer(inMemoryJvmBackingIndex)) { - // ensure we're not filtering out anything - override fun isActive(sourceId: String) = true - } + val inMemoryJvmSymbolIndex = + object : JvmSymbolIndex(inMemoryJvmBackingIndex, BackgroundIndexer(inMemoryJvmBackingIndex)) { + // ensure we're not filtering out anything + override fun isActive(sourceId: String) = true + } val inMemoryFileMetaBackingIndex = InMemoryIndex(KtFileMetadataDescriptor) val inMemoryFileMetaIndex = KtFileMetadataIndex(inMemoryFileMetaBackingIndex) @@ -161,25 +226,48 @@ internal class KtLspTestEnvironment( } /** - * Writes [content] to [relativePath] under the first source root, refreshes - * the VFS, and returns the corresponding [KtFile]. + * Writes [content] to [relativePath] under the first module's source root, refreshes the VFS, + * and returns the corresponding [KtFile]. */ fun createSourceFile( relativePath: String, content: String, - ): KtFile { - require(sourceRoots.isNotEmpty()) { "No source roots configured" } - val file = sourceRoots.first().resolve(relativePath) + ): KtFile = createSourceFile(moduleSpecs.first().name, relativePath, content) + + /** As above, but under the source root of the module named [moduleName]. */ + fun createSourceFile( + moduleName: String, + relativePath: String, + content: String, + ): KtFile = + createFile(moduleName, relativePath, content) as? KtFile + ?: error("Not a Kotlin file: $relativePath") + + /** + * Writes [content] to [relativePath] under the source root of the module named [moduleName], + * refreshes the VFS, and returns the corresponding [PsiFile]. + * + * Use this for `.java` sources, which are part of a Kotlin source module's content scope but have + * no [KtFile]. Kotlin callers want [createSourceFile], which narrows the result. + */ + fun createFile( + moduleName: String, + relativePath: String, + content: String, + ): PsiFile { + val root = rootByModule[moduleName] ?: error("No test source module named '$moduleName'") + val file = root.resolve(relativePath) file.parent.toFile().mkdirs() file.writeText(content) - val vf = localFileSystem.refreshAndFindFileByPath(file.pathString) - ?: error("VFS cannot find newly created file: $file") + val vf = + localFileSystem.refreshAndFindFileByPath(file.pathString) + ?: error("VFS cannot find newly created file: $file") modules.filterIsInstance().forEach { it.invalidateSearchScope() } return project.read { - PsiManager.getInstance(project).findFile(vf) as? KtFile + PsiManager.getInstance(project).findFile(vf) ?: error("PSI file not found for: $file") } } @@ -187,8 +275,18 @@ internal class KtLspTestEnvironment( /** * Runs [action] inside a [KaSession] for [file], acquiring the project read lock first. */ - inline fun analyze(file: KtFile, crossinline action: KaSession.() -> R): R = - project.read { ktAnalyze(file, action) } + inline fun analyze( + file: KtFile, + crossinline action: KaSession.() -> R, + ): R = project.read { ktAnalyze(file, action) } +} + +/** + * Disposes this environment. Disposing the project model requires an IntelliJ write action; our own + * `project.write` lock does not supply one, which is why plain `close()` used to fail here. + */ +internal fun KtLspTestEnvironment.closeInWriteAction() { + ApplicationManager.getApplication().runWriteAction { close() } } /** @@ -204,39 +302,45 @@ internal class KtLspTestEnvironment( private fun findIntellijPluginRoot(): Path { // Primary: scan the classpath for the well-known cached names. val classPath = System.getProperty("java.class.path") ?: "" - classPath.split(java.io.File.pathSeparator) + classPath + .split(java.io.File.pathSeparator) .map { Path.of(it) } .firstOrNull { // named 'kt-android.jar' when added by external assets plugins it.name == "kt-android.jar" || - // for local builds, named 'analysis-api-standalone-embeddable-for-ide-X.X.X-SNAPSHOT.jar' - it.name.matches("analysis-api-standalone-embeddable-for-ide.*\\.jar".toRegex()) - } - ?.let { return it } + // for local builds, named 'analysis-api-standalone-embeddable-for-ide-X.X.X-SNAPSHOT.jar' + it.name.matches("analysis-api-standalone-embeddable-for-ide.*\\.jar".toRegex()) + }?.let { return it } // Fallback to reflection. This works on a plain JVM where the classloader exposes // the code source, but may not work under Robolectric. return try { - val location = StandaloneProjectFactory::class.java.protectionDomain - ?.codeSource?.location - ?: error("code source is null") + val location = + StandaloneProjectFactory::class.java.protectionDomain + ?.codeSource + ?.location + ?: error("code source is null") val path = Path.of(location.toURI()) check(path.name.endsWith(".jar")) { "resolved to directory, not a JAR: $path" } path } catch (e: Exception) { error( "Cannot locate kt-android.jar on the test classpath. " + - "Ensure the subprojects.kotlinAnalysisApi dependency is included in testImplementation. " + - "Also verify that the JAR file name matches expected names. " + - "(reflection fallback also failed: ${e.message})" + "Ensure the subprojects.kotlinAnalysisApi dependency is included in testImplementation. " + + "Also verify that the JAR file name matches expected names. " + + "(reflection fallback also failed: ${e.message})", ) } } -private fun findKotlinStdlibJar(): Path? = try { - val location = KotlinVersion::class.java.protectionDomain?.codeSource?.location ?: return null - Path.of(location.toURI()).takeIf { it.name.endsWith(".jar") } -} catch (_: Exception) { - null -} +private fun findKotlinStdlibJar(): Path? = + try { + val location = + KotlinVersion::class.java.protectionDomain + ?.codeSource + ?.location ?: return null + Path.of(location.toURI()).takeIf { it.name.endsWith(".jar") } + } catch (_: Exception) { + null + } diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestRule.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestRule.kt index 00ca7b8159..92159a49bf 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestRule.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestRule.kt @@ -1,12 +1,23 @@ package com.itsaky.androidide.lsp.kotlin.fixtures -import com.itsaky.androidide.lsp.kotlin.compiler.write import org.junit.rules.TemporaryFolder import org.junit.rules.TestRule import org.junit.runner.Description import org.junit.runners.model.Statement -internal class KtLspTestRule : TestRule { +/** + * @param moduleSpecs Supplied as a lambda, not a value: JUnit evaluates rules after the test class + * is fully constructed, so this defers reading a subclass's `moduleSpecs` override until it has + * actually been initialized. + * @param enableParserEventSystem See [KtLspTestEnvironment]'s parameter of the same name. Also a + * lambda, for the same construction-order reason as [moduleSpecs]. + */ +internal class KtLspTestRule( + private val moduleSpecs: () -> List = { + listOf(TestSourceModuleSpec("src")) + }, + private val enableParserEventSystem: () -> Boolean = { false }, +) : TestRule { val tempDir = TemporaryFolder() lateinit var env: KtLspTestEnvironment private set @@ -18,20 +29,37 @@ internal class KtLspTestRule : TestRule { tempDir.apply( object : Statement() { override fun evaluate() { - try { - val sourceRoot = tempDir.newFolder("src").toPath() - env = KtLspTestEnvironment(listOf(sourceRoot)) + // Built before the try: a constructor throw closes itself (see KtLspTestEnvironment's + // init), so there is nothing here for a finally to clean up. + val environment = + KtLspTestEnvironment( + tempDir.root.toPath(), + moduleSpecs(), + enableParserEventSystem = enableParserEventSystem(), + ) + env = environment + var failure: Throwable? = null + try { statement?.evaluate() + } catch (e: Throwable) { + failure = e + throw e } finally { - if (::env.isInitialized) { - // Dispose each test's heavy Analysis-API environment (IntelliJ project, - // application env, background index workers). Without this the per-method - // environments accumulate for the JVM's lifetime and the suite eventually - // exhausts the heap (ADFA-4936). close() manages its own threading/locking, - // so it must NOT run inside project.write { } -- that nesting was the - // original "fails in test cases" failure. - env.close() + // Without disposal every test leaks a whole KotlinCoreApplicationEnvironment + // (refcounted, process-wide static) and the suite OOMs part-way through. A throw + // from disposal must not replace the test's own failure, though - JUnit would then + // report the fixture instead of the assertion that actually broke. + val disposal = + runCatching { + if (!environment.project.isDisposed) { + environment.closeInWriteAction() + } + }.exceptionOrNull() + val pending = failure + if (disposal != null) { + if (pending == null) throw disposal + pending.addSuppressed(disposal) } } } diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/TestSourceModuleSpec.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/TestSourceModuleSpec.kt new file mode 100644 index 0000000000..4f6abb999c --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/TestSourceModuleSpec.kt @@ -0,0 +1,15 @@ +package com.itsaky.androidide.lsp.kotlin.fixtures + +/** + * One source module in a test environment. + * + * @param name Module name, and the key other specs use in [dependsOn]. + * @param dirName Directory created for this module's source root, under the test's temp folder. + * @param dependsOn Names of other source modules this one depends on. Empty means it sees only + * the JDK, the stdlib and any extra jars. + */ +internal data class TestSourceModuleSpec( + val name: String, + val dirName: String = name, + val dependsOn: List = emptyList(), +) diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/FindDefinitionRequestTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/FindDefinitionRequestTest.kt new file mode 100644 index 0000000000..fb77ac74eb --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/FindDefinitionRequestTest.kt @@ -0,0 +1,137 @@ +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.DefinitionParams +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 + +class FindDefinitionRequestTest : KtLspTest() { + // The live-document regression test below drives resolution and range computation through a + // KtFile built by KtSymbolIndex.refreshToCurrent (KtPsiFactory.createFile), not through + // createSourceFile's on-disk file. That file is only a faithful stand-in for production's live + // document if it is physical the way production's is - see KtLspTestEnvironment's parameter of + // the same name. + override val enableParserEventSystem = true + + private val openedPaths = mutableListOf() + + @After + fun closeDocs() { + openedPaths.forEach { FileManager.onDocumentClose(DocumentCloseEvent(it)) } + openedPaths.clear() + } + + /** Registers [path] as an active document at version 1 with [content], as the editor does. */ + private fun openDocument( + path: Path, + content: String, + ) { + FileManager.onDocumentOpen(DocumentOpenEvent(path, content, 1)) + openedPaths.add(path) + } + + private fun requestAt( + file: Path, + text: String, + marker: String, + delta: Int = 0, + cancelChecker: ICancelChecker = ICancelChecker.NOOP, + ) = runBlocking { + val offset = + text.indexOf(marker).also { check(it >= 0) { "marker '$marker' not in source" } } + delta + val params = DefinitionParams(file, Position(0, 0, offset), cancelChecker) + context(env) { findDefinitionAt(params) } + } + + @Test + fun `a resolvable reference returns its declaration`() { + val text = "fun target() {}\nfun caller() { target() }" + val ktFile = createSourceFile("Request.kt", text) + val path = Path.of(ktFile.virtualFile.path) + + // "{ target(" not "target()": the bare form matches the declaration first. + val result = requestAt(path, text, "{ target()", delta = 3) + + assertThat(result.locations).hasSize(1) + assertThat( + result.locations[0] + .file.fileName + .toString(), + ).isEqualTo("Request.kt") + assertThat( + result.locations[0] + .range.start.index, + ).isEqualTo(text.indexOf("fun target") + 4) + } + + @Test + fun `a caret that names nothing returns an empty result`() { + val text = "fun target() {}\nfun caller() { }" + val ktFile = createSourceFile("Empty.kt", text) + val path = Path.of(ktFile.virtualFile.path) + + assertThat(requestAt(path, text, "{ }", delta = 2).locations).isEmpty() + } + + @Test + fun `a file the environment cannot load returns an empty result`() { + val text = "fun caller() { target() }" + val missing = env.sourceRoots.first().resolve("Missing.kt") + + assertThat(requestAt(missing, text, "target()", delta = 1).locations).isEmpty() + } + + @Test + fun `a cancelled request returns an empty result rather than throwing`() { + val text = "fun target() {}\nfun caller() { target() }" + val ktFile = createSourceFile("Cancel.kt", text) + val path = Path.of(ktFile.virtualFile.path) + + val result = + requestAt( + path, + text, + "{ target()", + delta = 3, + cancelChecker = ICancelChecker.CANCELLED, + ) + + // The caret is on a genuinely resolvable call, so an empty result can only come from + // cancellation - with the ambiguous "target()" marker this would have passed for the wrong + // reason, by landing on the declaration's own name. + assertThat(result.locations).isEmpty() + } + + @Test + fun `a same-file target found through the active document still resolves`() { + // Every other test in this file leaves the file un-opened, so getCurrentKtFile takes the + // disk fallback - a real CoreLocalFileSystem-backed KtFile whose virtualFile has protocol + // "file". That's exactly the path the production bug (ADFA-4823 finding 1) does NOT hit: + // opening the file makes getCurrentKtFile refresh a live KtFile instead + // (KtSymbolIndex.refreshToCurrent), whose virtualFile is a non-physical LightVirtualFile - + // locationOfPsi must resolve a path from backingFilePath instead, which is exactly what this + // test exercises. + val text = "fun target() {}\nfun caller() { target() }" + val ktFile = createSourceFile("Live.kt", text) + val path = Path.of(ktFile.virtualFile.path) + openDocument(path, text) + + // "{ target(" not "target()": the bare form matches the declaration first. + val result = requestAt(path, text, "{ target()", delta = 3) + + assertThat(result.locations).hasSize(1) + assertThat(result.locations[0].file).isEqualTo(path) + assertThat( + result.locations[0] + .range.start.index, + ).isEqualTo(text.indexOf("fun target") + 4) + } +} diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/GoToDefinitionTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/GoToDefinitionTest.kt new file mode 100644 index 0000000000..93e3fd190e --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/GoToDefinitionTest.kt @@ -0,0 +1,312 @@ +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.models.Location +import com.itsaky.androidide.progress.ICancelChecker +import org.junit.Test + +class GoToDefinitionTest : KtLspTest() { + override val moduleSpecs = + listOf( + TestSourceModuleSpec("lib"), + TestSourceModuleSpec("app", dependsOn = listOf("lib")), + ) + + /** Locations for the caret at `text.indexOf(marker) + delta` in a new "app" module file. */ + private fun locationsAt( + name: String, + text: String, + marker: String, + delta: Int = 0, + ): List { + val file = createSourceFile("app", name, text) + val offset = + text.indexOf(marker).also { check(it >= 0) { "marker '$marker' not in source" } } + delta + return analyze(file) { + val element = referenceAtCaret(file, offset) ?: return@analyze emptyList() + definitionLocations(element, ICancelChecker.NOOP) + } + } + + /** + * Asserts a single location whose range covers [name], searching [text] from the first + * occurrence of [declAnchor]. Index arithmetic on the source text is exact because the file + * content is that text verbatim. + */ + private fun assertNavigatesTo( + locations: List, + text: String, + declAnchor: String, + name: String, + ) { + val anchor = text.indexOf(declAnchor).also { check(it >= 0) { "anchor '$declAnchor' not in source" } } + val nameStart = text.indexOf(name, anchor).also { check(it >= 0) { "name '$name' not after anchor" } } + assertThat(locations).hasSize(1) + assertThat(locations[0].range.start.index).isEqualTo(nameStart) + assertThat(locations[0].range.end.index).isEqualTo(nameStart + name.length) + } + + @Test + fun `function call navigates to the function name`() { + // Anchor on "{ target(", not "target()": the latter matches the declaration first, so indexOf + // would put the caret on the declaration's own name and the call site would go untested. + val text = "fun target() {}\nfun caller() { target() }" + assertNavigatesTo(locationsAt("Fn.kt", text, "{ target()", 3), text, "fun target", "target") + } + + @Test + fun `caret one past an identifier navigates to the same declaration as inside it`() { + // R2/AC 8's actual guarantee, asserted where it is observable. Inside the identifier the caret + // resolves a name reference; on the following '(' it resolves the call expression instead, and + // the resolved-call fallback still lands on the same function. The guarantee is about the + // declaration reached, not the intermediate PSI node. + val text = "fun target() {}\nfun caller() { target() }" + val inside = locationsAt("Inside.kt", text, "{ target()", 3) + val onParen = locationsAt("OnParen.kt", text, "{ target()", 8) + + assertNavigatesTo(inside, text, "fun target", "target") + assertThat(onParen.map { it.range }).isEqualTo(inside.map { it.range }) + } + + @Test + fun `local variable navigates to its declaration`() { + val text = "fun caller() {\n\tval count = 1\n\tprintln(count)\n}" + assertNavigatesTo(locationsAt("Local.kt", text, "println(count)", 8), text, "val count", "count") + } + + @Test + fun `parameter navigates to its declaration`() { + val text = "fun caller(amount: Int) {\n\tprintln(amount)\n}" + assertNavigatesTo(locationsAt("Param.kt", text, "println(amount)", 8), text, "amount: Int", "amount") + } + + @Test + fun `property read navigates to the property`() { + val text = "class Holder {\n\tval label = \"x\"\n\tfun read() = label\n}" + assertNavigatesTo(locationsAt("Prop.kt", text, "= label", 2), text, "val label", "label") + } + + @Test + fun `type reference navigates to the classifier`() { + val text = "class Greeter\nfun make(): Greeter? = null" + assertNavigatesTo(locationsAt("Type.kt", text, ": Greeter", 2), text, "class Greeter", "Greeter") + } + + @Test + fun `constructor call navigates to the class when there is no explicit constructor`() { + val text = "class Greeter\nfun make() = Greeter()" + assertNavigatesTo(locationsAt("Ctor.kt", text, "= Greeter()", 2), text, "class Greeter", "Greeter") + } + + @Test + fun `explicit constructor call navigates to that constructor`() { + val text = "class Greeter constructor(val name: String)\nfun make() = Greeter(\"a\")" + val locations = locationsAt("Ctor2.kt", text, "= Greeter(", 2) + assertThat(locations).hasSize(1) + // The primary constructor has no name identifier, so the range collapses to its start. + val ctorStart = text.indexOf("constructor") + assertThat(locations[0].range.start.index).isEqualTo(ctorStart) + assertThat(locations[0].range.end.index).isEqualTo(ctorStart) + } + + @Test + fun `annotation navigates to the annotation class`() { + val text = "annotation class Marker\n@Marker\nfun caller() {}" + assertNavigatesTo(locationsAt("Anno.kt", text, "@Marker", 2), text, "class Marker", "Marker") + } + + @Test + fun `typealias navigates to the alias, not its expansion`() { + val text = "class Greeter\ntypealias Salute = Greeter\nfun make(): Salute? = null" + assertNavigatesTo(locationsAt("Alias.kt", text, ": Salute", 2), text, "typealias Salute", "Salute") + } + + @Test + fun `object reference navigates to the object`() { + val text = "object Registry { val count = 0 }\nfun read() = Registry.count" + assertNavigatesTo(locationsAt("Obj.kt", text, "= Registry", 2), text, "object Registry", "Registry") + } + + @Test + fun `named argument navigates to the parameter`() { + val text = "fun target(amount: Int) {}\nfun caller() { target(amount = 1) }" + assertNavigatesTo(locationsAt("Named.kt", text, "amount = 1", 1), text, "amount: Int", "amount") + } + + @Test + fun `import directive navigates to the imported declaration`() { + createSourceFile("lib", "lib/Greeter.kt", "package lib\n\nclass Greeter") + val text = "package app\n\nimport lib.Greeter\n\nfun make(): Greeter? = null" + val locations = locationsAt("Import.kt", text, "import lib.Greeter", 12) + assertThat(locations).hasSize(1) + assertThat(locations[0].file.fileName.toString()).isEqualTo("Greeter.kt") + } + + @Test + fun `package reference yields nothing`() { + val text = "package app\n\nfun caller() {}" + assertThat(locationsAt("Pkg.kt", text, "package app", 9)).isEmpty() + } + + @Test + fun `label navigates to the labelled expression`() { + val text = "fun caller() {\n\touter@ for (i in 1..2) { break@outer }\n}" + val locations = locationsAt("Label.kt", text, "break@outer", 7) + assertThat(locations).hasSize(1) + // The labelled for-loop has no name identifier, so the range collapses to the loop's start. + val forStart = text.indexOf("for (") + assertThat(locations[0].range.start.index).isEqualTo(forStart) + assertThat(locations[0].range.end.index).isEqualTo(forStart) + } + + @Test + fun `operator navigates to the operator function`() { + val text = + "class P {\n\toperator fun plus(other: P): P = this\n}\nfun caller(a: P, b: P) = a + b" + assertNavigatesTo(locationsAt("Op.kt", text, "a + b", 2), text, "fun plus", "plus") + } + + @Test + fun `index access navigates to the get function`() { + val text = "class Box {\n\toperator fun get(index: Int): Int = index\n}\nfun caller(b: Box) = b[0]" + assertNavigatesTo(locationsAt("Get.kt", text, "b[0]", 1), text, "fun get", "get") + } + + @Test + fun `invoke navigates to the invoke function`() { + val text = "class Runner {\n\toperator fun invoke(): Int = 1\n}\nfun caller(r: Runner) = r()" + assertNavigatesTo(locationsAt("Invoke.kt", text, "r()", 1), text, "fun invoke", "invoke") + } + + @Test + fun `destructuring entry navigates to its component function`() { + val text = + "class P {\n\toperator fun component1(): Int = 1\n\toperator fun component2(): Int = 2\n}\n" + + "fun caller(p: P) { val (first, second) = p }" + assertNavigatesTo( + locationsAt("Destructure.kt", text, "(first, second)", 1), + text, + "fun component1", + "component1", + ) + } + + @Test + fun `property delegate navigates to its getValue`() { + val text = + "import kotlin.reflect.KProperty\n" + + "class Delegate {\n\toperator fun getValue(thisRef: Any?, property: KProperty<*>): Int = 1\n}\n" + + "val number: Int by Delegate()" + val locations = locationsAt("Delegate.kt", text, " by Delegate", 1) + // setValue/provideDelegate may also resolve here as compiler details; assert getValue is + // present rather than pinning an exact count. + assertThat(locations).isNotEmpty() + val getValueName = text.indexOf("getValue", text.indexOf("fun getValue")) + assertThat(locations.map { it.range.start.index }).contains(getValueName) + } + + @Test + fun `for loop navigates to its iterator convention members`() { + val text = + "class Items {\n\toperator fun iterator(): Iterator = listOf(1).iterator()\n}\n" + + "fun caller(items: Items) { for (i in items) {} }" + val locations = locationsAt("For.kt", text, "in items", 1) + val iteratorName = text.indexOf("iterator", text.indexOf("fun iterator")) + // hasNext/next come from the stdlib Iterator and are dropped as non-workspace sources. + assertThat(locations.map { it.range.start.index }).contains(iteratorName) + } + + @Test + fun `KDoc link navigates to the linked declaration`() { + val text = "class Greeter\n\n/** See [Greeter]. */\nfun caller() {}" + assertNavigatesTo(locationsAt("KDoc.kt", text, "[Greeter]", 1), text, "class Greeter", "Greeter") + } + + @Test + fun `inter-file reference opens the sibling file`() { + createSourceFile("app", "app/Greeter.kt", "package app\n\nclass Greeter") + val text = "package app\n\nfun make(): Greeter? = null" + val locations = locationsAt("app/Sibling.kt", text, ": Greeter", 2) + assertThat(locations).hasSize(1) + assertThat(locations[0].file.fileName.toString()).isEqualTo("Greeter.kt") + } + + @Test + fun `inter-module reference opens the dependency module's file`() { + createSourceFile("lib", "lib/Greeter.kt", "package lib\n\nclass Greeter") + val text = "package app\n\nimport lib.Greeter\n\nfun make(): Greeter? = null" + val locations = locationsAt("app/CrossModule.kt", text, ": Greeter", 2) + assertThat(locations).hasSize(1) + assertThat(locations[0].file.fileName.toString()).isEqualTo("Greeter.kt") + assertThat( + locations[0] + .file.parent.fileName + .toString(), + ).isEqualTo("lib") + } + + @Test + fun `a workspace Java class is a valid target`() { + env.createFile("lib", "lib/JavaGreeter.java", "package lib;\npublic class JavaGreeter {}") + val text = "package app\n\nimport lib.JavaGreeter\n\nfun make(): JavaGreeter? = null" + val locations = locationsAt("app/FromJava.kt", text, ": JavaGreeter", 2) + assertThat(locations).hasSize(1) + assertThat(locations[0].file.fileName.toString()).isEqualTo("JavaGreeter.java") + } + + @Test + fun `a stdlib reference yields nothing`() { + val text = "fun caller() = listOf(1)" + assertThat(locationsAt("Stdlib.kt", text, "listOf(1)", 1)).isEmpty() + } + + @Test + fun `for loop over a workspace iterator yields three deduplicated locations in offset order`() { + // A resolvable call like `target(1)` resolves to exactly one symbol, so a test built on it + // never exercises distinctBy/sortedWith in definitionLocations - R5/AC7's actual + // multi-candidate case needs several *simultaneous* candidates. A for-loop's `in` resolves to + // iterator/hasNext/next together (KtForLoopInReference); giving the iterator a workspace + // return type - instead of the stdlib Iterator, as in the test above - keeps all three as + // workspace sources instead of two being filtered out. + val text = + "class WorkspaceIterator {\n" + + "\toperator fun hasNext(): Boolean = false\n" + + "\toperator fun next(): Int = 0\n" + + "}\n" + + "class Items {\n" + + "\toperator fun iterator(): WorkspaceIterator = WorkspaceIterator()\n" + + "}\n" + + "fun caller(items: Items) { for (i in items) {} }" + val locations = locationsAt("WorkspaceFor.kt", text, "in items", 1) + + val hasNextDecl = text.indexOf("hasNext", text.indexOf("fun hasNext")) + val nextDecl = text.indexOf("next", text.indexOf("fun next")) + val iteratorDecl = text.indexOf("iterator", text.indexOf("fun iterator")) + val offsets = locations.map { it.range.start.index } + + assertThat(offsets).containsExactly(hasNextDecl, nextDecl, iteratorDecl) + assertThat(offsets).containsNoDuplicates() + assertThat(offsets).isInOrder() + } + + // R6's collapse-to-start-offset branch is covered by `explicit constructor call navigates to + // that constructor` above: a KtPrimaryConstructor has no name identifier. Do not add a second + // test for the same branch. + + @Test + fun `a cancelled request resolves nothing`() { + val text = "fun target() {}\nfun caller() { target() }" + val file = createSourceFile("app", "Cancelled.kt", text) + val offset = text.indexOf("target()", startIndex = text.indexOf("caller")) + 1 + val locations = + analyze(file) { + val element = referenceAtCaret(file, offset) ?: return@analyze emptyList() + runCatching { + definitionLocations(element, ICancelChecker.CANCELLED) + }.getOrElse { emptyList() } + } + assertThat(locations).isEmpty() + } +} diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/ReferenceAtCaretTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/ReferenceAtCaretTest.kt new file mode 100644 index 0000000000..5b84e04554 --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/navigation/ReferenceAtCaretTest.kt @@ -0,0 +1,135 @@ +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.KtArrayAccessExpression +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtDestructuringDeclarationEntry +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtForExpression +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.KtOperationReferenceExpression +import org.jetbrains.kotlin.psi.KtPropertyDelegate +import org.junit.Test + +class ReferenceAtCaretTest : KtLspTest() { + /** Resolves the caret at `text.indexOf(marker) + delta` in a file containing [text]. */ + private fun refAt( + name: String, + text: String, + marker: String, + delta: Int = 0, + ): KtElement? { + val file = createSourceFile(name, text) + val offset = + text.indexOf(marker).also { check(it >= 0) { "marker '$marker' not in source" } } + delta + return env.project.read { referenceAtCaret(file, offset) } + } + + @Test + fun `caret inside an identifier finds its name reference`() { + // "target()" also matches the declaration; anchor on "{ target()" to land in the call. + val ref = refAt("A.kt", "fun target() {}\nfun caller() { target() }", "{ target()", delta = 3) + assertThat(ref).isInstanceOf(KtNameReferenceExpression::class.java) + assertThat(ref!!.text).isEqualTo("target") + } + + @Test + fun `caret immediately after an identifier retries one character back`() { + // The character after `value` is whitespace, which names nothing, so the offset-1 retry is + // what finds the reference. This is the case R2's retry exists for. + val text = "fun caller(value: Int) = value + 1" + val ref = refAt("B.kt", text, "value + 1", delta = 5) + assertThat(ref).isInstanceOf(KtNameReferenceExpression::class.java) + assertThat(ref!!.text).isEqualTo("value") + } + + @Test + fun `caret on a call's paren finds the call expression`() { + // LPAR is navigable for the invoke convention, so the primary lookup succeeds here and the + // offset-1 retry never fires. That is deliberate and consistent with `caret on an index + // bracket`: a caret on a convention token resolves the convention host, coarser than the + // identifier before it. R2's "one past an identifier behaves like inside it" guarantee still + // holds end to end, because resolving a KtCallExpression lands on the called function - that + // is asserted in GoToDefinitionTest, not here, since it needs an analysis session. + val text = "fun target() {}\nfun caller() { target() }" + val ref = refAt("B2.kt", text, "{ target()", delta = 8) + assertThat(ref).isInstanceOf(KtCallExpression::class.java) + } + + @Test + fun `caret on whitespace finds nothing`() { + assertThat(refAt("C.kt", "fun caller() { }", " ", delta = 1)).isNull() + } + + @Test + fun `caret in a comment finds nothing`() { + assertThat(refAt("D.kt", "// target here\nfun target() {}", "target here", delta = 1)).isNull() + } + + @Test + fun `caret on a non-navigable keyword finds nothing`() { + assertThat(refAt("E.kt", "fun target() {}", "fun", delta = 1)).isNull() + } + + @Test + fun `caret on a declaration's own name finds nothing`() { + assertThat(refAt("F.kt", "fun target() {}", "target", delta = 1)).isNull() + } + + @Test + fun `caret on a local declaration's name does not climb into the enclosing call`() { + // Without the climb cap this would reach the enclosing `run(...)` call and navigate there. + val ref = + refAt( + "G.kt", + "fun run(block: () -> Unit) {}\nfun caller() { run { fun inner() {} } }", + "inner", + delta = 1, + ) + assertThat(ref).isNull() + } + + @Test + fun `caret on an operator finds the operation reference`() { + val ref = + refAt( + "H.kt", + "class P { operator fun plus(other: P): P = this }\nfun caller(a: P, b: P) { a + b }", + "a + b", + delta = 2, + ) + assertThat(ref).isInstanceOf(KtOperationReferenceExpression::class.java) + } + + @Test + fun `caret on an index bracket finds the array access`() { + val ref = refAt("I.kt", "fun caller(list: List) { list[0] }", "[0]", delta = 0) + assertThat(ref).isInstanceOf(KtArrayAccessExpression::class.java) + } + + @Test + fun `caret on by finds the property delegate`() { + val ref = refAt("J.kt", "val value: Int by lazy { 1 }", "by", delta = 1) + assertThat(ref).isInstanceOf(KtPropertyDelegate::class.java) + } + + @Test + fun `caret on for-loop in finds the for expression`() { + val ref = refAt("K.kt", "fun caller(items: List) { for (i in items) {} }", "in items", delta = 1) + assertThat(ref).isInstanceOf(KtForExpression::class.java) + } + + @Test + fun `caret on a destructuring entry finds the entry`() { + val ref = + refAt( + "L.kt", + "data class P(val x: Int, val y: Int)\nfun caller(p: P) { val (x, y) = p }", + "(x, y)", + delta = 1, + ) + assertThat(ref).isInstanceOf(KtDestructuringDeclarationEntry::class.java) + } +}