Some checks are pending
Intrepid/Prometeu/Studio/pipeline/pr-master Build started...
JaCoCo Coverage #### Project Overview
No changes detected, that affect the code coverage.
* Line Coverage: 63.75% (19407/30442)
* Branch Coverage: 54.17% (7740/14289)
* Lines of Code: 30442
* Cyclomatic Complexity: 12365
#### Quality Gates Summary
Output truncated.
Test / Build skipped: 15, passed: 685
Intrepid/Prometeu/Studio/pipeline/head This commit looks good
Add PBS quick fixes as frontend-owned repairs transported by the LSP. Duplicate reserved attributes are deleted at the extra span. Interface InitAllowed attributes with arguments become [InitAllowed]. The server matches Diagnostic.code plus range, does not parse messages, and returns an empty list when the capability or the fix is absent. Housekeep DSC-0044 with LSN-0073.
131 lines
10 KiB
Markdown
131 lines
10 KiB
Markdown
---
|
|
id: LSN-0073
|
|
ticket: pbs-lsp-code-actions
|
|
title: PBS quick fixes are frontend repairs transported by the LSP
|
|
created: 2026-09-22
|
|
tags: [studio, lsp, vscode, compiler-pbs, editor, code-actions, quick-fix]
|
|
---
|
|
|
|
# PBS quick fixes are frontend repairs transported by the LSP
|
|
|
|
## Original Problem
|
|
|
|
PBS diagnostics already reach the editor, but the server did not advertise `codeActionProvider`. A user could see a duplicate attribute or a bad `InitAllowed` and still had to repair it by hand.
|
|
|
|
The tempting shortcut was a code-to-edit table in the LSP. That shortcut does not work. Compiler `Diagnostic` values carry a stable code, phase, template, placeholders, span, and related locations, but the reserved-attribute reports use the short constructor: empty placeholders, and the English sentence is the only fine discriminator. `E_SEM_MALFORMED_RESERVED_ATTRIBUTE` covers Doc without `markdown`, a wrong payload, empty markdown, an unknown argument, `InitAllowed` with arguments, an out-of-range `AssetLowering`, and a bad `Host` shape. A switch on the code cannot pick a safe edit without reading the message. Diagnostic wording is rendering, not identity. Code actions are an optional editorial capability: the frontend owns the meaning, and the LSP transports it.
|
|
|
|
## Consolidated Decision
|
|
|
|
PBS quick fix is one `WorkspaceEdit` tied to one diagnostic of the requested physical document. The frontend produces the repair from the syntax tree. The LSP does not invent it.
|
|
|
|
Durable locks from `DEC-0057` (AGD-0047: scope A, producer B, residence B, first wave C):
|
|
|
|
1. This wave is quick fix only. No organize-imports, source action, refactor, function or struct stub, code lens, or server command.
|
|
2. The server does not write files. The client applies the edit.
|
|
3. Each repair names the stable diagnostic code, the primary span of that diagnostic, and exactly one local text edit. The LSP must not keep a code-to-edit table and must not read the diagnostic message to build the edit.
|
|
4. The repair is computed on `textDocument/codeAction` from the current document analysis. It is not a required field of compiler `Diagnostic`, and it is not published inside `publishDiagnostics`, including `data`.
|
|
5. `publishDiagnostics` carries the stable compiler code on `Diagnostic.code` so the client can echo it. Source, severity, message, and related information stay as they were. Those presentation questions remain with `AGD-0054`.
|
|
6. A repair is returned only when the request context contains a diagnostic with the same code and the same primary range. An empty diagnostic list returns an empty action list. An `only` filter that does not include quick fix returns an empty action list.
|
|
7. Each item is a QuickFix `CodeAction`, not a `Command`. The edit touches one file, the requested physical document, and contains one `TextEdit`. There is no lazy resolve.
|
|
8. A path that is not a compiler-known regular file, including `/virtual/stdlib`, gets no edit.
|
|
9. If the frontend does not expose the capability, the server does not advertise `codeActionProvider`. A request that arrives anyway returns an empty list, not a protocol error. Absence of a fix is not a refused cursor.
|
|
10. PBS exposes the capability. Another frontend does not gain it merely by having a language service.
|
|
11. The first wave is two shapes, not three diagnostic codes.
|
|
- For every extra reserved attribute the declaration validator already reports as `E_SEM_DUPLICATE_RESERVED_ATTRIBUTE`, delete exactly that occurrence's span. Do not delete the first occurrence, the declaration, or neighboring whitespace.
|
|
- For an `InitAllowed` attribute with a non-empty argument list on a host signature, the shape the validator reports as `E_SEM_MALFORMED_RESERVED_ATTRIBUTE` when the file is an interface module, replace that attribute span with `[InitAllowed]`. Do not emit that repair for any other attribute that shares the code. `[InitAllowed()]` is not diagnosed today and gets no repair.
|
|
12. Nothing else gets a quick fix in this wave. In particular: inserting `[Doc(markdown = """""")]` or any other Doc rewrite, inserting an empty or incomplete `[Host]` or `[BuiltinType]`, `W_SEM_IGNORED_VALUE`, unresolved imports, an invalid reserved-attribute target, parse errors, unresolved overloads, and method stubs.
|
|
|
|
`AGD-0048` still owns formatting. `AGD-0049` still owns import assistance and organize-imports. `AGD-0054` still owns range, related locations, and diagnostic presentation. The PBS diagnostics spec does not gain a mandatory remediation field.
|
|
|
|
Spec 23 §8.3 now states the shared contract: code actions are quick fixes only; the frontend produces each repair from the document syntax; tooling transports it and does not parse the message or switch on codes; the published diagnostic carries the stable code; a repair is returned only for a matching context diagnostic; the edit is one physical file; the server does not write files, return a command, or require resolve; a missing capability is an empty list, not an error; virtual documents get no edit.
|
|
|
|
## Final Implementation
|
|
|
|
| Layer | What landed |
|
|
|---|---|
|
|
| PBS repairs | `PbsQuickFixCollector` walks the same attribute surfaces as `PbsDeclarationSemanticsValidator`, including the interface-module gate. Duplicate reserved attributes become an empty replacement of the extra span. The first `InitAllowed` with arguments on an interface host signature becomes `[InitAllowed]`. |
|
|
| Source kind | `PbsSemanticReadSurface.sourceKindByFile` records the kind used to parse each file. A missing kind means project rules, so a project `declare host` does not pretend the validator emitted the interface-only `InitAllowed` diagnostic. |
|
|
| Generic contract | `FrontendLanguageService.codeActionsSupported` defaults false. `codeActions` defaults empty. `FrontendCodeAction` is title, diagnostic code, path, offsets, and `newText`. Empty `newText` is a deletion and is allowed. |
|
|
| PBS mapping | `PBSFrontendLanguageService` exposes the capability and returns repairs only for the requested regular file. `/virtual/stdlib` fails `Files.isRegularFile` and produces nothing. |
|
|
| LSP | `BaselineDocumentIssue.code` is copied onto `Diagnostic.code`. `data` stays unset. The bridge drops repairs whose code and range do not match a context diagnostic. `PrometeuTextDocumentService.codeAction` honors the quick-fix `only` filter and never fails the request for “no fix”. |
|
|
| Protocol | `codeActionProvider` is `CodeActionOptions` with kind `quickfix` and `resolveProvider` false, and only when the frontend reports support. Mapped actions are `CodeAction` values with a `WorkspaceEdit`. They are not `Command`s. |
|
|
|
|
Titles are `Remove duplicate reserved attribute` and `Remove InitAllowed arguments`.
|
|
|
|
## Examples
|
|
|
|
Duplicate `Doc` on a project file. The first copy stays. Each later copy is deleted at its own span:
|
|
|
|
```pbs
|
|
[Doc(markdown = """
|
|
First.
|
|
""")]
|
|
[Doc(markdown = """
|
|
Second.
|
|
""")]
|
|
fn duplicated() -> void { return; }
|
|
```
|
|
|
|
The quick fix removes the second attribute and leaves `First.`. A context diagnostic with the same range but code `E_SEM_MALFORMED_RESERVED_ATTRIBUTE` produces no action. An empty diagnostic list produces no action.
|
|
|
|
Malformed or empty Doc is visible and not repairable here:
|
|
|
|
```pbs
|
|
[Doc(text = """
|
|
Wrong argument.
|
|
""")]
|
|
fn malformed() -> void { return; }
|
|
```
|
|
|
|
`InitAllowed` with arguments is repaired only on an interface-module host signature:
|
|
|
|
```pbs
|
|
declare host Gfx {
|
|
[Host(module = "gfx2d", name = "clear", version = 1)]
|
|
[InitAllowed(flag = 1)]
|
|
fn clear() -> void;
|
|
|
|
[Host(module = "gfx2d", name = "fill", version = 1)]
|
|
[InitAllowed()]
|
|
fn fill() -> void;
|
|
}
|
|
```
|
|
|
|
`clear` becomes `[InitAllowed]`. `fill` is already valid to the validator, so `[InitAllowed()]` is left alone. The same `InitAllowed(flag = 1)` inside a project file does not get that repair, because the validator does not emit the malformed-attribute diagnostic there.
|
|
|
|
The same collector deletes the extra copy of `BuiltinType`, `IntrinsicCall`, `BuiltinConst`, `Host`, and `AssetLowering` when the validator reports `E_SEM_DUPLICATE_RESERVED_ATTRIBUTE` for that surface. A malformed `Host` that merely shares the malformed-attribute code gets no edit.
|
|
|
|
## Pitfalls
|
|
|
|
Do not choose the edit in `prometeu-lsp` by switching on `E_SEM_MALFORMED_RESERVED_ATTRIBUTE` or by parsing the English message. The code is too coarse, and the message is not the identity.
|
|
|
|
Do not store the repair on compiler `Diagnostic` or in `Diagnostic.data`. Compilation and quick fix stay different surfaces. The published code exists so the client can echo it back.
|
|
|
|
Do not match on `Diagnostic.source` as if it were the long-term identity field. Today the bridge still copies the code into `source` for the existing presentation. The code-action match uses `Diagnostic.code`. `AGD-0054` may change `source` later.
|
|
|
|
Do not delete the first duplicate, and do not trim the whitespace around the deleted span. That cleanup is formatting and belongs to `AGD-0048`.
|
|
|
|
Do not insert `[Doc(markdown = """""")]`, an empty `[Host]`, or an empty `[BuiltinType]`. Those edits trade one diagnostic for another, or invent a declaration.
|
|
|
|
Do not treat `W_SEM_IGNORED_VALUE` as a source edit. Discard exists in lowering, not as PBS syntax.
|
|
|
|
Do not offer the `InitAllowed` rewrite for `[InitAllowed()]` or for a project file. The validator does not diagnose those shapes as “arguments not allowed”, so there is no diagnostic to attach.
|
|
|
|
Do not return a protocol error when there is no fix. Rename refuses a bad cursor with an error. Quick fix answers “nothing to do” with an empty list.
|
|
|
|
Do not advertise `codeActionProvider` for every frontend that has a language service. The default capability is unsupported.
|
|
|
|
Do not treat this lesson as closing formatting, import assistance, diagnostic presentation, snapshots, folding, document links, or hierarchy.
|
|
|
|
## References
|
|
|
|
- Agenda: `AGD-0047` (scope A, producer B, residence B, first wave C)
|
|
- Decision: `DEC-0057`
|
|
- Plan: `PLN-0136`
|
|
- Spec 23 §8.3 — optional code actions; frontend-owned repair; match by code and range; empty list when unsupported
|
|
- Spec: `docs/specs/compiler-languages/pbs/12. Diagnostics Specification.md` — wording is not identity; remediation stays outside the v1 diagnostic minimum
|
|
- `LSN-0056` — code actions are an optional editorial capability
|
|
- `LSN-0058` — generic `FrontendLanguageService` for the LSP
|
|
- `LSN-0067` — no new editorial SPI without a real consumer; PBS implements the existing language service
|
|
- Still open: `AGD-0048` formatting, `AGD-0049` import assistance, `AGD-0054` diagnostics UX, `AGD-0051` snapshots, `AGD-0056` hierarchy
|