Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ vite.config.ts.timestamp-*
tests-dist/
.rsdoctor/

# F5 playground's materialized Node (see .vscode/tasks.json "playground node")
packages/vscode/.playground/

# E2E fixtures install published npm versions on demand; only
# their manifests and configs are tracked.
packages/vscode/e2e/fixtures/*/node_modules/
Expand Down
14 changes: 13 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,20 @@
"--disable-updates",
"${workspaceFolder}/packages/vscode/${input:playgroundTarget}"
],
// The "playground node" task materialized a floor-satisfying Node here;
// prepending it makes the extension's PATH probe find it, regardless of
// what node the desktop session carries (GUI launches never run the
// shell hooks that honor .nvmrc).
"env": {
"PATH": "${workspaceFolder}/packages/vscode/.playground/node-bin:${env:PATH}"
},
"windows": {
"env": {
"PATH": "${workspaceFolder}\\packages\\vscode\\.playground\\node-bin;${env:PATH}"
}
},
"outFiles": ["${workspaceFolder}/packages/vscode/dist/**/*.js"],
"preLaunchTask": "extension watch",
"preLaunchTask": "playground",
// Off by default: attaching to every spawned worker slows runs down and
// child-process sourcemaps are unreliable (same setting upstream).
"autoAttachChildProcesses": false
Expand Down
26 changes: 26 additions & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,32 @@
{
"version": "2.0.0",
"tasks": [
{
// Materializes a floor-satisfying Node into .playground/node-bin/ so
// launch.json can prepend it to the dev host's PATH. A GUI-launched
// VS Code skips the contributor's shell hooks (fnm/nvm on .nvmrc), so
// without this the dev host sees the desktop session's node — often
// below the extension's runtime floor, and Rslint reports
// "version mismatch" instead of linting.
"label": "playground node",
"type": "shell",
"command": "node",
"args": ["packages/vscode/scripts/playgroundNode.mjs"],
"group": "build",
"problemMatcher": [],
"presentation": {
"reveal": "silent",
"panel": "shared"
}
},
{
// What F5 depends on: materialize the playground Node, then start the
// watch build. Kept separate from "extension watch" so a plain rebuild
// from the task palette does not run the playground helper.
"label": "playground",
"dependsOrder": "sequence",
"dependsOn": ["playground node", "extension watch"]
},
{
// Background watch build the F5 launch depends on. `--env-mode dev`
// (`watch:local`) so breakpoints in src/ bind inside the dev host.
Expand Down
2 changes: 2 additions & 0 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Glossary of terms used across rstack-editor. Code, docs, commit messages and rev
- **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).
- **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.
- **Native folder** — a workspace folder whose lint runs against its own `rslint.config.*`, exactly as the standalone Rslint extension would.
- **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.
- **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.

## fmt

Expand Down
22 changes: 22 additions & 0 deletions docs/adr/0004-no-bundled-rule-knowledge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
status: accepted
---

# Self-documenting diagnostics carry no bundled rule knowledge

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`.

**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.

## Considered options

- **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.
- **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.

## Consequences

- Hover and Problems-panel entries show the rule id and its docs link, **no prose description**, until upstream exposes rule metadata.
- 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.
- `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.
- 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.
- 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).
5 changes: 3 additions & 2 deletions packages/vscode/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
## The copies are intentional

- `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.
- 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.
- 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.
- **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.

## The seven adaptations
## The eight adaptations

1. **Shell activation** — stacks never self-activate; `register()` returns fast and never blocks on starting a server/worker.
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).
Expand All @@ -17,6 +17,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
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.
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.
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`.
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.

## Rules

Expand Down
14 changes: 14 additions & 0 deletions packages/vscode/e2e/fixtures/rslint/rslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ import localPlugin from './local-plugin.mjs';
* A real Rslint flat config: it is loaded by the language server through the
* project's own `@rslint/core` (this extension ships none), and by the
* plugin-host regression smoke test through `createPluginLintHost`.
*
* Besides the local plugin rule the smoke test asserts on, two native rules
* are enabled so the F5 playground shows diagnostics whose derived docs links
* resolve to real pages on rslint.rs (a local plugin rule has no docs page).
* A config entry takes either community plugin instances or built-in plugin
* names, never both, so the two live in separate entries.
*/
export default [
{
Expand All @@ -13,4 +19,12 @@ export default [
'local/no-null': 'error',
},
},
{
files: ['src/**/*.ts'],
plugins: ['@typescript-eslint'],
rules: {
'no-console': 'error',
'@typescript-eslint/no-explicit-any': 'error',
},
},
];
61 changes: 58 additions & 3 deletions packages/vscode/e2e/fixtures/rslint/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,61 @@
// The lintable issue this fixture exists for: the `null` literal below is
// reported by `local/no-null` (see `rslint.config.mjs`). Exactly one `null`
// literal — the smoke test asserts on the diagnostic count.
// This fixture doubles as the F5 playground: open this file in the dev host
// and every lint capability of the extension is observable directly below.
// Constraint: the smoke test asserts on `local/no-null` — keep exactly one
// `null` literal in this file.

// #region Diagnostics — clickable rule docs in the Problems panel
// Every rslint diagnostic in the Problems panel (Cmd+Shift+M) shows its rule
// id as a clickable link, and the message carries no `[rule-id]` prefix — the
// extension lifts the id into the diagnostic's code.

// `local/no-null` is this fixture's own plugin rule (see local-plugin.mjs).
// Its derived docs link is a deliberate 404: a user-local rule has no page on
// rslint.rs. This is the diagnostic the smoke test asserts on.
export function getValue() {
return null;
}

// Native rules link to real pages: `no-console` → /rules/eslint/no-console,
// `@typescript-eslint/no-explicit-any` → /rules/typescript-eslint/no-explicit-any.
export function debugValue(value: any) {
console.log(value);
}
// #endregion

// #region Inline directives — hover, underline, Ctrl+click
// Rule ids inside a disable comment are underlined. Hovering one shows
// `Rslint(rule-id)` with the id linking to its docs page; Ctrl+click
// (Cmd+click on macOS) opens the page directly. The directive keyword itself
// has no hover — only the rule ids do.

// rslint-disable-next-line no-console
console.log('suppressed — hover the underlined rule id above');

// Comma-separated ids are each their own hover target; the ` -- ` trailer is
// free-form description and is not parsed.
// rslint-disable-next-line no-console, @typescript-eslint/no-explicit-any -- demo: two rule ids and a trailer
export const logAny = (value: any) => console.log(value);
// #endregion

// #region Directive forms — disable-line, eslint- prefix, wildcard
// `rslint-disable-line` suppresses its own line, and works from a trailing
// comment too.
console.log('suppressed inline'); // rslint-disable-line no-console

// The `eslint-` prefix is an exact equivalent of `rslint-`.
// eslint-disable-next-line no-console
console.log('suppressed via the eslint- prefix');

// A bare directive suppresses every rule; with no rule id there is nothing to
// hover.
// rslint-disable-next-line
console.log('suppressed by the wildcard directive');
// #endregion

// #region Pitfall — a mistyped rule id
// A mistyped id suppresses nothing: the squiggle below survives, which is the
// signal the directive missed. Its hover link still derives (and 404s) — the
// extension deliberately validates nothing against a rule list (ADR 0004).
// rslint-disable-next-line no-consle
console.log('NOT suppressed — the rule id above is mistyped');
// #endregion
7 changes: 7 additions & 0 deletions packages/vscode/e2e/lint/fixtures/hover/rslint.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default [
{
rules: {
'no-console': 'error',
},
},
];
8 changes: 8 additions & 0 deletions packages/vscode/e2e/lint/fixtures/hover/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// rslint-disable-next-line no-console
console.log('suppressed');
console.log('reported');

export function getValue() {
// rslint-disable-next-line local/no-null
return null;
}
10 changes: 10 additions & 0 deletions packages/vscode/e2e/lint/fixtures/hover/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"compilerOptions": {
"target": "ES2020",
"module": "ESNext",
"moduleResolution": "bundler",
"strict": true,
"skipLibCheck": true
},
"include": ["src/**/*.ts"]
}
5 changes: 5 additions & 0 deletions packages/vscode/e2e/lint/runTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,11 @@ async function main(): Promise<void> {
workspace: fixture('rule-option-types'),
tests: suiteDir('suite-rule-option-types'),
},
{
name: 'Self-documenting diagnostics tests',
workspace: fixture('hover'),
tests: suiteDir('suite-hover'),
},
{
name: 'import/no-cycle tests',
workspace: fixture('import-cycle'),
Expand Down
3 changes: 2 additions & 1 deletion packages/vscode/e2e/lint/suite-bridge/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import path from 'node:path';
import * as vscode from 'vscode';
import { findPackageJsonUncached } from '../../../src/shared/packageResolve';
import {
diagnosticRuleIdIncludes,
getRslintDiagnostics,
waitForRslintDiagnostics,
waitForRslintDiagnosticsCount,
Expand Down Expand Up @@ -51,7 +52,7 @@ async function openLintTarget(): Promise<vscode.TextDocument> {

function hasNoDebugger(diagnostics: readonly vscode.Diagnostic[]): boolean {
return diagnostics.some((diagnostic) =>
diagnostic.message.includes('no-debugger'),
diagnosticRuleIdIncludes(diagnostic, 'no-debugger'),
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import path from 'node:path';
import { waitForContentChange } from '../suite/fixall-helpers';
import { saveDocumentOnce } from '../utils/codeActionRegistry';
import { withCodeActionsOnSave } from '../utils/configuration';
import { waitForRslintDiagnostics } from '../utils/diagnostics';
import {
diagnosticRuleIdIncludes,
waitForRslintDiagnostics,
} from '../utils/diagnostics';
import {
closeAndDeleteTemporaryDocument,
temporaryFilePath,
Expand All @@ -31,8 +34,6 @@ suite('rslint object-form plugins integration', function () {
return workspaceFolder.uri.fsPath;
}

// LSP diagnostic messages are formatted as `[<ruleName>] <description>`
// (see internal/lsp/service.go), so ruleName is matchable on `.message`.
function messages(diags: vscode.Diagnostic[]): string {
return diags.map((d) => d.message).join(' | ');
}
Expand Down Expand Up @@ -69,11 +70,13 @@ suite('rslint object-form plugins integration', function () {
const diagnostics = await waitForRslintDiagnostics(
openedDocument,
(diags) =>
diags.some((d) => d.message.includes('local/prefer-array-some')),
diags.some((d) =>
diagnosticRuleIdIncludes(d, 'local/prefer-array-some'),
),
);
assert.ok(
diagnostics.some((d) =>
d.message.includes('local/prefer-array-some'),
diagnosticRuleIdIncludes(d, 'local/prefer-array-some'),
),
`prefer-array-some did not appear; cannot exercise fixAll. Got: ${messages(diagnostics)}`,
);
Expand Down Expand Up @@ -124,22 +127,24 @@ suite('rslint object-form plugins integration', function () {
await vscode.window.showTextDocument(doc);

const diagnostics = await waitForRslintDiagnostics(doc, (diags) =>
diags.some((d) => d.message.includes('local/no-null')),
diags.some((d) => diagnosticRuleIdIncludes(d, 'local/no-null')),
);
const msgs = messages(diagnostics);

// Both plugin rules must come back from the worker...
assert.ok(
diagnostics.some((d) => d.message.includes('local/no-null')),
diagnostics.some((d) => diagnosticRuleIdIncludes(d, 'local/no-null')),
`Expected local/no-null. Got: ${msgs}`,
);
assert.ok(
diagnostics.some((d) => d.message.includes('local/prefer-array-some')),
diagnostics.some((d) =>
diagnosticRuleIdIncludes(d, 'local/prefer-array-some'),
),
`Expected local/prefer-array-some. Got: ${msgs}`,
);
// ...alongside the natively-linted rule, proving the merge.
assert.ok(
diagnostics.some((d) => d.message.includes('no-console')),
diagnostics.some((d) => diagnosticRuleIdIncludes(d, 'no-console')),
`Expected native no-console merged with plugin diagnostics. Got: ${msgs}`,
);
});
Expand Down
Loading