From 533023a7b2d6f9523518b8d605df6ee9cef4bf67 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 25 Jun 2026 16:27:41 +0000 Subject: [PATCH] refactor: simplify scan orchestration cleanup Co-authored-by: Aiden Bai --- packages/core/src/apply-ignore-overrides.ts | 7 +-- packages/core/src/schemas.ts | 8 +-- packages/deslop-js/README.md | 4 +- packages/deslop-js/src/collect/entries.ts | 9 --- packages/deslop-js/src/constants.ts | 2 - packages/deslop-js/src/index.ts | 7 +-- packages/deslop-js/tests/semantic.test.ts | 6 +- .../src/plugin/react-doctor-plugin.ts | 13 ++--- .../src/cli/utils/render-project-detection.ts | 13 ----- .../react-doctor/src/cli/utils/run-explain.ts | 2 + packages/react-doctor/src/inspect.ts | 58 +++---------------- .../tests/namespace-hooks.test.ts | 7 ++- .../react-doctor/tests/run-oxlint/_helpers.ts | 6 ++ packages/react-doctor/vite.config.ts | 3 + 14 files changed, 42 insertions(+), 103 deletions(-) delete mode 100644 packages/react-doctor/src/cli/utils/render-project-detection.ts diff --git a/packages/core/src/apply-ignore-overrides.ts b/packages/core/src/apply-ignore-overrides.ts index 353f04d31..aa20219e9 100644 --- a/packages/core/src/apply-ignore-overrides.ts +++ b/packages/core/src/apply-ignore-overrides.ts @@ -12,9 +12,6 @@ interface CompiledIgnoreOverride { const isStringArray = (value: unknown): value is string[] => Array.isArray(value) && value.every((entry) => typeof entry === "string"); -const collectStringList = (value: unknown): string[] => - Array.isArray(value) ? value.filter((entry): entry is string => typeof entry === "string") : []; - const validateOverrideEntry = (entry: unknown, index: number): ReactDoctorIgnoreOverride | null => { if (!isPlainObject(entry)) { warnConfigIssue( @@ -52,11 +49,11 @@ export const compileIgnoreOverrides = ( return overrides.flatMap((entry, index) => { const validated = validateOverrideEntry(entry, index); if (!validated) return []; - const filePatterns = compileGlobPatternsLenient(collectStringList(validated.files), (error) => + const filePatterns = compileGlobPatternsLenient(validated.files, (error) => warnConfigIssue(`ignore.overrides[${index}]: ${error.message}`), ); if (filePatterns.length === 0) return []; - const ruleIds = new Set(collectStringList(validated.rules)); + const ruleIds = new Set(validated.rules ?? []); return [{ filePatterns, ruleIds }]; }); }; diff --git a/packages/core/src/schemas.ts b/packages/core/src/schemas.ts index f98386d8b..389d43f94 100644 --- a/packages/core/src/schemas.ts +++ b/packages/core/src/schemas.ts @@ -99,11 +99,9 @@ export class JsonReportProjectEntry extends Schema.Class }) {} /** - * Versioned JsonReport schema. `JsonReport` is a `Schema.Union` so we - * can add `schemaVersion: 2` later as one new union member without - * breaking existing v1 consumers (the GitHub Action keys off the - * version literal). Today's union is single-arm; the shape is - * intentional. + * Versioned JsonReport schema. `JsonReport` stays a `Schema.Union` so + * consumers can branch on the version literal while v1 and baseline v2 + * payloads remain independently decodable. */ export class JsonReportV1 extends Schema.Class("JsonReportV1")({ schemaVersion: Schema.Literal(1), diff --git a/packages/deslop-js/README.md b/packages/deslop-js/README.md index 20f5e4844..c97a0d17a 100644 --- a/packages/deslop-js/README.md +++ b/packages/deslop-js/README.md @@ -164,7 +164,7 @@ const config = defineConfig({ | `reportTypes` | `boolean` | `false` | Include type-only exports in `unusedExports` | | `includeEntryExports` | `boolean` | `false` | Report unused exports from entry files | | `reportRedundancy` | `boolean` | `true` | Emit the redundancy / DRY findings listed above | -| `semantic` | `SemanticConfig` | `undefined` | Opt-in TypeScript type-aware analysis (see below) | +| `semantic` | `SemanticConfig` | `{ enabled: true }` | TypeScript type-aware analysis (see below) | Path aliases are auto-detected by default — from `tsconfig` `paths`, Vite (`resolve.alias`), webpack, Babel (`module-resolver`), and Jest (`moduleNameMapper`) configs, plus the workspace layout (a `@scope/` import resolves to the matching workspace package even when its `package.json` name differs). Use `paths` / `--paths` only for mappings none of those cover. @@ -189,7 +189,7 @@ const config = defineConfig({ | Option | Default | Notes | | --------------------------------- | ----------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `enabled` | `false` | Master switch; semantic analysis loads the TS program and adds ~1–3s per scan | +| `enabled` | `true` | Master switch; semantic analysis loads the TS program and adds ~1–3s per scan | | `reportUnusedTypes` | `true` | Type aliases / interfaces / type-only exports never referenced | | `reportUnusedEnumMembers` | `true` | Enum members no reference site reads or writes | | `reportUnusedClassMembers` | **`false`** | Subclass overrides, framework method-by-name invocation (`@HttpGet`, lifecycle hooks) produce too many stylistic FPs to enable by default. Opt in selectively. | diff --git a/packages/deslop-js/src/collect/entries.ts b/packages/deslop-js/src/collect/entries.ts index a97dc1971..8dbd8f0ac 100644 --- a/packages/deslop-js/src/collect/entries.ts +++ b/packages/deslop-js/src/collect/entries.ts @@ -10,7 +10,6 @@ import { SCRIPT_FILE_PATTERN, SCRIPT_EXTENSIONLESS_FILE_PATTERN, SCRIPT_CONFIG_FILE_PATTERN, - SCRIPT_ENTRY_PATTERNS, SHALLOW_WORKSPACE_MAX_DEPTH, SOURCE_EXTENSIONS as IMPORTABLE_SOURCE_EXTENSIONS, } from "../constants.js"; @@ -894,14 +893,6 @@ const extractScriptEntries = (directory: string): string[] => { } } catch {} - const scriptDirectoryFiles = fg.sync(SCRIPT_ENTRY_PATTERNS, { - cwd: directory, - absolute: true, - onlyFiles: true, - ignore: ["**/node_modules/**"], - }); - entries.push(...scriptDirectoryFiles); - return entries; }; diff --git a/packages/deslop-js/src/constants.ts b/packages/deslop-js/src/constants.ts index 15bf0419e..4e6fc0721 100644 --- a/packages/deslop-js/src/constants.ts +++ b/packages/deslop-js/src/constants.ts @@ -49,8 +49,6 @@ export const SCRIPT_EXTENSIONLESS_FILE_PATTERN = export const SCRIPT_CONFIG_FILE_PATTERN = /--config\s+([\w./@-]+\.(?:ts|tsx|js|jsx|mts|mjs|cts|cjs))/; -export const SCRIPT_ENTRY_PATTERNS: string[] = []; - export const DEFAULT_ENTRY_GLOBS = [ "src/index.{ts,tsx,js,jsx}", "src/main.{ts,tsx,js,jsx}", diff --git a/packages/deslop-js/src/index.ts b/packages/deslop-js/src/index.ts index 63274c25d..9af1eec9a 100644 --- a/packages/deslop-js/src/index.ts +++ b/packages/deslop-js/src/index.ts @@ -200,10 +200,9 @@ export type { * - `reportRedundancy: true` — on because redundancy findings are mostly * high-signal and the detectors carry their own confidence tiers. * - * - `duplicateBlocks: undefined` — token-based copy-paste detection (suffix - * array + LCP) is opt-in. It re-parses every source - * file to emit a token stream and adds significant runtime to the scan. - * Pass `duplicateBlocks: { enabled: true }` to turn it on. + * - `duplicateBlocks.enabled: true` — token-based copy-paste detection is + * part of the default code-quality pass. Disable `reportCodeQuality` or + * pass `duplicateBlocks: { enabled: false }` to skip the extra token pass. */ const fillSemanticConfig = ( semanticOverrides: Partial | undefined, diff --git a/packages/deslop-js/tests/semantic.test.ts b/packages/deslop-js/tests/semantic.test.ts index 6557d22c8..6af6c9ac0 100644 --- a/packages/deslop-js/tests/semantic.test.ts +++ b/packages/deslop-js/tests/semantic.test.ts @@ -22,8 +22,8 @@ const scanFixtureWithSemantic = async ( const unusedTypeNames = (result: ScanResult): string[] => result.unusedTypes.map((unusedType) => unusedType.name).sort(); -describe("semantic (Phase 0)", () => { - it("populates unusedTypes as [] by default (semantic disabled)", async () => { +describe("semantic analysis", () => { + it("populates unusedTypes as [] on fixtures without a tsconfig", async () => { const result = await analyze(defineConfig({ rootDir: resolve(FIXTURES_DIR, "simple-app") })); assert.deepEqual(result.unusedTypes, []); }); @@ -36,7 +36,7 @@ describe("semantic (Phase 0)", () => { }), ); assert.ok(Array.isArray(result.unusedTypes), "unusedTypes must be an array"); - assert.equal(result.unusedTypes.length, 0, "Phase 0 returns no findings yet"); + assert.equal(result.unusedTypes.length, 0, "projects without tsconfig return no findings"); }); it("preserves all pre-existing ScanResult fields when semantic is enabled", async () => { diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/react-doctor-plugin.ts b/packages/oxlint-plugin-react-doctor/src/plugin/react-doctor-plugin.ts index fb92cc435..63f33d34a 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/react-doctor-plugin.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/react-doctor-plugin.ts @@ -1,7 +1,6 @@ import { ruleRegistry } from "./rule-registry.js"; import type { Rule } from "./utils/rule.js"; -import type { HostRule } from "./utils/rule-plugin.js"; -import type { RulePlugin } from "./utils/rule-plugin.js"; +import type { HostRule, RulePlugin } from "./utils/rule-plugin.js"; import { wrapReactNativeRule } from "./utils/wrap-react-native-rule.js"; import { wrapWithSemanticContext } from "./utils/wrap-with-semantic-context.js"; @@ -26,11 +25,11 @@ const applyFrameworkRuleWrappers = (registry: Record): Record/.ts`. Adding a new -// rule is a single-file operation: create the rule, set its `id`, run -// `pnpm gen`. +// and by `packages/eslint-plugin-react-doctor/src/index.ts`. Rules are +// sourced from the codegen-built `rule-registry.ts`, which scans every +// `defineRule({ id: "...", ... })` declaration under +// `src/plugin/rules//.ts`. Adding a new rule is a single-file +// operation: create the rule, set its `id`, run `pnpm gen`. const plugin: RulePlugin = { meta: { name: "react-doctor" }, rules: applyFrameworkRuleWrappers(ruleRegistry), diff --git a/packages/react-doctor/src/cli/utils/render-project-detection.ts b/packages/react-doctor/src/cli/utils/render-project-detection.ts deleted file mode 100644 index 1eb767840..000000000 --- a/packages/react-doctor/src/cli/utils/render-project-detection.ts +++ /dev/null @@ -1,13 +0,0 @@ -import * as Effect from "effect/Effect"; -import type { ProjectInfo, ReactDoctorConfig } from "@react-doctor/core"; - -export interface PrintProjectDetectionInput { - readonly projectInfo: ProjectInfo; - readonly userConfig: ReactDoctorConfig | null; - readonly isDiffMode: boolean; - readonly includePaths: ReadonlyArray; - readonly lintSourceFileCount: number | undefined; -} - -export const printProjectDetection = (_input: PrintProjectDetectionInput): Effect.Effect => - Effect.void; diff --git a/packages/react-doctor/src/cli/utils/run-explain.ts b/packages/react-doctor/src/cli/utils/run-explain.ts index b5fdb6d99..f827116ba 100644 --- a/packages/react-doctor/src/cli/utils/run-explain.ts +++ b/packages/react-doctor/src/cli/utils/run-explain.ts @@ -49,8 +49,10 @@ export const runExplain = async ( const scanResult = await inspect(targetDirectory, { ...context.scanOptions, + deadCode: false, silent: true, noScore: true, + suppressRendering: true, configOverride: context.userConfig, }); diff --git a/packages/react-doctor/src/inspect.ts b/packages/react-doctor/src/inspect.ts index c4e8e1a9f..089876837 100644 --- a/packages/react-doctor/src/inspect.ts +++ b/packages/react-doctor/src/inspect.ts @@ -62,7 +62,6 @@ import { shouldRecordOnboarding, } from "./cli/utils/onboarding-pacing.js"; import { hasCompletedOnboarding, markOnboardingComplete } from "./cli/utils/onboarding-state.js"; -import { printProjectDetection } from "./cli/utils/render-project-detection.js"; import { printBrandingOnlyHeader, printNoScoreHeader, @@ -308,14 +307,11 @@ export const inspect = async ( const options = mergeInspectOptions(inputOptions, userConfig); - // HACK: spinner.ts still has module-level silent state (used by - // printProjectDetection's internal spinner() calls). Mirror the - // silent flag here until that file moves to a Progress service in - // a follow-up PR. Console-side silent is handled by swapping the - // global Console reference for `silentConsole` inside the program - // (see `runInspectWithRuntime`). Concurrent batch members never touch - // the shared flag — overlapping save/restore pairs would race — so the - // pool owner (the CLI) silences spinners once around the whole batch. + // HACK: spinner.ts still has module-level silent state. Mirror the + // silent flag here until every spinner is owned by the Progress service. + // Concurrent batch members never touch the shared flag — overlapping + // save/restore pairs would race — so the pool owner silences spinners + // once around the whole batch. const ownsSpinnerSilence = options.silent && !isConcurrentScan; const wasSpinnerSilent = isSpinnerSilent(); if (ownsSpinnerSilence) setSpinnerSilent(true); @@ -508,12 +504,6 @@ const runInspectWithRuntime = async ( concurrentScan: options.concurrentScan, }); recordCount(METRIC.projectDetected, 1); - await renderCachedProjectDetection({ - payload: cachedPayload, - options, - userConfig, - isDiffMode, - }); const baselineDegraded = Boolean(options.baseline) && isDiffMode && cachedPayload.baselineDelta === undefined; const result = await renderAndRecordScan({ @@ -576,8 +566,8 @@ const runInspectWithRuntime = async ( concurrentScan: options.concurrentScan, }, { - beforeLint: (projectInfo, lintIncludePaths) => - Effect.gen(function* () { + beforeLint: (projectInfo) => + Effect.sync(() => { // Attach the discovered project shape to Sentry as early as possible // (this hook fires right after project discovery) so crashes, the run // transaction, and every subsequent metric carry it. No-op when @@ -586,15 +576,6 @@ const runInspectWithRuntime = async ( concurrentScan: options.concurrentScan, }); recordCount(METRIC.projectDetected, 1); - if (options.scoreOnly || options.suppressRendering) return; - const lintSourceFileCount = lintIncludePaths?.length ?? projectInfo.sourceFileCount; - yield* printProjectDetection({ - projectInfo, - userConfig, - isDiffMode, - includePaths: options.includePaths, - lintSourceFileCount, - }); }), }, ); @@ -750,13 +731,6 @@ interface FinalizeInput { baselineDelta: InspectResult["baselineDelta"]; } -interface RenderCachedProjectDetectionInput { - readonly payload: CachedScanPayload; - readonly options: ResolvedInspectOptions; - readonly userConfig: ReactDoctorConfig | null; - readonly isDiffMode: boolean; -} - interface RenderAndRecordScanInput { readonly payload: CachedScanPayload; readonly options: ResolvedInspectOptions; @@ -782,24 +756,6 @@ const runMaybeSilent = ( ): Effect.Effect => silent ? effect.pipe(Effect.provideService(Console.Console, silentConsole)) : effect; -const renderCachedProjectDetection = async ( - input: RenderCachedProjectDetectionInput, -): Promise => { - if (input.options.scoreOnly || input.options.suppressRendering) return; - await Effect.runPromise( - runMaybeSilent( - printProjectDetection({ - projectInfo: input.payload.project, - userConfig: input.userConfig, - isDiffMode: input.isDiffMode, - includePaths: input.options.includePaths, - lintSourceFileCount: input.payload.scannedFileCount, - }), - input.options.silent, - ), - ); -}; - const renderAndRecordScan = async (input: RenderAndRecordScanInput): Promise => { const finalizeInput: FinalizeInput = { options: input.options, diff --git a/packages/react-doctor/tests/namespace-hooks.test.ts b/packages/react-doctor/tests/namespace-hooks.test.ts index baf9bcc4d..c7d6b6ce5 100644 --- a/packages/react-doctor/tests/namespace-hooks.test.ts +++ b/packages/react-doctor/tests/namespace-hooks.test.ts @@ -1,5 +1,5 @@ import * as path from "node:path"; -import { describe, expect, it } from "vite-plus/test"; +import { beforeAll, describe, expect, it } from "vite-plus/test"; import type { Diagnostic } from "@react-doctor/core"; import { runOxlint } from "@react-doctor/core"; import { buildTestProject } from "./regressions/_helpers.js"; @@ -26,7 +26,7 @@ const findDiagnosticsInFile = ( let diagnostics: Diagnostic[]; describe("namespace hook detection (React.useEffect, React.useState, etc.)", () => { - it("loads diagnostics from namespace-hooks fixture", async () => { + beforeAll(async () => { diagnostics = await runOxlint({ rootDirectory: BASIC_REACT_DIRECTORY, project: buildTestProject({ @@ -34,6 +34,9 @@ describe("namespace hook detection (React.useEffect, React.useState, etc.)", () hasTanStackQuery: true, }), }); + }); + + it("loads diagnostics from namespace-hooks fixture", () => { expect(diagnostics.length).toBeGreaterThan(0); }); diff --git a/packages/react-doctor/tests/run-oxlint/_helpers.ts b/packages/react-doctor/tests/run-oxlint/_helpers.ts index 85f82018e..262baefba 100644 --- a/packages/react-doctor/tests/run-oxlint/_helpers.ts +++ b/packages/react-doctor/tests/run-oxlint/_helpers.ts @@ -26,6 +26,9 @@ export const USER_OXLINT_CONFIG_BROKEN_DIRECTORY = path.join( const findDiagnosticsByRule = (diagnostics: Diagnostic[], rule: string): Diagnostic[] => diagnostics.filter((diagnostic) => diagnostic.rule === rule); +const pathIncludesFixture = (filePath: string, fixture: string): boolean => + filePath.split(path.sep).join("/").replaceAll("\\", "/").includes(fixture); + export interface RuleTestCase { fixture: string; ruleSource: string; @@ -43,6 +46,9 @@ export const describeRules = ( it(`${ruleName} (${testCase.fixture} → ${testCase.ruleSource})`, () => { const issues = findDiagnosticsByRule(getDiagnostics(), ruleName); expect(issues.length).toBeGreaterThan(0); + expect(issues.some((issue) => pathIncludesFixture(issue.filePath, testCase.fixture))).toBe( + true, + ); if (testCase.severity) expect(issues[0].severity).toBe(testCase.severity); if (testCase.category) expect(issues[0].category).toBe(testCase.category); }); diff --git a/packages/react-doctor/vite.config.ts b/packages/react-doctor/vite.config.ts index 3940e2ddd..5cc82625d 100644 --- a/packages/react-doctor/vite.config.ts +++ b/packages/react-doctor/vite.config.ts @@ -194,6 +194,9 @@ export default defineConfig({ dts: false, target: "node20", platform: "node", + env: { + VERSION: process.env.VERSION ?? packageJson.version, + }, fixedExtension: false, }, ],