-
-
Notifications
You must be signed in to change notification settings - Fork 34
ADFA-4614: K2-LSP code action — Organize imports #1502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
0bece05
editor: make SignatureHelpWindow tolerant of malformed and immutable …
itsaky-adfa 8963238
lsp/kotlin: add CallAtCursorFinder to locate the call at the cursor
itsaky-adfa ea857a7
lsp/kotlin: extract shared renderName and build SignatureInformation …
itsaky-adfa 2a6b6b7
lsp/kotlin: compute active parameter index with named-argument remapping
itsaky-adfa 810ef88
lsp/kotlin: assemble SignatureHelp from resolved call candidates
itsaky-adfa e7d4845
lsp/kotlin: enable signature help in KotlinLanguageServer
itsaky-adfa a5b82e8
lsp/kotlin: add signature-help test coverage (named-arg via buildSign…
itsaky-adfa 044dfe3
editor: dismiss signature-help popup instead of showing it empty
itsaky-adfa 9b7204a
lsp/kotlin: guard requireIndex() inside doSignatureHelp try block
itsaky-adfa d60f8a0
lsp/kotlin: add debug logging throughout signature help
itsaky-adfa 502a446
lsp/kotlin: resolve signature help against live document contents
itsaky-adfa 85db15b
ADFA-3322: add version-stamped current-KtFile cache with single-flight
itsaky-adfa b6228ca
ADFA-3322: drain current-file refresh executor on close; document ver…
itsaky-adfa 8dd1e87
ADFA-3322: invalidate FIR session and reindex inside current-file ref…
itsaky-adfa 42020b0
ADFA-3322: pull-model file refresh — onFileContentChanged only schedules
itsaky-adfa dea75ac
ADFA-3322: route all LSP features through getCurrentKtFile
itsaky-adfa d6b9541
ADFA-3322: remove openedFiles map, drain refresh executor on close
itsaky-adfa 51130e5
ADFA-3322: add current-KtFile cache regression tests and fix diagnost…
itsaky-adfa eedc5f7
ADFA-3322: avoid blocking parse on UI thread in edit handler; fix sta…
itsaky-adfa f230235
chore: simplify comments
itsaky-adfa e6e738b
refactor: reformat
itsaky-adfa 48be771
ADFA-4614: register no-op Organize Imports action skeleton
itsaky-adfa f96beec
ADFA-4614: add pure import-organizing logic with tests
itsaky-adfa 386b28d
ADFA-4614: collect used importable names via Analysis API
itsaky-adfa f2cc9f0
ADFA-4614: compute and apply organized imports in the action
itsaky-adfa 93d2922
ADFA-4614: extract computeOrganizeEdit helper and cover it end-to-end
itsaky-adfa e88dc5a
ADFA-4614: cover convention imports, harden action against analysis f…
itsaky-adfa 55b6e39
Merge branch 'stage' into feat/ADFA-4614
itsaky-adfa 7b63515
Merge branch 'stage' into feat/ADFA-4614
itsaky-adfa 058c6aa
fix: keep unresolved references when organizing imports
itsaky-adfa 34d4e64
ADFA-4614: keep imports used only via a constructor
itsaky-adfa 485b14f
Merge branch 'stage' into feat/ADFA-4614
itsaky-adfa c44d0fb
chore: remove specs/plans
itsaky-adfa 93ca137
refactor: reformat
itsaky-adfa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
1,040 changes: 0 additions & 1,040 deletions
1,040
docs/superpowers/plans/2026-06-25-enhance-ai-assistant-plugin-ui.md
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
93 changes: 93 additions & 0 deletions
93
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| package com.itsaky.androidide.lsp.kotlin.actions | ||
|
|
||
| import com.itsaky.androidide.actions.ActionData | ||
| import com.itsaky.androidide.actions.get | ||
| import com.itsaky.androidide.actions.requireFile | ||
| import com.itsaky.androidide.lsp.kotlin.KotlinLanguageServer | ||
| import com.itsaky.androidide.lsp.kotlin.compiler.AbstractCompilationEnvironment | ||
| import com.itsaky.androidide.lsp.kotlin.compiler.modules.analyzeMaybeDangling | ||
| import com.itsaky.androidide.lsp.kotlin.compiler.read | ||
| import com.itsaky.androidide.lsp.kotlin.utils.collectImportUsage | ||
| import com.itsaky.androidide.lsp.kotlin.utils.organizedImportBlock | ||
| import com.itsaky.androidide.lsp.kotlin.utils.toRange | ||
| 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.DocumentChange | ||
| import com.itsaky.androidide.lsp.models.TextEdit | ||
| import com.itsaky.androidide.models.Range | ||
| import com.itsaky.androidide.resources.R | ||
| import org.slf4j.LoggerFactory | ||
| import java.nio.file.Path | ||
|
|
||
| class OrganizeImportsAction : BaseKotlinCodeAction() { | ||
| override var titleTextRes: Int = R.string.action_organize_imports | ||
| override val id: String = "ide.editor.lsp.kt.organizeImports" | ||
| override var label: String = "" | ||
|
|
||
| companion object { | ||
| private val logger = LoggerFactory.getLogger(OrganizeImportsAction::class.java) | ||
| } | ||
|
|
||
| override suspend fun execAction(data: ActionData): List<TextEdit> { | ||
| val server = data.get<KotlinLanguageServer>() ?: return emptyList() | ||
| val nioPath = data.requireFile().toPath() | ||
| val env = server.compilationEnvironmentFor(nioPath) ?: return emptyList() | ||
| return computeOrganizeEdit(env, nioPath) | ||
| } | ||
|
|
||
| /** | ||
| * Computes the text edits that organize the imports of the file at [nioPath] within [env]. | ||
| * The current [org.jetbrains.kotlin.psi.KtFile] is fetched BEFORE entering [read] (deadlock | ||
| * rule: never block on `getCurrentKtFile(...).get()` inside `project.read`). Returns an empty | ||
| * list when there is nothing to do (no imports, already organized, or no usable range) *and* | ||
| * whenever anything in this pipeline (the `.get()`, analysis, or PSI access) 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. Degrading to zero edits is | ||
| * always safe -- it just leaves the imports as-is, never produces a partial/incorrect rewrite. | ||
| */ | ||
| internal fun computeOrganizeEdit( | ||
| env: AbstractCompilationEnvironment, | ||
| nioPath: Path, | ||
| ): List<TextEdit> = | ||
| runCatching { | ||
| val ktFile = env.ktSymbolIndex.getCurrentKtFile(nioPath).get() ?: return emptyList() | ||
| if (ktFile.importDirectives.isEmpty()) return emptyList() | ||
| env.project.read { | ||
| val usage = analyzeMaybeDangling(ktFile) { collectImportUsage(ktFile) } | ||
| val newText = organizedImportBlock(ktFile, usage) ?: return@read emptyList() | ||
| val range = ktFile.importList?.textRange?.toRange(ktFile) ?: return@read emptyList() | ||
| if (range == Range.NONE) return@read emptyList() | ||
| listOf(TextEdit(range, newText)) | ||
| } | ||
| }.getOrElse { e -> | ||
| logger.warn("Failed to organize imports", e) | ||
| emptyList() | ||
| } | ||
|
|
||
| override fun postExec( | ||
| data: ActionData, | ||
| result: Any, | ||
| ) { | ||
| super.postExec(data, result) | ||
| if (result !is List<*> || result.isEmpty()) return | ||
|
|
||
| @Suppress("UNCHECKED_CAST") | ||
| result as List<TextEdit> | ||
|
|
||
| val client = | ||
| data.languageClient ?: run { | ||
| logger.warn("No language client set. Cannot organize imports.") | ||
| return | ||
| } | ||
| val file = data.requireFile() | ||
| client.performCodeAction( | ||
| CodeActionItem( | ||
| title = label, | ||
| changes = listOf(DocumentChange(file = file.toPath(), edits = result)), | ||
| kind = CodeActionKind.QuickFix, | ||
| command = Command("", ""), // no post-action command (no CMD_FORMAT_CODE) | ||
| ), | ||
| ) | ||
| } | ||
| } |
102 changes: 102 additions & 0 deletions
102
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizer.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| package com.itsaky.androidide.lsp.kotlin.utils | ||
|
|
||
| import org.jetbrains.kotlin.com.intellij.psi.util.PsiTreeUtil | ||
| import org.jetbrains.kotlin.kdoc.psi.api.KDoc | ||
| import org.jetbrains.kotlin.psi.KtFile | ||
| import org.jetbrains.kotlin.psi.KtImportDirective | ||
|
|
||
| /** | ||
| * What a file's body actually uses, expressed as plain strings so nothing crosses an `analyze` | ||
| * lifetime boundary. Produced by [collectImportUsage]. | ||
| * | ||
| * @property usedFqNames importable fully-qualified names referenced by the body. | ||
| * @property usedPackages parent packages/objects of used symbols (for wildcard matching). | ||
| * @property unresolvedNames short names of body references that failed to resolve; an import | ||
| * matching one of these is kept, since a resolution failure can't prove the import unused. | ||
| */ | ||
| internal data class ImportUsage( | ||
| val usedFqNames: Set<String>, | ||
| val usedPackages: Set<String>, | ||
| val unresolvedNames: Set<String> = emptySet(), | ||
| ) | ||
|
|
||
| /** JVM packages that Kotlin imports with a wildcard by default; explicit named imports from these are redundant. */ | ||
| internal val DEFAULT_STAR_PACKAGES: Set<String> = | ||
| setOf( | ||
| "kotlin", | ||
| "kotlin.annotation", | ||
| "kotlin.collections", | ||
| "kotlin.comparisons", | ||
| "kotlin.io", | ||
| "kotlin.ranges", | ||
| "kotlin.sequences", | ||
| "kotlin.text", | ||
| "kotlin.jvm", | ||
| "java.lang", | ||
| ) | ||
|
|
||
| private val KDOC_LINK = Regex("""\[([^\]\s]+)]""") | ||
|
|
||
| /** | ||
| * Computes the canonical import block for [ktFile] given [usage]: unused/redundant imports removed, | ||
| * survivors deduped and lexicographically sorted. Returns null when the imports are already in that | ||
| * exact form (no edit needed). The returned text has no surrounding newlines. | ||
| */ | ||
| internal fun organizedImportBlock( | ||
| ktFile: KtFile, | ||
| usage: ImportUsage, | ||
| ): String? { | ||
| val directives = ktFile.importDirectives | ||
| if (directives.isEmpty()) return null | ||
|
|
||
| val filePackage = ktFile.packageFqName.asString() | ||
| val kdocNames = collectKDocLinkNames(ktFile) | ||
|
|
||
| val newLines = | ||
| directives | ||
| .filter { keepImport(it, usage, filePackage, kdocNames) } | ||
| .mapNotNull { it.importPath?.let { path -> "import $path" } } | ||
| .distinct() | ||
| .sorted() | ||
|
|
||
| val currentLines = directives.mapNotNull { it.importPath?.let { path -> "import $path" } } | ||
|
|
||
| if (newLines == currentLines) return null | ||
| return newLines.joinToString(System.lineSeparator()) | ||
| } | ||
|
|
||
| private fun keepImport( | ||
| directive: KtImportDirective, | ||
| usage: ImportUsage, | ||
| filePackage: String, | ||
| kdocNames: Set<String>, | ||
| ): Boolean { | ||
| val fqName = directive.importedFqName ?: return true // malformed import -> keep (conservative) | ||
| val fqNameStr = fqName.asString() | ||
| val alias = directive.aliasName | ||
| val shortName = alias ?: fqName.shortName().asString() | ||
|
|
||
| // Conservative: keep anything referenced by short name/alias in a KDoc link. | ||
| if (shortName in kdocNames) return true | ||
|
|
||
| // Conservative: an unresolved body reference by this short name can't prove the import dead. | ||
| if (shortName in usage.unresolvedNames) return true | ||
|
|
||
| if (directive.isAllUnder) { | ||
| // Wildcard: keep iff some used symbol lives in this package/object. | ||
| return fqNameStr in usage.usedPackages | ||
| } | ||
|
|
||
| val parentPackage = fqName.parent().asString() | ||
| // Redundant named imports (only when not aliased — an alias is meaningful). | ||
| if (alias == null && parentPackage in DEFAULT_STAR_PACKAGES) return false | ||
| if (alias == null && parentPackage == filePackage) return false | ||
|
|
||
| return fqNameStr in usage.usedFqNames | ||
| } | ||
|
|
||
| private fun collectKDocLinkNames(ktFile: KtFile): Set<String> = | ||
| PsiTreeUtil | ||
| .collectElementsOfType(ktFile, KDoc::class.java) | ||
| .flatMap { kdoc -> KDOC_LINK.findAll(kdoc.text).map { it.groupValues[1].substringAfterLast('.') } } | ||
| .toSet() | ||
105 changes: 105 additions & 0 deletions
105
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollector.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| package com.itsaky.androidide.lsp.kotlin.utils | ||
|
|
||
| import org.jetbrains.kotlin.analysis.api.KaSession | ||
| import org.jetbrains.kotlin.analysis.api.resolution.successfulFunctionCallOrNull | ||
| import org.jetbrains.kotlin.analysis.api.resolution.symbol | ||
| import org.jetbrains.kotlin.analysis.api.symbols.KaCallableSymbol | ||
| import org.jetbrains.kotlin.analysis.api.symbols.KaClassLikeSymbol | ||
| import org.jetbrains.kotlin.analysis.api.symbols.KaConstructorSymbol | ||
| import org.jetbrains.kotlin.analysis.api.symbols.KaSymbol | ||
| import org.jetbrains.kotlin.idea.references.mainReference | ||
| import org.jetbrains.kotlin.psi.KtArrayAccessExpression | ||
| import org.jetbrains.kotlin.psi.KtCallExpression | ||
| import org.jetbrains.kotlin.psi.KtDestructuringDeclaration | ||
| import org.jetbrains.kotlin.psi.KtDestructuringDeclarationEntry | ||
| import org.jetbrains.kotlin.psi.KtElement | ||
| import org.jetbrains.kotlin.psi.KtFile | ||
| import org.jetbrains.kotlin.psi.KtForExpression | ||
| import org.jetbrains.kotlin.psi.KtImportList | ||
| import org.jetbrains.kotlin.psi.KtNameReferenceExpression | ||
| import org.jetbrains.kotlin.psi.KtOperationReferenceExpression | ||
| import org.jetbrains.kotlin.psi.KtPropertyDelegate | ||
| import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType | ||
| import org.jetbrains.kotlin.psi.psiUtil.getParentOfType | ||
|
|
||
| /** | ||
| * Collects the importable fq-names (and their packages) referenced by [ktFile]'s body. MUST be | ||
| * called inside [analyzeMaybeDangling]. Returns only plain strings, so nothing escapes the analyze | ||
| * lifetime. A reference that fails to resolve doesn't join the used set; instead its short name is | ||
| * recorded in [ImportUsage.unresolvedNames] so its import is kept. Both paths are safe: they lead to | ||
| * keeping an import, never removing a used one. | ||
| */ | ||
| internal fun KaSession.collectImportUsage(ktFile: KtFile): ImportUsage { | ||
| val usedFqNames = HashSet<String>() | ||
| val usedPackages = HashSet<String>() | ||
| val unresolvedNames = HashSet<String>() | ||
|
|
||
| fun record(symbol: KaSymbol?) { | ||
| val fq = symbol?.importableFqNameString() ?: return | ||
| usedFqNames += fq | ||
| val pkg = fq.substringBeforeLast('.', missingDelimiterValue = "") | ||
| if (pkg.isNotEmpty()) usedPackages += pkg | ||
| } | ||
|
|
||
| fun recordAll(symbols: Collection<KaSymbol>?) { | ||
| symbols?.forEach(::record) | ||
| } | ||
|
|
||
| // 1) Plain name / type references (excluding the import list itself). A null (or thrown) | ||
| // resolution is treated as unresolved and its short name kept, so a used-but-unresolvable | ||
| // reference never drops its import. A non-null, non-importable symbol (local, param) is a | ||
| // clean resolve: it records nothing and is not unresolved. | ||
| ktFile.collectDescendantsOfType<KtNameReferenceExpression>().forEach { ref -> | ||
| if (ref.getParentOfType<KtImportList>(strict = false) != null) return@forEach | ||
| val symbol = runCatching { ref.mainReference.resolveToSymbol() }.getOrNull() | ||
| if (symbol != null) record(symbol) else unresolvedNames += ref.getReferencedName() | ||
| } | ||
|
|
||
| // 1b) Implicit-convention references that carry more than one resolution target and so don't | ||
| // resolve through `resolveToSymbol()` (singular; returns null when ambiguous) but do resolve | ||
| // through `resolveToSymbols()` (plural). Confirmed empirically: | ||
| // - KtForExpression: resolves to [iterator(), hasNext(), next()] -- iterator is the | ||
| // user-importable one for a `for (x in foo)` loop. | ||
| // - KtDestructuringDeclarationEntry (one per destructured variable): resolves to that | ||
| // variable's own componentN() symbol. | ||
| // - KtPropertyDelegate: resolves to the delegate's getValue()/setValue() symbol(s). | ||
| // Recording every returned symbol is safe: extra (e.g. stdlib Iterator.next) symbols only ever | ||
| // keep an import, never drop a used one. | ||
| ktFile.collectDescendantsOfType<KtForExpression>().forEach { forExpr -> | ||
| runCatching { recordAll(forExpr.mainReference?.resolveToSymbols()) } | ||
| } | ||
| ktFile.collectDescendantsOfType<KtDestructuringDeclarationEntry>().forEach { entry -> | ||
| runCatching { recordAll(entry.mainReference?.resolveToSymbols()) } | ||
| } | ||
| ktFile.collectDescendantsOfType<KtPropertyDelegate>().forEach { delegate -> | ||
| runCatching { recordAll(delegate.mainReference?.resolveToSymbols()) } | ||
| } | ||
|
|
||
| // 2) Convention / operator call sites (no textual name reference). | ||
| ktFile.collectDescendantsOfType<KtElement>().forEach { element -> | ||
| val isConvention = | ||
| element is KtOperationReferenceExpression || | ||
| element is KtArrayAccessExpression || | ||
| element is KtCallExpression || | ||
| element is KtForExpression || | ||
| element is KtDestructuringDeclaration || | ||
| element is KtPropertyDelegate | ||
| if (!isConvention) return@forEach | ||
| runCatching { | ||
| record(element.resolveToCall()?.successfulFunctionCallOrNull()?.symbol) | ||
| } | ||
| } | ||
|
|
||
| return ImportUsage(usedFqNames, usedPackages, unresolvedNames) | ||
| } | ||
|
|
||
| private fun KaSymbol.importableFqNameString(): String? = | ||
| when (this) { | ||
| // A constructor's own callableId is null, so it must map to its containing class -- the name | ||
| // that's actually imported. Covers `Foo()` calls and `@Foo` annotations (both resolve to the | ||
| // constructor). Must precede the KaCallableSymbol branch, which a constructor also matches. | ||
| is KaConstructorSymbol -> containingClassId?.asSingleFqName()?.asString() | ||
| is KaClassLikeSymbol -> classId?.asSingleFqName()?.asString() | ||
| is KaCallableSymbol -> callableId?.asSingleFqName()?.asString() | ||
| else -> null | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.