Skip to content

Commit 3b6b729

Browse files
authored
ADFA-4823: Kotlin go-to-definition in the K2 LSP (#1597)
1 parent fdbf04b commit 3b6b729

21 files changed

Lines changed: 1635 additions & 127 deletions

File tree

‎build.gradle.kts‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,16 @@ buildscript {
6767
}
6868
}
6969

70+
// `jacocoAggregateReport` dependsOn every subproject's `testV8DebugUnitTest` and `sonarqube`
71+
// dependsOn that, so a hard test failure skips both - even under `--continue` - and the analysis
72+
// run uploads an empty coverage artifact with no Sonar analysis at all. That, not any individual
73+
// module's broken suite, is the only reason `ignoreFailures` exists here: keep test failures
74+
// non-fatal when the analysis chain is what was asked for, and let every ordinary build gate on them.
75+
val analysisRun =
76+
gradle.startParameter.taskNames.any {
77+
it.substringAfterLast(':') in setOf("sonar", "sonarqube", "jacocoAggregateReport")
78+
}
79+
7080
subprojects {
7181
plugins.apply("jacoco")
7282

@@ -78,14 +88,28 @@ subprojects {
7888
FDroidConfig.load(project)
7989

8090
tasks.withType<Test> {
81-
// Continue even if tests fail, so coverage data is written
82-
ignoreFailures = true
91+
ignoreFailures = analysisRun
92+
93+
// Gradle's default test-worker heap is 512m, too small for the Robolectric +
94+
// Kotlin Analysis API suites (:lsp:kotlin peaks near 240m and keeps growing).
95+
// Keep it explicit so the suites fail on a real regression, not on the default.
96+
maxHeapSize = "1g"
8397

8498
// Backstop: kill any individual Test task that runs longer than 10 minutes.
8599
// Prevents a single hung test JVM (e.g. the Tooling API child) from burning
86100
// the entire CI job budget.
87101
timeout.set(Duration.ofMinutes(10))
88102

103+
// A test worker's default working dir is the module directory, so an unpathed
104+
// -XX:+HeapDumpOnOutOfMemoryError drops a heap dump of up to maxHeapSize into the source
105+
// tree, untracked and not gitignored. Keep dumps under build/ instead, one per task.
106+
val heapDumpFile =
107+
layout.buildDirectory
108+
.file("test-heapdumps/$name.hprof")
109+
.get()
110+
.asFile
111+
doFirst { heapDumpFile.parentFile.mkdirs() }
112+
89113
// JPMS opens required by the unit-test stack on JDK 17+:
90114
// - jdk.unsupported/sun.misc: HiddenApiBypass.<clinit> reflectively
91115
// resolves sun.misc.Unsafe; without this the IDEApplication
@@ -105,6 +129,14 @@ subprojects {
105129
"--add-opens=java.base/java.util=ALL-UNNAMED",
106130
"--add-opens=java.base/java.util.concurrent=ALL-UNNAMED",
107131
"--add-opens=jdk.unsupported/sun.misc=ALL-UNNAMED",
132+
// An OutOfMemoryError inside a test deadlocks Gradle's TestWorker: the
133+
// OOM unwinds the runQueue.take() loop, whose finally needs the TestWorker
134+
// monitor that the in-flight stop() already holds while blocked on
135+
// runQueue.put() (capacity 1). The worker then never exits and the build
136+
// hangs. Exiting on the first OOM turns that hang into a task failure.
137+
"-XX:+ExitOnOutOfMemoryError",
138+
"-XX:+HeapDumpOnOutOfMemoryError",
139+
"-XX:HeapDumpPath=${heapDumpFile.absolutePath}",
108140
)
109141

110142
// Attach jacoco agent
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# 0010. Kotlin navigation resolves via the Analysis API, not the symbol index
2+
3+
- **Status:** Proposed
4+
- **Date:** 2026-07-27
5+
- **Deciders:** Code On The Go team
6+
7+
## Context
8+
9+
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".
10+
11+
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.
12+
13+
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.
14+
15+
## Decision
16+
17+
**Kotlin navigation resolves through the Analysis API and PSI only. The symbol indexes are not consulted.**
18+
19+
- 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.
20+
- Convert each resolved symbol to its PSI declaration, and the declaration to a file plus a name-identifier range.
21+
- Correctness of scoping - module dependencies, content scopes, visibility - is delegated to the analysis session rather than reimplemented over index rows.
22+
23+
## Consequences
24+
25+
**Positive**
26+
27+
- Results are *resolved*, not name-matched: the right overload, the right receiver, the right module.
28+
- 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`.
29+
- One code path for all three resolution scopes (same-file, inter-file, inter-module), so the test matrix covers behaviour rather than plumbing.
30+
31+
**Negative / costs**
32+
33+
- **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.
34+
- 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.
35+
- 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.
36+
37+
## Alternatives considered
38+
39+
- **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.
40+
- **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.
41+
- **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.
42+
43+
## Related
44+
45+
- [docs/features/kotlin-goto-definition.md](../features/kotlin-goto-definition.md) - the first feature built on this decision
46+
- [ADR 0001](0001-prefer-room-for-persistence.md) - persistence choices for the indexes this ADR declines to use

‎docs/adr/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,4 @@ Format is lightweight **MADR / Nygard**: Context → Decision → Consequences
2323
| [0007](0007-strictmode-whitelist-engine.md) | Enforce StrictMode via a custom whitelist engine | Proposed |
2424
| [0008](0008-retain-androidide-namespace.md) | Retain the `com.itsaky.androidide` namespace after rebrand | Proposed |
2525
| [0009](0009-jetpack-compose-for-new-ui.md) | Build new UI in Jetpack Compose, not XML Views | Proposed |
26+
| [0010](0010-navigation-resolves-via-analysis-api.md) | Kotlin navigation resolves via the Analysis API, not the symbol index | Proposed |

0 commit comments

Comments
 (0)