ADFA-4614: K2-LSP code action — Organize imports - #1502
Merged
Conversation
…from a function symbol
…atureHelp, vararg active param, empty documentation)
Adds SLF4J debug breadcrumbs across findEnclosingCall, buildSignatureHelp, computeActiveParameter and buildSignatureInformation so the signature-help pipeline can be traced from logs, without changing any behavior.
Signature help resolved the cursor offset against the KtFile returned by getOpenedKtFile(), which is only refreshed by the ASYNC onDocumentChange subscriber. The editor computes the cursor index against its live in-memory buffer, so when the opened KtFile lagged behind (by one or more keystrokes) the offset landed on the wrong element and findEnclosingCall returned null, silently dropping signature help. Build the analysis KtFile from FileManager.getDocumentContents() (the live document) instead, mirroring what KotlinCompletions.doComplete already does, so the offset and the parsed tree always agree.
…ic-message assertion scope
Adds the reparse-after-invalidate regression test, peek-branch coverage
for getCurrentKtFileIfPresent/getKtFile (hit and miss cases), and moves
the diagnostic .defaultMessage mapping inside env.analyze{} so a real
regression fails with a clean assertion diff instead of
KaInaccessibleLifetimeOwnerAccessException.
…le comment Replace the blocking getCurrentKtFile(...).get() in AdvancedKotlinEditHandler with the non-blocking getCurrentKtFileIfPresent(...) peek, since this method runs on the editor UI thread and a version-miss would otherwise drive a synchronous parse + project.write there (ANR risk). The completion just ran against this file, so the current instance is essentially always cached; a peek-miss falls back to the existing "File not open" log/return. Also correct the refreshToCurrent comment in KtSymbolIndex to reason about the actual write order in FileManager.onDocumentContentChange (version before content, both unsynchronized) rather than an under-specified version/content staleness claim.
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
![claude[bot]](https://cdn.github.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
itsaky-adfa
marked this pull request as draft
July 9, 2026 19:24
Contributor
Author
|
Marked as draft because this depends on #1484 |
Contributor
📝 WalkthroughWalkthroughAdds a Kotlin organize-imports code action with analysis-based usage collection, canonical import filtering and formatting, language-client edit submission, menu registration, and unit/end-to-end coverage. ChangesKotlin organize imports
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant KotlinCodeActionsMenu
participant OrganizeImportsAction
participant CompilationEnvironment
participant ImportUsageCollector
participant ImportOrganizer
participant LanguageClient
KotlinCodeActionsMenu->>OrganizeImportsAction: invoke organize imports
OrganizeImportsAction->>CompilationEnvironment: retrieve current Kotlin file
OrganizeImportsAction->>ImportUsageCollector: collect import usage
ImportUsageCollector-->>OrganizeImportsAction: return ImportUsage
OrganizeImportsAction->>ImportOrganizer: compute organized import block
ImportOrganizer-->>OrganizeImportsAction: return TextEdit
OrganizeImportsAction->>LanguageClient: submit QuickFix edits
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/AdvancedKotlinEditHandler.kt (1)
26-30: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAvoid aborting completion edits when the current-file cache hasn’t refreshed yet
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/AdvancedKotlinEditHandler.kt:26-30
getCurrentKtFileIfPresent()only sees a completed refresh, so an open file can still hit thenullpath right after typing and skip the edit. Keep a fallback to the current open document or parsed file instead of returning early.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/AdvancedKotlinEditHandler.kt` around lines 26 - 30, The current early return in AdvancedKotlinEditHandler’s edit path drops completion edits when getCurrentKtFileIfPresent() has not refreshed yet. Update the managed-file lookup logic in the same branch to fall back to the current open document or parsed file from analysisContext/file state before giving up, and only log and return if no usable file source exists. Keep the existing flow in AdvancedKotlinEditHandler and use the existing managedFile handling to preserve edits during transient cache misses.
🧹 Nitpick comments (3)
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizer.kt (1)
57-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
"\n"instead ofSystem.lineSeparator()for import line joining.
System.lineSeparator()returns the platform default (\r\non Windows), but the LSP protocol and the test expectations inImportOrganizerTest.kt(line 41:"import a.b.Apple${'\n'}import a.b.Zebra") use\n. On Android this happens to coincide, but hardcoding"\n"ensures test portability and LSP-compliant output.♻️ Proposed fix
- return newLines.joinToString(System.lineSeparator()) + return newLines.joinToString("\n")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizer.kt` at line 57, The import joining in ImportOrganizer’s line assembly currently uses the platform newline via System.lineSeparator(), which can produce non-LSP output on some systems. Update the join in ImportOrganizer to use a fixed "\n" separator so the generated import text matches ImportOrganizerTest expectations and remains portable across platforms.lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt (1)
75-79: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
await()here instead of.get()It suspends cleanly and preserves the original failure cause.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt` around lines 75 - 79, The current Kt file lookup in AddImportAction is blocking on a future with .get(), which should be replaced with a suspending await() call so the coroutine flow stays non-blocking and failure causes are preserved. Update the ktFile retrieval in the AddImportAction path that calls env.ktSymbolIndex.getCurrentKtFile(nioPath) to use await() instead of .get(), keeping the existing null fallback behavior intact.lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt (1)
25-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix indentation inconsistency in
KotlinSignatureHelp.kt.Function bodies in
buildSignatureHelpanddoSignatureHelphave many lines starting at column 0 (e.g., lines 26, 33, 39, 45, 53, 76, 78, 91, 107), while adjacent lines and all other files in the same package use tab indentation. PMD also reports a parse error on this file (Begin column must be >= 1, got 0), likely related to the zero-column lines. Reformatting for consistency would improve readability and resolve the PMD warning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt` around lines 25 - 112, The bodies of KotlinSignatureHelp’s buildSignatureHelp and doSignatureHelp are inconsistently indented, with several top-level statements starting at column 0 and triggering the PMD parse warning. Reformat those function bodies to match the surrounding tab-based indentation used elsewhere in the package, keeping the control flow and statements aligned consistently inside buildSignatureHelp, doSignatureHelp, and the logger-related blocks.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt`:
- Around line 49-63: The organize-imports flow is swallowing coroutine
cancellation inside computeOrganizeEdit, which should be preserved instead of
turning into emptyList(). Update the runCatching/getOrElse block in
OrganizeImportsAction.computeOrganizeEdit to detect CancellationException and
re-throw it before logging, while still logging other failures and returning
emptyList() for them; apply the same cancellation-preserving pattern in
ImportUsageCollector as well.
---
Outside diff comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/AdvancedKotlinEditHandler.kt`:
- Around line 26-30: The current early return in AdvancedKotlinEditHandler’s
edit path drops completion edits when getCurrentKtFileIfPresent() has not
refreshed yet. Update the managed-file lookup logic in the same branch to fall
back to the current open document or parsed file from analysisContext/file state
before giving up, and only log and return if no usable file source exists. Keep
the existing flow in AdvancedKotlinEditHandler and use the existing managedFile
handling to preserve edits during transient cache misses.
---
Nitpick comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt`:
- Around line 75-79: The current Kt file lookup in AddImportAction is blocking
on a future with .get(), which should be replaced with a suspending await() call
so the coroutine flow stays non-blocking and failure causes are preserved.
Update the ktFile retrieval in the AddImportAction path that calls
env.ktSymbolIndex.getCurrentKtFile(nioPath) to use await() instead of .get(),
keeping the existing null fallback behavior intact.
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.kt`:
- Around line 25-112: The bodies of KotlinSignatureHelp’s buildSignatureHelp and
doSignatureHelp are inconsistently indented, with several top-level statements
starting at column 0 and triggering the PMD parse warning. Reformat those
function bodies to match the surrounding tab-based indentation used elsewhere in
the package, keeping the control flow and statements aligned consistently inside
buildSignatureHelp, doSignatureHelp, and the logger-related blocks.
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizer.kt`:
- Line 57: The import joining in ImportOrganizer’s line assembly currently uses
the platform newline via System.lineSeparator(), which can produce non-LSP
output on some systems. Update the join in ImportOrganizer to use a fixed "\n"
separator so the generated import text matches ImportOrganizerTest expectations
and remains portable across platforms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18cd3a63-dd4e-4f61-beff-b57807930226
📒 Files selected for processing (28)
editor/src/main/java/com/itsaky/androidide/editor/ui/SignatureHelpWindow.javaeditor/src/test/java/com/itsaky/androidide/editor/ui/SignatureHelpWindowTest.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionsMenu.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/AbstractCompilationEnvironment.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/AdvancedKotlinEditHandler.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/ActiveParameterResolver.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/CallAtCursorFinder.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelp.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/SignatureInfoBuilder.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizer.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollector.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/TypeRendering.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/index/CurrentKtFileCacheTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/ActiveParameterResolverTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/CallAtCursorFinderTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/KotlinSignatureHelpTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/signaturehelp/SignatureInfoBuilderTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizerTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollectorTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt
itsaky-adfa
marked this pull request as ready for review
July 13, 2026 17:27
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollector.kt (1)
47-89: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRemove broad exception swallowing, redundant convention checks, and exclude package directives.
- Broad exception swallowing: Based on learnings, prefer narrow exception handling that catches only the specific exception type reported in crashes (such as
IllegalArgumentException) instead of a broad catch-all (e.g.,catch (e: Exception)). TherunCatchingblock additionally swallowsProcessCanceledException(PCE) andCancellationException, which breaks cancellation and drastically degrades IDE responsiveness when the user types. Please removerunCatchingand allow the underlying calls to fail-fast.- Redundant convention checks:
KtForExpression,KtDestructuringDeclaration, andKtPropertyDelegateare already explicitly resolved in section 1b. Checking them again in section 2 adds redundant analysis overhead.- Package directive exclusion: The design spec correctly requires excluding the package directive to prevent its segments from being falsely recorded as unresolved references, which leads to over-retaining unrelated imports.
Please apply the diff below, and add the missing import outside this range:
import org.jetbrains.kotlin.psi.KtPackageDirective🛠️ Proposed fixes for logic and performance
- // 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 + // 1) Plain name / type references (excluding the import list itself). A null + // 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 (ref.getParentOfType<KtImportList>(strict = false) != null || ref.getParentOfType<KtPackageDirective>(strict = false) != null) return@forEach + val symbol = ref.mainReference.resolveToSymbol() 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()) } + recordAll(forExpr.mainReference?.resolveToSymbols()) } ktFile.collectDescendantsOfType<KtDestructuringDeclarationEntry>().forEach { entry -> - runCatching { recordAll(entry.mainReference?.resolveToSymbols()) } + recordAll(entry.mainReference?.resolveToSymbols()) } ktFile.collectDescendantsOfType<KtPropertyDelegate>().forEach { delegate -> - runCatching { recordAll(delegate.mainReference?.resolveToSymbols()) } + 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 + element is KtCallExpression if (!isConvention) return@forEach - runCatching { - record(element.resolveToCall()?.successfulFunctionCallOrNull()?.symbol) - } + record(element.resolveToCall()?.successfulFunctionCallOrNull()?.symbol) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollector.kt` around lines 47 - 89, Update ImportUsageCollector’s resolution paths to remove runCatching wrappers and let resolution failures, including cancellation, propagate normally. Exclude references whose parent is KtPackageDirective alongside import-list references, using the KtPackageDirective import. Remove KtForExpression, KtDestructuringDeclaration, and KtPropertyDelegate from section 2’s convention checks because section 1b already resolves them explicitly.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollector.kt`:
- Around line 47-89: Update ImportUsageCollector’s resolution paths to remove
runCatching wrappers and let resolution failures, including cancellation,
propagate normally. Exclude references whose parent is KtPackageDirective
alongside import-list references, using the KtPackageDirective import. Remove
KtForExpression, KtDestructuringDeclaration, and KtPropertyDelegate from section
2’s convention checks because section 1b already resolves them explicitly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c51ae228-71aa-4216-a1d6-afedc86c60b7
📒 Files selected for processing (6)
docs/superpowers/plans/2026-07-09-kotlin-organize-imports.mddocs/superpowers/specs/2026-07-09-kotlin-organize-imports-design.mdlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizer.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollector.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizerTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollectorTest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizerTest.kt
- lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizer.kt
dara-abijo-adfa
approved these changes
Jul 14, 2026
An import referenced only through a constructor call (`Foo()`), an annotation (`@Foo`), or a typealias used as a constructor (`Alias()`) was dropped by Organize Imports, breaking compilation. All three resolve to a KaConstructorSymbol, whose own callableId is null, so importableFqNameString returned null and the class was never recorded as used. Map a constructor to its containing class instead. Add regression tests for the collector and end-to-end behavior.
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt (1)
63-66: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRethrow
CancellationExceptionto preserve coroutine cancellation.
runCatchingcatches allThrowables, includingCancellationException. Swallowing it prevents the coroutine framework from cleanly canceling the job and turns a canceled organize-imports request intoemptyList(). As per coding guidelines and previous review comments,CancellationExceptionmust be rethrown.🐛 Proposed fix to rethrow CancellationException
}.getOrElse { e -> + if (e is kotlin.coroutines.cancellation.CancellationException) throw e logger.warn("Failed to organize imports", e) emptyList() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt` around lines 63 - 66, Update the getOrElse error handler in OrganizeImportsAction to detect CancellationException and rethrow it before logging and returning emptyList(); preserve the existing warning and fallback behavior for all other failures.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt`:
- Around line 63-66: Update the getOrElse error handler in OrganizeImportsAction
to detect CancellationException and rethrow it before logging and returning
emptyList(); preserve the existing warning and fallback behavior for all other
failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e237b544-2828-4a66-9a99-dbff53456c48
📒 Files selected for processing (9)
docs/superpowers/plans/2026-06-25-enhance-ai-assistant-plugin-ui.mdlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionsMenu.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizer.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollector.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizerTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollectorTest.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt
💤 Files with no reviewable changes (2)
- docs/superpowers/plans/2026-06-25-enhance-ai-assistant-plugin-ui.md
- lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/OrganizeImportsEndToEndTest.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionsMenu.kt
- lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizerTest.kt
- lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/utils/ImportUsageCollectorTest.kt
- lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/utils/ImportOrganizer.kt
- 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Adds an Organize imports code action for
.ktfiles (K2 LSP): removes unused/redundant imports, de-dupes, and sorts the block. Usage is computed in-process via the Analysis API.Jira: ADFA-4614
Note: unlike the ticket's "driven by diagnostics" phrasing, this is always-available and computes usage directly from the Analysis API.
Behavior
x[i],for, destructuring,bydelegation).Design
OrganizeImportsAction→computeOrganizeEdit(fetchesKtFilebeforeproject.read).ImportUsageCollector— sole Analysis-API seam; returns onlyStrings.ImportOrganizer— pure PSI/string logic (sort, redundancy, KDoc-keep, no-op).Testing
./gradlew :lsp:kotlin:testV7DebugUnitTest→ 91 tests, 0 failures. Manual QA: see ticket.