ADFA-3944: ensure KtSymbolIndex.getKtFile does not fail for non-Kotlin files

[itsaky-adfa]

See ADFA-3944 for more details.

[coderabbitai]

📝 Walkthrough

Release Notes: ADFA-3944 - Ensure KtSymbolIndex.getKtFile Returns Null for Non-Kotlin Files

Changes

• KtSymbolIndex.getKtFile() API Change: Function signature changed from fun getKtFile(vf: VirtualFile): KtFile to fun getKtFile(vf: VirtualFile): KtFile?. The method now performs file type validation using DocumentUtils.isKotlinFile(path) and returns null when the provided VirtualFile is not a Kotlin file, preventing attempts to parse non-Kotlin files as Kotlin source code.

• DeclarationsProvider Updates: Updated ktFilesForPackage() to use mapNotNull when calling index.getKtFile(it), properly filtering out any null results from non-Kotlin files.

• DirectInheritorsProvider Updates: Updated computeIndex() to use mapNotNull when retrieving Kotlin files, ensuring null results from getKtFile are skipped during index traversal.

• AnnotationsResolver Already Compliant: The AnnotationsResolver service already uses mapNotNull when calling getKtFile(), indicating it was designed with nullable returns in mind.

Risks and Considerations

• Breaking API Change: The return type change from non-nullable KtFile to nullable KtFile? is a breaking change for all callers. Code audit confirms the three identified callers in the codebase have been properly updated, but external consumers of this API will need updates.

• Silent Filtering of Non-Kotlin Files: The change silently filters out non-Kotlin files rather than explicitly handling them. Callers may inadvertently ignore unexpected file types, potentially masking configuration or data integrity issues.

• LSP Service Stability: Since this affects core Kotlin language server components (DeclarationsProvider, DirectInheritorsProvider, AnnotationsResolver), the change impacts symbol resolution and inheritance analysis. Proper null handling is critical for maintaining language server stability.

Walkthrough

KtSymbolIndex's getKtFile() function now validates Kotlin file types and returns nullable KtFile?. DeclarationsProvider and DirectInheritorsProvider adapt by using mapNotNull instead of map to safely filter null results from the updated API contract.

Changes

Kotlin File Type Validation

Layer / File(s)	Summary
API Contract and File-Type Validation lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.kt	getKtFile(vf: VirtualFile) now filters by Kotlin file type using DocumentUtils.isKotlinFile(path), returns null for non-Kotlin files, and changes return type from KtFile to KtFile?. Cached lookup and lazy-load logic remains within the nullable-return contract.
Null-Safe Consumer Integration lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/services/DeclarationsProvider.kt, lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/services/DirectInheritorsProvider.kt	DeclarationsProvider.ktFilesForPackage and DirectInheritorsProvider.computeIndex() switch from map() to mapNotNull() when calling index.getKtFile(), filtering out null KtFile results during file pipeline processing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

• Daniel-ADFA
• dara-abijo-adfa

Poem

🐰 A file walks in, does it speak Kotlin's tongue?
We check, we validate—null or KtFile done!
The mapNotNull skips what shouldn't be there,
Type-safe and filtered with hop and with care! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: ensuring KtSymbolIndex.getKtFile handles non-Kotlin files by returning null instead of failing.
Description check	✅ Passed	The description references the issue ADFA-3944 which provides context, though minimal detail is included in the PR description itself.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/ADFA-3944

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Microsoft Presidio Analyzer (2.2.362)

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.kt

Microsoft Presidio Analyzer failed to scan this file

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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/index/KtSymbolIndex.kt (1)

183-192: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

loadKtFile has an unsafe cast that can still crash after the isKotlinFile guard.

PsiManager.findFile(vf) returns PsiFile? — it can return null (e.g., the file isn't indexed yet) or a non-KtFile PsiFile (e.g., parsed as a stub). The hard as KtFile cast at line 191 will throw NullPointerException or ClassCastException in both cases, even though the PR's intent is for getKtFile to return null for unresolvable cases.

🛡️ Proposed fix: make `loadKtFile` nullable and propagate null

-	private fun loadKtFile(vf: VirtualFile): KtFile = project.read {
+	private fun loadKtFile(vf: VirtualFile): KtFile? = project.read {
 		PsiManager.getInstance(project)
-			.findFile(vf) as KtFile
+			.findFile(vf) as? KtFile
 	}

And update the call site in getKtFile:

-		val ktFile = loadKtFile(vf)
-
-		ktFileCache.put(path, ktFile)
-		return ktFile
+		val ktFile = loadKtFile(vf) ?: return null
+		ktFileCache.put(path, ktFile)
+		return ktFile

🤖 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/compiler/index/KtSymbolIndex.kt`
around lines 183 - 192, Change loadKtFile to return a nullable KtFile (signature
loadKtFile(vf: VirtualFile): KtFile?) and stop using the unsafe cast; inside
project.read return PsiManager.getInstance(project).findFile(vf) as? KtFile so
it yields null when the file is missing or not a KtFile. Update getKtFile to
accept and propagate the nullable result (do not forcibly cast), only cache when
the returned ktFile is non-null, and handle the null path the same way the guard
intended (return null for unresolvable cases).

🤖 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/compiler/index/KtSymbolIndex.kt`:
- Around line 183-192: Change loadKtFile to return a nullable KtFile (signature
loadKtFile(vf: VirtualFile): KtFile?) and stop using the unsafe cast; inside
project.read return PsiManager.getInstance(project).findFile(vf) as? KtFile so
it yields null when the file is missing or not a KtFile. Update getKtFile to
accept and propagate the nullable result (do not forcibly cast), only cache when
the returned ktFile is non-null, and handle the null path the same way the guard
intended (return null for unresolvable cases).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6b43f332-4906-4533-9ea2-62d2ca444305

📥 Commits

Reviewing files that changed from the base of the PR and between 69e03bf and 004f607.

📒 Files selected for processing (3)

• lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.kt
• lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/services/DeclarationsProvider.kt
• lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/services/DirectInheritorsProvider.kt