Skip to content

ADFA-3319: add basic K2-based Kotlin diagnostic provider#1112

Merged
itsaky-adfa merged 21 commits intostagefrom
feat/ADFA-3319-basic-diagnostics-provider
Apr 18, 2026
Merged

ADFA-3319: add basic K2-based Kotlin diagnostic provider#1112
itsaky-adfa merged 21 commits intostagefrom
feat/ADFA-3319-basic-diagnostics-provider

Conversation

@itsaky-adfa
Copy link
Copy Markdown
Contributor

@itsaky-adfa itsaky-adfa commented Mar 24, 2026

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>
@itsaky-adfa itsaky-adfa self-assigned this Mar 24, 2026
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>
@itsaky-adfa itsaky-adfa force-pushed the feat/ADFA-3319-basic-diagnostics-provider branch from 191fdbb to 2ba6d7e Compare March 25, 2026 10:34
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
@itsaky-adfa itsaky-adfa force-pushed the feat/ADFA-3319-basic-diagnostics-provider branch from 2ba6d7e to 90a3de1 Compare March 25, 2026 10:58
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
@itsaky-adfa itsaky-adfa force-pushed the feat/ADFA-3319-basic-diagnostics-provider branch from 90a3de1 to 54ca7a9 Compare March 26, 2026 11:06
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>
@itsaky-adfa itsaky-adfa changed the base branch from stage to fix/ADFA-3318-setup-K2-LSP-infra April 15, 2026 04:33
@itsaky-adfa itsaky-adfa marked this pull request as ready for review April 15, 2026 04:33
@dara-abijo-adfa dara-abijo-adfa requested a review from a team April 15, 2026 11:43
Base automatically changed from fix/ADFA-3318-setup-K2-LSP-infra to stage April 17, 2026 16:56
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 17, 2026

📝 Walkthrough

Release Notes: K2-based Kotlin Diagnostic Provider (ADFA-3319)

Features

  • New Kotlin Diagnostic Provider: Added KotlinDiagnosticProvider for on-demand K2-based diagnostics with:

    • Per-file analysis timestamp caching to avoid redundant re-analysis
    • Automatic parsing of Kotlin source files with K2 pipeline
    • Diagnostic severity mapping (ERROR/WARNING/INFO) and text range conversion to LSP ranges
    • Proper resource cleanup and cancellation support
  • Event-driven Diagnostics Publishing: Enhanced KotlinLanguageServer with:

    • Debounced analysis (400ms delay) on document open/change events
    • Automatic diagnostic clearing on document close
    • Coroutine-based async infrastructure using SupervisorJob
  • Parser Configuration: Added enableParserEventSystem parameter to control parser event handling in CompilationEnvironment and exposed via Compiler.defaultKotlinParser property

  • Bytecode Class Reference Rewriting: Added ReplaceClassRef data class for normalized class name transformations (dot vs. slash notation) with serialization support

  • Kotlin Analysis Artifact Update: Updated kotlin-analysis-api external dependency to newer K2 release tag

Dependencies

  • Prerequisite PRs: This PR must be merged after #1097, #1098, and #1105

⚠️ Risks & Best Practices Considerations

  • Session Reinitialization: Modified setupWithProject() to always call recreateSession() without guarding on prior initialization—verify this doesn't cause unnecessary overhead or resource leaks during repeated project setup
  • Artifact Checksum Change: Updated kotlin-analysis-api SHA256 checksum for new k2-android release—ensure artifact integrity is verified in deployment pipeline
  • Debounce Timing: 400ms debounce delay on diagnostics may impact perceived responsiveness for fast typists or automated analysis workflows; consider if this aligns with IDE performance expectations
  • Concurrent State Management: Diagnostics provider uses ConcurrentHashMap for timestamps—verify thread safety under high concurrency scenarios with multiple file edits
  • PSI Document Synchronization: Relies on PsiDocumentManager to translate K2 text ranges to LSP ranges; ensure document state consistency is maintained during async analysis

Walkthrough

The pull request introduces on-demand Kotlin diagnostics capabilities to the language server through a new KotlinDiagnosticProvider class, adds coroutine-based event-driven analysis with debouncing, updates the compilation environment to support parser event system configuration, exposes parser access through the compiler, adds a new ReplaceClassRef data class for bytecode rewriting, and updates an external dependency checksum.

Changes

