From a2fcc0114994e4dfea6b462cc6bb0fb17d500574 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 17 Jul 2026 12:57:19 +0000 Subject: [PATCH 1/6] ADFA-4747: Fix Add-import action never appearing on unresolved references AddImportAction.prepare() gated visibility on findSymbolBySimpleName(name, 0), but the helper ended with Sequence.take(limit) and take(0) returns empty, so the action was invisible for every unresolved-reference diagnostic - dead since ADFA-3754. Treat limit <= 0 as unbounded, honoring the documented ReadableIndex.query contract the helper was violating. Also extract hasImportableClassifier and a non-suspend computeImportCandidates (runCatching, KtFile fetched before project.read) for testability and crash-safety, and drop the no-op CMD_FORMAT_CODE. Adds AddImportActionTest (visibility-gate regression) and EditExtsTest. --- .../lsp/kotlin/actions/AddImportAction.kt | 80 +++++++---- .../kotlin/compiler/index/KtSymbolIndex.kt | 10 +- .../lsp/kotlin/actions/AddImportActionTest.kt | 125 ++++++++++++++++++ .../lsp/kotlin/utils/EditExtsTest.kt | 102 ++++++++++++++ 4 files changed, 291 insertions(+), 26 deletions(-) create mode 100644 lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt create mode 100644 lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/EditExtsTest.kt diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt index 9896343851..784034c8a8 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt @@ -7,7 +7,9 @@ import com.itsaky.androidide.actions.newDialogBuilder import com.itsaky.androidide.actions.require import com.itsaky.androidide.actions.requireFile import com.itsaky.androidide.idetooltips.TooltipTag +import com.itsaky.androidide.lsp.kotlin.compiler.AbstractCompilationEnvironment import com.itsaky.androidide.lsp.kotlin.compiler.index.findSymbolBySimpleName +import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.diagnostic.KotlinDiagnosticExtra import com.itsaky.androidide.lsp.kotlin.utils.insertImport import com.itsaky.androidide.lsp.models.CodeActionItem @@ -17,8 +19,8 @@ import com.itsaky.androidide.lsp.models.DiagnosticItem import com.itsaky.androidide.lsp.models.DocumentChange import com.itsaky.androidide.lsp.models.TextEdit import com.itsaky.androidide.resources.R -import org.appdevforall.codeonthego.indexing.jvm.JvmSymbol import org.slf4j.LoggerFactory +import java.nio.file.Path class AddImportAction : BaseKotlinCodeAction() { override var titleTextRes: Int = R.string.action_import_classes @@ -51,39 +53,65 @@ class AddImportAction : BaseKotlinCodeAction() { return } - val env = extra.compilationEnv - val hasImportableSymbols = - env.ktSymbolIndex - .findSymbolBySimpleName(reference, limit = 0) - .any { it.kind.isClassifier } - - if (!hasImportableSymbols) { + // Known main-thread I/O: prepare() runs on the UI thread (menu build) and this resolves + // against the SQLite-backed symbol index synchronously. Pre-existing (ADFA-3754); tracked + // as a follow-up to move the visibility lookup off the main thread. Keep it cheap here. + if (!hasImportableClassifier(extra.compilationEnv, reference)) { markInvisible() return } } - override suspend fun execAction(data: ActionData): Map> { + /** + * True when [reference] resolves to at least one importable classifier in [env]'s indexes. + * This is the exact predicate that gates the action's visibility in [prepare]. + */ + internal fun hasImportableClassifier( + env: AbstractCompilationEnvironment, + reference: String, + ): Boolean = + env.ktSymbolIndex + .findSymbolBySimpleName(reference, limit = 0) + .any { it.kind.isClassifier } + + override suspend fun execAction(data: ActionData): Map> { val (reference, env) = data.require().extra as? KotlinDiagnosticExtra ?: return emptyMap() if (reference == null) return emptyMap() - val file = data.requireFile() - val nioPath = file.toPath() - val ktFile = - env.ktSymbolIndex - .getCurrentKtFile(nioPath) - .get() - ?: return emptyMap() - - return env.ktSymbolIndex - .findSymbolBySimpleName(reference, limit = 0) - .filter { it.kind.isClassifier } - .associateWith { symbol -> insertImport(ktFile, symbol.fqName) } + return computeImportCandidates(env, data.requireFile().toPath(), reference) } + /** + * Computes, for the unresolved [reference] in the file at [nioPath] within [env], a map from + * each importable classifier's fully-qualified name to the edits that add its import in sorted + * position. The [org.jetbrains.kotlin.psi.KtFile] is fetched BEFORE entering [read] (deadlock + * rule: never block on `getCurrentKtFile(...).get()` inside `project.read`). Keying by FQN + * collapses the duplicate a symbol picks up from being present in both the source and library + * indexes. Returns an empty map when there is nothing to import *and* whenever anything in this + * pipeline throws: the action framework only catches [IllegalArgumentException] and this runs on + * a coroutine scope with no exception handler, so an uncaught throw here would crash the app. + */ + internal fun computeImportCandidates( + env: AbstractCompilationEnvironment, + nioPath: Path, + reference: String, + ): Map> = + runCatching { + val ktFile = env.ktSymbolIndex.getCurrentKtFile(nioPath).get() ?: return emptyMap() + env.project.read { + env.ktSymbolIndex + .findSymbolBySimpleName(reference, limit = 0) + .filter { it.kind.isClassifier } + .associate { symbol -> symbol.fqName to insertImport(ktFile, symbol.fqName) } + } + }.getOrElse { e -> + logger.warn("Failed to compute import candidates for '{}'", reference, e) + emptyMap() + } + override fun postExec( data: ActionData, result: Any, @@ -95,7 +123,7 @@ class AddImportAction : BaseKotlinCodeAction() { } @Suppress("UNCHECKED_CAST") - result as Map> + result as Map> if (result.isEmpty()) { logger.warn("No classifiers to import.") @@ -113,12 +141,14 @@ class AddImportAction : BaseKotlinCodeAction() { val nioPath = file.toPath() val actions = result - .map { (symbol, edits) -> + .map { (fqName, edits) -> CodeActionItem( - title = symbol.fqName, + title = fqName, changes = listOf(DocumentChange(file = nioPath, edits = edits)), kind = CodeActionKind.QuickFix, - command = Command.CMD_FORMAT_CODE, + // Imports are column-0 text; emit final text ourselves. CMD_FORMAT_CODE is a + // no-op for Kotlin, so use an empty (no-op) post-action command. + command = Command("", ""), ) } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.kt index a89a19e7e8..2b36a45cd7 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.kt @@ -336,8 +336,16 @@ internal fun KtSymbolIndex.filesForPackage(packageFqn: String) = fileIndex.getFi internal fun KtSymbolIndex.subpackageNames(packageFqn: String) = fileIndex.getSubpackageNames(packageFqn) +/** + * Returns source- and library-index symbols whose simple name equals [name]. + * + * [limit] `<= 0` means unbounded, honoring the same convention as + * [org.appdevforall.codeonthego.indexing.api.ReadableIndex.query] ("If IndexQuery.limit is 0, all + * matches are emitted"). A plain `take(limit)` would turn the common `limit = 0` call into + * `take(0)`, silently yielding no results. + */ internal fun KtSymbolIndex.findSymbolBySimpleName( name: String, limit: Int, ) = (sourceIndex.findBySimpleName(name, 0) + libraryIndex.findBySimpleName(name, 0)) - .take(limit) + .let { if (limit <= 0) it else it.take(limit) } diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt new file mode 100644 index 0000000000..811cdf9181 --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt @@ -0,0 +1,125 @@ +package com.itsaky.androidide.lsp.kotlin.actions + +import com.itsaky.androidide.lsp.kotlin.compiler.index.findSymbolBySimpleName +import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest +import kotlinx.coroutines.runBlocking +import org.appdevforall.codeonthego.indexing.jvm.JvmClassInfo +import org.appdevforall.codeonthego.indexing.jvm.JvmFunctionInfo +import org.appdevforall.codeonthego.indexing.jvm.JvmSourceLanguage +import org.appdevforall.codeonthego.indexing.jvm.JvmSymbol +import org.appdevforall.codeonthego.indexing.jvm.JvmSymbolKind +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class AddImportActionTest : KtLspTest() { + + private val mainPath get() = env.sourceRoots.first().resolve("Main.kt") + + private fun classSymbol( + pkg: String, + shortName: String, + ) = symbol(pkg, shortName, JvmSymbolKind.CLASS, JvmClassInfo()) + + private fun funSymbol( + pkg: String, + shortName: String, + ) = symbol(pkg, shortName, JvmSymbolKind.FUNCTION, JvmFunctionInfo()) + + private fun symbol( + pkg: String, + shortName: String, + kind: JvmSymbolKind, + data: org.appdevforall.codeonthego.indexing.jvm.JvmSymbolInfo, + ): JvmSymbol { + val internalName = "${pkg.replace('.', '/')}/$shortName" + return JvmSymbol( + key = "$internalName#${kind.name}", + sourceId = "test", + name = internalName, + shortName = shortName, + packageName = pkg, + kind = kind, + language = JvmSourceLanguage.KOTLIN, + data = data, + ) + } + + private fun index(vararg symbols: JvmSymbol) = + runBlocking { symbols.forEach { env.ktSymbolIndex.sourceIndex.insert(it) } } + + @Test + fun `resolves a single classifier candidate by simple name`() { + index(classSymbol("lib", "Foo")) + createSourceFile("Main.kt", "package p\nimport lib.Bar\nfun f(x: Foo) {}") + + val candidates = AddImportAction().computeImportCandidates(env, mainPath, "Foo") + + assertEquals(setOf("lib.Foo"), candidates.keys) + val edit = candidates.getValue("lib.Foo").single() + assertEquals("import lib.Foo", edit.newText.trim()) + } + + @Test + fun `offers every matching classifier for a multi-candidate reference`() { + index(classSymbol("a", "Foo"), classSymbol("b", "Foo")) + createSourceFile("Main.kt", "package p\nfun f(x: Foo) {}") + + val candidates = AddImportAction().computeImportCandidates(env, mainPath, "Foo") + + assertEquals(setOf("a.Foo", "b.Foo"), candidates.keys) + candidates.values.forEach { assertEquals(1, it.size) } + } + + @Test + fun `filters out non-classifier symbols`() { + index(classSymbol("a", "Foo"), funSymbol("b", "Foo")) + createSourceFile("Main.kt", "package p\nfun f(x: Foo) {}") + + val candidates = AddImportAction().computeImportCandidates(env, mainPath, "Foo") + + assertEquals(setOf("a.Foo"), candidates.keys) + } + + @Test + fun `returns no candidates for an unknown reference`() { + createSourceFile("Main.kt", "package p\nfun f() {}") + + assertTrue(AddImportAction().computeImportCandidates(env, mainPath, "Nope").isEmpty()) + } + + /** + * The user-facing bug: the action never appeared on an unresolved-reference diagnostic. + * prepare() gates visibility on exactly this predicate; `take(0)` made it always false. Guards + * that the action goes VISIBLE when the reference resolves to an importable classifier. + */ + @Test + fun `visibility gate is true when the reference resolves to an importable classifier`() { + index(classSymbol("lib", "Foo")) + + assertTrue(AddImportAction().hasImportableClassifier(env, "Foo")) + } + + @Test + fun `visibility gate is false for an unknown or non-classifier reference`() { + index(funSymbol("lib", "Bar")) + + assertFalse(AddImportAction().hasImportableClassifier(env, "Bar")) + assertFalse(AddImportAction().hasImportableClassifier(env, "Unknown")) + } + + /** + * Regression guard: `findSymbolBySimpleName` is called with `limit = 0` (unbounded). A plain + * `take(0)` would return nothing, silently disabling the whole Add-import action. + */ + @Test + fun `findSymbolBySimpleName treats limit 0 as unbounded and a positive limit as a cap`() { + index(classSymbol("a", "Foo"), classSymbol("b", "Foo"), classSymbol("c", "Foo")) + + val unbounded = env.ktSymbolIndex.findSymbolBySimpleName("Foo", limit = 0).toList() + assertEquals(setOf("a.Foo", "b.Foo", "c.Foo"), unbounded.map { it.fqName }.toSet()) + + assertEquals(2, env.ktSymbolIndex.findSymbolBySimpleName("Foo", limit = 2).toList().size) + } +} diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/EditExtsTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/EditExtsTest.kt new file mode 100644 index 0000000000..7bf7d48758 --- /dev/null +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/EditExtsTest.kt @@ -0,0 +1,102 @@ +package com.itsaky.androidide.lsp.kotlin.utils + +import com.itsaky.androidide.lsp.kotlin.compiler.read +import com.itsaky.androidide.lsp.kotlin.fixtures.KtLspTest +import com.itsaky.androidide.lsp.models.TextEdit +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** Tests for [insertImport]: the sorted-insertion + dedup logic the Add-import action relies on. */ +class EditExtsTest : KtLspTest() { + + private val nl = System.lineSeparator() + + private fun insert( + source: String, + fqn: String, + ): List { + val ktFile = createSourceFile("Main.kt", source) + return env.project.read { insertImport(ktFile, fqn) } + } + + @Test + fun `inserts before the first import that sorts after it`() { + val edit = + insert( + """ + package p + import a.A + import c.C + """.trimIndent(), + "b.B", + ).single() + // Pure insertion at the start of `import c.C` (line 2, col 0). + assertEquals(edit.range.start, edit.range.end) + assertEquals(2, edit.range.start.line) + assertEquals(0, edit.range.start.column) + assertEquals("import b.B$nl", edit.newText) + } + + @Test + fun `inserts before the very first import`() { + val edit = + insert( + """ + package p + import b.B + """.trimIndent(), + "a.A", + ).single() + assertEquals(1, edit.range.start.line) + assertEquals(0, edit.range.start.column) + assertEquals("import a.A$nl", edit.newText) + } + + @Test + fun `appends after the last import when it sorts last`() { + val edit = + insert( + """ + package p + import a.A + import b.B + """.trimIndent(), + "z.Z", + ).single() + // Pure insertion at the end of `import b.B` (line 2, col 10). + assertEquals(edit.range.start, edit.range.end) + assertEquals(2, edit.range.start.line) + assertEquals(10, edit.range.start.column) + assertEquals("${nl}import z.Z", edit.newText) + } + + @Test + fun `skips an exact duplicate import`() { + assertTrue( + insert( + """ + package p + import a.A + """.trimIndent(), + "a.A", + ).isEmpty(), + ) + } + + @Test + fun `inserts after the package statement when there are no imports`() { + val edit = + insert( + """ + package p + + class X + """.trimIndent(), + "a.A", + ).single() + assertEquals(0, edit.range.start.line) + assertEquals(9, edit.range.start.column) + assertEquals("${nl}import a.A", edit.newText) + } +} From b3c52da2a9be8781c0791f1484c6628f2c3d9a9f Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 17 Jul 2026 14:00:22 +0000 Subject: [PATCH 2/6] ADFA-4747: Make AddImportAction.prepare() in-memory (no main-thread I/O) Replace the SQLite-backed visibility gate with an in-memory check on the unresolved-reference marker. prepare() runs synchronously on the UI thread during menu build, so resolving against the symbol index there was main-thread disk I/O; that resolution now happens only in the background execAction. --- .../lsp/kotlin/actions/AddImportAction.kt | 31 +++---------------- .../lsp/kotlin/actions/AddImportActionTest.kt | 21 ------------- 2 files changed, 4 insertions(+), 48 deletions(-) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt index 784034c8a8..2a3fab2561 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt @@ -41,39 +41,16 @@ class AddImportAction : BaseKotlinCodeAction() { return } + // Optimistic visibility: decide from the in-memory unresolved-reference marker only. The + // importable-classifier resolution runs in the background execAction; doing it here would be + // main-thread SQLite I/O, because fillMenu() calls prepare() synchronously on the UI thread. val extra = data.require().extra as? KotlinDiagnosticExtra - if (extra == null) { - markInvisible() - return - } - - val reference = extra.unresolvedReference - if (reference == null) { - markInvisible() - return - } - - // Known main-thread I/O: prepare() runs on the UI thread (menu build) and this resolves - // against the SQLite-backed symbol index synchronously. Pre-existing (ADFA-3754); tracked - // as a follow-up to move the visibility lookup off the main thread. Keep it cheap here. - if (!hasImportableClassifier(extra.compilationEnv, reference)) { + if (extra?.unresolvedReference == null) { markInvisible() return } } - /** - * True when [reference] resolves to at least one importable classifier in [env]'s indexes. - * This is the exact predicate that gates the action's visibility in [prepare]. - */ - internal fun hasImportableClassifier( - env: AbstractCompilationEnvironment, - reference: String, - ): Boolean = - env.ktSymbolIndex - .findSymbolBySimpleName(reference, limit = 0) - .any { it.kind.isClassifier } - override suspend fun execAction(data: ActionData): Map> { val (reference, env) = data.require().extra as? KotlinDiagnosticExtra diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt index 811cdf9181..44355d8b1c 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt @@ -9,7 +9,6 @@ import org.appdevforall.codeonthego.indexing.jvm.JvmSourceLanguage import org.appdevforall.codeonthego.indexing.jvm.JvmSymbol import org.appdevforall.codeonthego.indexing.jvm.JvmSymbolKind import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test @@ -89,26 +88,6 @@ class AddImportActionTest : KtLspTest() { assertTrue(AddImportAction().computeImportCandidates(env, mainPath, "Nope").isEmpty()) } - /** - * The user-facing bug: the action never appeared on an unresolved-reference diagnostic. - * prepare() gates visibility on exactly this predicate; `take(0)` made it always false. Guards - * that the action goes VISIBLE when the reference resolves to an importable classifier. - */ - @Test - fun `visibility gate is true when the reference resolves to an importable classifier`() { - index(classSymbol("lib", "Foo")) - - assertTrue(AddImportAction().hasImportableClassifier(env, "Foo")) - } - - @Test - fun `visibility gate is false for an unknown or non-classifier reference`() { - index(funSymbol("lib", "Bar")) - - assertFalse(AddImportAction().hasImportableClassifier(env, "Bar")) - assertFalse(AddImportAction().hasImportableClassifier(env, "Unknown")) - } - /** * Regression guard: `findSymbolBySimpleName` is called with `limit = 0` (unbounded). A plain * `take(0)` would return nothing, silently disabling the whole Add-import action. From e83b550b0b3aa4c31c8be9ed0c5fc62369aa8b0e Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 17 Jul 2026 14:17:55 +0000 Subject: [PATCH 3/6] ADFA-4747: Report no-imports-found for optimistic Add-import action --- .../com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt | 2 ++ resources/src/main/res/values/strings.xml | 1 + 2 files changed, 3 insertions(+) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt index 2a3fab2561..59d05fe5fc 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt @@ -19,6 +19,7 @@ import com.itsaky.androidide.lsp.models.DiagnosticItem import com.itsaky.androidide.lsp.models.DocumentChange import com.itsaky.androidide.lsp.models.TextEdit import com.itsaky.androidide.resources.R +import com.itsaky.androidide.utils.flashError import org.slf4j.LoggerFactory import java.nio.file.Path @@ -104,6 +105,7 @@ class AddImportAction : BaseKotlinCodeAction() { if (result.isEmpty()) { logger.warn("No classifiers to import.") + flashError(R.string.msg_no_imports_found) return } diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index fa547343a5..a5f070e1d3 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -94,6 +94,7 @@ No references found + No imports found Diagnostics Installation failed Failed to install assets From bcbc6ea6da94ccb184e8e0cdc8246518372a7372 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 17 Jul 2026 20:23:51 +0530 Subject: [PATCH 4/6] refactor: reformat Signed-off-by: Akash Yadav --- .../lsp/kotlin/actions/AddImportAction.kt | 13 ++++++++++--- .../lsp/kotlin/actions/AddImportActionTest.kt | 12 ++++++++---- .../androidide/lsp/kotlin/utils/EditExtsTest.kt | 1 - 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt index 59d05fe5fc..06eac6c86b 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt @@ -132,9 +132,15 @@ class AddImportAction : BaseKotlinCodeAction() { } when (actions.size) { - 0 -> logger.error("No code actions found. Cannot completion action.") - 1 -> client.performCodeAction(actions[0]) - else -> + 0 -> { + logger.error("No code actions found. Cannot completion action.") + } + + 1 -> { + client.performCodeAction(actions[0]) + } + + else -> { newDialogBuilder(data) .setTitle(label) .setItems(actions.map { it.title }.toTypedArray()) { dialog, which -> @@ -144,6 +150,7 @@ class AddImportAction : BaseKotlinCodeAction() { logger.error("Index $which is out of bounds for actions of size ${actions.size}") } }.show() + } } } } diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt index 44355d8b1c..4edbe11662 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt @@ -13,7 +13,6 @@ import org.junit.Assert.assertTrue import org.junit.Test class AddImportActionTest : KtLspTest() { - private val mainPath get() = env.sourceRoots.first().resolve("Main.kt") private fun classSymbol( @@ -45,8 +44,7 @@ class AddImportActionTest : KtLspTest() { ) } - private fun index(vararg symbols: JvmSymbol) = - runBlocking { symbols.forEach { env.ktSymbolIndex.sourceIndex.insert(it) } } + private fun index(vararg symbols: JvmSymbol) = runBlocking { symbols.forEach { env.ktSymbolIndex.sourceIndex.insert(it) } } @Test fun `resolves a single classifier candidate by simple name`() { @@ -99,6 +97,12 @@ class AddImportActionTest : KtLspTest() { val unbounded = env.ktSymbolIndex.findSymbolBySimpleName("Foo", limit = 0).toList() assertEquals(setOf("a.Foo", "b.Foo", "c.Foo"), unbounded.map { it.fqName }.toSet()) - assertEquals(2, env.ktSymbolIndex.findSymbolBySimpleName("Foo", limit = 2).toList().size) + assertEquals( + 2, + env.ktSymbolIndex + .findSymbolBySimpleName("Foo", limit = 2) + .toList() + .size, + ) } } diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/EditExtsTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/EditExtsTest.kt index 7bf7d48758..b1357e5a9a 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/EditExtsTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/EditExtsTest.kt @@ -9,7 +9,6 @@ import org.junit.Test /** Tests for [insertImport]: the sorted-insertion + dedup logic the Add-import action relies on. */ class EditExtsTest : KtLspTest() { - private val nl = System.lineSeparator() private fun insert( From 454bd62496b4ac571739249efa3e2e2b435f0afb Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Thu, 23 Jul 2026 11:10:51 +0000 Subject: [PATCH 5/6] fix: make KotlinDiagnosticExtra type-safe Signed-off-by: Akash Yadav --- .../androidide/idetooltips/TooltipTag.kt | 1 + .../lsp/kotlin/actions/AddImportAction.kt | 26 +++++------------- .../kotlin/actions/BaseKotlinCodeAction.kt | 27 +++++++++++++++++++ .../kotlin/actions/ImplementMembersAction.kt | 5 ---- .../lsp/kotlin/actions/NullSafetyAction.kt | 27 +++++-------------- .../diagnostic/KotlinDiagnosticProvider.kt | 24 +++++++++++++---- 6 files changed, 61 insertions(+), 49 deletions(-) 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 3fd18b46be..cacf5cc30e 100644 --- a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt +++ b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt @@ -79,6 +79,7 @@ object TooltipTag { const val EDITOR_CODE_ACTIONS_GEN_TO_STRING_DIALOG = "editor.codeactions.gentostring.dialog" const val EDITOR_CODE_ACTIONS_UNUSED_IMPORTS = "editor.codeactions.unusedimports" const val EDITOR_CODE_ACTIONS_ORGANIZE_IMPORTS = "editor.codeactions.organizeimports" + const val EDITOR_CODE_ACTIONS_KT_FIX_IMPORTS = "editor.codeactions.kotlin.fiximports" const val EXIT_TO_MAIN = "exit.to.main" diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt index a4a478cf6d..362c5effa4 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt @@ -4,12 +4,10 @@ import com.itsaky.androidide.actions.ActionData import com.itsaky.androidide.actions.has import com.itsaky.androidide.actions.markInvisible import com.itsaky.androidide.actions.newDialogBuilder -import com.itsaky.androidide.actions.require import com.itsaky.androidide.actions.requireFile import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.lsp.kotlin.compiler.index.findSymbolBySimpleName import com.itsaky.androidide.lsp.kotlin.diagnostic.DiagnosticAction -import com.itsaky.androidide.lsp.kotlin.diagnostic.KotlinDiagnosticExtra import com.itsaky.androidide.lsp.kotlin.utils.insertImport import com.itsaky.androidide.lsp.models.CodeActionItem import com.itsaky.androidide.lsp.models.CodeActionKind @@ -22,19 +20,14 @@ import com.itsaky.androidide.utils.flashError import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import org.appdevforall.codeonthego.indexing.jvm.JvmSymbol -import org.slf4j.LoggerFactory class AddImportAction : BaseKotlinCodeAction() { override var titleTextRes: Int = R.string.action_import_classes - override var tooltipTag: String = TooltipTag.EDITOR_CODE_ACTIONS_FIX_IMPORTS + override var tooltipTag: String = TooltipTag.EDITOR_CODE_ACTIONS_KT_FIX_IMPORTS override val id: String = "ide.editor.lsp.kt.diagnostics.addImport" override var label: String = "" - companion object { - private val logger = LoggerFactory.getLogger(AddImportAction::class.java) - } - override fun prepare(data: ActionData) { super.prepare(data) @@ -46,25 +39,20 @@ class AddImportAction : BaseKotlinCodeAction() { // Optimistic visibility: decide from the in-memory unresolved-reference marker only. The // importable-classifier resolution runs in the background execAction; doing it here would be // main-thread SQLite I/O, because fillMenu() calls prepare() synchronously on the UI thread. - val extra = data.require().extra as? KotlinDiagnosticExtra - if (extra == null) { - markInvisible() - return - } - - if (extra.action !is DiagnosticAction.ResolveReference) { + val resolveReferenceActionDiagnostic = + data.findDiagnosticExtra() + if (resolveReferenceActionDiagnostic == null) { markInvisible() return } } override suspend fun execAction(data: ActionData): Map> { - val (env, action) = - data.require().extra as? KotlinDiagnosticExtra + val (_, extra) = + data.findDiagnosticExtra() ?: return emptyMap() - if (action !is DiagnosticAction.ResolveReference) return emptyMap() - + val (env, action) = extra val file = data.requireFile() val nioPath = file.toPath() val ktFile = diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/BaseKotlinCodeAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/BaseKotlinCodeAction.kt index 8cf859863c..191c665976 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/BaseKotlinCodeAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/BaseKotlinCodeAction.kt @@ -13,6 +13,11 @@ import com.itsaky.androidide.actions.requireContext import com.itsaky.androidide.actions.requireFile import com.itsaky.androidide.lsp.api.ILanguageClient import com.itsaky.androidide.lsp.kotlin.KotlinLanguageServer +import com.itsaky.androidide.lsp.kotlin.diagnostic.DiagnosticAction +import com.itsaky.androidide.lsp.kotlin.diagnostic.KotlinDiagnosticExtra +import com.itsaky.androidide.lsp.kotlin.diagnostic.asAction +import com.itsaky.androidide.lsp.models.DiagnosticItem +import com.itsaky.androidide.lsp.models.DiagnosticsInSelection import com.itsaky.androidide.utils.DocumentUtils import org.slf4j.Logger import org.slf4j.LoggerFactory @@ -58,4 +63,26 @@ abstract class BaseKotlinCodeAction : EditorActionItem { get() = get() ?.client + + internal inline fun DiagnosticItem.ktExtra(): KotlinDiagnosticExtra? = + (extra as? KotlinDiagnosticExtra<*>)?.asAction() + + /** + * Find the first [DiagnosticItem] in the current selection matching the given [predicate]. + * Prefers [DiagnosticsInSelection] (any matching diagnostic in the selected area); falls back + * to the at-selection-start [DiagnosticItem] when no container is present. + */ + protected inline fun ActionData.findFirstDiagnosticItem(predicate: (DiagnosticItem) -> Boolean): DiagnosticItem? = + get() + ?.let { return it.diagnostics.firstOrNull(predicate) } + ?: get()?.takeIf(predicate) + + /** + * Find the first in-selection [DiagnosticItem] whose extra carries a [T] action, paired with the + * typed [KotlinDiagnosticExtra]. Carries the type evidence through so callers don't re-extract. + */ + internal inline fun ActionData.findDiagnosticExtra(): Pair>? { + val item = findFirstDiagnosticItem { it.ktExtra() != null } ?: return null + return item to item.ktExtra()!! + } } diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt index 5c851f3c4a..a432403754 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt @@ -26,7 +26,6 @@ import org.jetbrains.kotlin.com.intellij.openapi.util.TextRange import org.jetbrains.kotlin.com.intellij.psi.util.PsiTreeUtil import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtFile -import org.slf4j.LoggerFactory import java.nio.file.Path class ImplementMembersAction : BaseKotlinCodeAction() { @@ -35,10 +34,6 @@ class ImplementMembersAction : BaseKotlinCodeAction() { override val id: String = "ide.editor.lsp.kt.implementMembers" override var label: String = "" - companion object { - private val logger = LoggerFactory.getLogger(ImplementMembersAction::class.java) - } - // Intentionally no prepare() visibility gate: the action is visible on any Kotlin file (BaseKotlinCodeAction // only checks the file type) and simply produces no edit when the enclosing class/object has nothing to // implement. Deciding that up front needs a K2 analysis session, which is too costly for prepare() (UI diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt index 8b09511e09..4223da8166 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt @@ -1,14 +1,12 @@ package com.itsaky.androidide.lsp.kotlin.actions import com.itsaky.androidide.actions.ActionData -import com.itsaky.androidide.actions.get import com.itsaky.androidide.actions.markInvisible import com.itsaky.androidide.actions.newDialogBuilder import com.itsaky.androidide.actions.requireContext import com.itsaky.androidide.actions.requireFile import com.itsaky.androidide.lsp.kotlin.compiler.read import com.itsaky.androidide.lsp.kotlin.diagnostic.DiagnosticAction -import com.itsaky.androidide.lsp.kotlin.diagnostic.KotlinDiagnosticExtra import com.itsaky.androidide.lsp.kotlin.utils.NullSafetyKind import com.itsaky.androidide.lsp.kotlin.utils.NullSafetyVariant import com.itsaky.androidide.lsp.kotlin.utils.findNullableMemberAccess @@ -16,8 +14,6 @@ import com.itsaky.androidide.lsp.kotlin.utils.nullSafetyVariants import com.itsaky.androidide.lsp.models.CodeActionItem import com.itsaky.androidide.lsp.models.CodeActionKind import com.itsaky.androidide.lsp.models.Command -import com.itsaky.androidide.lsp.models.DiagnosticItem -import com.itsaky.androidide.lsp.models.DiagnosticsInSelection import com.itsaky.androidide.lsp.models.DocumentChange import com.itsaky.androidide.resources.R import kotlinx.coroutines.Dispatchers @@ -38,23 +34,13 @@ class NullSafetyAction : BaseKotlinCodeAction() { override val id: String = "ide.editor.lsp.kt.diagnostics.nullSafety" override var label: String = "" - /** - * The first null-safety-fixable diagnostic in the selection. Prefers [DiagnosticsInSelection] - * (any matching diagnostic in the selected region); falls back to the at-selection-start - * [DiagnosticItem] when no container is present. - */ - private fun ActionData.nullSafetyDiagnostic(): DiagnosticItem? { - val predicate = { d: DiagnosticItem -> - (d.extra as? KotlinDiagnosticExtra)?.action == DiagnosticAction.NullSafetyFix - } - get()?.let { return it.diagnostics.firstOrNull(predicate) } - return get()?.takeIf(predicate) - } - override fun prepare(data: ActionData) { super.prepare(data) - if (!visible || data.nullSafetyDiagnostic() == null) { + val nullSafetyFixDiagnostic = + data.findDiagnosticExtra() + + if (!visible || nullSafetyFixDiagnostic == null) { markInvisible() return } @@ -62,8 +48,9 @@ class NullSafetyAction : BaseKotlinCodeAction() { override suspend fun execAction(data: ActionData): List = runCatching { - val diagnostic = data.nullSafetyDiagnostic() ?: return emptyList() - val extra = diagnostic.extra as? KotlinDiagnosticExtra ?: return emptyList() + val (diagnostic, extra) = + data.findDiagnosticExtra() + ?: return emptyList() val nioPath = data.requireFile().toPath() diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt index 87d18b7137..7805c62800 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt @@ -23,11 +23,15 @@ import java.nio.file.Path private val logger = LoggerFactory.getLogger("KotlinDiagnosticProvider") -internal data class KotlinDiagnosticExtra( +internal data class KotlinDiagnosticExtra( val compilationEnv: CompilationEnvironment, - val action: DiagnosticAction, + val action: ActionT, ) +@Suppress("UNCHECKED_CAST") +internal inline fun KotlinDiagnosticExtra<*>?.asAction(): KotlinDiagnosticExtra? = + if (this?.action is T) this as KotlinDiagnosticExtra else null + internal sealed interface DiagnosticAction { data object None : DiagnosticAction @@ -98,9 +102,19 @@ private fun doAnalyze( // the KaLifetimeOwner diagnostic escape (see KotlinDiagnosticExtra). val action = when (diagnostic) { - is KaFirDiagnostic.UnresolvedReference -> DiagnosticAction.ResolveReference(diagnostic.reference) - is KaFirDiagnostic.UnsafeCall -> DiagnosticAction.NullSafetyFix - else -> DiagnosticAction.None + is KaFirDiagnostic.UnresolvedReference -> { + DiagnosticAction.ResolveReference( + diagnostic.reference, + ) + } + + is KaFirDiagnostic.UnsafeCall -> { + DiagnosticAction.NullSafetyFix + } + + else -> { + DiagnosticAction.None + } } add( From 9858bb352368e36eb6ba3b0b7a731e4a32d8a534 Mon Sep 17 00:00:00 2001 From: Akash Yadav Date: Fri, 24 Jul 2026 18:05:38 +0530 Subject: [PATCH 6/6] fix: rethrow CancellationException in NullSafetyAction Signed-off-by: Akash Yadav --- .../itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt index 4223da8166..3d8f21d435 100644 --- a/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt +++ b/lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt @@ -16,6 +16,7 @@ import com.itsaky.androidide.lsp.models.CodeActionKind import com.itsaky.androidide.lsp.models.Command import com.itsaky.androidide.lsp.models.DocumentChange import com.itsaky.androidide.resources.R +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -72,6 +73,7 @@ class NullSafetyAction : BaseKotlinCodeAction() { nullSafetyVariants(qe) } }.getOrElse { e -> + if (e is CancellationException) throw e logger.warn("Failed to compute null-safety fixes", e) emptyList() }