Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f97a398
ADFA-4826: Enable Compose in lsp/kotlin
itsaky-adfa Aug 10, 2026
2712672
ADFA-4826: Add extract-variable analysis, plan and rewrite
itsaky-adfa Aug 10, 2026
410f9db
ADFA-4826: Add the extract-variable Compose sheet
itsaky-adfa Aug 10, 2026
96c4365
ADFA-4826: Wire up the extract-variable code action
itsaky-adfa Aug 10, 2026
9c94ed0
ADFA-4826: Document the extract-variable requirements
itsaky-adfa Aug 10, 2026
bc9cb11
ADFA-4826: Stop offering the lambda that wraps the expression
itsaky-adfa Aug 12, 2026
42c231b
ADFA-4826: Label a block rung by the construct that owns it
itsaky-adfa Aug 12, 2026
b84ad40
ADFA-4826: Fix misleading KDoc and add else block test
itsaky-adfa Aug 12, 2026
79ed3fc
ADFA-4826: Write the return type when converting an expression body
itsaky-adfa Aug 12, 2026
a69b679
ADFA-4826: Anchor the declaration in the scope the user picked
itsaky-adfa Aug 12, 2026
1a89480
ADFA-4826: Cover contentSpanOf and fix a nested-block fixture
itsaky-adfa Aug 12, 2026
b9877f8
ADFA-4826: Expand a block written on one line
itsaky-adfa Aug 12, 2026
f9de1e8
ADFA-4826: Expand only a block that is really written on one line
itsaky-adfa Aug 12, 2026
75944bd
ADFA-4826: Split the type-text renderer from its catching form
itsaky-adfa Aug 12, 2026
9d8ca53
ADFA-4826: Decline a block whose statement shares the brace line
itsaky-adfa Aug 12, 2026
d00742d
ADFA-4826: Tidy the expression-body conversion and its docs
itsaky-adfa Aug 12, 2026
cb43457
chore: remove plan docs
itsaky-adfa Aug 18, 2026
dd298b4
ADFA-4826: Refuse an unhostable block rung at plan time
itsaky-adfa Aug 18, 2026
8238b88
ADFA-4826: Keep replace-all off an unhostable anchor
itsaky-adfa Aug 18, 2026
f634421
ADFA-4826: Validate the variable name against names actually in scope
itsaky-adfa Aug 18, 2026
70cf174
ADFA-4826: Decide Unit-ness from the type text that gets written
itsaky-adfa Aug 18, 2026
f699e7c
ADFA-4826: Resolve a whitespace-only selection like a caret
itsaky-adfa Aug 18, 2026
d942f1c
ADFA-4826: Offer the expression chooser for an exact selection too
itsaky-adfa Aug 18, 2026
921dd25
ADFA-4826: Apply Spotless formatting
itsaky-adfa Aug 18, 2026
7fc37ce
ADFA-4826: Address whole-branch review findings
itsaky-adfa Aug 18, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions docs/adr/0013-refactoring-ui-lives-in-the-owning-lsp-module.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# 0012. Refactoring UI lives in the owning LSP module

- **Status:** Proposed
- **Date:** 2026-08-03
- **Deciders:** Code On The Go team

## Context

The K2 Kotlin LSP is gaining interactive refactorings: extract variable and extract method (ADFA-4826), inline variable (ADFA-4827), semantic rename (ADFA-4825). Unlike every existing Kotlin code action, these cannot be a single fire-and-forget edit — the user has to choose an expression, a name, a target scope, and whether to replace other occurrences. That is a real UI surface, not a `DialogUtils` one-liner.

[ADR 0009](0009-jetpack-compose-for-new-ui.md) settles *what* that UI is built with (Compose, UDF, `ViewModel` + `StateFlow`). It says nothing about *where* language-specific UI lives, and the module graph makes that a genuine question:

- `editor` depends on `lsp/kotlin` (`editor/build.gradle.kts`), so the dependency flows **LSP -> editor**. An LSP module cannot reach the editor or `app`.
- `ActionData` carries only a `Context` and the editor; there is no service-lookup mechanism for an LSP module to call *up* into a UI layer.
- `lsp/java` already owns UI code today — `AutoFixImportsAction` builds and shows a `DialogUtils` chooser directly.

