Skip to content

Commit 0be5644

Browse files
fi3eworkfi3ework
authored
feat(lint): self-documenting Rslint diagnostics — hover, clickable rule docs, playground (#28)
* docs: record self-documenting diagnostics design (ADR 0004) * feat(vscode): make Rslint diagnostics self-documenting in the editor Client-side half of #27. New lint-stack modules parse Inline directives (rslint-/eslint- prefixes, all four forms, comma lists, ' -- ' trailers) into per-rule-id hover (rendered as Rslint(rule-id) with the id linking to its docs page), DocumentLink and persistent-underline affordances, sharing one memoized parse per document version. The router middleware lifts the '[rule-id] ' message prefix into Diagnostic.code + codeDescription-equivalent target so the Problems panel shows a clickable rule id; it yields automatically once the server publishes code itself and passes unmatched messages through untouched. Everything derives from one base URL plus the rule id — no bundled rule metadata, no network, no configuration (ADR 0004). The hover provider stands down if a future server advertises hoverProvider. Recorded as the eighth adaptation in AGENTS.md. * test(vscode): cover self-documenting diagnostics in E2E New suite-hover (isolated fixture) asserts the Rslint(rule-id) hover on line-0 and mid-file Inline directives, the DocumentLink range over the rule id token, and the enriched Diagnostic.code with its docs target and stripped message prefix. The ported suites migrate rule identification from message-substring matching to the shared diagnosticRuleIdIncludes accessor, since the rule id now lives in the diagnostic code instead of the message prose. * build(vscode): give the F5 playground a floor-satisfying Node A GUI-launched VS Code never runs the contributor's shell hooks, so the dev host inherits the desktop session's node — often below the lint worker's runtime floor, at which point Rslint reports version mismatch and lints nothing. A new preLaunchTask chain (playground node -> watch) materializes a floor-satisfying Node into a gitignored .playground/node-bin/ (PATH node if compliant, else the highest satisfying fnm/nvm/volta/asdf install, filtered by directory name so only one probe spawns), and launch.json prepends that directory to the dev host's PATH only — nothing machine-wide changes. The floor is read from versionCheck.ts, the single source of truth. * test(vscode): turn the rslint playground fixture into a feature showcase The fixture the F5 playground opens now demonstrates every lint capability in region-separated sections: clickable rule docs in the Problems panel (a local plugin rule whose derived link deliberately 404s beside native rules that resolve to real pages), Inline-directive hover/underline/ctrl+click, every directive form (next-line, trailing disable-line, eslint- prefix, bare wildcard, comma lists with a description trailer), and the mistyped-id pitfall. Native rules join the config in a second entry — one entry takes either community plugin instances or built-in plugin names, never both. The smoke-test contract is preserved: exactly one null literal, asserted rule set unchanged. * fix(vscode): address PR review findings on runtime mirror and code guard - Prune the controller's runtime capability mirror identity-safely via an optional Rslint.onClosed hook, so a same-key runtime replacement created during an in-flight close is no longer deleted by the old runtime's cleanup (RuntimeManager removes its entry before the async close ends). - Yield diagnostic enrichment only to an object code (the converted server-published codeDescription shape); a future primitive server code is preserved as the value while the docs target is still derived and the [rule-id] prefix stripped. - Document the accepted raw-text false-positive trade-off on the inline directive comment scanner.
1 parent 96e08ae commit 0be5644

42 files changed

Lines changed: 1246 additions & 199 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ vite.config.ts.timestamp-*
147147
tests-dist/
148148
.rsdoctor/
149149

150+
# F5 playground's materialized Node (see .vscode/tasks.json "playground node")
151+
packages/vscode/.playground/
152+
150153
# E2E fixtures install published npm versions on demand; only
151154
# their manifests and configs are tracked.
152155
packages/vscode/e2e/fixtures/*/node_modules/

‎.vscode/launch.json‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,20 @@
3333
"--disable-updates",
3434
"${workspaceFolder}/packages/vscode/${input:playgroundTarget}"
3535
],
36+
// The "playground node" task materialized a floor-satisfying Node here;
37+
// prepending it makes the extension's PATH probe find it, regardless of
38+
// what node the desktop session carries (GUI launches never run the
39+
// shell hooks that honor .nvmrc).
40+
"env": {
41+
"PATH": "${workspaceFolder}/packages/vscode/.playground/node-bin:${env:PATH}"
42+
},
43+
"windows": {
44+
"env": {
45+
"PATH": "${workspaceFolder}\\packages\\vscode\\.playground\\node-bin;${env:PATH}"
46+
}
47+
},
3648
"outFiles": ["${workspaceFolder}/packages/vscode/dist/**/*.js"],
37-
"preLaunchTask": "extension watch",
49+
"preLaunchTask": "playground",
3850
// Off by default: attaching to every spawned worker slows runs down and
3951
// child-process sourcemaps are unreliable (same setting upstream).
4052
"autoAttachChildProcesses": false

‎.vscode/tasks.json‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,32 @@
11
{
22
"version": "2.0.0",
33
"tasks": [
4+
{
5+
// Materializes a floor-satisfying Node into .playground/node-bin/ so
6+
// launch.json can prepend it to the dev host's PATH. A GUI-launched
7+
// VS Code skips the contributor's shell hooks (fnm/nvm on .nvmrc), so
8+
// without this the dev host sees the desktop session's node — often
9+
// below the extension's runtime floor, and Rslint reports
10+
// "version mismatch" instead of linting.
11+
"label": "playground node",
12+
"type": "shell",
13+
"command": "node",
14+
"args": ["packages/vscode/scripts/playgroundNode.mjs"],
15+
"group": "build",
16+
"problemMatcher": [],
17+
"presentation": {
18+
"reveal": "silent",
19+
"panel": "shared"
20+
}
21+
},
22+
{
23+
// What F5 depends on: materialize the playground Node, then start the
24+
// watch build. Kept separate from "extension watch" so a plain rebuild
25+
// from the task palette does not run the playground helper.
26+
"label": "playground",
27+
"dependsOrder": "sequence",
28+
"dependsOn": ["playground node", "extension watch"]
29+
},
430
{
531
// Background watch build the F5 launch depends on. `--env-mode dev`
632
// (`watch:local`) so breakpoints in src/ bind inside the dev host.

‎CONTEXT.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ Glossary of terms used across rstack-editor. Code, docs, commit messages and rev
3434
- **Lint worker** — the process the extension ships and runs for one lint server: it hosts Rslint's JS side (config evaluation, plugin rules) on a User Node runtime with its cwd at the workspace folder root, and fronts the Go `rslint --lsp` process it spawns, so the editor sees one language server. _Avoid_: lint host, lint proxy, lint server (that is what the worker presents, not what it is).
3535
- **Bridged folder** — a workspace folder whose lint runs against the Rstack config: no native `rslint.config.*` anywhere in the folder, a `rstack.config.*` at its root, and the lint worker pinned to rstack's shipped shim for its whole lifetime. _Avoid_: bridged workspace, rstack folder.
3636
- **Native folder** — a workspace folder whose lint runs against its own `rslint.config.*`, exactly as the standalone Rslint extension would.
37+
- **Inline directive** — a source comment that toggles lint rules for a scope: `rslint-disable`, `rslint-enable`, `rslint-disable-line`, `rslint-disable-next-line`, with the `eslint-` prefix accepted as an exact equivalent. Rule ids are comma-separated; a bare directive applies to all rules. _Avoid_: disable comment, suppression comment.
38+
- **Rule docs link** — the documentation URL derived from a rule id alone (one base URL plus the id, no per-rule data). Best-effort by design: a mistyped or brand-new rule id yields a dead link, never an error. _Avoid_: rule doc URL, docs href.
3739

3840
## fmt
3941

‎docs/adr/0004-no-bundled-rule-knowledge.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
status: accepted
3+
---
4+
5+
# Self-documenting diagnostics carry no bundled rule knowledge
6+
7+
Issue #27 wants Rslint diagnostics to explain themselves in the editor: hover over rule ids in inline directives, clickable rule ids in the Problems panel, faded/struck-through rendering for dead-code rules. The Go server today gives the client almost nothing to build on: `textDocument/publishDiagnostics` sets only range, severity, source and a message of the form `[rule-id] description` — no `code`, no `codeDescription`, no `tags` — the server advertises no `hoverProvider`, exposes no rule-metadata request, and the rule type itself has no description field. The tempting fix is to bundle what the server won't say: a rule list (there are ~500), per-rule descriptions scraped from the docs site, a hand-maintained set of "unused-variable-like" rules for `DiagnosticTag.Unnecessary`.
8+
9+
**Decision.** The extension carries **no per-rule data of any kind**. Everything it shows is either **derived from the rule id by one formula** — `https://rslint.rs/rules/<plugin prefix minus '@', or 'eslint' for core rules>/<rule-name>`, the same formula as upstream's `getRuleDocUrl` — or **parsed from server output**: the lint middleware reads the `[rule-id] ` prefix off each published diagnostic's message, synthesizes `code` + `codeDescription.href`, and strips the prefix (if the message doesn't match, the diagnostic passes through untouched). No network requests either: links are best-effort, so a mistyped or brand-new rule id yields a dead docs link, not a validation round-trip.
10+
11+
## Considered options
12+
13+
- **Bundle rule metadata** (scrape `rslint.rs/llms.txt` or vendor the per-rule `.md` files at build time) — rejected: a standing sync pipeline whose failure mode is showing _stale_ descriptions, worse than showing none; the docs link already lands on the authoritative text.
14+
- **Validate links over the network** (HEAD-check with cache, suppress hover on 404) — rejected: makes an editor affordance depend on connectivity; offline/intranet kills the feature.
15+
16+
## Consequences
17+
18+
- Hover and Problems-panel entries show the rule id and its docs link, **no prose description**, until upstream exposes rule metadata.
19+
- A mistyped rule id in an inline directive stays **silent** — the extension cannot know it is unknown without a rule list. The user's signal is the squiggle the directive failed to suppress. Proper reporting (unused/mistyped directive diagnostics) is upstream work.
20+
- `DiagnosticTag` rendering is **not attempted client-side** — only rules know whether they are dead-code-like, and encoding that in the extension is exactly the bundled knowledge this ADR forbids.
21+
- The message-prefix synthesis is **transitional by design**: once the Go server publishes `code`/`codeDescription` natively, the middleware synthesis is deleted, not kept as a fallback. If upstream changes the message format first, the guard makes the feature degrade to the status quo silently.
22+
- The client-side hover provider registers only while the server does not advertise `hoverProvider`; the day it does, the client yields (the fmt precedent: never fight a server-registered capability).

‎packages/vscode/AGENTS.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
55
## The copies are intentional
66

77
- `stacks/lint` and `stacks/test` are deliberate near-verbatim copies of the upstream extensions, kept close to upstream so changes can be synced by diffing. Do NOT deduplicate or refactor across the two stacks — the duplication is the point; consolidation is a later, explicit phase.
8-
- The copies diverge from upstream in exactly seven ways (the "adaptations" below). When syncing upstream, preserve them. An eighth divergence is either a bug or must be added to this list.
8+
- The copies diverge from upstream in exactly eight ways (the "adaptations" below). When syncing upstream, preserve them. A ninth divergence is either a bug or must be added to this list.
99
- **Tracked upstream state.** `stacks/lint` is synced to web-infra-dev/rslint `packages/vscode-extension` at **39536fd6** (#1617 — per-document core resolution, `CoreResolver` + `RuntimeManager`, `corePath`, PnP removed) and **892482e0** (#1630 — `configPath` on `rslint/configRefresh`). `CoreResolver.ts` / `RuntimeManager.ts` / `WorkspaceDocumentRouter.ts` / `Rslint.ts` are the files to diff when syncing further; record the new commits here when you do.
1010

11-
## The seven adaptations
11+
## The eight adaptations
1212

1313
1. **Shell activation** — stacks never self-activate; `register()` returns fast and never blocks on starting a server/worker.
1414
2. **Namespace** — everything user-visible is `rstack.*`. Legacy `rslint.*` / `rstest.*` settings and command ids are not read, aliased or migrated (breaking old settings and keybindings was an accepted cost).
@@ -17,6 +17,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
1717
5. **Worker-cwd decoupling** (test) — a project's cwd is explicit, not derived from the config file path; for native configs behavior stays byte-identical to upstream.
1818
6. **Node runtime selection** (lint, test, fmt) — the Node a project-loading child process runs on is a **User Node runtime** chosen by the extension against one uniform floor, never assumed from PATH; the recovery path is the user's own shell, and the dividing line is the **load bound** (terms in CONTEXT.md; the full rule and rationale in `docs/adr/0001-node-runtime-selection.md`). All three callers — the lint worker, the rstest worker and the `rs fmt --lsp` server — take the decision from the one shared module (`shared/nodeResolution.ts`) and share one escape hatch, the resource-scoped `rstack.nodeExecutable` (`shared/nodeExecutableSetting.ts`); each appends its own consequence to the shared preflight message.
1919
7. **Lint worker and Rstack bridge** — the extension host is only Rslint's language client. One vscode-free, editor-shipped lint worker per **Lint runtime** (one Rslint core inside one workspace folder — CONTEXT.md) runs on the User Node runtime, owns the Go LSP plus all five reverse requests, and derives the binary/config/plugin pieces from one explicit `@rslint/core` directory. Upstream's `CoreResolver` loads that core in the extension host; ours only walks to the directory (`fs.stat` + `package.json` + semver) and hands the path to the worker, and its `CoreInstallation` therefore carries paths, not module factories; upstream's installation cache goes with the module loading it memoized (`clear()` is a no-op kept for the `RuntimeManager` contract). A bridged folder passes only rstack's published `dist/rslintConfig.js` shim; neither the extension nor the worker re-implements Rstack config semantics. Because protocol 2 locks `configPath` per process, the shim is part of the runtime key (`folder + core identity + shim`), which upstream — having no bridge — keys on the core alone. Why: `docs/adr/0003-lint-through-editor-worker.md`.
20+
8. **Self-documenting Rslint diagnostics** — client-side providers parse Inline directives into per-rule hover, DocumentLink and underline-decoration affordances (the hover renders `Rslint(rule-id)`, the shape VS Code gives the published diagnostics), and the router enriches today's `[rule-id] message` diagnostics with a derived Rule docs link. No rule metadata or network lookup is bundled (ADR 0004). The hover provider yields whenever the owning language client's resolved capabilities advertise `hoverProvider`; an optional `Rslint.onClosed` hook identity-safely prunes the controller's capability mirror; the diagnostic synthesis is removed once upstream publishes `code` / `codeDescription` natively.
2021

2122
## Rules
2223

‎packages/vscode/e2e/fixtures/rslint/rslint.config.mjs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ import localPlugin from './local-plugin.mjs';
44
* A real Rslint flat config: it is loaded by the language server through the
55
* project's own `@rslint/core` (this extension ships none), and by the
66
* plugin-host regression smoke test through `createPluginLintHost`.
7+
*
8+
* Besides the local plugin rule the smoke test asserts on, two native rules
9+
* are enabled so the F5 playground shows diagnostics whose derived docs links
10+
* resolve to real pages on rslint.rs (a local plugin rule has no docs page).
11+
* A config entry takes either community plugin instances or built-in plugin
12+
* names, never both, so the two live in separate entries.
713
*/
814
export default [
915
{
@@ -13,4 +19,12 @@ export default [
1319
'local/no-null': 'error',
1420
},
1521
},
22+
{
23+
files: ['src/**/*.ts'],
24+
plugins: ['@typescript-eslint'],
25+
rules: {
26+
'no-console': 'error',
27+
'@typescript-eslint/no-explicit-any': 'error',
28+
},
29+
},
1630
];

‎packages/vscode/e2e/fixtures/rslint/src/index.ts‎

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,61 @@
1-
// The lintable issue this fixture exists for: the `null` literal below is
2-
// reported by `local/no-null` (see `rslint.config.mjs`). Exactly one `null`
3-
// literal — the smoke test asserts on the diagnostic count.
1+
// This fixture doubles as the F5 playground: open this file in the dev host
2+
// and every lint capability of the extension is observable directly below.
3+
// Constraint: the smoke test asserts on `local/no-null` — keep exactly one
4+
// `null` literal in this file.
5+
6+
// #region Diagnostics — clickable rule docs in the Problems panel
7+
// Every rslint diagnostic in the Problems panel (Cmd+Shift+M) shows its rule
8+
// id as a clickable link, and the message carries no `[rule-id]` prefix — the
9+
// extension lifts the id into the diagnostic's code.
10+
11+
// `local/no-null` is this fixture's own plugin rule (see local-plugin.mjs).
12+
// Its derived docs link is a deliberate 404: a user-local rule has no page on
13+
// rslint.rs. This is the diagnostic the smoke test asserts on.
414
export function getValue() {
515
return null;
616
}
17+
18+
// Native rules link to real pages: `no-console` → /rules/eslint/no-console,
19+
// `@typescript-eslint/no-explicit-any` → /rules/typescript-eslint/no-explicit-any.
20+
export function debugValue(value: any) {
21+
console.log(value);
22+
}
23+
// #endregion
24+
25+
// #region Inline directives — hover, underline, Ctrl+click
26+
// Rule ids inside a disable comment are underlined. Hovering one shows
27+
// `Rslint(rule-id)` with the id linking to its docs page; Ctrl+click
28+
// (Cmd+click on macOS) opens the page directly. The directive keyword itself
29+
// has no hover — only the rule ids do.
30+
31+
// rslint-disable-next-line no-console
32+
console.log('suppressed — hover the underlined rule id above');
33+
34+
// Comma-separated ids are each their own hover target; the ` -- ` trailer is
35+
// free-form description and is not parsed.
36+
// rslint-disable-next-line no-console, @typescript-eslint/no-explicit-any -- demo: two rule ids and a trailer
37+
export const logAny = (value: any) => console.log(value);
38+
// #endregion
39+
40+
// #region Directive forms — disable-line, eslint- prefix, wildcard
41+
// `rslint-disable-line` suppresses its own line, and works from a trailing
42+
// comment too.
43+
console.log('suppressed inline'); // rslint-disable-line no-console
44+
45+
// The `eslint-` prefix is an exact equivalent of `rslint-`.
46+
// eslint-disable-next-line no-console
47+
console.log('suppressed via the eslint- prefix');
48+
49+
// A bare directive suppresses every rule; with no rule id there is nothing to
50+
// hover.
51+
// rslint-disable-next-line
52+
console.log('suppressed by the wildcard directive');
53+
// #endregion
54+
55+
// #region Pitfall — a mistyped rule id
56+
// A mistyped id suppresses nothing: the squiggle below survives, which is the
57+
// signal the directive missed. Its hover link still derives (and 404s) — the
58+
// extension deliberately validates nothing against a rule list (ADR 0004).
59+
// rslint-disable-next-line no-consle
60+
console.log('NOT suppressed — the rule id above is mistyped');
61+
// #endregion

‎packages/vscode/e2e/lint/fixtures/hover/rslint.config.mjs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default [
2+
{
3+
rules: {
4+
'no-console': 'error',
5+
},
6+
},
7+
];

‎packages/vscode/e2e/lint/fixtures/hover/src/index.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// rslint-disable-next-line no-console
2+
console.log('suppressed');
3+
console.log('reported');
4+
5+
export function getValue() {
6+
// rslint-disable-next-line local/no-null
7+
return null;
8+
}

0 commit comments

Comments
 (0)