Cohort / File(s) Summary
Desugaring DSL
composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt
New data class ReplaceClassRef for bytecode class-reference rewriting, accepting fromClass and toClass parameters with computed getters for internal ASM notation; implements Serializable with serialVersionUID.
Kotlin Diagnostic Provider
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt
New class providing on-demand Kotlin diagnostics with per-file analysis caching, PSI resolution, and severity mapping via Kotlin Analysis API; implements AutoCloseable.
Kotlin Language Server Infrastructure
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt, lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/Compiler.kt, lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt
Added coroutine infrastructure with debounced analysis triggering, KotlinDiagnosticProvider lifecycle management, parser event system configuration parameter, and defaultKotlinParser property exposure.
Build Configuration
subprojects/kotlin-analysis-api/build.gradle.kts
Updated external kt-android JAR asset reference tag and sha256 checksum.

Sequence Diagram(s)

sequenceDiagram
    actor Editor as Editor (Client)
    participant LSR as Language Server
    participant KDP as KotlinDiagnosticProvider
    participant CMP as Compiler
    participant PRJ as Kotlin Project
    
    Editor->>LSR: Document Open/Change
    LSR->>LSR: Cancel previous analyzeJob
    LSR->>LSR: Schedule analysis (400ms debounce)
    
    Note over LSR: Wait 400ms<br/>(debounce delay)
    
    LSR->>KDP: analyze(selectedFile)
    activate KDP
    
    KDP->>CMP: Refresh & resolve PSI
    CMP->>PRJ: Get KtFile
    CMP-->>KDP: Return resolved PsiFile
    
    KDP->>KDP: Create in-memory copy
    KDP->>PRJ: Run analyzeCopy()<br/>(PREFER_SELF + ONLY_COMMON_CHECKERS)
    PRJ-->>KDP: Return KaDiagnostics
    
    KDP->>KDP: Map diagnostics to items<br/>(severity, ranges)
    KDP-->>LSR: Return DiagnosticResult
    deactivate KDP
    
    LSR->>Editor: Publish Diagnostics
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PR #1097: Modifies the same ReplaceClassRef data class in the desugaring DSL, indicating parallel or dependent work on bytecode class-reference rewriting features.
  • PR #1098: Updates Kotlin analysis API integration in subprojects/kotlin-analysis-api, which complements the diagnostic provider infrastructure added in this PR.

Suggested reviewers

  • jatezzz
  • Daniel-ADFA
  • jomen-adfa

Poem

🐰 A rabbit hops through code so keen,
With diagnostics now on screen,
Debouncing dreams and coroutines fast,
Kotlin analysis built to last! 🚀

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a basic K2-based Kotlin diagnostic provider, which aligns with the implementation of KotlinDiagnosticProvider and related infrastructure.
Description check ✅ Passed The description provides relevant context by referencing the related issue ADFA-3319 and specifying merge dependencies, which is related to the changeset's purpose and scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 feat/ADFA-3319-basic-diagnostics-provider

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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

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

338-342: ⚠️ Potential issue | 🟡 Minor

onDocumentChange analyzes selectedFile, not event.changedFile.

The Kotlin-file guard checks event.changedFile, but debouncingAnalyze() uses whatever selectedFile currently is. If a background change event fires for a Kotlin file that isn't the currently selected one (e.g. formatter, external write, multi-document workflows), you'll (a) re-analyze the wrong file and (b) skip the actually-changed one. Consider passing the changed file through and/or making selectedFile the source of truth only for open/selected events.

🤖 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/KotlinLanguageServer.kt`
around lines 338 - 342, onDocumentChange currently checks
DocumentUtils.isKotlinFile(event.changedFile) but calls debouncingAnalyze()
which uses the global selectedFile, causing the wrong file to be analyzed;
change debouncingAnalyze to accept a File/URI parameter (e.g.,
debouncingAnalyze(changedFile: VirtualFile/URI)) and update onDocumentChange to
pass event.changedFile (DocumentChangeEvent.changedFile) to it, and update other
callers (e.g., selection/open handlers) to call debouncingAnalyze(selectedFile)
so the analyzed file is always the one provided rather than relying on the
selectedFile field.
🧹 Nitpick comments (4)
composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt (1)

15-18: Clarify parameter docs to match accepted input formats.

Class KDoc states both dot and slash notations are accepted, but these field docs currently describe dot-notation only. Align wording to avoid DSL misuse.

Doc-only tweak
-	/** The class whose references should be replaced (dot-notation). */
+	/** The class whose references should be replaced (dot or slash notation). */
 	val fromClass: String,
-	/** The class that should replace all [fromClass] references (dot-notation). */
+	/** The class that should replace all [fromClass] references (dot or slash notation). */
 	val toClass: String,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt`
