Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
666191b
ADFA-4823: Let test source modules depend on each other
itsaky-adfa Jul 28, 2026
c80ef98
ADFA-4823: Fix lsp:kotlin unit-test OOM (worker heap + env disposal)
itsaky-adfa Jul 28, 2026
698a0e1
ADFA-4823: Map a Kotlin caret offset to the reference it names
itsaky-adfa Jul 28, 2026
db57f9e
ADFA-4823: Resolve a Kotlin reference to its declaration's location
itsaky-adfa Jul 28, 2026
92e4f20
ADFA-4823: Cover convention references in go-to-definition tests
itsaky-adfa Jul 28, 2026
8ff4577
ADFA-4823: Classify source modules structurally so Java roots are ind…
itsaky-adfa Jul 28, 2026
9116f14
ADFA-4823: Cover resolution scopes, candidates and ranges
itsaky-adfa Jul 28, 2026
2941605
ADFA-4823: Propagate analysis cancellation instead of swallowing it
itsaky-adfa Jul 28, 2026
7f74fe8
ADFA-4823: Implement KotlinLanguageServer.findDefinition
itsaky-adfa Jul 28, 2026
c7acd90
ADFA-4823: Add Go to definition to the Kotlin code actions menu
itsaky-adfa Jul 28, 2026
ea64281
ADFA-4823: Document Kotlin navigation's requirements and ADR
itsaky-adfa Jul 28, 2026
80586e7
ADFA-4823: Fix go-to-definition returning nothing for the open file
itsaky-adfa Jul 28, 2026
bc54742
ADFA-4823: Retry preempted definitions, tighten a vacuous test, fix s…
itsaky-adfa Jul 28, 2026
a9af680
Merge branch 'stage' into worktree/ADFA-4823
itsaky-adfa Jul 28, 2026
87b30cd
Merge branch 'stage' into worktree/ADFA-4823
itsaky-adfa Jul 29, 2026
d3f15d2
ADFA-4823: Keep coverage and Sonar analysis running when unit tests fail
itsaky-adfa Jul 29, 2026
53b02df
ADFA-4823: Write test heap dumps under build/, not the source tree
itsaky-adfa Jul 29, 2026
bc72b0a
ADFA-4823: Harden go-to-definition against scripts, throwing referenc…
itsaky-adfa Jul 29, 2026
3251f2c
ADFA-4823: Stop the LSP test fixture masking failures and leaking on …
itsaky-adfa Jul 29, 2026
3f53365
Merge branch 'stage' into worktree/ADFA-4823
itsaky-adfa Jul 31, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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")

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

tasks.withType<Test> {
// 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.<clinit> reflectively
// resolves sun.misc.Unsafe; without this the IDEApplication
Expand All @@ -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
Expand Down
46 changes: 46 additions & 0 deletions docs/adr/0010-navigation-resolves-via-analysis-api.md
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions docs/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Loading
Loading