diff --git a/.env.example b/.env.example index 9536558737..c9b3042c9f 100644 --- a/.env.example +++ b/.env.example @@ -68,28 +68,28 @@ GITTENSORY_REVIEW_ENRICHMENT=false # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber -# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,i18n -# commitLint +# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y +# i18n,commitLint # # Profile defaults: # fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency # hardcodedUrl,actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild # testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker -# debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,i18n +# debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y,i18n # balanced (default): dependency,dependencyDiff,lockfileDrift,secret,license,installScript # heavyDependency,hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight # typosquat,commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication # churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch # commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology # todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting -# errorSwallow,unsafeAny,i18n,commitLint +# errorSwallow,unsafeAny,a11y,i18n,commitLint # deep: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency # hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber -# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,i18n -# commitLint +# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y +# i18n,commitLint # END GENERATED REES ANALYZERS # Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep diff --git a/apps/gittensory-ui/src/lib/rees-analyzers.ts b/apps/gittensory-ui/src/lib/rees-analyzers.ts index 9620f8fdf9..960b4ab37f 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -1071,6 +1071,29 @@ export const REES_ANALYZERS = [ "Structural regex only (no type-checker). String literals and comment lines are skipped; findings are capped.", }, }, + { + name: "a11y", + title: "Accessibility regressions", + category: "quality", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files"], + limits: { + maxFindings: 25, + maxLineChars: 2000, + }, + docs: { + summary: "Flags common accessibility regressions in newly added JSX/HTML markup lines.", + looksAt: + "Added lines in changed non-test .jsx/.tsx/.html/.vue files for missing img alt text, click-only handlers, unlabeled controls, and positive tabindex.", + reports: + "File, line, and rule: img-alt, click-events-have-key-events, label-control, or positive-tabindex.", + network: "Pure local analyzer. No external network call.", + notes: + "Structural markup heuristics only — buttons/links and controls with aria-label/id associations are not flagged.", + }, + }, { name: "i18n", title: "i18n regressions", diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index e06196056c..eb783794b9 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -1213,6 +1213,32 @@ "notes": "Structural regex only (no type-checker). String literals and comment lines are skipped; findings are capped." } }, + { + "name": "a11y", + "title": "Accessibility regressions", + "category": "quality", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files" + ], + "limits": { + "maxFindings": 25, + "maxLineChars": 2000 + }, + "docs": { + "summary": "Flags common accessibility regressions in newly added JSX/HTML markup lines.", + "looksAt": "Added lines in changed non-test .jsx/.tsx/.html/.vue files for missing img alt text, click-only handlers, unlabeled controls, and positive tabindex.", + "reports": "File, line, and rule: img-alt, click-events-have-key-events, label-control, or positive-tabindex.", + "network": "Pure local analyzer. No external network call.", + "notes": "Structural markup heuristics only — buttons/links and controls with aria-label/id associations are not flagged." + } + }, { "name": "i18n", "title": "i18n regressions", diff --git a/review-enrichment/src/analyzers/a11y-regression.ts b/review-enrichment/src/analyzers/a11y-regression.ts new file mode 100644 index 0000000000..156a5fdf2e --- /dev/null +++ b/review-enrichment/src/analyzers/a11y-regression.ts @@ -0,0 +1,124 @@ +// Accessibility regression analyzer (#2026). Flags common a11y regressions in newly added JSX/HTML markup: +// images without alt text, click-only handlers on non-interactive elements, unlabeled form controls, and +// positive tabindex values. Pure compute over added lines in .jsx/.tsx/.html/.vue files, no network. +import type { A11yFinding, EnrichRequest } from "../types.js"; +import { isTestPath } from "./test-ratio.js"; + +const MAX_FINDINGS = 25; +const MAX_LINE_CHARS = 2000; + +const MARKUP_PATH_RE = /\.(?:tsx|jsx|html|vue)$/i; + +const POSITIVE_TABINDEX_RE = + /\btabIndex\s*=\s*\{\s*([1-9]\d*)\s*\}|\btabindex\s*=\s*["']([1-9]\d*)["']/i; +const IMG_TAG_RE = / MAX_LINE_CHARS) return null; + + if (POSITIVE_TABINDEX_RE.test(line)) return "positive-tabindex"; + + if (IMG_TAG_RE.test(line) && !ALT_ATTR_RE.test(line)) return "img-alt"; + + if (ON_CLICK_RE.test(line)) { + const hasKeyboard = KEYBOARD_HANDLER_RE.test(line); + const isInteractive = + INTERACTIVE_TAG_RE.test(line) || INTERACTIVE_ROLE_RE.test(line); + if ( + !hasKeyboard && + !isInteractive && + NON_INTERACTIVE_CLICK_TARGET_RE.test(line) + ) { + return "click-events-have-key-events"; + } + } + + if (FORM_CONTROL_RE.test(line) && !LABEL_ASSOC_RE.test(line)) { + return "label-control"; + } + + return null; +} + +type ScanLimits = { + maxFindings?: number; + signal?: AbortSignal; +}; + +/** Scan one file patch's added lines for a11y regressions, line-cited via hunk headers. Pure. */ +export function scanPatchForA11yRegression( + path: string, + patch: string, + limits: ScanLimits = {}, +): A11yFinding[] { + const maxFindings = limits.maxFindings ?? MAX_FINDINGS; + if (maxFindings <= 0 || !isMarkupPath(path)) return []; + + const findings: A11yFinding[] = []; + let newLine = 0; + let inHunk = false; + + for (const line of patch.split("\n")) { + if (limits.signal?.aborted) throw new Error("analyzer_aborted"); + const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); + if (hunk) { + newLine = Number(hunk[1]); + inHunk = true; + continue; + } + if (!inHunk) continue; + if (line.startsWith("+")) { + const body = line.slice(1); + const rule = detectA11yRegression(body); + if (rule) { + findings.push({ file: path, line: newLine, rule }); + if (findings.length >= maxFindings) return findings; + } + newLine++; + } else if (!line.startsWith("-") && !line.startsWith("\\")) { + newLine++; + } + } + + return findings; +} + +/** Analyzer entrypoint: scan markup files for accessibility regressions. */ +export async function scanA11yRegression( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + const findings: A11yFinding[] = []; + for (const file of req.files ?? []) { + if (signal?.aborted) throw new Error("analyzer_aborted"); + if (!file.patch) continue; + for (const finding of scanPatchForA11yRegression(file.path, file.patch, { + maxFindings: MAX_FINDINGS - findings.length, + signal, + })) { + findings.push(finding); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + return findings; +} diff --git a/review-enrichment/src/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index 2f64a03cb5..24bb153e14 100644 --- a/review-enrichment/src/analyzers/registry.ts +++ b/review-enrichment/src/analyzers/registry.ts @@ -37,6 +37,7 @@ import { scanI18nRegression } from "./i18n-regression.js"; import { scanErrorSwallow } from "./error-swallow.js"; import { scanFloatingPromise } from "./floating-promise.js"; import { scanSizeSmell } from "./size-smell.js"; +import { scanA11yRegression } from "./a11y-regression.js"; import { scanCommitLint } from "./commit-lint.js"; import { scanUnsafeAny } from "./unsafe-any.js"; import { scanTerminology } from "./terminology.js"; @@ -1167,6 +1168,37 @@ export const ANALYZER_DESCRIPTORS = [ }, run: (req, { signal }) => scanUnsafeAny(req, signal), }), + descriptor({ + name: "a11y", + title: "Accessibility regressions", + category: "quality", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxLineChars: 2000 }, + docs: { + summary: + "Flags common accessibility regressions in newly added JSX/HTML markup lines.", + looksAt: + "Added lines in changed non-test .jsx/.tsx/.html/.vue files for missing img alt text, click-only handlers, unlabeled controls, and positive tabindex.", + reports: + "File, line, and rule: img-alt, click-events-have-key-events, label-control, or positive-tabindex.", + network: "Pure local analyzer. No external network call.", + notes: + "Structural markup heuristics only — buttons/links and controls with aria-label/id associations are not flagged.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Accessibility regressions (added markup)"]; + for (const item of findings) { + lines.push( + `- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${helpers.safeCodeSpan(item.rule)}`, + ); + } + return lines; + }, + run: (req, { signal }) => scanA11yRegression(req, signal), + }), descriptor({ name: "i18n", title: "i18n regressions", diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 5039c10652..63b97b26b5 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -491,6 +491,7 @@ export function renderBrief( lines.push(...renderDescriptorSection("deepNesting", findings.deepNesting)); lines.push(...renderDescriptorSection("errorSwallow", findings.errorSwallow)); lines.push(...renderDescriptorSection("unsafeAny", findings.unsafeAny)); + lines.push(...renderDescriptorSection("a11y", findings.a11y)); lines.push(...renderDescriptorSection("i18n", findings.i18n)); lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl)); lines.push(...renderDescriptorSection("commitLint", findings.commitLint)); diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index c2183577be..1fb604edf1 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -536,6 +536,18 @@ export interface UnsafeAnyFinding { kind: "annotation" | "cast" | "assertion"; } +/** A common accessibility regression in newly added markup (#2026, part of #1499). + * Reports file, line, and rule only — never string content. */ +export interface A11yFinding { + file: string; + line: number; + rule: + | "img-alt" + | "click-events-have-key-events" + | "label-control" + | "positive-tabindex"; +} + /** An absolute HTTP(S) URL or raw IP:port endpoint hardcoded in non-test, non-config source (#2027, part of #1499). * Reports location, kind, and a redacted/truncated host — never full paths or query strings. */ export interface HardcodedUrlFinding { @@ -597,6 +609,7 @@ export interface BriefFindings { deepNesting?: DeepNestingFinding[]; errorSwallow?: ErrorSwallowFinding[]; unsafeAny?: UnsafeAnyFinding[]; + a11y?: A11yFinding[]; i18n?: I18nFinding[]; hardcodedUrl?: HardcodedUrlFinding[]; commitLint?: CommitLintFinding[]; diff --git a/review-enrichment/test/a11y-regression.test.ts b/review-enrichment/test/a11y-regression.test.ts new file mode 100644 index 0000000000..95dc52ea3f --- /dev/null +++ b/review-enrichment/test/a11y-regression.test.ts @@ -0,0 +1,89 @@ +// Units for the a11y regression analyzer (#2026). Own file so concurrent analyzer PRs don't collide. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + detectA11yRegression, + scanA11yRegression, + scanPatchForA11yRegression, +} from "../dist/analyzers/a11y-regression.js"; +import { renderBrief } from "../dist/render.js"; + +const patchOf = (lines: string[]) => + `@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`; + +test("detectA11yRegression: flags each rule on representative markup", () => { + assert.equal(detectA11yRegression(''), "img-alt"); + assert.equal( + detectA11yRegression('
open()}>Open
'), + "click-events-have-key-events", + ); + assert.equal(detectA11yRegression(''), "label-control"); + assert.equal(detectA11yRegression('
Skip
'), "positive-tabindex"); +}); + +test("detectA11yRegression: does not flag compliant markup", () => { + assert.equal(detectA11yRegression('Logo'), null); + assert.equal(detectA11yRegression(''), null); + assert.equal( + detectA11yRegression('
open()}>Open
'), + null, + ); + assert.equal( + detectA11yRegression('
open()} onKeyDown={handleKey}>Open
'), + null, + ); + assert.equal(detectA11yRegression(''), null); + assert.equal(detectA11yRegression(""), null); + assert.equal(detectA11yRegression('
Focusable
'), null); +}); + +test("scanPatchForA11yRegression: flags added lines with correct locations", () => { + const findings = scanPatchForA11yRegression( + "src/Widget.tsx", + patchOf([ + "export function Widget() {", + ' return ;', + "}", + ]), + ); + assert.deepEqual(findings, [ + { file: "src/Widget.tsx", line: 2, rule: "img-alt" }, + ]); +}); + +test("scanPatchForA11yRegression: skips test files and non-markup paths", () => { + assert.deepEqual( + scanPatchForA11yRegression("src/Widget.test.tsx", patchOf([''])), + [], + ); + assert.deepEqual( + scanPatchForA11yRegression("src/worker.ts", patchOf([''])), + [], + ); +}); + +test("scanPatchForA11yRegression: respects the findings cap", () => { + const lines = Array.from({ length: 10 }, () => ''); + assert.equal( + scanPatchForA11yRegression("src/a.tsx", patchOf(lines), { maxFindings: 2 }).length, + 2, + ); +}); + +test("scanA11yRegression: aggregates across files and renders a public-safe brief", async () => { + const findings = await scanA11yRegression({ + repoFullName: "owner/repo", + prNumber: 1, + files: [ + { path: "src/a.tsx", patch: patchOf(['
Skip
']) }, + { path: "src/b.html", patch: patchOf(['']) }, + ], + }); + assert.equal(findings.length, 2); + assert.equal(findings[0]?.rule, "positive-tabindex"); + assert.equal(findings[1]?.rule, "label-control"); + const { promptSection } = renderBrief({ a11y: findings }); + assert.match(promptSection, /Accessibility regressions/); + assert.match(promptSection, /positive-tabindex/); + assert.doesNotMatch(promptSection, /Skip/); +}); diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index 9db05f6470..133ab89d9a 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -53,6 +53,7 @@ const EXPECTED_ANALYZERS = [ "deepNesting", "errorSwallow", "unsafeAny", + "a11y", "i18n", "commitLint", ]; diff --git a/src/review/enrichment-analyzer-names.ts b/src/review/enrichment-analyzer-names.ts index 7b22b22aff..8c7113c29e 100644 --- a/src/review/enrichment-analyzer-names.ts +++ b/src/review/enrichment-analyzer-names.ts @@ -47,6 +47,7 @@ export const REES_ANALYZER_NAMES = [ "deepNesting", "errorSwallow", "unsafeAny", + "a11y", "i18n", "commitLint", ] as const;