diff --git a/.changeset/file-context-labels.md b/.changeset/file-context-labels.md new file mode 100644 index 000000000..fe288289d --- /dev/null +++ b/.changeset/file-context-labels.md @@ -0,0 +1,5 @@ +--- +"react-doctor": patch +--- + +Diagnostics in test, spec, fixture, and Storybook files are now labeled with their file context. The terminal report and the per-rule text dumps tag those sites as `(test file)` / `(story file)` so a finding in a spec doesn't read as a production problem, and each diagnostic in the JSON report carries an optional `fileContext` field (`"test"` / `"story"`; omitted for production files). The classification reuses the same path heuristics that already drive test-noise auto-suppression, so the label and the suppression can never disagree. diff --git a/packages/core/src/build-diagnostic-pipeline.ts b/packages/core/src/build-diagnostic-pipeline.ts index df6b867c2..019786dba 100644 --- a/packages/core/src/build-diagnostic-pipeline.ts +++ b/packages/core/src/build-diagnostic-pipeline.ts @@ -1,5 +1,10 @@ import reactDoctorPlugin from "oxlint-plugin-react-doctor"; -import type { Diagnostic, ReactDoctorConfig, RuleSeverityOverride } from "./types/index.js"; +import type { + Diagnostic, + DiagnosticFileContext, + ReactDoctorConfig, + RuleSeverityOverride, +} from "./types/index.js"; import { compileIgnoreOverrides, isDiagnosticIgnoredByOverrides, @@ -9,7 +14,7 @@ import { buildRuleSeverityControls } from "./build-rule-severity-controls.js"; import { evaluateSuppression } from "./evaluate-suppression.js"; import { getDiagnosticRuleIdentity } from "./get-diagnostic-rule-identity.js"; import { compileIgnoredFilePatterns, isFileIgnoredByPatterns } from "./is-ignored-file.js"; -import { isTestFilePath } from "./is-test-file.js"; +import { classifyFileContext } from "./classify-file-context.js"; import { resolveRuleSeverityOverride } from "./resolve-rule-severity-override.js"; import { isSameRuleKey } from "./rule-key-aliases.js"; import { APP_ONLY_RULE_KEYS } from "./constants.js"; @@ -60,6 +65,8 @@ const collectStringSet = (values: unknown): ReadonlySet => { * 5. `rn-no-raw-text` suppression via configured `textComponents` and * `rawTextWrapperComponents` (config-driven JSX enclosure checks) * 6. inline suppressions (`// react-doctor-disable-next-line ...`) + * 7. file-context stamping (`fileContext: "test" | "story"` on + * survivors in non-production files, so renderers can label them) * * Returns `null` when the diagnostic is dropped, the (possibly * severity-restamped) diagnostic otherwise. @@ -87,7 +94,7 @@ export const buildDiagnosticPipeline = ( const hasTextComponents = textComponentNames.size > 0; const hasRawTextWrappers = rawTextWrapperComponentNames.size > 0; const fileLinesCache = new Map(); - const testFileCache = new Map(); + const fileContextCache = new Map(); const libraryFileCache = new Map(); // App-only rules (`static-components`, `no-render-prop-children`) describe @@ -113,11 +120,11 @@ export const buildDiagnosticPipeline = ( return lines; }; - const isTest = (filePath: string): boolean => { - let cached = testFileCache.get(filePath); + const getFileContext = (filePath: string): DiagnosticFileContext => { + let cached = fileContextCache.get(filePath); if (cached === undefined) { - cached = isTestFilePath(filePath); - testFileCache.set(filePath, cached); + cached = classifyFileContext(filePath); + fileContextCache.set(filePath, cached); } return cached; }; @@ -127,7 +134,7 @@ export const buildDiagnosticPipeline = ( const rule = reactDoctorPlugin.rules[diagnostic.rule]; if (!rule?.tags?.includes("test-noise")) return false; if (rule.tags.includes("migration-hint")) return false; - return isTest(diagnostic.filePath); + return getFileContext(diagnostic.filePath) !== "production"; }; const isRuleIgnored = (ruleIdentifier: string): boolean => { @@ -235,6 +242,11 @@ export const buildDiagnosticPipeline = ( } } + const fileContext = getFileContext(current.filePath); + if (fileContext !== "production") { + current = { ...current, fileContext }; + } + return current; }, }; diff --git a/packages/core/src/calculate-score.ts b/packages/core/src/calculate-score.ts index a126ede9c..963c16f85 100644 --- a/packages/core/src/calculate-score.ts +++ b/packages/core/src/calculate-score.ts @@ -24,8 +24,13 @@ const ScoreApiResponseSchema = Schema.Struct({ const parseScoreResult = (value: unknown): ScoreResult | null => Option.getOrNull(Schema.decodeUnknownOption(ScoreApiResponseSchema)(value)); -const stripFilePaths = (diagnostics: Diagnostic[]): Omit[] => - diagnostics.map(({ filePath: _filePath, ...rest }) => rest); +// `filePath` never leaves the machine, and `fileContext` is derived +// from it — neither rides the Score API payload, which keeps the +// request shape identical to what the server has always received. +const stripLocalFileFields = ( + diagnostics: Diagnostic[], +): Omit[] => + diagnostics.map(({ filePath: _filePath, fileContext: _fileContext, ...rest }) => rest); const isAbortError = (error: unknown): boolean => error instanceof Error && (error.name === "AbortError" || error.name === "TimeoutError"); @@ -66,7 +71,7 @@ export const calculateScore = async ( try { const requestBody = JSON.stringify({ - diagnostics: stripFilePaths(diagnostics), + diagnostics: stripLocalFileFields(diagnostics), ...(options.metadata?.repo ? { repo: options.metadata.repo } : {}), ...(options.metadata?.sha ? { sha: options.metadata.sha } : {}), ...(options.metadata?.framework ? { framework: options.metadata.framework } : {}), diff --git a/packages/core/src/classify-file-context.ts b/packages/core/src/classify-file-context.ts new file mode 100644 index 000000000..7cdb9e5b5 --- /dev/null +++ b/packages/core/src/classify-file-context.ts @@ -0,0 +1,83 @@ +import type { DiagnosticFileContext } from "./types/index.js"; + +// HACK: most rules in react-doctor's curated set encode "you wouldn't +// want this in production code" — but tests intentionally exercise +// bad patterns to lock in regression coverage (an array-index key in a +// test fixture, a giant fixture component, a `forwardRef` to verify +// ref forwarding). Surface-level path matching is enough to catch the +// near-universal test conventions: `.test.*` / `.spec.*` suffix, or +// living under a `__tests__` / `tests` / `test` directory. +// +// We use combined regexes against the forward-slash relative path so +// the match is allocation-free per diagnostic. The story and test +// suffix patterns share one extension grammar so they can never drift +// apart — `isTestFilePath` is derived from this classifier, making +// "the label and the suppression always agree" structural. +const SCRIPT_EXTENSION_FRAGMENT = "[cm]?[jt]sx?"; +const STORY_FILE_SUFFIX_PATTERN = new RegExp( + `\\.(?:stories|story)\\.(?:${SCRIPT_EXTENSION_FRAGMENT})$`, +); +const TEST_FILE_SUFFIX_PATTERN = new RegExp( + `\\.(?:test|spec|fixture|fixtures)\\.(?:${SCRIPT_EXTENSION_FRAGMENT})$`, +); +const TEST_FILE_DIRECTORY_PATTERN = + /(?:^|\/)(?:__tests__|__test__|tests|test|__mocks__|cypress|e2e|playwright)\//; + +// "Source root" markers — once a path contains `/src/`, `/app/`, +// `/lib/`, `/pages/`, etc., everything BELOW that is production code +// regardless of how the project is laid out above. Critical for test +// fixture projects (`tests/fixtures//src/...`) so the FIXTURE +// source files don't get auto-suppressed just because the outer wrap +// happens to have `/tests/` or `/fixtures/` in the path. +// +// We only strip when a `/fixtures/` (or `/__fixtures__/`) segment is +// present, because that's the unambiguous signal of "fixture project — +// the inner source root is the real production code under lint". +// For regular layouts like `tests/app/setup.ts` or `e2e/components/ +// helpers.ts`, the `app` / `components` segment is just a sub-folder +// of the test directory, NOT a real source root, so stripping would +// incorrectly drop the `tests/` / `e2e/` prefix and the file would +// no longer be detected as a test. +const FIXTURE_PROJECT_PATTERN = /\/(?:fixtures|__fixtures__)\//; +const SOURCE_ROOT_PATTERN = + /\/(?:src|app|lib|components|pages|features|modules|packages|apps|frontend|client)\//g; + +const stripAboveSourceRoot = (relativePath: string): string => { + const fixtureMatch = FIXTURE_PROJECT_PATTERN.exec(relativePath); + if (fixtureMatch === null) return relativePath; + let lastIdx = -1; + for (const match of relativePath.matchAll(SOURCE_ROOT_PATTERN)) { + if (match.index !== undefined && match.index > lastIdx) lastIdx = match.index; + } + if (lastIdx >= 0) return relativePath.slice(lastIdx); + // No inner source-root marker — strip up through the fixture segment + // so the outer `tests/` / `e2e/` prefix can't re-trigger the test + // heuristic on fixture production files like + // `tests/fixtures/my-app/Component.tsx`. The fixture itself is the + // unit under test; its contents are production-shaped code. + return relativePath.slice(fixtureMatch.index + fixtureMatch[0].length - 1); +}; + +/** + * Classifies where a file sits relative to shipped code. A finding in a + * `.stories.tsx` or `.spec.ts` file never runs in front of users, so + * renderers label those sites instead of framing them as production + * impact (`rn-no-raw-text` in a spec doesn't say users crash). + * + * `"story"` is the `.stories.*` / `.story.*` suffix; `"test"` is the + * test/spec/fixture suffixes and test directories; `"production"` is + * the default. + */ +export const classifyFileContext = (relativePath: string): DiagnosticFileContext => { + if (relativePath.length === 0) return "production"; + const forwardSlashed = relativePath.replaceAll("\\", "/"); + // The SUFFIX checks (.stories/.test/.spec etc.) are on the FULL path — + // unambiguous regardless of context. + if (STORY_FILE_SUFFIX_PATTERN.test(forwardSlashed)) return "story"; + if (TEST_FILE_SUFFIX_PATTERN.test(forwardSlashed)) return "test"; + // The DIRECTORY check (`tests/`, `__tests__/`, `cypress/`, etc.) + // scopes to the source-root-below path so that fixture-project + // source files don't get falsely auto-suppressed. + const scoped = stripAboveSourceRoot(forwardSlashed); + return TEST_FILE_DIRECTORY_PATTERN.test(scoped) ? "test" : "production"; +}; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a87996da8..35c68bb07 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -40,6 +40,7 @@ export * from "./check-react-native-project.js"; export * from "./check-react-server-components-advisory.js"; export * from "./check-reduced-motion.js"; export * from "./check-supply-chain.js"; +export * from "./classify-file-context.js"; export * from "./collect-ignore-patterns.js"; export * from "./compute-diagnostic-delta.js"; export * from "./constants.js"; diff --git a/packages/core/src/is-test-file.ts b/packages/core/src/is-test-file.ts index 1a9d2a8d9..b6dd5cde7 100644 --- a/packages/core/src/is-test-file.ts +++ b/packages/core/src/is-test-file.ts @@ -1,62 +1,8 @@ -// HACK: most rules in react-doctor's curated set encode "you wouldn't -// want this in production code" — but tests intentionally exercise -// bad patterns to lock in regression coverage (an array-index key in a -// test fixture, a giant fixture component, a `forwardRef` to verify -// ref forwarding). Surface-level path matching is enough to catch the -// near-universal test conventions: `.test.*` / `.spec.*` suffix, or -// living under a `__tests__` / `tests` / `test` directory. -// -// We use a single combined regex against the forward-slash relative -// path so the match is allocation-free per diagnostic. -const TEST_FILE_DIRECTORY_PATTERN = - /(?:^|\/)(?:__tests__|__test__|tests|test|__mocks__|cypress|e2e|playwright)\//; -const TEST_FILE_SUFFIX_PATTERN = - /\.(?:test|spec|stories|story|fixture|fixtures)\.(?:[cm]?[jt]sx?)$/; +import { classifyFileContext } from "./classify-file-context.js"; -// "Source root" markers — once a path contains `/src/`, `/app/`, -// `/lib/`, `/pages/`, etc., everything BELOW that is production code -// regardless of how the project is laid out above. Critical for test -// fixture projects (`tests/fixtures//src/...`) so the FIXTURE -// source files don't get auto-suppressed just because the outer wrap -// happens to have `/tests/` or `/fixtures/` in the path. -// -// We only strip when a `/fixtures/` (or `/__fixtures__/`) segment is -// present, because that's the unambiguous signal of "fixture project — -// the inner source root is the real production code under lint". -// For regular layouts like `tests/app/setup.ts` or `e2e/components/ -// helpers.ts`, the `app` / `components` segment is just a sub-folder -// of the test directory, NOT a real source root, so stripping would -// incorrectly drop the `tests/` / `e2e/` prefix and the file would -// no longer be detected as a test. -const FIXTURE_PROJECT_PATTERN = /\/(?:fixtures|__fixtures__)\//; -const SOURCE_ROOT_PATTERN = - /\/(?:src|app|lib|components|pages|features|modules|packages|apps|frontend|client)\//g; - -const stripAboveSourceRoot = (relativePath: string): string => { - const fixtureMatch = FIXTURE_PROJECT_PATTERN.exec(relativePath); - if (fixtureMatch === null) return relativePath; - let lastIdx = -1; - for (const match of relativePath.matchAll(SOURCE_ROOT_PATTERN)) { - if (match.index !== undefined && match.index > lastIdx) lastIdx = match.index; - } - if (lastIdx >= 0) return relativePath.slice(lastIdx); - // No inner source-root marker — strip up through the fixture segment - // so the outer `tests/` / `e2e/` prefix can't re-trigger the test - // heuristic on fixture production files like - // `tests/fixtures/my-app/Component.tsx`. The fixture itself is the - // unit under test; its contents are production-shaped code. - return relativePath.slice(fixtureMatch.index + fixtureMatch[0].length - 1); -}; - -export const isTestFilePath = (relativePath: string): boolean => { - if (relativePath.length === 0) return false; - const forwardSlashed = relativePath.replaceAll("\\", "/"); - // The SUFFIX check (.test/.spec/.stories etc.) is on the FULL path — - // unambiguous regardless of context. - if (TEST_FILE_SUFFIX_PATTERN.test(forwardSlashed)) return true; - // The DIRECTORY check (`tests/`, `__tests__/`, `cypress/`, etc.) - // scopes to the source-root-below path so that fixture-project - // source files don't get falsely auto-suppressed. - const scoped = stripAboveSourceRoot(forwardSlashed); - return TEST_FILE_DIRECTORY_PATTERN.test(scoped); -}; +// Derived from `classifyFileContext` so the test/story path heuristics +// live in exactly one place: any non-production context (a story file +// is still non-shipping code) counts as a test path for test-noise +// auto-suppression. +export const isTestFilePath = (relativePath: string): boolean => + classifyFileContext(relativePath) !== "production"; diff --git a/packages/core/src/schemas.ts b/packages/core/src/schemas.ts index 2c72a56b7..3fba1d331 100644 --- a/packages/core/src/schemas.ts +++ b/packages/core/src/schemas.ts @@ -32,6 +32,7 @@ export class Diagnostic extends Schema.Class("Diagnostic")({ endLine: Schema.optional(Schema.Number), endColumn: Schema.optional(Schema.Number), category: Schema.String, + fileContext: Schema.optional(Schema.Literals(["test", "story"])), suppressionHint: Schema.optional(Schema.String), relatedLocations: Schema.optional(Schema.Array(DiagnosticRelatedLocation)), }) {} diff --git a/packages/core/src/types/diagnostic.ts b/packages/core/src/types/diagnostic.ts index a99c29fe3..8c04152da 100644 --- a/packages/core/src/types/diagnostic.ts +++ b/packages/core/src/types/diagnostic.ts @@ -48,6 +48,8 @@ export interface DiagnosticRelatedLocation { message: string; } +export type DiagnosticFileContext = "test" | "story" | "production"; + export interface Diagnostic { filePath: string; plugin: string; @@ -77,6 +79,12 @@ export interface Diagnostic { /** 1-indexed end column of the primary span, when derivable. */ endColumn?: number; category: string; + /** + * Set when the file never ships to users (`"test"` / `"story"`), so + * renderers can label the site instead of implying production impact. + * Omitted for production files (the default). + */ + fileContext?: Exclude; suppressionHint?: string; /** Secondary source locations (oxlint's non-primary labels). */ relatedLocations?: DiagnosticRelatedLocation[]; diff --git a/packages/core/src/types/index.ts b/packages/core/src/types/index.ts index 729aab8b4..60173d349 100644 --- a/packages/core/src/types/index.ts +++ b/packages/core/src/types/index.ts @@ -21,6 +21,7 @@ export type { export type { CleanedDiagnostic, Diagnostic, + DiagnosticFileContext, DiagnosticRelatedLocation, OxlintOutput, } from "./diagnostic.js"; diff --git a/packages/core/tests/classify-file-context.test.ts b/packages/core/tests/classify-file-context.test.ts new file mode 100644 index 000000000..8ac779246 --- /dev/null +++ b/packages/core/tests/classify-file-context.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "vite-plus/test"; +import { classifyFileContext } from "@react-doctor/core"; + +describe("classifyFileContext", () => { + it("classifies `.stories.*` / `.story.*` suffixes as story", () => { + expect(classifyFileContext("src/components/Button.stories.tsx")).toBe("story"); + expect(classifyFileContext("src/components/Button.story.tsx")).toBe("story"); + expect(classifyFileContext("src/components/Button.stories.jsx")).toBe("story"); + expect(classifyFileContext("src/components/Button.story.mts")).toBe("story"); + }); + + it("classifies story files inside test directories as story, not test", () => { + expect(classifyFileContext("__tests__/Button.stories.tsx")).toBe("story"); + expect(classifyFileContext("tests/Button.story.tsx")).toBe("story"); + }); + + it("classifies `.test.*` / `.spec.*` suffixes as test", () => { + expect(classifyFileContext("src/utils/foo.test.ts")).toBe("test"); + expect(classifyFileContext("src/utils/foo.spec.tsx")).toBe("test"); + expect(classifyFileContext("src/utils/foo.test.mjs")).toBe("test"); + }); + + it("classifies fixture suffixes and test directories as test", () => { + expect(classifyFileContext("src/components/Button.fixture.tsx")).toBe("test"); + expect(classifyFileContext("src/utils/__tests__/foo.ts")).toBe("test"); + expect(classifyFileContext("cypress/e2e/login.cy.ts")).toBe("test"); + expect(classifyFileContext("e2e/checkout.ts")).toBe("test"); + }); + + it("normalizes Windows-style backslashes", () => { + expect(classifyFileContext("src\\components\\Button.stories.tsx")).toBe("story"); + expect(classifyFileContext("src\\utils\\__tests__\\foo.ts")).toBe("test"); + }); + + it("classifies production source files as production", () => { + expect(classifyFileContext("src/components/Button.tsx")).toBe("production"); + expect(classifyFileContext("packages/ui/src/index.ts")).toBe("production"); + expect(classifyFileContext("app/page.tsx")).toBe("production"); + }); + + it("classifies fixture-project source files under `fixtures/` as production", () => { + expect(classifyFileContext("tests/fixtures/sample/src/Button.tsx")).toBe("production"); + expect(classifyFileContext("tests/__fixtures__/repo/Component.tsx")).toBe("production"); + }); + + it("classifies empty input as production", () => { + expect(classifyFileContext("")).toBe("production"); + }); +}); diff --git a/packages/core/tests/filter-diagnostics.test.ts b/packages/core/tests/filter-diagnostics.test.ts index 363613520..c1f6adfc4 100644 --- a/packages/core/tests/filter-diagnostics.test.ts +++ b/packages/core/tests/filter-diagnostics.test.ts @@ -370,3 +370,65 @@ describe("mergeAndFilterDiagnostics — ignore rules / files / overrides", () => expect(filtered[0].rule).toBe("no-cascading-set-state"); }); }); + +describe("mergeAndFilterDiagnostics — file-context stamping", () => { + it('stamps `fileContext: "test"` on diagnostics in spec files', () => { + const diagnostics = [createDiagnostic({ filePath: "src/utils/foo.spec.ts" })]; + const filtered = filterIgnoredDiagnostics( + diagnostics, + {}, + TEST_ROOT_DIRECTORY, + testReadFileLines, + ); + expect(filtered).toHaveLength(1); + expect(filtered[0].fileContext).toBe("test"); + }); + + it('stamps `fileContext: "story"` on diagnostics in story files', () => { + const diagnostics = [createDiagnostic({ filePath: "src/components/Button.stories.tsx" })]; + const filtered = filterIgnoredDiagnostics( + diagnostics, + {}, + TEST_ROOT_DIRECTORY, + testReadFileLines, + ); + expect(filtered).toHaveLength(1); + expect(filtered[0].fileContext).toBe("story"); + }); + + it("leaves `fileContext` unset on diagnostics in production files", () => { + const diagnostics = [createDiagnostic({ filePath: "src/components/Button.tsx" })]; + const filtered = filterIgnoredDiagnostics( + diagnostics, + {}, + TEST_ROOT_DIRECTORY, + testReadFileLines, + ); + expect(filtered).toHaveLength(1); + expect(filtered[0].fileContext).toBeUndefined(); + }); + + it("still auto-suppresses test-noise rules (rn-no-raw-text in a spec never shows)", () => { + const diagnostics = [ + createDiagnostic({ + plugin: "react-doctor", + rule: "rn-no-raw-text", + filePath: "src/components/Button.spec.tsx", + }), + createDiagnostic({ + plugin: "react-doctor", + rule: "rn-no-raw-text", + filePath: "src/components/Button.tsx", + }), + ]; + const filtered = filterIgnoredDiagnostics( + diagnostics, + {}, + TEST_ROOT_DIRECTORY, + testReadFileLines, + ); + expect(filtered).toHaveLength(1); + expect(filtered[0].filePath).toBe("src/components/Button.tsx"); + expect(filtered[0].fileContext).toBeUndefined(); + }); +}); diff --git a/packages/react-doctor/src/cli/utils/build-run-event.ts b/packages/react-doctor/src/cli/utils/build-run-event.ts index 5a97686f8..21fa3f198 100644 --- a/packages/react-doctor/src/cli/utils/build-run-event.ts +++ b/packages/react-doctor/src/cli/utils/build-run-event.ts @@ -139,6 +139,13 @@ const buildOutcomeAttributes = (input: RunEventInput): RunEventAttributes => { } } + let diagnosticsInTestFiles = 0; + let diagnosticsInStoryFiles = 0; + for (const diagnostic of result.diagnostics) { + if (diagnostic.fileContext === "test") diagnosticsInTestFiles += 1; + if (diagnostic.fileContext === "story") diagnosticsInStoryFiles += 1; + } + const attributes: RunEventAttributes = { outcome, exitCode: wouldBlock ? 1 : 0, @@ -149,6 +156,8 @@ const buildOutcomeAttributes = (input: RunEventInput): RunEventAttributes => { errorCount: summary.errorCount, warningCount: summary.warningCount, affectedFiles: summary.affectedFileCount, + diagnosticsInTestFiles, + diagnosticsInStoryFiles, distinctRulesFired: countByRule.size, topRule, scannedFileCount: result.scannedFileCount ?? null, diff --git a/packages/react-doctor/src/cli/utils/render-diagnostics.ts b/packages/react-doctor/src/cli/utils/render-diagnostics.ts index adf852977..0ee35c33d 100644 --- a/packages/react-doctor/src/cli/utils/render-diagnostics.ts +++ b/packages/react-doctor/src/cli/utils/render-diagnostics.ts @@ -44,6 +44,11 @@ interface VerboseSiteEntry { suppressionHint?: string; } +interface VerboseFileEntry { + contextTag: string; + sites: VerboseSiteEntry[]; +} + interface CategoryDiagnosticGroup { category: string; diagnostics: Diagnostic[]; @@ -56,16 +61,25 @@ interface SourceRootResolver { (diagnostic: Diagnostic): string; } -const buildVerboseSiteMap = (diagnostics: Diagnostic[]): Map => { - const fileSites = new Map(); +// Dim "(test file)" / "(story file)" tag after a site's location, so a +// finding in a spec or story reads as non-shipping code rather than a +// production problem. Empty for production files (the default). +const formatFileContextTag = (diagnostic: Diagnostic): string => + diagnostic.fileContext ? ` (${diagnostic.fileContext} file)` : ""; + +const buildVerboseFileEntries = (diagnostics: Diagnostic[]): Map => { + const fileEntries = new Map(); for (const diagnostic of diagnostics) { - const sites = fileSites.get(diagnostic.filePath) ?? []; + let entry = fileEntries.get(diagnostic.filePath); + if (entry === undefined) { + entry = { contextTag: formatFileContextTag(diagnostic), sites: [] }; + fileEntries.set(diagnostic.filePath, entry); + } if (diagnostic.line > 0) { - sites.push({ line: diagnostic.line, suppressionHint: diagnostic.suppressionHint }); + entry.sites.push({ line: diagnostic.line, suppressionHint: diagnostic.suppressionHint }); } - fileSites.set(diagnostic.filePath, sites); } - return fileSites; + return fileEntries; }; const formatSiteCountBadge = (count: number): string => (count > 1 ? `×${count}` : ""); @@ -267,11 +281,12 @@ const clusterNearbyDiagnostics = (diagnostics: Diagnostic[]): DiagnosticCluster[ }; const formatClusterLocation = (cluster: DiagnosticCluster): string => { - const { filePath } = cluster.diagnostics[0]!; - if (cluster.startLine <= 0) return filePath; + const lead = cluster.diagnostics[0]!; + const contextTag = formatFileContextTag(lead); + if (cluster.startLine <= 0) return `${lead.filePath}${contextTag}`; if (cluster.endLine > cluster.startLine) - return `${filePath}:${cluster.startLine}-${cluster.endLine}`; - return `${filePath}:${cluster.startLine}`; + return `${lead.filePath}:${cluster.startLine}-${cluster.endLine}${contextTag}`; + return `${lead.filePath}:${cluster.startLine}${contextTag}`; }; // The location + inline code frame for a cluster of nearby same-file @@ -679,17 +694,17 @@ export const formatRuleSummary = (ruleKey: string, ruleDiagnostics: Diagnostic[] } sections.push("", "Files:"); - const fileSites = buildVerboseSiteMap(ruleDiagnostics); - for (const [filePath, sites] of fileSites) { + const fileEntries = buildVerboseFileEntries(ruleDiagnostics); + for (const [filePath, { contextTag, sites }] of fileEntries) { if (sites.length > 0) { for (const site of sites) { - sections.push(` ${filePath}:${site.line}`); + sections.push(` ${filePath}:${site.line}${contextTag}`); if (site.suppressionHint) { sections.push(` ${site.suppressionHint}`); } } } else { - sections.push(` ${filePath}`); + sections.push(` ${filePath}${contextTag}`); } } diff --git a/packages/react-doctor/tests/render-file-context-tag.test.ts b/packages/react-doctor/tests/render-file-context-tag.test.ts new file mode 100644 index 000000000..7ad1c1550 --- /dev/null +++ b/packages/react-doctor/tests/render-file-context-tag.test.ts @@ -0,0 +1,74 @@ +import * as Effect from "effect/Effect"; +import { describe, expect, it, vi } from "vite-plus/test"; +import type { Diagnostic } from "@react-doctor/core"; +import { formatRuleSummary, printDiagnostics } from "../src/cli/utils/render-diagnostics.js"; + +const makeDiagnostic = (overrides: Partial = {}): Diagnostic => ({ + filePath: "src/App.tsx", + plugin: "react-doctor", + rule: "no-array-index-as-key", + severity: "error", + title: "Array index used as a key", + message: "Reordering the list re-renders the wrong rows.", + help: "Use a stable id as the key.", + line: 3, + column: 1, + category: "Correctness", + ...overrides, +}); + +const ANSI = new RegExp(String.raw`\u001B\[[0-?]*[ -/]*[@-~]`, "g"); +const stripAnsi = (text: string): string => text.replace(ANSI, ""); + +const captureOutput = async (run: () => Promise): Promise => { + const writes: string[] = []; + const logSpy = vi.spyOn(console, "log").mockImplementation((...args: unknown[]) => { + writes.push(`${args.join(" ")}\n`); + }); + try { + await run(); + } finally { + logSpy.mockRestore(); + } + return stripAnsi(writes.join("")); +}; + +describe("file-context tags in rendered diagnostics", () => { + it("labels sites in test files so they don't read as production impact", async () => { + const diagnostics = [ + makeDiagnostic({ filePath: "src/utils/foo.spec.tsx", fileContext: "test" }), + ]; + const output = await captureOutput(() => + Effect.runPromise(printDiagnostics(diagnostics, false, "/nonexistent")), + ); + expect(output).toContain("src/utils/foo.spec.tsx:3 (test file)"); + }); + + it("labels sites in story files", async () => { + const diagnostics = [ + makeDiagnostic({ filePath: "src/Button.stories.tsx", fileContext: "story" }), + ]; + const output = await captureOutput(() => + Effect.runPromise(printDiagnostics(diagnostics, false, "/nonexistent")), + ); + expect(output).toContain("src/Button.stories.tsx:3 (story file)"); + }); + + it("leaves production sites untagged", async () => { + const diagnostics = [makeDiagnostic()]; + const output = await captureOutput(() => + Effect.runPromise(printDiagnostics(diagnostics, false, "/nonexistent")), + ); + expect(output).toContain("src/App.tsx:3"); + expect(output).not.toContain("file)"); + }); + + it("tags file sites in the per-rule text summary", () => { + const summary = formatRuleSummary("react-doctor/no-array-index-as-key", [ + makeDiagnostic({ filePath: "src/utils/foo.spec.tsx", fileContext: "test" }), + makeDiagnostic(), + ]); + expect(summary).toContain("src/utils/foo.spec.tsx:3 (test file)"); + expect(summary).toContain("src/App.tsx:3\n"); + }); +});