ADFA-3739: collect and report Kotlin syntax errors

[itsaky-adfa]

See ADFA-3739 for more details.

[coderabbitai]

📝 Walkthrough

Release Notes - ADFA-3739: Kotlin Syntax Error Collection and Reporting

Features

• Kotlin syntax errors now reported: KotlinDiagnosticProvider now collects and reports PsiErrorElement instances from Kotlin files in addition to Kotlin analysis diagnostics, improving syntax error visibility in the IDE
• Aggregated diagnostics reporting: Diagnostics are now sourced from two channels—syntax errors (PsiErrorElement) and Kotlin analysis diagnostics—providing more comprehensive error reporting

Changes

• KotlinSnippetRepository refactored to use lazy initialization: snippet map now computed on-demand via SnippetRegistry rather than eagerly populated during initialization
• KotlinDiagnosticProvider.doAnalyze() refactored to use a shared diagnosticItem() helper for consistent diagnostic range/source construction

Risks & Considerations

• Timing concerns: The shift from eager to lazy initialization in KotlinSnippetRepository could introduce subtle timing issues if snippets are accessed before the registry is fully initialized; verify that access patterns don't have undocumented initialization dependencies
• Diagnostic source format change: Diagnostic source field changed from "Kotlin" (capitalized) to "kotlin" (lowercase); confirm that IDE clients and external integrations that filter or parse diagnostics by source name are compatible with this change
• Duplicate diagnostics possible: Aggregating diagnostics from both PsiErrorElement and Kotlin analysis could theoretically produce duplicates if both sources report the same syntax error; recommend testing to ensure deduplication is working correctly
• Performance impact: Iterating over all PsiErrorElement instances during every analysis run may impact performance on files with many syntax errors; monitor performance metrics post-deployment

Walkthrough

The PR refactors Kotlin LSP components: snippet management shifts from eager parsing to lazy retrieval through a shared registry, while diagnostic collection expands to include PSI error elements alongside Kotlin analysis diagnostics.

Changes

Cohort / File(s)	Summary
Snippet Management lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinSnippetRepository.kt	Changed snippets from mutable lateinit var to immutable val with computed getter. init() now delegates to SnippetRegistry.initBuiltIn() instead of direct parsing, enabling lazy retrieval on demand.
Diagnostic Collection lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt	Enhanced doAnalyze to collect both PsiErrorElement instances and Kotlin analysis diagnostics. Refactored toDiagnosticItem() to use shared diagnosticItem() helper; diagnostic source changed to lowercase "kotlin".

Sequence Diagram(s)

sequenceDiagram
    participant KSR as KotlinSnippetRepository
    participant SR as SnippetRegistry
    participant Client as LSP Client

    Note over KSR,SR: Old Flow (Eager Loading)
    Client->>KSR: init()
    KSR->>SR: SnippetParser.parse("kt", scopes)
    SR-->>KSR: Populate snippets map
    Client->>KSR: getSnippets()
    KSR-->>Client: Return cached snippets

    Note over KSR,SR: New Flow (Lazy Loading)
    Client->>KSR: init()
    KSR->>SR: initBuiltIn("kt", scopes)
    SR-->>KSR: OK
    Client->>KSR: snippets getter
    KSR->>SR: getSnippets("kt", scope) per scope
    SR-->>KSR: Retrieve snippet map
    KSR-->>Client: Return computed snippets

Loading

sequenceDiagram
    participant KDP as KotlinDiagnosticProvider
    participant PSI as PSI File
    participant KA as Kotlin Analysis
    participant DI as DiagnosticItem

    Note over KDP,DI: Enhanced Diagnostic Collection
    KDP->>PSI: Collect PsiErrorElement instances
    PSI-->>KDP: Return error elements
    KDP->>DI: Create ERROR DiagnosticItem per element
    DI-->>KDP: Return diagnostic items
    
    KDP->>KA: collectDiagnostics(EXTENDED_AND_COMMON_CHECKERS)
    KA-->>KDP: Return KaDiagnostic results
    KDP->>DI: toDiagnosticItem() via helper
    DI-->>KDP: Return diagnostic items
    
    KDP-->>KDP: Aggregate both sources

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• ADFA-3319: add basic K2-based Kotlin diagnostic provider #1112: Modifies the Kotlin diagnostic provider implementation with changes to diagnostic collection and conversion logic.

Suggested reviewers

• dara-abijo-adfa

Poem

