ADFA-4747: Fix Add-import code action not appearing on unresolved references

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,9 @@ 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 com.itsaky.androidide.utils.flashError
 import org.slf4j.LoggerFactory
+import java.nio.file.Path
 
 class AddImportAction : BaseKotlinCodeAction() {
 	override var titleTextRes: Int = R.string.action_import_classes
@@ -39,51 +42,54 @@ 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<DiagnosticItem>().extra as? KotlinDiagnosticExtra
-		if (extra == null) {
-			markInvisible()
-			return
-		}
-
-		val reference = extra.unresolvedReference
-		if (reference == null) {
-			markInvisible()
-			return
-		}
-
-		val env = extra.compilationEnv
-		val hasImportableSymbols =
-			env.ktSymbolIndex
-				.findSymbolBySimpleName(reference, limit = 0)
-				.any { it.kind.isClassifier }
-
-		if (!hasImportableSymbols) {
+		if (extra?.unresolvedReference == null) {
 			markInvisible()
 			return
 		}
 	}
 
-	override suspend fun execAction(data: ActionData): Map<JvmSymbol, List<TextEdit>> {
+	override suspend fun execAction(data: ActionData): Map<String, List<TextEdit>> {
 		val (reference, env) =
 			data.require<DiagnosticItem>().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<String, List<TextEdit>> =
+		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,10 +101,11 @@ class AddImportAction : BaseKotlinCodeAction() {
 		}
 
 		@Suppress("UNCHECKED_CAST")
-		result as Map<JvmSymbol, List<TextEdit>>
+		result as Map<String, List<TextEdit>>
 
 		if (result.isEmpty()) {
 			logger.warn("No classifiers to import.")
+			flashError(R.string.msg_no_imports_found)
 			return
 		}
 
@@ -113,19 +120,27 @@ 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("", ""),
 					)
 				}
 
 		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 ->
@@ -135,6 +150,7 @@ class AddImportAction : BaseKotlinCodeAction() {
 								logger.error("Index $which is out of bounds for actions of size ${actions.size}")
 							}
 					}.show()
+			}
 		}
 	}
 }

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) }

lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportActionTest.kt

@@ -0,0 +1,108 @@
+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.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())
+	}
+
+	/**
+	 * 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,
+		)
+	}
+}

lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/EditExtsTest.kt

@@ -0,0 +1,101 @@
+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<TextEdit> {
+		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)
+	}
+}

resources/src/main/res/values/strings.xml

@@ -94,6 +94,7 @@
 
 	<!-- Build Tools Installation -->
 	<string name="msg_no_references">No references found</string>
+	<string name="msg_no_imports_found">No imports found</string>
 	<string name="view_diags">Diagnostics</string>
 	<string name="title_installation_failed">Installation failed</string>
 	<string name="error_installation_failed">Failed to install assets</string>