So a refactoring in `lsp/kotlin` either renders its own UI, or a new inversion mechanism has to be invented for it.

## Decision

**A language server module owns the UI for its own refactorings.** `lsp/kotlin` enables Compose and hosts the refactoring bottom sheets; the same applies to any future `lsp/*` module that grows an interactive refactoring.

- Compose is enabled per-module exactly as `flamegraph`, `floating-window` and `profiler` do it: the `kotlin-compose` plugin, `compose = true`, and the Compose BOM with `ui`/`foundation`/`material3`.
- The UI is a `BottomSheetDialogFragment` hosting a `ComposeView`. The hosting `FragmentActivity` is found by walking `ContextWrapper.baseContext` up from `ActionData`'s `Context` — no new `ActionData` key, no change to the `editor` module.
- **The analysis/UI split is enforced by data, not by module boundaries.** The action's background pass produces a plain-data plan (candidate expressions, scope chains, occurrence ranges, suggested name, document version); the sheet performs no analysis and holds no PSI. All refactoring logic lives in pure functions, unit-testable without an editor, an activity, or Compose.
- ADR 0009 otherwise applies unchanged: `ViewModel` + `StateFlow<UiState>`, sealed `UiEvent`, `collectAsStateWithLifecycle()`.

## Consequences

**Positive**
- No new indirection: one module, one PR per refactoring, no interface to register or resolve.
- Consistent with `lsp/java` already owning its dialogs, so there is one rule for LSP-owned UI rather than two.
- The plain-data plan boundary keeps the valuable logic testable regardless of where the UI sits, so the placement decision does not compromise test coverage.

**Negative / costs**
- A language server module gains a UI surface, which is a layering smell: `lsp/kotlin` is no longer purely a language service.
- Compose and `lifecycle-viewmodel` are added to a module that previously had neither, growing its build surface and bringing ktlint's compose-rules ruleset to bear on it.
- Walking the `ContextWrapper` chain for a `FragmentActivity` is an implicit dependency on how the editor is hosted; a future change to that hosting breaks it at runtime rather than at compile time.
- If three or more `lsp/*` modules end up with Compose UI, extracting a shared UI module becomes worthwhile and this decision will need revisiting.

## Alternatives considered

- **Render in `editor`, invert via an interface.** Declare a refactoring-UI interface in `editorApi` or `lsp/models`, implement it in `editor`, have `lsp/kotlin` call up through it. Cleanest layering. Rejected: nothing registers such an implementation today, so it means inventing a service-lookup mechanism for one sheet, and the interface would be guessed from a single client.
- **Render in `app`.** `app` is the integration point and already hosts `BottomSheetDialogFragment`s and `ILanguageClient`. Rejected: same inversion problem, and it puts Kotlin-specific refactoring UI in the module where nothing else language-specific lives.
- **A new `lsp/kotlin-ui` module.** Keeps Compose out of `lsp/kotlin` without inverting. Rejected for now: a new Gradle module in a ~80-module build is disproportionate for one sheet. Reconsider once extract-method and inline-variable have landed and the UI surface is known.

## Related

- [ADR 0009](0009-jetpack-compose-for-new-ui.md) — Compose for new UI; this ADR answers *where*, not *what*.
- [ADR 0006](0006-koin-dependency-injection.md) — Koin DI, unchanged.
- [ADR 0010](0010-navigation-resolves-via-analysis-api.md) — the K2 Analysis API as the Kotlin semantic source of truth.
- [ARCHITECTURE.md](../../ARCHITECTURE.md) — module map, layering, UDF.
1 change: 1 addition & 0 deletions docs/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ Format is lightweight **MADR / Nygard**: Context → Decision → Consequences
| [0010](0010-navigation-resolves-via-analysis-api.md) | Kotlin navigation resolves via the Analysis API, not the symbol index | Proposed |
| [0011](0011-command-analysis-priority.md) | User-invoked commands get their own analysis priority | Proposed |
| [0012](0012-volatile-build-metadata-out-of-abis.md) | Keep volatile build metadata out of module ABIs | Proposed |
| [0013](0012-refactoring-ui-lives-in-the-owning-lsp-module.md) | Refactoring UI lives in the owning LSP module | Proposed |
Loading
Loading