around lines 15 - 18, The KDoc for ReplaceClassRef accepts both dot and slash
notation but the property docs for fromClass and toClass only mention
dot-notation; update the property comments for fromClass and toClass to state
they accept either dot (e.g. java.lang.String) or slash (e.g. java/lang/String)
notation and mention which form the DSL will consume/emit if applicable so users
aren't confused when using the ReplaceClassRef class.
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt (1)

46-57: Nit: broad catch (Throwable) — narrow it if possible.

The re-throw of CancellationException is correct, but catching Throwable swallows Errors (e.g. StackOverflowError, OutOfMemoryError) and obscures genuine VM-level problems. Consider narrowing to Exception (and still re-throwing CancellationException). Based on learnings from actions/src/main/java/com/itsaky/androidide/actions/internal/DefaultActionsRegistry.kt, prefer narrow exception handling in Kotlin files across this project.

🤖 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/diagnostic/KotlinDiagnosticProvider.kt`
around lines 46 - 57, The analyze function currently catches Throwable which can
swallow VM Errors; change the catch to catch Exception instead while preserving
the CancellationException rethrow logic: in analyze (and around doAnalyze and
logger.error), catch Exception, if it is a CancellationException rethrow,
otherwise log the error and return DiagnosticResult.NO_UPDATE; keep the same
logger.error call and return behavior so only non-fatal Exceptions are handled.
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt (1)

316-322: Minor: non-atomic cancel/reassign of analyzeJob under ASYNC subscribers.

onDocumentOpen / onDocumentChange run on the EventBus ASYNC pool, so two events landing concurrently can both read the old analyzeJob, cancel it, and launch — leaking one job past the debounce window. Functionally still "eventually consistent", but worth guarding (e.g. a mutex, AtomicReference, or serializing via a single-dispatcher actor).

🤖 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/KotlinLanguageServer.kt`
around lines 316 - 322, The debouncingAnalyze() currently cancels and reassigns
analyzeJob non-atomically which can race when onDocumentOpen/onDocumentChange
run on the EventBus ASYNC pool; make the cancel+assign atomic by serializing
access to analyzeJob in debouncingAnalyze — e.g. use an AtomicReference<Job?>
for analyzeJob and update via getAndSet/cancel on the previous job, or protect
the block with a Mutex, or route all debounce requests through a single-threaded
coroutine actor/dispatcher; update the debouncingAnalyze method (and the
analyzeJob field) to use one of these approaches so concurrent callers cannot
leak a launched Job past the debounce window.
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt (1)

38-45: Nit: document the new enableParserEventSystem parameter in the class KDoc.

