From 1be745b69c2a3d64014797342492d13ba12b92b0 Mon Sep 17 00:00:00 2001 From: SG Command Date: Sat, 22 Aug 2026 23:19:10 +0200 Subject: [PATCH] docs(lsp): add capability inventory, gaps, and next steps (issue #3) --- docs/lsp-status.md | 116 +++++++++++++++++++++++++++++++++++++++++++++ docs/todo.md | 1 + 2 files changed, 117 insertions(+) create mode 100644 docs/lsp-status.md diff --git a/docs/lsp-status.md b/docs/lsp-status.md new file mode 100644 index 0000000..7cf9d57 --- /dev/null +++ b/docs/lsp-status.md @@ -0,0 +1,116 @@ +# PgTidy LSP Status & Roadmap + +This document describes the current state of the PgTidy LSP server (`pkg/lsp`, +`cmd/pgtidy/lsp.go`), what it actually provides today, and concrete next steps. +It was written as part of issue #3 ("See what we can provide for LSP"). + +> Note: The V3 milestone is already implemented and shipping (`docs/todo.md` marks +> V3 done), so this is an inventory + gap analysis, not a greenfield proposal. + +## What the server provides today + +Verified by hand against the built binary (`pgtidy lsp`, JSON-RPC 2.0 over stdio, +`Content-Length` framing) — no external LSP library, all wire types hand-rolled. + +### Advertised capabilities (`initialize` → `capabilities`) + +| Capability | Value | Where | +|---|---|---| +| `textDocumentSync` | `1` (full sync) | `serverCaps` | +| `documentFormattingProvider` | `true` | `handle("initialize")` | +| `documentRangeFormattingProvider` | `true` | `handle("initialize")` | +| `codeActionProvider` | `true` | `handle("initialize")` | + +### Supported methods + +| Method | Direction | Behavior | +|---|---|---| +| `initialize` / `shutdown` / `exit` / `initialized` | req/resp + notif | Lifecycle. `exit`/`shutdown` acknowledged with `null` result. | +| `textDocument/didOpen` | notif | Stores document text; triggers `publishDiagnostics`. | +| `textDocument/didChange` | notif | Stores latest content version; triggers `publishDiagnostics`. | +| `textDocument/didClose` | notif | Drops text + fix cache; clears diagnostics with empty list. | +| `textDocument/formatting` | req/resp | Full-doc format via `pkg/format`; returns one `fullReplace` `TextEdit`. | +| `textDocument/rangeFormatting` | req/resp | Formats doc, returns minimal edit over the selected line range. | +| `textDocument/codeAction` | req/resp | Returns quick-fix `WorkspaceEdit`s for fixable diagnostics overlapping the range. | +| `textDocument/publishDiagnostics` | notif | Sent on every open/change; diagnostic code = `RuleID`, source = `pgtidy`. | +| `$/cancelRequest` | req | Ignored (per LSP, no response). | +| unknown | req/resp | `-32601 method not found` (when the request has an `id`). | + +### Verified at runtime (e2e smoke test) + +- `initialize` returns the capability block above. +- `didOpen` on `select * from t;` → `publishDiagnostics` with `COR001` + ("SELECT * is fragile…", severity 4 = hint, code `COR001`). +- `textDocument/formatting` on that input → edit replacing with + `SELECT *\nFROM t;\n` (keyword casing + clause-per-line applied). +- `textDocument/hover` → `-32601 method not found` (not implemented — correct). + +## What the server does NOT provide (gaps) + +These are the most useful, well-scoped gaps to fill next. None are blockers for the +current shipping state. + +1. **No `hover`.** `textDocument/hover` is unimplemented and returns `-32601`. + A natural first add: return the `RuleID` + a short explanation for diagnostics + on the hovered range, or a keyword/type doc for `hover` on SQL identifiers. +2. **No `documentSymbol` / `documentLink`.** No outline/symbol tree. For a formatter + that already parses `CREATE FUNCTION`/`PROCEDURE` headers into a CST, a symbol + provider listing functions/procedures would be low-cost and high-value in large + schema files. +3. **`hover`-style diagnostics shape.** Diagnostics currently use a `Range` whose + `end.character` is `start.character + 1` (a 1-char caret), not the actual + offending span. A real highlight range would improve editor UX. +4. **`textDocument/willSave` / `willSaveWaitUntil` / `didSave`.** No save hooks — + "format-on-save" must currently be driven by the client binding + `textDocument/formatting` to the editor's save event. A `willSaveWaitUntil` + handler would let the server own format-on-save. +5. **No `completion`.** `textDocument/completion` is not implemented. Not urgent for + a formatter/linter, but relevant if PL/pgSQL autocompletion (keywords, types) is + ever in scope. +6. **No diagnostics debounce/coalescing beyond full-sync.** Every `didChange` + re-runs the full lint engine. Fine for now; a debounce + incremental re-check + becomes relevant on large files. +7. **`initializationOptions` / workspace config.** `initialize` params are parsed + nowhere — no way to pass style overrides or a config path over the protocol. +8. **No `textDocument/prepareRename`, `rename`, `references`, `foldingRange`.** + Low priority; would be natural extensions once symbol info exists. + +## Conventions to keep consistent + +- **One core, many frontends.** The LSP reuses `pkg/diagnostics.Diagnostic` and + `pkg/lint` directly — no parallel diagnostic model. New LSP features should reuse + these, not fork them. +- **Safety gate is non-negotiable.** Both `formatting` and `rangeFormatting` call + `format.SemanticallyEqual(src, out)` before returning edits; on failure they return + an empty edit (keep original). Any new code path that formats must honor this + invariant (invariant #5 in `AGENTS.md`). +- **No new external deps.** The wire layer is intentionally dependency-free. New + protocol types should be added as local structs, not pulled in from an LSP library. + +## Concrete next steps (recommended, smallest-first) + +Ranked by effort/value for the smallest useful delta: + +1. **Add a real diagnostic highlight range** (swap the 1-char caret for the actual + offending span) — ~1 file, no new method, immediate UX win. Reuses existing + `RuleID`/severity data. +2. **Add `textDocument/hover`** returning the rule explanation for the hovered + diagnostic, or a keyword/type glossary. Reuses `pkg/lint` rule metadata. +3. **Add `documentSymbol`** listing `CREATE FUNCTION`/`PROCEDURE` signatures. + Reuses the existing CST header parse in `pkg/format`. +4. **Add `willSaveWaitUntil`** to own format-on-save instead of relying on client + binding. + +> Do NOT: expand the LSP surface into a broad design (workspace features, + incremental parsing, custom `textDocument/*` extensions) as part of this issue. + Keep any change scoped to the above and evidence-backed by a `pkg/lsp` test + (see `server_test.go` for the framed-request/response harness). + +## Verification + +- `go build ./cmd/pgtidy` succeeds. +- `go test ./...` passes (LSP unit tests in `pkg/lsp/server_test.go` exercise + `initialize`, formatting, range formatting, `didClose` diagnostics clearing). +- Runtime e2e smoke test (framed JSON-RPC over stdio) confirmed `initialize` + capabilities, `COR001` diagnostics, and a formatting edit; `hover` correctly + returns `-32601`. diff --git a/docs/todo.md b/docs/todo.md index ba12ba4..e1d0b68 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -119,6 +119,7 @@ Legend: ✅ done · 🚧 in progress · ⬜ not started - `cmd/pgtidy/lsp.go`: `pgtidy lsp` subcommand; config discovered from cwd. - `editors/vscode/`: TS extension using `vscode-languageclient`; launches `pgtidy lsp` via stdio; `.pgsql` mapped to `sql` language; `pgtidy.path` / `pgtidy.enable` settings. - _Range formatting: future._ +- Full capability inventory, runtime-verified gaps, and ranked next steps in `docs/lsp-status.md` (issue #3). ## ✅ V4 — DataGrip - `editors/datagrip/`: Gradle-based JetBrains plugin targeting DataGrip 2024.3+ via LSP4IJ. -- 2.54.0