🐰 Snippets now lazy, diagnostics born anew,
Registry whispers, errors peek through,
No eager parsing, just ask when you need,
LSP flows faster—swift bunny speed! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly describes the main change: collecting and reporting Kotlin syntax errors, which aligns with the KotlinDiagnosticProvider modifications that now collect PsiErrorElement instances.
Description check	✅ Passed	The description references the Jira issue ADFA-3739 which provides details about the change; it is contextually related to the changeset's objective of collecting and reporting Kotlin syntax errors.

_{✏️ 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-3739-k2-doesnt-report-syntax-errors

---

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]

🧹 Nitpick comments (2)

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinSnippetRepository.kt (2)

3-5: SnippetParser import appears unused after this change.

init() now delegates to SnippetRegistry.initBuiltIn, and SnippetParser is no longer referenced in this file. Worth dropping the import if your linter/IDE doesn't already flag it.

🤖 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/completion/KotlinSnippetRepository.kt`
around lines 3 - 5, The file KotlinSnippetRepository.kt still imports
SnippetParser but the init() implementation now delegates to
SnippetRegistry.initBuiltIn and no longer references SnippetParser; remove the
unused import statement for SnippetParser to clean up the code and satisfy the
linter, leaving the imports for ISnippet and SnippetRegistry intact and ensuring
KotlinSnippetRepository.init() continues to call SnippetRegistry.initBuiltIn.

---

8-11: Getter rebuilds the full map on every access.

Every read of snippets now iterates all KotlinSnippetScope.entries, invokes SnippetRegistry.getSnippets(...) per scope (each taking a read lock and merging built-in + user + plugin snippets — see SnippetRegistry.kt:51-64), and allocates a fresh Map. Per KotlinCompletions.kt:430-456, this property is read twice on every completion request — once for GLOBAL and again for the context-specific scope — so you do N scope lookups and build a full N-entry map just to read two entries.

Cheap win: skip the intermediate map and look scopes up on demand.

♻️ Proposed refactor

-	val snippets: Map<KotlinSnippetScope, List<ISnippet>>
-		get() = KotlinSnippetScope.entries.associateWith { scope ->
-			SnippetRegistry.getSnippets("kt", scope.filename)
-		}
+	fun snippetsFor(scope: KotlinSnippetScope): List<ISnippet> =
+		SnippetRegistry.getSnippets("kt", scope.filename)

Then update the two call sites in KotlinCompletions.collectSnippetCompletions to use KotlinSnippetRepository.snippetsFor(scope) instead of KotlinSnippetRepository.snippets[scope].

🤖 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/completion/KotlinSnippetRepository.kt`
around lines 8 - 11, The current KotlinSnippetRepository.snippets getter
rebuilds the entire map on every access; replace it with a lightweight lookup
method (e.g., KotlinSnippetRepository.snippetsFor(scope: KotlinSnippetScope))
that returns SnippetRegistry.getSnippets("kt", scope.filename) for the given
scope on demand (avoiding allocating the full Map and repeated reads), and
update call sites in KotlinCompletions.collectSnippetCompletions to call
snippetsFor(scope) instead of accessing snippets[scope]; keep KotlinSnippetScope
and SnippetRegistry.getSnippets(...) usage intact and remove or deprecate the
old full-map getter.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinSnippetRepository.kt`:
- Around line 3-5: The file KotlinSnippetRepository.kt still imports
SnippetParser but the init() implementation now delegates to
SnippetRegistry.initBuiltIn and no longer references SnippetParser; remove the
unused import statement for SnippetParser to clean up the code and satisfy the
linter, leaving the imports for ISnippet and SnippetRegistry intact and ensuring
KotlinSnippetRepository.init() continues to call SnippetRegistry.initBuiltIn.
- Around line 8-11: The current KotlinSnippetRepository.snippets getter rebuilds
the entire map on every access; replace it with a lightweight lookup method
(e.g., KotlinSnippetRepository.snippetsFor(scope: KotlinSnippetScope)) that
returns SnippetRegistry.getSnippets("kt", scope.filename) for the given scope on
demand (avoiding allocating the full Map and repeated reads), and update call
sites in KotlinCompletions.collectSnippetCompletions to call snippetsFor(scope)
instead of accessing snippets[scope]; keep KotlinSnippetScope and
SnippetRegistry.getSnippets(...) usage intact and remove or deprecate the old
full-map getter.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0678c908-d70e-4e57-990c-0aa1f6ad3f29

📥 Commits

Reviewing files that changed from the base of the PR and between cc21a2b and 7f99f43.

📒 Files selected for processing (2)

• lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinSnippetRepository.kt
• lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt