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 6c57c89955..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 @@ -18,22 +16,18 @@ 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 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) @@ -42,37 +36,23 @@ class AddImportAction : BaseKotlinCodeAction() { return } - val extra = data.require().extra as? KotlinDiagnosticExtra - if (extra == null) { - markInvisible() - return - } - - val action = extra.action - if (action !is DiagnosticAction.ResolveReference) { - markInvisible() - return - } - - val env = extra.compilationEnv - val hasImportableSymbols = - env.ktSymbolIndex - .findSymbolBySimpleName(action.referenceName, limit = 0) - .any { it.kind.isClassifier } - - if (!hasImportableSymbols) { + // 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 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 = @@ -104,6 +84,7 @@ class AddImportAction : BaseKotlinCodeAction() { if (result.isEmpty()) { logger.warn("No classifiers to import.") + flashError(R.string.msg_no_imports_found) return } 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..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 @@ -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,10 +14,9 @@ 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.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -38,23 +35,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 +49,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() @@ -85,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() } 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/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( 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..4edbe11662 --- /dev/null +++ b/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, + ) + } +} 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..b1357e5a9a --- /dev/null +++ b/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 { + 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) + } +} diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index a03ea5596d..dee0dff70d 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