ADFA-3320: add support for completing non imported symbol completions#1163
ADFA-3320: add support for completing non imported symbol completions#1163itsaky-adfa merged 32 commits intostagefrom
Conversation
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
fixes duplicate class errors for org.antrl.v4.* classes Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
It is now included in the embeddable JAR (named UnsafeAndroid) with proper relocations. Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
…ject re-sync Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Handles document events to manage instances of in-memory KtFile that can be used by various Kt LSP components (like diagnostics provider, code completions) to re-use already parsed KtFile instances Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
This ensures that extension functions whose receiver type is not available in the current scope are not suggested for scope completions Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
📝 WalkthroughRelease NotesFeatures
Improvements
Risk Indicators
Notes
WalkthroughThis PR implements a build event bus system, enhances Kotlin completion with JVM library symbol integration, and refactors compilation environment property naming. It adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
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/compiler/CompilationEnvironment.kt (1)
134-142:⚠️ Potential issue | 🟠 MajorRebind
fileManagerafter rebuilding the analysis session.
rebuildSession()replacessessionandparser, butfileManagerstill holds the old parser/PSI managers from init. After a structure change, completions may operate on stale session-scoped services.♻️ Proposed fix
private fun rebuildSession() { logger.info("Rebuilding analysis session") + fileManager.close() disposable.dispose() disposable = Disposer.newDisposable() session = buildSession() parser = KtPsiFactory(session.project, eventSystemEnabled = enableParserEventSystem) + fileManager = KtFileManager(parser, psiManager, psiDocumentManager) logger.info("Analysis session rebuilt") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt` around lines 134 - 142, rebuildSession() currently replaces session and parser but leaves fileManager bound to old session-scoped services; update rebuildSession() to rebind fileManager after creating the new session and parser (i.e., after session = buildSession() and parser = KtPsiFactory(...)) so fileManager uses the new session/psi managers—locate the fileManager initialization helper or constructor (the same logic used in init) and invoke it here to recreate or reassign fileManager to the new session and parser.
🧹 Nitpick comments (1)
lsp/models/src/main/java/com/itsaky/androidide/lsp/edits/DefaultEditHandler.kt (1)
125-132: Narrow the reflective command exception handling.Line 130 catches
Throwable, which can suppress fatal VM/runtime errors. Keep this best-effort path limited to reflection/security failures.♻️ Proposed refactor
try { val klass = editor::class.java val method = klass.getMethod("executeCommand", Command::class.java) method.isAccessible = true method.invoke(editor, command) - } catch (th: Throwable) { - log.error("Unable to invoke 'executeCommand(Command) method in IDEEditor.", th) + } catch (e: ReflectiveOperationException) { + log.error("Unable to invoke 'executeCommand(Command) method in IDEEditor.", e) + } catch (e: SecurityException) { + log.error("Unable to access 'executeCommand(Command) method in IDEEditor.", e) }Based on learnings, in Kotlin files across AndroidIDE, prefer narrow exception handling over broad catch-all handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/models/src/main/java/com/itsaky/androidide/lsp/edits/DefaultEditHandler.kt` around lines 125 - 132, The current catch(Throwable) around the reflective call to editor::class.java.getMethod("executeCommand", Command::class.java) and method.invoke(editor, command) is too broad; replace it with targeted catches for reflection/security failures (e.g., NoSuchMethodException, SecurityException, IllegalAccessException, InvocationTargetException, and IllegalArgumentException) so only those errors are logged by log.error("Unable to invoke 'executeCommand(Command) method in IDEEditor.", e); remove the catch(Throwable) fallback (or at most keep a catch(Exception) that rethrows non-reflection errors) so fatal VM/runtime errors are not swallowed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@editor/src/main/java/com/itsaky/androidide/editor/adapters/CompletionListAdapter.kt`:
- Around line 93-97: The async API-info callback can update a recycled row and
show stale data; modify showApiInfoIfNeeded to attach a stable identifier to the
view (e.g., view.setTag(item.uniqueId or item.computeStableKey())) before
launching the async fetch, capture that tag in the callback, and only write to
binding.completionApiInfo if the current view.tag still equals the captured id;
alternatively cancel/ignore outstanding requests for that view when convertView
is rebound. Apply this guard wherever showApiInfoIfNeeded is invoked (including
the block around binding.completionApiInfo visibility and the region referenced
at lines 141-203) so updates only apply to the row currently bound to the
original item.
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt`:
- Around line 331-340: The branch handling JvmSymbolKind.FUNCTION and
JvmSymbolKind.CONSTRUCTOR misses indexed extension functions, so update the when
on symbol.kind in KotlinCompletions.kt to also handle
JvmSymbolKind.EXTENSION_FUNCTION (or use the kindOf() mapped value) the same as
FUNCTION: cast symbol.data to JvmFunctionInfo, set item.detail =
data.signatureDisplay, call item.setInsertTextForFunction(symbol.shortName,
data.parameterCount > 0, partial), and preserve the constructor-specific
overrideTypeText assignment for CONSTRUCTOR; ensure the same snippet/parameter
handling applies to extension functions so they show signature detail and
parameter hints.
- Around line 293-306: The call to librarySymbolIndex.findByPrefix(partial) in
KotlinCompletions.kt can run for empty or very short partials and stream the
whole index; guard it by returning early when partial is null/empty or shorter
than a chosen min length (e.g., 2–3 chars) and/or apply a result cap before
collecting (use a Flow operator like take(n) on the returned flow) so collect
only a bounded number of symbols; update usages around librarySymbolIndex,
findByPrefix(partial), collect and ensure the required Flow operator import
(e.g., take) is added.
- Around line 315-354: The scope-completion items for non-class JVM symbols are
not setting completion data or import-resolution handlers, leaving item.data
null for functions/properties/type-aliases and preventing proper resolve/import;
update the code in KotlinCompletions.kt where ktCompletionItem is built (inside
the branch that handles JvmSymbolKind.FUNCTION, PROPERTY, TYPE_ALIAS and the
earlier non-extension check) to populate item.data with appropriate structured
payloads (e.g., MethodCompletionData for top-level functions/properties and
ClassCompletionData for type-aliases) and ensure functions still call
setInsertTextForFunction; also implement the TODO by filtering extension
functions against implicit receiver applicability before creating items so only
applicable extensions are suggested, and ensure resolveCompletionItem expects
and handles these structured completion data types (FQN in the same fields you
populate) for import/resolve handling.
In
`@lsp/models/src/main/java/com/itsaky/androidide/lsp/edits/DefaultEditHandler.kt`:
- Around line 85-91: The code opens a batch edit with text.beginBatchEdit() at
the top but mistakenly calls text.beginBatchEdit() again instead of closing it;
replace the second call with text.endBatchEdit() so the batch started before
performing edits (in the block that calls
item.additionalEditHandler!!.performEdits(...) or
RewriteHelper.performEdits(...)) is properly closed and the editor's batch-edit
state remains balanced.
---
Outside diff comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt`:
- Around line 134-142: rebuildSession() currently replaces session and parser
but leaves fileManager bound to old session-scoped services; update
rebuildSession() to rebind fileManager after creating the new session and parser
(i.e., after session = buildSession() and parser = KtPsiFactory(...)) so
fileManager uses the new session/psi managers—locate the fileManager
initialization helper or constructor (the same logic used in init) and invoke it
here to recreate or reassign fileManager to the new session and parser.
---
Nitpick comments:
In
`@lsp/models/src/main/java/com/itsaky/androidide/lsp/edits/DefaultEditHandler.kt`:
- Around line 125-132: The current catch(Throwable) around the reflective call
to editor::class.java.getMethod("executeCommand", Command::class.java) and
method.invoke(editor, command) is too broad; replace it with targeted catches
for reflection/security failures (e.g., NoSuchMethodException,
SecurityException, IllegalAccessException, InvocationTargetException, and
IllegalArgumentException) so only those errors are logged by log.error("Unable
to invoke 'executeCommand(Command) method in IDEEditor.", e); remove the
catch(Throwable) fallback (or at most keep a catch(Exception) that rethrows
non-reflection errors) so fatal VM/runtime errors are not swallowed.
🪄 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: 188e5ab5-70dd-42e8-9ece-e65d168b13c4
📒 Files selected for processing (12)
app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kteditor/src/main/java/com/itsaky/androidide/editor/adapters/CompletionListAdapter.kteditor/src/main/java/com/itsaky/androidide/editor/language/CommonCompletionProvider.kteditor/src/main/java/com/itsaky/androidide/editor/language/IDELanguage.kteventbus-events/src/main/java/com/itsaky/androidide/eventbus/events/BuildEvent.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/Compiler.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/IncrementalModificationTracker.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/KotlinProjectModel.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.ktlsp/models/src/main/java/com/itsaky/androidide/lsp/edits/DefaultEditHandler.kt
See ADFA-3320 for more details.