The KDoc block above the class enumerates the other parameters; enableParserEventSystem is now public API and should be described there too (its semantics matter for anyone writing callers that don't need live PSI events).

🤖 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 38 - 45, Add a KDoc entry describing the new parameter
enableParserEventSystem on the CompilationEnvironment class: explain that
enableParserEventSystem controls whether the parser event/PSI live event system
is enabled (true by default), its effect on callers that do not need live PSI
events, and any notable semantics (e.g., default value, thread-safety or
lifecycle implications). Update the KDoc parameter list in the class header to
include `@param` enableParserEventSystem with this concise description so public
API consumers of CompilationEnvironment see its purpose.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt`:
- Around line 21-23: Replace the public `@JvmField` val serialVersionUID in the
companion object of ReplaceClassRef with a private compile-time constant by
changing it to private const val serialVersionUID to make it a true static
constant, restrict visibility, and remove the need for `@JvmField`; update the
companion object in ReplaceClassRef.kt accordingly.

In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt`:
- Around line 35-38: KotlinDiagnosticProvider.close() currently cancels the
externally provided scope (scope) which destroys the caller's server-wide
SupervisorJob; instead, make KotlinDiagnosticProvider own a child scope and only
cancel that in close(): create an internal CoroutineScope in the
KotlinDiagnosticProvider constructor derived from the passed-in scope (e.g.,
CoroutineScope(scope.coroutineContext +
SupervisorJob(scope.coroutineContext[Job]))) and replace uses of scope with
this.internalScope, then change close() to cancel only the internal child scope
(or keep close() a no-op and cancel per-analysis jobs explicitly); ensure
references to KotlinLanguageServer.recreateSession and
diagnosticProvider?.close() no longer terminate the server-wide scope.
- Around line 98-106: The code logs full source text and noisy per-keystroke
info at info level; in KotlinDiagnosticProvider.kt change the logger calls
around analyzeCopy (specifically the logger.info("ktFile.text={}",
inMemoryPsi.text) and other per-change messages like "Analyzing file" / "Found
{} diagnostics") to use logger.debug or logger.trace and remove or stop logging
inMemoryPsi.text (do not log full file contents); keep only minimal contextual
info (e.g., file id or path) at debug/trace so analyzeCopy, inMemoryPsi,
collectDiagnostics and the final diagnostics count do not emit sensitive/noisy
info at info level.

In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt`:
- Around line 131-133: The problem: calling diagnosticProvider?.close() from
recreateSession causes KotlinDiagnosticProvider.close() to cancel the shared
SupervisorJob (member scope) so subsequent debouncingAnalyze()/analyzeSelected()
launches are dropped; to fix, change KotlinDiagnosticProvider.close() so it does
NOT call scope.cancelIfActive(...) on the shared scope: instead give the
provider its own private CoroutineScope or Job for internal work and cancel that
(or cancel only its internal debounce/job fields), leaving the class member
scope (SupervisorJob) untouched; update KotlinDiagnosticProvider.close() to
cancel the provider-owned scope/job (or clear jobs) and remove any call that
cancels the shared scope; keep recreateSession calling
diagnosticProvider?.close() as-is.

---

Outside diff comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt`:
- Around line 338-342: onDocumentChange currently checks
DocumentUtils.isKotlinFile(event.changedFile) but calls debouncingAnalyze()
which uses the global selectedFile, causing the wrong file to be analyzed;
change debouncingAnalyze to accept a File/URI parameter (e.g.,
debouncingAnalyze(changedFile: VirtualFile/URI)) and update onDocumentChange to
pass event.changedFile (DocumentChangeEvent.changedFile) to it, and update other
callers (e.g., selection/open handlers) to call debouncingAnalyze(selectedFile)
so the analyzed file is always the one provided rather than relying on the
selectedFile field.

---

Nitpick comments:
In
`@composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt`:
- Around line 15-18: The KDoc for ReplaceClassRef accepts both dot and slash
notation but the property docs for fromClass and toClass only mention
dot-notation; update the property comments for fromClass and toClass to state
they accept either dot (e.g. java.lang.String) or slash (e.g. java/lang/String)
notation and mention which form the DSL will consume/emit if applicable so users
aren't confused when using the ReplaceClassRef class.

In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt`:
- Around line 38-45: Add a KDoc entry describing the new parameter
enableParserEventSystem on the CompilationEnvironment class: explain that
enableParserEventSystem controls whether the parser event/PSI live event system
is enabled (true by default), its effect on callers that do not need live PSI
events, and any notable semantics (e.g., default value, thread-safety or
lifecycle implications). Update the KDoc parameter list in the class header to
include `@param` enableParserEventSystem with this concise description so public
API consumers of CompilationEnvironment see its purpose.

In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt`:
- Around line 46-57: The analyze function currently catches Throwable which can
swallow VM Errors; change the catch to catch Exception instead while preserving
the CancellationException rethrow logic: in analyze (and around doAnalyze and
logger.error), catch Exception, if it is a CancellationException rethrow,
otherwise log the error and return DiagnosticResult.NO_UPDATE; keep the same
logger.error call and return behavior so only non-fatal Exceptions are handled.

In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt`:
- Around line 316-322: The debouncingAnalyze() currently cancels and reassigns
analyzeJob non-atomically which can race when onDocumentOpen/onDocumentChange
run on the EventBus ASYNC pool; make the cancel+assign atomic by serializing
access to analyzeJob in debouncingAnalyze — e.g. use an AtomicReference<Job?>
for analyzeJob and update via getAndSet/cancel on the previous job, or protect
the block with a Mutex, or route all debounce requests through a single-threaded
coroutine actor/dispatcher; update the debouncingAnalyze method (and the
analyzeJob field) to use one of these approaches so concurrent callers cannot
leak a launched Job past the debounce window.
🪄 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: 4048de85-ad9a-4763-b153-328f5f72dda4

📥 Commits

Reviewing files that changed from the base of the PR and between fb067ea and f79b396.

📒 Files selected for processing (6)
  • composite-builds/build-logic/desugaring/src/main/java/com/itsaky/androidide/desugaring/dsl/ReplaceClassRef.kt
  • lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.kt
  • lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.kt
  • lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/Compiler.kt
  • lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt
  • subprojects/kotlin-analysis-api/build.gradle.kts

@itsaky-adfa itsaky-adfa merged commit 43684b2 into stage Apr 18, 2026
2 checks passed
@itsaky-adfa itsaky-adfa deleted the feat/ADFA-3319-basic-diagnostics-provider branch April 18, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants