ADFA-2956: Improve kotlin Library Indexing: Member-Level Completions from Dependencies#983
Merged
Conversation
…from Dependencies
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Kotlin extension-function detection and indexing (metadata parsing and MethodInfo.isExtension), registers extensions by receiver type, surfaces extensions and inherited members to completion, tightens syntax-error boundary logic, refines external-type checks, and removes noisy logging. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant CP as "CompletionProvider"
participant SR as "SymbolResolver"
participant PR as "ProjectIndex"
participant CI as "ClasspathIndex"
participant CK as "ClasspathIndexer"
User->>CP: request completions at cursor
CP->>SR: resolve receiver name -> FQN(s)
SR->>CI: query isExternal / findMembers(fqName)
CI-->>SR: symbol info + members
SR-->>CP: resolved FQN(s)
CP->>PR: findExtensions(receiverType)
PR->>CI: findExtensionsFor(receiverType)
CI-->>PR: extension symbols
PR-->>CP: aggregated extension symbols
CP->>CI: request inherited members for FQN
CI->>CK: use parsed metadata to mark extensions/traverse hierarchy
CK-->>CI: collected members
CI-->>CP: deduplicated members
CP->>User: return completion items (local + inherited + extensions)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ClasspathIndexer.kt`:
- Around line 111-118: detectExtensionReceiver is currently matching
extensionFunctionNames only by method.name which misclassifies non-extension
overloads; update the detection to require a full signature match (e.g., compare
method descriptor or parameter type list plus name) or use a per-method
extension flag from metadata instead of name-only lookup. Specifically, modify
detectExtensionReceiver (and the similar logic around the other occurrence) to
consult MethodInfo's full parameter type sequence or descriptor when checking
against extensionFunctionNames, or add/read an "isExtension" flag in MethodInfo
parsed from Kotlin metadata and use that to decide whether to drop the first
parameter and clear containingClass.
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt`:
- Around line 772-777: The isFilteredMember function currently discards
all-uppercase names which removes valid enum constants; update the filter so it
does not suppress enum entries by checking the receiver type before applying the
all-uppercase rule (e.g., when determining completion candidates in the code
path that calls isFilteredMember, allow names that are all-uppercase if the
receiver is an enum) or tighten the condition to only match synthetic patterns
(e.g., strictly synthetic prefixes/suffixes) instead of the broad name.all {
it.isUpperCase() || it == '_' || it.isDigit() } rule; locate isFilteredMember
and the call sites that pass the receiver (or add a new overload taking a
receiver type) and adjust the logic to preserve uppercase enum constants while
still filtering true synthetic members like "<init>", "<clinit>", and "access$".
- Around line 649-677: resolveToFqName currently falls back to
classpathIndex/stdlibIndex before checking the local/project symbols; update it
to prefer local/project types first by querying the projectIndex (and the
symbolTable's declared types) for matches before consulting classpathIndex and
stdlibIndex. Specifically, in resolveToFqName use symbolTable (declared/owned
types) and projectIndex lookups for simple-name matches and return a unique
project/local fqName if found (or prefer project matches when multiple), only
then iterate symbolTable.imports star imports and consult classpathIndex and
stdlibIndex; ensure you still preserve existing import alias logic and the final
fallback to the original typeName.
- Around line 679-735: The dedupe key in both findStdlibMembersWithInheritance
and findClasspathMembersWithInheritance currently omits the member kind, causing
collisions between different kinds (e.g., a property vs a parameterless
function); update the key construction to include the member.kind (e.g., val key
= member.kind.toString() + ":" + member.name + "(" +
member.parameters.joinToString(",") { it.type } + ")") so the filter
distinguishes symbols by kind as well as name+params.
🧹 Nitpick comments (2)
lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/symbol/SymbolBuilder.kt (1)
890-896: Preserve qualifiers when inferring constructor call types.For navigation expressions like
com.foo.Bar(), the inferred type becomesBar, which can mis-resolve to a different localBar. Consider deriving a qualified name from the navigation expression when available.lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ClasspathIndex.kt (1)
9-9: Deduplicate extension lookups to keep the public API stable.Extensions are registered under both full and simple receiver names, so
findExtensionsForcan return duplicates for qualified types. Returning a distinct list avoids surprises for callers that don’t dedupe.Suggested tweak
- return results + return results.distinctBy { it.fqName }Also applies to: 59-67, 99-105
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt (1)
341-346: 🛠️ Refactor suggestion | 🟠 MajorInconsistent deduplication:
items.nonevsaddedMemberNamesset.Lines 341–346 use
items.none { it.label == ext.name }(O(n) linear scan) for dedup, while all other paths use the O(1)addedMemberNamesset. This is both a performance inconsistency and a correctness risk — names inaddedMemberNamesthat were filtered fromitemsby the prefix check (line 349) won't prevent re-addition here.♻️ Use addedMemberNames consistently
val stdlibExtensions = stdlibIndex?.findExtensions(typeName, emptyList()) ?: emptyList() stdlibExtensions.forEach { ext -> - if (items.none { it.label == ext.name }) { + if (ext.name !in addedMemberNames) { + addedMemberNames.add(ext.name) items.add(createCompletionItemFromIndexed(ext, CompletionItemPriority.STDLIB)) } }
🤖 Fix all issues with AI agents
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt`:
- Around line 630-647: patchDanglingDots currently rewrites every line ending
with a dot which breaks valid multi-line chained calls; modify patchDanglingDots
so it only patches the line at the cursor (or at least skips lines that are
continued on the next non-blank line). Specifically, in patchDanglingDots,
instead of operating over all indices, detect the target line (use the
cursor/offset passed into the caller or add a parameter) or for each candidate
line check the next non-blank line: if the trimmed next line starts with '.' (or
is a valid continuation), skip patching that line; otherwise apply the existing
logic that replaces the dangling dot with "toString()". Update any callers to
pass the cursor line if you choose the cursor-line approach and keep function
name patchDanglingDots and local variables (lines, trimmed, patched) intact for
easy location.
- Around line 26-27: The shared field completionParser holds a non-thread-safe
KotlinParser and causes data races when provideCompletions() is invoked
concurrently; fix by eliminating the shared instance and either instantiate a
new KotlinParser inside provideCompletions() (use a local val parser =
KotlinParser() and use that for parsing) or guard all accesses to
completionParser with synchronization (e.g., wrap parse calls in a
synchronized(this) block or use a Mutex around parse/visit calls); update any
references to completionParser in CompletionProvider to use the chosen approach.
🧹 Nitpick comments (3)
lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ClasspathIndexer.kt (1)
383-386: O(n²) byte-array concatenation in fold.
d1Bytes.fold(ByteArray(0)) { acc, b -> acc + b }copies the accumulator on every iteration. For classes with large metadata (e.g., stdlib files), this can become a meaningful cost during bulk indexing.♻️ Use ByteArrayOutputStream or pre-sized array
- val combined = d1Bytes.fold(ByteArray(0)) { acc, b -> acc + b } + val combined = ByteArray(d1Bytes.sumOf { it.size }).also { buf -> + var off = 0 + for (b in d1Bytes) { b.copyInto(buf, off); off += b.size } + }lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt (2)
695-723:findStdlibMembersWithInheritancetraverses by simple name, which can collide.The
visitedset and BFS queue use simple names (e.g.,"Comparable"). If two stdlib classes share a simple name in different packages, visiting one would prevent visiting the other. In practice this is unlikely for stdlib, but the classpath variant correctly uses FQ names. Consider aligning the stdlib variant to also track by FQ name for correctness.
274-293: Property inference from getters doesn't check member kind.The property-name synthesis loop iterates all
classpathMembersincluding properties and constructors, not just functions. While theaddedMemberNamescheck (line 283) prevents duplicates from being added, the loop still runs the string checks unnecessarily for non-function members. Consider adding amember.kind == IndexedSymbolKind.FUNCTIONguard for clarity and minor efficiency.♻️ Add kind check
for (member in classpathMembers) { + if (member.kind != IndexedSymbolKind.FUNCTION) continue val name = member.name val propName = when {
dara-abijo-adfa
requested changes
Feb 17, 2026
dara-abijo-adfa
requested changes
Feb 17, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ProjectIndex.kt (2)
71-77:findInProjectFilesdoesn't deduplicate results unlikefindBySimpleName.Other public lookup methods (e.g.,
findBySimpleNameat line 68) calldistinctBy { it.fqName }. This method can return duplicates if the same symbol appears in multiple file indexes (e.g., re-indexed or overlapping declarations).Suggested fix
fun findInProjectFiles(name: String): List<IndexedSymbol> { val results = mutableListOf<IndexedSymbol>() for (fileIndex in fileIndexes.values) { results.addAll(fileIndex.findBySimpleName(name)) } - return results + return results.distinctBy { it.fqName } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ProjectIndex.kt` around lines 71 - 77, findInProjectFiles collects symbols from all fileIndexes via fileIndex.findBySimpleName(name) but does not deduplicate, causing duplicate IndexedSymbol entries when the same fqName appears in multiple file indexes; update findInProjectFiles to return results.distinctBy { it.fqName } (or apply distinctBy before returning) so its behavior matches findBySimpleName and eliminates duplicates by fqName.
234-237:includeAnyExtensionsis not forwarded toclasspathIndex.When
includeAnyExtensionsistrue, the method queriesextensionIndexandstdlibIndexforAny/Any?extensions, but theclasspathIndexblock only queries forreceiverTypeand explicitsupertypes. Extensions defined onAnyin classpath dependencies won't be found here. This may be intentional iffindExtensionsFordoesn't support it, but it's an asymmetry worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ProjectIndex.kt` around lines 234 - 237, The classpathIndex block in ProjectIndex.kt does not consider includeAnyExtensions, causing Any/Any? extensions in classpath dependencies to be missed; update the classpathIndex handling (the cpIndex block that calls cpIndex.findExtensionsFor with receiverType and supertypes) to also call cpIndex.findExtensionsFor for Any and Any? when includeAnyExtensions is true, mirroring the logic used for extensionIndex and stdlibIndex so extensions declared on Any in classpath deps are included.lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/symbol/SymbolBuilder.kt (2)
1137-1138: BothERRORandelsebranches now callprocessBody(child)identically.This causes every unrecognized node type in a function body to be recursively traversed, which broadens error recovery. This is likely intentional, but note that it makes the explicit
SyntaxKind.ERRORcase on line 1137 redundant — it could be folded into theelsebranch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/symbol/SymbolBuilder.kt` around lines 1137 - 1138, The when-branch handling SyntaxKind.ERROR is redundant because both SyntaxKind.ERROR and the else branch call processBody(child); update the match in SymbolBuilder (the when over child.kind) to remove the explicit SyntaxKind.ERROR case and let the else branch handle it (or merge the ERROR arm into the else) so there is no duplicated branch; ensure the surrounding logic still calls processBody(child) for unrecognized kinds.
1488-1524: RewrittenextractReceiverTypelooks correct but has a minor quadratic scan.Line 1502:
node.namedChildren.indexOf(child)is O(n) inside the for-loop, making this block O(n²) over named children. For typical AST sizes this is fine, but if you want to tighten it:Minor optimization
- for (child in node.namedChildren) { - if (child.kind != SyntaxKind.USER_TYPE && child.kind != SyntaxKind.SIMPLE_USER_TYPE) continue - val nextSibling = node.namedChildren.getOrNull(node.namedChildren.indexOf(child) + 1) + val named = node.namedChildren + for (i in named.indices) { + val child = named[i] + if (child.kind != SyntaxKind.USER_TYPE && child.kind != SyntaxKind.SIMPLE_USER_TYPE) continue + val nextSibling = named.getOrNull(i + 1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/symbol/SymbolBuilder.kt` around lines 1488 - 1524, The extractReceiverType function has an O(n²) hotspot from calling node.namedChildren.indexOf(child) inside the for-loop; replace that loop with an indexed iteration (e.g., iterate over node.namedChildren.indices or maintain an explicit index counter) so you can directly access the next sibling with getOrNull(i + 1) instead of indexOf; update the branch that currently uses indexOf(child) to use the loop index to compute nextSibling and keep the existing checks and calls to extractType unchanged.lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ClasspathIndexer.kt (1)
449-527: Consider usingkotlin-metadata-jvmfor reading Kotlin metadata instead of hand-rolled protobuf parsing.The
extractExtensionFunctionsmethod parses Kotlin metadata using hardcoded protobuf field numbers (9 for function declarations, 2 for name index, 5 for value params, 6 for receiver). This manual parsing is brittle and couples the implementation to Kotlin's internal metadata schema.The
kotlin-metadata-jvmlibrary provides a stable public API (stable since Kotlin 2.0.0) for reading Kotlin metadata. However, the library version must be kept in sync with the Kotlin compiler version to ensure compatibility across Kotlin releases. The current pragmatic approach withtry-catchfallback is reasonable if adding a dependency is not feasible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ClasspathIndexer.kt` around lines 449 - 527, The extractExtensionFunctions function implements fragile, manual protobuf parsing of Kotlin metadata (hardcoded field numbers like 9, 2, 5, 6) which is brittle; replace this logic with the kotlin-metadata-jvm library to read KotlinClassHeader/KotlinClassMetadata and iterate over functions to detect extension receiver and parameter counts (update extractExtensionFunctions to use kotlin-metadata-jvm APIs), keep the current try-catch fallback only if adding the dependency is infeasible, and ensure you reference the same output shape (Map<String, MutableList<Int>>) so callers remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ClasspathIndexer.kt`:
- Around line 306-318: The current matching in markedMethods (using
extensionFunctions, remaining, methods and method.parameters.size - 1) can
mis-mark overloads that share name + param count; update the logic to also
verify the extension receiver type matches the method's first parameter type
before treating it as an extension. Concretely, augment extensionFunctions (or
its stored entries in remaining/valueCounts) to include the expected receiver
type and, when mapping methods, check method.parameters.first().type (or
equivalent) equals that receiver type in addition to param count before removing
from valueCounts and returning method.copy(isExtension = true); if metadata
doesn't include receiver type, derive a stable signature (fully qualified
first-param type) to disambiguate instead of relying solely on counts.
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt`:
- Around line 272-291: The property-name derivation incorrectly strips the "is"
prefix for boolean getters; in the classpathMembers loop change the
"is"-prefixed branch so propName is the original method name (e.g., name)
instead of lowercasing the next char. Update the when branch that checks
name.startsWith("is") to set propName = name (leave get-prefixed handling
unchanged), so addedMemberNames and the CompletionItem creation use the correct
"isX" property name.
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/symbol/SymbolBuilder.kt`:
- Around line 819-825: The code calls calleeName.first() which will throw if
calleeName is an empty string; update the branch in SymbolBuilder (where
TypeReference(calleeName) is constructed) to guard against empty/blank names by
checking calleeName.isNotEmpty() or calleeName.isNotBlank() (or using
calleeName.firstOrNull()?.isUpperCase() == true) before accessing the first
character, and only construct TypeReference(calleeName) when that check passes;
leave the else -> null behavior otherwise.
---
Nitpick comments:
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ClasspathIndexer.kt`:
- Around line 449-527: The extractExtensionFunctions function implements
fragile, manual protobuf parsing of Kotlin metadata (hardcoded field numbers
like 9, 2, 5, 6) which is brittle; replace this logic with the
kotlin-metadata-jvm library to read KotlinClassHeader/KotlinClassMetadata and
iterate over functions to detect extension receiver and parameter counts (update
extractExtensionFunctions to use kotlin-metadata-jvm APIs), keep the current
try-catch fallback only if adding the dependency is infeasible, and ensure you
reference the same output shape (Map<String, MutableList<Int>>) so callers
remain unchanged.
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ProjectIndex.kt`:
- Around line 71-77: findInProjectFiles collects symbols from all fileIndexes
via fileIndex.findBySimpleName(name) but does not deduplicate, causing duplicate
IndexedSymbol entries when the same fqName appears in multiple file indexes;
update findInProjectFiles to return results.distinctBy { it.fqName } (or apply
distinctBy before returning) so its behavior matches findBySimpleName and
eliminates duplicates by fqName.
- Around line 234-237: The classpathIndex block in ProjectIndex.kt does not
consider includeAnyExtensions, causing Any/Any? extensions in classpath
dependencies to be missed; update the classpathIndex handling (the cpIndex block
that calls cpIndex.findExtensionsFor with receiverType and supertypes) to also
call cpIndex.findExtensionsFor for Any and Any? when includeAnyExtensions is
true, mirroring the logic used for extensionIndex and stdlibIndex so extensions
declared on Any in classpath deps are included.
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/symbol/SymbolBuilder.kt`:
- Around line 1137-1138: The when-branch handling SyntaxKind.ERROR is redundant
because both SyntaxKind.ERROR and the else branch call processBody(child);
update the match in SymbolBuilder (the when over child.kind) to remove the
explicit SyntaxKind.ERROR case and let the else branch handle it (or merge the
ERROR arm into the else) so there is no duplicated branch; ensure the
surrounding logic still calls processBody(child) for unrecognized kinds.
- Around line 1488-1524: The extractReceiverType function has an O(n²) hotspot
from calling node.namedChildren.indexOf(child) inside the for-loop; replace that
loop with an indexed iteration (e.g., iterate over node.namedChildren.indices or
maintain an explicit index counter) so you can directly access the next sibling
with getOrNull(i + 1) instead of indexOf; update the branch that currently uses
indexOf(child) to use the loop index to compute nextSibling and keep the
existing checks and calls to extractType unchanged.
dara-abijo-adfa
approved these changes
Feb 17, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt (1)
294-313:⚠️ Potential issue | 🟠 MajorFix Java
isX()getter projection and guard property synthesis to zero-arg methods.Line 299 currently maps
isEmpty()toempty, but Kotlin property naming keepsisEmpty. Also, property synthesis should only apply to zero-parameter functions to avoid false property completions.Suggested fix
for (member in classpathMembers) { val name = member.name val propName = when { - name.startsWith("get") && name.length > 3 && name[3].isUpperCase() -> + member.kind == IndexedSymbolKind.FUNCTION && + member.parameters.isEmpty() && + name.startsWith("get") && name.length > 3 && name[3].isUpperCase() -> name[3].lowercase() + name.substring(4) - name.startsWith("is") && name.length > 2 && name[2].isUpperCase() -> - name[2].lowercase() + name.substring(3) + member.kind == IndexedSymbolKind.FUNCTION && + member.parameters.isEmpty() && + name.startsWith("is") && name.length > 2 && name[2].isUpperCase() -> + name else -> null }In Kotlin Java interop, what Kotlin property name is produced for a Java boolean getter named isEmpty()?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt` around lines 294 - 313, The current projection logic for Java getters in CompletionProvider.kt incorrectly maps boolean isX() getters to "x" and also applies property synthesis to methods with parameters; update the propName computation to only synthesize properties for zero-argument methods (check the member's parameter count, e.g., member.parameterCount or member.parameters.isEmpty()) and change the "is" branch so boolean getters preserve the original name (e.g., for names starting with "is" and matching the uppercase check set propName = name) instead of stripping the "is" prefix; keep the existing handling for "get" getters and the rest of the flow that adds into addedMemberNames and items (the loop over classpathMembers, member.name, addedMemberNames, items.add(CompletionItem { ... })).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt`:
- Around line 328-329: The code is using substringAfterLast('.') and
findBySimpleName (e.g. simpleTypeName and
findStdlibMembersWithInheritance(stdlibIndex)) which causes unrelated classes
with the same simple name to be merged; replace the simple-name based lookup
with fully-qualified-name (FQ) based resolution end-to-end: obtain the FQ name
for the type (do not use substringAfterLast), query the stdlib index with that
FQ name (use or add a findByFqName/findExactFqLookup on stdlibIndex or filter
members by exactFqName), and then traverse inheritance only for the exact FQ
match in findStdlibMembersWithInheritance and the other affected call sites
(around lines referenced: where simpleTypeName is computed and uses
findBySimpleName). Ensure all member-computation paths use the exact FQ identity
rather than simple names.
- Around line 749-753: The current dedupe uses the mutableSet seen and always
adds every member key in the result.filter lambda, which misses applying the
isFilteredMember predicate and can leak synthetic stdlib completions; update the
filter so that before adding the constructed key (member.kind.name + ":" +
member.name + "(" + member.parameters.joinToString(",") { it.type } + ")") you
first call isFilteredMember(member) and skip (return false) when it returns
true, only add the key and allow the member through when it is not filtered and
not already seen (i.e., combine the isFilteredMember check with the existing
dedupe logic in the same filter block used during the inheritance merge).
- Around line 906-958: resolveMemberReturnType currently only queries
classpathIndex and stdlibIndex so chained lookups can miss members defined in
the local/project symbols; update the BFS to check local/project symbols before
external indexes by: after computing fqName (resolveToFqName) and inside the
loop for each current, first consult symbolTable (and projectIndex's project
index if available) to find members for current and return TypeReference when a
matching member or getter is found, then proceed to classpathIndex and
stdlibIndex as fallback; ensure you also traverse superTypes from the
project/local index (like you do for classpathIndex/stdlibIndex) and keep
existing behavior around get${memberName.capitalize()} and returning
TypeReference(match.returnType).
---
Duplicate comments:
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt`:
- Around line 294-313: The current projection logic for Java getters in
CompletionProvider.kt incorrectly maps boolean isX() getters to "x" and also
applies property synthesis to methods with parameters; update the propName
computation to only synthesize properties for zero-argument methods (check the
member's parameter count, e.g., member.parameterCount or
member.parameters.isEmpty()) and change the "is" branch so boolean getters
preserve the original name (e.g., for names starting with "is" and matching the
uppercase check set propName = name) instead of stripping the "is" prefix; keep
the existing handling for "get" getters and the rest of the flow that adds into
addedMemberNames and items (the loop over classpathMembers, member.name,
addedMemberNames, items.add(CompletionItem { ... })).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5997beaf-2f9a-4dc5-b131-4084f694ec87
📒 Files selected for processing (1)
lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt (3)
294-301:⚠️ Potential issue | 🟠 MajorKeep
isXJava getters asisXKotlin properties.Line 299-Line 300 converts
isEmptytoempty. In Kotlin interop, boolean getters likeisEmpty()map to property nameisEmpty, notempty.Suggested fix
val propName = when { name.startsWith("get") && name.length > 3 && name[3].isUpperCase() -> name[3].lowercase() + name.substring(4) name.startsWith("is") && name.length > 2 && name[2].isUpperCase() -> - name[2].lowercase() + name.substring(3) + name else -> null }In Kotlin Java interop, what Kotlin property name is exposed for a Java method `boolean isEmpty()`?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt` around lines 294 - 301, The code currently converts Java boolean getters like `isEmpty` into `empty` by stripping the "is" prefix; update the logic in CompletionProvider (loop over `classpathMembers`, variable `name` and `propName`) so that when `name` startsWith "is" and `name[2].isUpperCase()` you set `propName` to the original `name` (e.g., "isEmpty") instead of removing the "is" prefix; keep the existing handling for "get" getters unchanged and ensure the `else -> null` branch remains intact.
329-333:⚠️ Potential issue | 🟡 MinorApply
isFilteredMemberon stdlib member merges too.Line 329-Line 333 and Line 749-Line 753 currently dedupe without filtering. This can surface synthetic members (
<init>,access$, etc.) in completion.Suggested fix
stdlibMembers.forEach { member -> - if (member.name !in addedMemberNames) { + if (member.name !in addedMemberNames && !isFilteredMember(member.name)) { addedMemberNames.add(member.name) items.add(createCompletionItemFromIndexed(member, CompletionItemPriority.MEMBER)) } } val seen = mutableSetOf<String>() return result.filter { member -> + if (isFilteredMember(member.name)) return@filter false val key = member.kind.name + ":" + member.name + "(" + member.parameters.joinToString(",") { it.type } + ")" seen.add(key) }Also applies to: 749-753
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt` around lines 329 - 333, The stdlib merging loops add stdlibMembers into items after only checking dedupe via addedMemberNames, which lets synthetic names (like <init>, access$) slip through; update both loops (the one iterating stdlibMembers at the block using addedMemberNames and the similar block around lines 749-753) to also call isFilteredMember before adding/deduping—i.e., skip the member if isFilteredMember(member.name) (or the appropriate member identifier) returns true, and only then add to addedMemberNames and call createCompletionItemFromIndexed with CompletionItemPriority.MEMBER.
326-329:⚠️ Potential issue | 🟠 MajorUse fully-qualified type identity for stdlib inheritance traversal.
Line 326 and Line 1036 convert to simple names, and Line 739-Line 745 traverse via
findBySimpleName. This can merge unrelated classes with the same simple name and return incorrect members.Suggested fix
- val simpleTypeName = typeName.substringAfterLast('.') - - val stdlibMembers = findStdlibMembersWithInheritance(simpleTypeName, stdlibIndex) + val stdlibTypeFqName = resolveToFqName(typeName, symbolTable) + val stdlibMembers = findStdlibMembersWithInheritance(stdlibTypeFqName, stdlibIndex) - private fun findStdlibMembersWithInheritance( - simpleTypeName: String, + private fun findStdlibMembersWithInheritance( + typeFqName: String, stdlibIndex: StdlibIndex ): List<IndexedSymbol> { val visited = mutableSetOf<String>() val result = mutableListOf<IndexedSymbol>() val queue = ArrayDeque<String>() - queue.add(simpleTypeName) + queue.add(typeFqName) while (queue.isNotEmpty()) { val current = queue.removeFirst() if (!visited.add(current)) continue - val fqCandidates = stdlibIndex.findBySimpleName(current).filter { it.kind.isClass } - for (classSymbol in fqCandidates) { - result.addAll(stdlibIndex.findMembers(classSymbol.fqName)) - classSymbol.superTypes.forEach { superType -> - val superSimple = superType.substringAfterLast('.') - if (superSimple !in visited) queue.add(superSimple) - } - } + val classSymbol = stdlibIndex.findByFqName(current) ?: continue + if (!classSymbol.kind.isClass) continue + result.addAll(stdlibIndex.findMembers(classSymbol.fqName)) + classSymbol.superTypes.forEach { superType -> + if (superType !in visited) queue.add(superType) + } } - findStdlibMembersWithInheritance(superFqName.substringAfterLast('.'), stIndex).forEach { member -> + findStdlibMembersWithInheritance(superFqName, stIndex).forEach { member ->Also applies to: 726-747, 976-984, 1036-1037
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt` around lines 326 - 329, The code is using simpleTypeName (derived from typeName.substringAfterLast('.')) and findBySimpleName during stdlib inheritance traversal, which conflates types with the same simple name; update findStdlibMembersWithInheritance and any callers (e.g., where simpleTypeName is computed) to use the fully-qualified type identity (use typeName as-is or a normalized FQN) instead of simpleTypeName, and change lookups that call stdlibIndex.findBySimpleName to a lookup that matches the fully-qualified name in stdlibIndex (or add a map keyed by FQN); ensure functions referenced (findStdlibMembersWithInheritance, stdlibIndex lookups, and any call sites around typeName/simpleTypeName at the noted locations) are adjusted accordingly so inheritance traversal uses the FQN.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt`:
- Around line 906-968: The resolveMemberReturnType function currently only
checks class members via projectIndex.findMembersInProjectFiles /
classpathIndex.findMembers / stdlibIndex.findMembers and therefore misses
extension functions whose receiver is indexed by type; update the method to also
query each index for extension functions keyed by the receiver type (e.g. call
projectIndex.findExtensionsInProjectFiles(current) or
projectIndex.findExtensionsByReceiver(current), and analogous
classpathIndex/stdlibIndex extension lookup APIs), look for an extension with
name == memberName or getterName, and if found return its return type wrapped in
TypeReference (matching the existing projectMatch/match/stdMatch handling); if
your indexes lack such helper APIs, add equivalent extension-search helpers that
return extension function signatures so resolveMemberReturnType can include them
in its BFS lookup.
---
Duplicate comments:
In
`@lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt`:
- Around line 294-301: The code currently converts Java boolean getters like
`isEmpty` into `empty` by stripping the "is" prefix; update the logic in
CompletionProvider (loop over `classpathMembers`, variable `name` and
`propName`) so that when `name` startsWith "is" and `name[2].isUpperCase()` you
set `propName` to the original `name` (e.g., "isEmpty") instead of removing the
"is" prefix; keep the existing handling for "get" getters unchanged and ensure
the `else -> null` branch remains intact.
- Around line 329-333: The stdlib merging loops add stdlibMembers into items
after only checking dedupe via addedMemberNames, which lets synthetic names
(like <init>, access$) slip through; update both loops (the one iterating
stdlibMembers at the block using addedMemberNames and the similar block around
lines 749-753) to also call isFilteredMember before adding/deduping—i.e., skip
the member if isFilteredMember(member.name) (or the appropriate member
identifier) returns true, and only then add to addedMemberNames and call
createCompletionItemFromIndexed with CompletionItemPriority.MEMBER.
- Around line 326-329: The code is using simpleTypeName (derived from
typeName.substringAfterLast('.')) and findBySimpleName during stdlib inheritance
traversal, which conflates types with the same simple name; update
findStdlibMembersWithInheritance and any callers (e.g., where simpleTypeName is
computed) to use the fully-qualified type identity (use typeName as-is or a
normalized FQN) instead of simpleTypeName, and change lookups that call
stdlibIndex.findBySimpleName to a lookup that matches the fully-qualified name
in stdlibIndex (or add a map keyed by FQN); ensure functions referenced
(findStdlibMembersWithInheritance, stdlibIndex lookups, and any call sites
around typeName/simpleTypeName at the noted locations) are adjusted accordingly
so inheritance traversal uses the FQN.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d387d6f-2063-4e93-a903-c180c0e3adfe
📒 Files selected for processing (3)
lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/FileIndex.ktlsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ProjectIndex.ktlsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/server/providers/CompletionProvider.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- lsp/kotlin-core/src/main/java/org/appdevforall/codeonthego/lsp/kotlin/index/ProjectIndex.kt
This was referenced Apr 20, 2026
Closed
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.
Screen.Recording.2026-02-16.at.14.23.34.mov