diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee7bb80..6af70e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,17 @@ permissions: jobs: check: - runs-on: ubuntu-latest + # The extension is cross-platform; the LSP child-process spawn + # path, the bundle loader, and several file-path helpers are all + # Windows-sensitive. Running the gate on three OSes catches the + # LF/CRLF and path-separator bugs that single-OS CI silently + # ignores. The matrix shares the same step list — only the vsix + # upload and the network-bound npm audit are pinned to Linux. + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v6 @@ -45,16 +55,19 @@ jobs: run: npm run smoke - name: npm audit (prod deps, high+) + # Network-bound and platform-independent; one run is enough. + if: matrix.os == 'ubuntu-latest' run: npm audit --omit=dev --audit-level=high - name: Verify vsix packs cleanly # vsce is a pinned devDependency (see package.json) — Dependabot # bumps it via the npm config. `npm ci` has already installed it. - run: | - npx vsce package --out pipeline-check.vsix - ls -lh pipeline-check.vsix + run: npx vsce package --out pipeline-check.vsix - uses: actions/upload-artifact@v7 + # Single artefact upload from the Linux job; identical-name + # uploads from the matrix would collide. + if: matrix.os == 'ubuntu-latest' with: name: vsix path: pipeline-check.vsix diff --git a/ROADMAP.md b/ROADMAP.md index 3051f2a..a666534 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -391,3 +391,105 @@ on their own. The 50px of vertical space we'd save up front is small relative to the structural cost of having to add the header back the first time we want a second view. + +--- + +## Review pass (2026-05-19) — improvements from in-depth code review + +The findings below came out of a holistic review of the codebase after +v0.1.1 shipped. Categories cluster related work into reviewable PRs. + +### Code-level fixes (cheap wins) + +- [ ] **R1** Reorder the `filterByThreshold` import in + [src/extension.ts:72](src/extension.ts#L72) up to the rest of the + import block. Currently sits after a function declaration. +- [ ] **R2** "Restart language server" toast at + [src/extension.ts:223](src/extension.ts#L223) fires even when + `startClient()` failed. Guard with `if (client)`. +- [ ] **R3** Add a timeout to `stopClient()` + ([src/extension.ts:185-192](src/extension.ts#L185-L192)) — + a deadlocked LSP child holds the deactivate path indefinitely. +- [ ] **R4** `groupByFile` does `key = f.uri.toString()` then + `vscode.Uri.parse(key)` to get the Uri back + ([src/findingsView.ts:357-385](src/findingsView.ts#L357-L385)). + Use a `Map` keyed on the Uri directly. +- [ ] **R5** `compareByLocation` + ([src/findingsView.ts:413](src/findingsView.ts#L413)) sorts on + `uri.toString()` which includes the `file://` scheme prefix. + Sort on `fsPath` instead. + +### Performance + +- [ ] **R6** Cache `collectFindings()` per refresh. Currently called + twice per refresh (`buildRoot` + `updateBadge`); each walks every + diagnostic in the workspace. +- [ ] **R7** Filter `onDidChangeDiagnostics` by the event's `uris` + payload — skip the refresh when none of the changed URIs carry a + pipeline-check diagnostic. + +### UX gaps + +- [ ] **R8** Wire `Diagnostic.code.target` into the leaf tooltip. The + server publishes the rule's docs URL; we read `code.value` and + throw the URL away. Add a docs link at the bottom of the leaf + tooltip (set `isTrusted = true` on the `MarkdownString`). +- [ ] **R9** Status bar item showing critical/high counts. Click + reveals the Findings panel. +- [ ] **R10** Rename `pipelineCheck.findings.refresh` — re-renders + from current diagnostics, doesn't trigger a fresh scan. Once + scan-workspace lands, have refresh call `scanWorkspace()`. +- [ ] **R11** `CodeAction` provider for suppression comments. + Right-click a finding → "Suppress GHA-001 for this line" that + inserts the CLI's suppression syntax. +- [ ] **R12** "Go to next/previous finding" commands with keybindings. +- [ ] **R13** Set `Diagnostic.tags` for `Deprecated` / + `Unnecessary` where the rule indicates it. Server-side change. + +### Architecture + +- [ ] **R14** Extract the candidate-file pattern list from + `activationEvents`, `documentSelector`, and (post-merge) + `SCAN_PATTERNS` into a single shared TS module. Today the list + lives in three places. +- [ ] **R15** Add `onCommand:pipelineCheck.scanWorkspace` to + `activationEvents` so opening an isolated file from outside the + workspace can still trigger activation. +- [ ] **R16** Client-side structured logging into the output channel + with a `[client]` prefix. Breadcrumbs for bug reports. + +### Testing + +- [ ] **R17** `@vscode/test-electron` integration tests covering real + LSP publish and tree render. +- [ ] **R18** Extract the `vi.mock("vscode", ...)` factory into a + shared `src/__testStubs__/vscode.ts`. + +### Marketplace + +- [ ] **R19** **Ship the screenshots** the HTML comment at + [README.md:13-25](README.md#L13-L25) has been waiting for since + v0.1.0. Highest-leverage marketplace conversion improvement. +- [ ] **R20** CI check that `package.json#description` stays ≤ 145 + characters (the marketplace-search truncation limit). + +### CI / release + +- [ ] **R21** Test matrix: + `[ubuntu-latest, windows-latest, macos-latest]`. The + `LF → CRLF` warnings on every git operation hint at the bugs. +- [ ] **R22** Finish the eslint flat-config migration so the + drift between eslint v8 and TS 6 / esbuild 0.28 stops widening. +- [ ] **R23** Resolve the CodeQL default-setup conflict — disable + default setup or delete codeql.yml. Don't ship with a + permanently-red required check. +- [ ] **R24** `vsce publish --pre-release` channel for the next + minor bump. + +### Strategic + +- [ ] **R25** Per-provider toggles in settings. +- [ ] **R26** Inline `CodeLens` summarising findings per file. +- [ ] **R27** Workspace-level config file shared with the CLI. +- [ ] **R28** Opt-in telemetry (`vscode.env.isTelemetryEnabled`). +- [ ] **R29** Scan-on-save mode. diff --git a/src/extension.ts b/src/extension.ts index 32ad27f..d14cd0b 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -19,6 +19,8 @@ import { TransportKind, } from "vscode-languageclient/node"; import { FindingsTreeProvider, GroupMode } from "./findingsView"; +import { filterByThreshold } from "./severityFilter"; +import { registerStatusBar } from "./statusBar"; // Group-mode options offered by the Findings panel's "Change // Grouping" button. Labels are user-facing; descriptions are the @@ -69,8 +71,6 @@ async function changeGrouping( provider.setGroupMode(choice.mode); } } -import { filterByThreshold } from "./severityFilter"; - const LANGUAGE_ID = "pipelineCheck"; const LANGUAGE_NAME = "Pipeline-Check"; const OUTPUT_CHANNEL = "Pipeline-Check"; @@ -182,13 +182,34 @@ async function startClient(): Promise { } } +// Hard ceiling on how long deactivate / restart waits for the LSP +// child to shut down cleanly. A deadlocked server would otherwise +// hold the deactivate path indefinitely and VS Code reports "Window +// not responding". +const STOP_TIMEOUT_MS = 2000; + async function stopClient(): Promise { if (!client) { return; } const local = client; client = undefined; - await local.stop(); + let timer: NodeJS.Timeout | undefined; + try { + await Promise.race([ + local.stop(), + new Promise((resolve) => { + timer = setTimeout(resolve, STOP_TIMEOUT_MS); + }), + ]); + } finally { + if (timer) { + clearTimeout(timer); + } + // If stop() didn't win the race the client is stranded; dispose + // explicitly so its subscriptions don't outlive us. + local.dispose?.(); + } } export async function activate( @@ -208,6 +229,10 @@ export async function activate( // badge. Handing the view back closes the loop and triggers an // initial badge update. findingsProvider.setTreeView(findingsView); + // Status bar item lives at the bottom-left and shows the per- + // severity tally. Click reveals the Findings panel. registerStatusBar + // pushes the item onto context.subscriptions internally. + registerStatusBar(context); context.subscriptions.push( findingsView, vscode.commands.registerCommand("pipelineCheck.findings.refresh", () => @@ -220,7 +245,12 @@ export async function activate( vscode.commands.registerCommand("pipelineCheck.restart", async () => { await stopClient(); await startClient(); - vscode.window.showInformationMessage("Language server restarted."); + // Only confirm success when startClient left a live client behind. + // If start failed it surfaced its own error toast; we'd otherwise + // show "failed to start" and "restarted" at the same time. + if (client) { + vscode.window.showInformationMessage("Language server restarted."); + } }), vscode.commands.registerCommand("pipelineCheck.showLog", () => { if (client?.outputChannel) { diff --git a/src/findingsView.test.ts b/src/findingsView.test.ts index 2830242..a98d150 100644 --- a/src/findingsView.test.ts +++ b/src/findingsView.test.ts @@ -49,7 +49,13 @@ vi.mock("vscode", () => { ) {} } class MarkdownString { - constructor(public readonly value: string) {} + isTrusted = false; + supportThemeIcons = false; + constructor(public value: string) {} + appendMarkdown(s: string): this { + this.value += s; + return this; + } } const Uri = { parse: (s: string) => { @@ -75,9 +81,26 @@ vi.mock("vscode", () => { uri.fsPath ?? uri.path ?? "", }, languages: { - getDiagnostics: () => - (globalThis as { __stubDiagnostics?: unknown[] }).__stubDiagnostics ?? - [], + // Two call shapes: + // - `getDiagnostics()` returns every [uri, diagnostic[]] pair + // - `getDiagnostics(uri)` returns just that uri's diagnostics + // The provider's batch-skip path uses the second form; the + // collection path uses the first. + getDiagnostics: (uri?: { toString: () => string }) => { + const all = + ( + globalThis as { + __stubDiagnostics?: Array<[ + { toString: () => string }, + unknown[], + ]>; + } + ).__stubDiagnostics ?? []; + if (uri === undefined) return all; + const key = uri.toString(); + const match = all.find(([u]) => u.toString() === key); + return match ? match[1] : []; + }, onDidChangeDiagnostics: () => ({ dispose: () => undefined }), }, commands: { executeCommand: () => Promise.resolve() }, @@ -99,6 +122,7 @@ type FakeFinding = { rule: string; severity?: string; line?: number; + docsUrl?: string; }; function setStubDiagnostics(findings: FakeFinding[]): void { @@ -109,7 +133,10 @@ function setStubDiagnostics(findings: FakeFinding[]): void { arr.push({ source: "pipeline-check", message: `${f.rule} title\n\nThe long description.\n\nFix: do X.`, - code: { value: f.rule, target: { toString: () => "" } }, + code: { + value: f.rule, + target: { toString: () => f.docsUrl ?? "" }, + }, range: { start: { line: f.line ?? 0, character: 0 }, end: { line: f.line ?? 0, character: 0 }, @@ -481,3 +508,81 @@ describe("FindingsTreeProvider — group mode behaviour", () => { expect(p.getGroupMode()).toBe("file"); }); }); + +describe("FindingsTreeProvider — rule docs link in tooltip", () => { + // When the server publishes ``Diagnostic.code.target`` (the rule's + // documentation URL), the leaf tooltip should carry a clickable + // "Read more" link below the message body. When the URL is absent + // or empty, the tooltip is just the message. The link target is + // exactly what the server published — we don't synthesise URLs. + + it("appends a docs link when the server publishes one", () => { + setStubDiagnostics([ + { + file: "a.yml", + rule: "GHA-001", + severity: "HIGH", + docsUrl: "https://example.com/rules/gha-001", + }, + ]); + const p = new FindingsTreeProvider(ctx); + p.setGroupMode("severity"); + const roots = p.getChildren(); + const leaves = p.getChildren(roots[0]); + const item = p.getTreeItem(leaves[0]); + const tip = item.tooltip as { value: string; isTrusted: boolean }; + expect(tip.value).toContain("GHA-001 title"); + expect(tip.value).toContain( + "[$(book) GHA-001 documentation](https://example.com/rules/gha-001)", + ); + expect(tip.isTrusted).toBe(true); + }); + + it("leaves the tooltip clean when the server publishes no URL", () => { + setStubDiagnostics([ + { + file: "a.yml", + rule: "GHA-001", + severity: "HIGH", + // no docsUrl + }, + ]); + const p = new FindingsTreeProvider(ctx); + p.setGroupMode("severity"); + const roots = p.getChildren(); + const leaves = p.getChildren(roots[0]); + const item = p.getTreeItem(leaves[0]); + const tip = item.tooltip as { value: string }; + expect(tip.value).toContain("GHA-001 title"); + expect(tip.value).not.toContain("documentation"); + }); +}); + +describe("FindingsTreeProvider — findings cache invalidation", () => { + // refresh() drops the cached findings list so the next render sees + // any new diagnostic publishes. Without invalidation, a freshly + // published finding wouldn't appear in the tree until the user + // toggled the group mode or restarted VS Code. The test exercises + // the path end-to-end: render once, swap the stub data, refresh, + // render again. + + it("refresh() picks up newly-published diagnostics", () => { + setStubDiagnostics([ + { file: "a.yml", rule: "GHA-001", severity: "HIGH" }, + ]); + const p = new FindingsTreeProvider(ctx); + p.setGroupMode("severity"); + expect(p.getChildren()).toHaveLength(1); + + setStubDiagnostics([ + { file: "a.yml", rule: "GHA-001", severity: "HIGH" }, + { file: "b.yml", rule: "GHA-002", severity: "CRITICAL" }, + ]); + // Without refresh() the second publish would be invisible. + p.refresh(); + const roots = p.getChildren(); + expect(roots).toHaveLength(2); + // CRITICAL is leftmost in the sort. + expect(roots[0].kind === "group" && roots[0].label).toBe("CRITICAL"); + }); +}); diff --git a/src/findingsView.ts b/src/findingsView.ts index 59c2127..a87318c 100644 --- a/src/findingsView.ts +++ b/src/findingsView.ts @@ -71,6 +71,10 @@ type Finding = { readonly diagnostic: vscode.Diagnostic; readonly severity: SeverityName; readonly ruleId: string; + // The rule's documentation URL, when the server published one via + // ``Diagnostic.code.target``. Used to render a "Read more" link in + // the leaf tooltip — same prose the CLI's `--explain` shows. + readonly docsUrl: string | undefined; }; type GroupNode = { @@ -103,14 +107,30 @@ export class FindingsTreeProvider implements vscode.TreeDataProvider { // makes the pre-wiring window explicit — refresh() called before // setTreeView simply skips the badge update. private treeView: vscode.TreeView | undefined; + // Memoised result of ``collectFindings()``. A single refresh used to + // walk the global diagnostic store twice (once from buildRoot, once + // from updateBadge); now both paths read from this cache and one + // walk is enough. ``refresh()`` invalidates by setting it to null. + private cachedFindings: Finding[] | null = null; + // URIs that carried a pipeline-check diagnostic at the last refresh. + // Used to detect *clears*: when a URI in this set shows up in an + // onDidChangeDiagnostics batch with no remaining pipeline-check + // diagnostic, we still need to refresh so the stale leaf vanishes. + private lastFindingUris = new Set(); constructor(context: vscode.ExtensionContext) { // VS Code does not expose a per-source filter on the diagnostic- - // change event, so we re-render on every publish. collectFindings - // re-filters by source, so the rendered tree only changes when a - // pipeline-check publish actually arrives. + // change event, so we have to subscribe to all of them and bounce + // the ones we don't care about. We DO get the list of changed URIs + // on the event, so the early-out below skips the tree rebuild when + // no pipeline-check publish landed in this batch — keystroke-rate + // ESLint / mypy / redhat.yaml chatter no longer wakes us up. context.subscriptions.push( - vscode.languages.onDidChangeDiagnostics(() => this.refresh()), + vscode.languages.onDidChangeDiagnostics((e) => { + if (this.batchTouchesUs(e.uris)) { + this.refresh(); + } + }), ); // Seed the context key the title-bar menu reads to highlight the // active group mode. Must run after the view is registered to @@ -146,10 +166,49 @@ export class FindingsTreeProvider implements vscode.TreeDataProvider { } refresh(): void { + // Drop the memo so the next read sees the fresh diagnostics. + this.cachedFindings = null; this._onDidChangeTreeData.fire(); this.updateBadge(); } + /** + * Cached accessor used everywhere a refresh path needs the current + * findings. Walks the workspace diagnostic store once per refresh + * instead of once per consumer, and rebuilds the "URIs we had + * findings for" set so the next batch-skip check has fresh data. + */ + private findings(): Finding[] { + if (this.cachedFindings === null) { + this.cachedFindings = collectFindings(); + this.lastFindingUris = new Set( + this.cachedFindings.map((f) => f.uri.toString()), + ); + } + return this.cachedFindings; + } + + /** + * Returns true if any of the changed URIs in this batch either + * carries a pipeline-check diagnostic right now (publish or update) + * or carried one at the last refresh (clear). The second branch is + * what stops a stale leaf from outliving a cleared file. + */ + private batchTouchesUs(uris: readonly vscode.Uri[]): boolean { + for (const uri of uris) { + if (this.lastFindingUris.has(uri.toString())) { + return true; + } + const diags = vscode.languages.getDiagnostics(uri); + for (const d of diags) { + if (d.source === DIAGNOSTIC_SOURCE) { + return true; + } + } + } + return false; + } + getTreeItem(node: TreeNode): vscode.TreeItem { if (node.kind === "group") { const item = new vscode.TreeItem( @@ -189,7 +248,7 @@ export class FindingsTreeProvider implements vscode.TreeDataProvider { ); item.iconPath = SEVERITY_ICON[f.severity]; item.description = composeLeafDescription(f, this.groupMode); - item.tooltip = new vscode.MarkdownString(f.diagnostic.message); + item.tooltip = composeLeafTooltip(f); item.command = { command: "vscode.open", title: "Reveal finding", @@ -207,7 +266,7 @@ export class FindingsTreeProvider implements vscode.TreeDataProvider { } private buildRoot(): TreeNode[] { - const all = collectFindings(); + const all = this.findings(); if (all.length === 0) { return []; } @@ -225,7 +284,7 @@ export class FindingsTreeProvider implements vscode.TreeDataProvider { if (!this.treeView) { return; } - const total = collectFindings().length; + const total = this.findings().length; this.treeView.badge = total === 0 ? undefined @@ -246,6 +305,23 @@ export class FindingsTreeProvider implements vscode.TreeDataProvider { // // The ``file.yml:23`` form mirrors what compilers emit and what // VS Code's status bar uses — familiar at a glance. +function composeLeafTooltip(f: Finding): vscode.MarkdownString { + // The server composes the message as "title\n\ndescription\n\nFix: ...". + // Markdown renders the paragraph rhythm; we append a docs link + // below it when the server published one. The MarkdownString must + // be ``isTrusted = true`` for the link to be clickable inside a + // TreeItem tooltip. + const md = new vscode.MarkdownString(f.diagnostic.message); + if (f.docsUrl) { + md.appendMarkdown( + `\n\n[$(book) ${f.ruleId || "Rule"} documentation](${f.docsUrl})`, + ); + } + md.isTrusted = true; + md.supportThemeIcons = true; + return md; +} + function composeLeafDescription(f: Finding, mode: GroupMode): string { const line = f.diagnostic.range.start.line + 1; const fileRef = `${basenameFromUri(f.uri)}:${line}`; @@ -278,12 +354,31 @@ function collectFindings(): Finding[] { diagnostic: diag, severity: readSeverity(diag), ruleId: readRuleId(diag), + docsUrl: readDocsUrl(diag), }); } } return out; } +function readDocsUrl(diag: vscode.Diagnostic): string | undefined { + // ``Diagnostic.code.target`` is the rule's documentation URL when + // ``codeDescription.href`` is set on the server side. We surface it + // as a "Read more" link in the leaf tooltip so the editor closes + // the loop on the marketplace copy that promises "hover descriptions + // with --explain prose". Anything not a URL-shaped object falls + // through and the link is just omitted from the tooltip. + if ( + diag.code && + typeof diag.code === "object" && + "target" in diag.code && + diag.code.target + ) { + return diag.code.target.toString(); + } + return undefined; +} + function readSeverity(diag: vscode.Diagnostic): SeverityName { // diagnostics.py stuffs the upstream severity NAME into // ``Diagnostic.data["severity"]``. vscode-languageclient passes @@ -354,17 +449,23 @@ function groupBySeverity(findings: readonly Finding[]): GroupNode[] { } function groupByFile(findings: readonly Finding[]): GroupNode[] { - const buckets = new Map(); + // Key the bucket on the stringified URI so a Map keeps stable lookup + // (vscode.Uri values from different findings can be distinct objects + // even when they point at the same file), but also store the original + // Uri so we don't have to parse it back out for the group node. + const buckets = new Map(); for (const f of findings) { const key = f.uri.toString(); - const arr = buckets.get(key) ?? []; - arr.push(f); - buckets.set(key, arr); + const bucket = buckets.get(key); + if (bucket) { + bucket.items.push(f); + } else { + buckets.set(key, { uri: f.uri, items: [f] }); + } } return [...buckets] .sort((a, b) => a[0].localeCompare(b[0])) - .map(([key, items]): GroupNode => { - const uri = vscode.Uri.parse(key); + .map(([, { uri, items }]): GroupNode => { items.sort(compareByLocation); // Parent dir moves to the tooltip so the description column // stays count-only across all three group modes — a uniform @@ -411,8 +512,11 @@ function groupByRule(findings: readonly Finding[]): GroupNode[] { } function compareByLocation(a: Finding, b: Finding): number { - const lhs = a.uri.toString(); - const rhs = b.uri.toString(); + // Sort on fsPath instead of the full Uri string so cross-scheme + // entries (file:// vs vscode-untitled:// etc.) don't bunch + // pathologically at one end. Within the same file, line order wins. + const lhs = a.uri.fsPath; + const rhs = b.uri.fsPath; if (lhs !== rhs) { return lhs.localeCompare(rhs); } diff --git a/src/statusBar.test.ts b/src/statusBar.test.ts new file mode 100644 index 0000000..b8d5a48 --- /dev/null +++ b/src/statusBar.test.ts @@ -0,0 +1,145 @@ +import { describe, it, expect, vi } from "vitest"; + +// statusBar.ts imports `vscode` for the runtime wiring; the pure +// helpers (formatStatusBarText, formatStatusBarTooltip, +// countDiagnostics) don't touch it but the module-level import has to +// resolve. Tiny stub covers it. +vi.mock("vscode", () => ({ + StatusBarAlignment: { Left: 1, Right: 2 }, + window: {}, + languages: {}, +})); + +import { + countDiagnostics, + formatStatusBarText, + formatStatusBarTooltip, +} from "./statusBar"; + +// Helpers +const make = (sev?: string) => ({ + source: "pipeline-check", + message: "x", + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, + }, + severity: 0, + data: sev ? { severity: sev } : undefined, +}); + +describe("formatStatusBarText", () => { + it("returns 'clean' when there are no findings", () => { + expect( + formatStatusBarText({ CRITICAL: 0, HIGH: 0, MEDIUM: 0, LOW: 0, INFO: 0 }), + ).toBe("$(shield) clean"); + }); + + it("leads with critical count when present", () => { + expect( + formatStatusBarText({ CRITICAL: 3, HIGH: 0, MEDIUM: 0, LOW: 0, INFO: 0 }), + ).toBe("$(shield) 3C"); + }); + + it("pairs critical with high when both present", () => { + expect( + formatStatusBarText({ CRITICAL: 3, HIGH: 1, MEDIUM: 9, LOW: 0, INFO: 0 }), + ).toBe("$(shield) 3C 1H"); + }); + + it("shows high alone when no critical", () => { + expect( + formatStatusBarText({ CRITICAL: 0, HIGH: 4, MEDIUM: 0, LOW: 0, INFO: 0 }), + ).toBe("$(shield) 4H"); + }); + + it("pairs high with medium when no critical", () => { + expect( + formatStatusBarText({ CRITICAL: 0, HIGH: 4, MEDIUM: 2, LOW: 9, INFO: 9 }), + ).toBe("$(shield) 4H 2M"); + }); + + it("collapses to a total when only medium/low/info present", () => { + expect( + formatStatusBarText({ CRITICAL: 0, HIGH: 0, MEDIUM: 2, LOW: 3, INFO: 1 }), + ).toBe("$(shield) 6"); + }); +}); + +describe("formatStatusBarTooltip", () => { + it("reports 'no findings' on a clean workspace", () => { + expect( + formatStatusBarTooltip({ CRITICAL: 0, HIGH: 0, MEDIUM: 0, LOW: 0, INFO: 0 }), + ).toBe("Pipeline-Check: no findings"); + }); + + it("breaks down every nonzero bucket", () => { + const tip = formatStatusBarTooltip({ + CRITICAL: 1, + HIGH: 2, + MEDIUM: 0, + LOW: 3, + INFO: 0, + }); + expect(tip).toContain("Pipeline-Check: 6 findings"); + expect(tip).toContain("CRITICAL: 1"); + expect(tip).toContain("HIGH: 2"); + expect(tip).toContain("LOW: 3"); + expect(tip).not.toContain("MEDIUM"); + expect(tip).not.toContain("INFO"); + expect(tip).toContain("Click to open the Findings panel."); + }); + + it("singular form for one finding", () => { + expect( + formatStatusBarTooltip({ CRITICAL: 0, HIGH: 1, MEDIUM: 0, LOW: 0, INFO: 0 }), + ).toContain("1 finding"); + }); +}); + +describe("countDiagnostics", () => { + it("ignores diagnostics whose source is not pipeline-check", () => { + const iter: Array<[unknown, unknown[]]> = [ + ["uri", [{ ...make("HIGH"), source: "eslint" }]], + ]; + expect( + countDiagnostics( + iter as unknown as Iterable, + ), + ).toEqual({ CRITICAL: 0, HIGH: 0, MEDIUM: 0, LOW: 0, INFO: 0 }); + }); + + it("tallies pipeline-check diagnostics by severity", () => { + const iter: Array<[unknown, unknown[]]> = [ + ["a", [make("CRITICAL"), make("HIGH"), make("HIGH")]], + ["b", [make("LOW")]], + ]; + expect( + countDiagnostics( + iter as unknown as Iterable, + ), + ).toEqual({ CRITICAL: 1, HIGH: 2, MEDIUM: 0, LOW: 1, INFO: 0 }); + }); + + it("falls back to INFO for missing/unknown severity", () => { + const iter: Array<[unknown, unknown[]]> = [ + ["a", [make(), make("BOGUS")]], + ]; + expect( + countDiagnostics( + iter as unknown as Iterable, + ), + ).toEqual({ CRITICAL: 0, HIGH: 0, MEDIUM: 0, LOW: 0, INFO: 2 }); + }); + + it("normalises lowercase severity names", () => { + const iter: Array<[unknown, unknown[]]> = [ + ["a", [make("high"), make("critical")]], + ]; + expect( + countDiagnostics( + iter as unknown as Iterable, + ), + ).toEqual({ CRITICAL: 1, HIGH: 1, MEDIUM: 0, LOW: 0, INFO: 0 }); + }); +}); diff --git a/src/statusBar.ts b/src/statusBar.ts new file mode 100644 index 0000000..f818613 --- /dev/null +++ b/src/statusBar.ts @@ -0,0 +1,159 @@ +// Status bar item that surfaces pipeline-check finding counts at the +// bottom-left of the window, so users get glanceable feedback without +// opening the Findings panel. Clicking the item reveals the panel. +// +// The visible-counts logic lives in `formatStatusBarText` as a pure +// function so the tests can pin the copy without booting VS Code. + +import * as vscode from "vscode"; + +const DIAGNOSTIC_SOURCE = "pipeline-check"; + +// What we read off ``Diagnostic.data.severity``. Mirrors the +// SEVERITY_ORDER in findingsView.ts; kept local here so the status +// bar can ship without a circular import. +type SeverityName = "CRITICAL" | "HIGH" | "MEDIUM" | "LOW" | "INFO"; + +export interface SeverityCounts { + readonly CRITICAL: number; + readonly HIGH: number; + readonly MEDIUM: number; + readonly LOW: number; + readonly INFO: number; +} + +const ZERO_COUNTS: SeverityCounts = { + CRITICAL: 0, + HIGH: 0, + MEDIUM: 0, + LOW: 0, + INFO: 0, +}; + +/** + * Render the status bar text from a per-severity tally. The output + * leans on the highest two severities present so the bar stays short + * (status-bar items steal horizontal space from everything to their + * right). Specifically: + * + * - no findings → "$(shield) clean" + * - critical present → "$(shield) 3C 1H" (or just "3C") + * - high but no crit → "$(shield) 4H 2M" + * - medium and below → "$(shield) 5" (total count, no letter) + */ +export function formatStatusBarText(c: SeverityCounts): string { + if (c.CRITICAL === 0 && c.HIGH === 0 && c.MEDIUM === 0 && c.LOW === 0 && c.INFO === 0) { + return "$(shield) clean"; + } + const parts: string[] = []; + if (c.CRITICAL > 0) { + parts.push(`${c.CRITICAL}C`); + if (c.HIGH > 0) { + parts.push(`${c.HIGH}H`); + } + } else if (c.HIGH > 0) { + parts.push(`${c.HIGH}H`); + if (c.MEDIUM > 0) { + parts.push(`${c.MEDIUM}M`); + } + } else { + // No critical or high — collapse to a single total so the bar + // doesn't shout for noise. + const total = c.MEDIUM + c.LOW + c.INFO; + parts.push(String(total)); + } + return `$(shield) ${parts.join(" ")}`; +} + +/** + * Render the tooltip — a longer breakdown shown on hover. Always + * lists every nonzero bucket, so the abbreviated bar text never + * hides information; just makes it less prominent. + */ +export function formatStatusBarTooltip(c: SeverityCounts): string { + const total = c.CRITICAL + c.HIGH + c.MEDIUM + c.LOW + c.INFO; + if (total === 0) { + return "Pipeline-Check: no findings"; + } + const lines = [ + `Pipeline-Check: ${total} finding${total === 1 ? "" : "s"}`, + ]; + for (const sev of ["CRITICAL", "HIGH", "MEDIUM", "LOW", "INFO"] as const) { + if (c[sev] > 0) { + lines.push(` ${sev}: ${c[sev]}`); + } + } + lines.push("Click to open the Findings panel."); + return lines.join("\n"); +} + +/** + * Tally pipeline-check diagnostics across the workspace by severity. + * Falls back to INFO when ``data.severity`` is missing or unknown + * (same rule the Findings tree uses). + */ +export function countDiagnostics( + iter: Iterable, +): SeverityCounts { + const counts = { ...ZERO_COUNTS } as { -readonly [K in SeverityName]: number }; + for (const [, diags] of iter) { + for (const d of diags) { + if (d.source !== DIAGNOSTIC_SOURCE) continue; + const name = readSeverity(d); + counts[name] += 1; + } + } + return counts; +} + +function readSeverity(diag: vscode.Diagnostic): SeverityName { + const data = (diag as vscode.Diagnostic & { + data?: { severity?: string }; + }).data; + const name = (data?.severity ?? "").toUpperCase(); + switch (name) { + case "CRITICAL": + case "HIGH": + case "MEDIUM": + case "LOW": + case "INFO": + return name; + default: + return "INFO"; + } +} + +/** + * Wire up the status bar item. Returns a Disposable the caller pushes + * onto the extension's subscriptions so the item is removed on + * deactivate. The item itself rewires on every diagnostic change and + * navigates to the Findings panel on click. + */ +export function registerStatusBar( + context: vscode.ExtensionContext, +): vscode.StatusBarItem { + const item = vscode.window.createStatusBarItem( + vscode.StatusBarAlignment.Left, + 100, + ); + // Click reveals the Findings panel; same action as the activity-bar + // icon, just from a different surface. + item.command = "pipelineCheck.findings.focus"; + item.name = "Pipeline-Check"; + + const update = () => { + const counts = countDiagnostics(vscode.languages.getDiagnostics()); + item.text = formatStatusBarText(counts); + item.tooltip = formatStatusBarTooltip(counts); + item.show(); + }; + + // Seed and subscribe. + update(); + context.subscriptions.push( + item, + vscode.languages.onDidChangeDiagnostics(update), + ); + + return item; +}