diff --git a/.changeset/issue-149-empty-pattern-knip.md b/.changeset/issue-149-empty-pattern-knip.md new file mode 100644 index 0000000..edaa8aa --- /dev/null +++ b/.changeset/issue-149-empty-pattern-knip.md @@ -0,0 +1,18 @@ +--- +"react-doctor": patch +--- + +fix(react-doctor): strip empty pattern strings from knip config so dead-code analysis no longer aborts with `Expected pattern to be a non-empty string` (#149) + +`knip` funnels every entry, project, ignore, and plugin pattern through +`picomatch`, which throws `Expected pattern to be a non-empty string` if +any value is `""` or whitespace-only. Empty patterns can sneak in via +`tsconfig.json` `include`, plugin shorthand resolution, or hand-written +`knip.json` entries — knocking out the entire dead-code step with the +single-line "Dead code detection failed (non-fatal, skipping)" message. + +`runKnip` now walks the parsed knip config (top-level keys, nested +plugin objects, and per-workspace overrides) and removes empty / +whitespace-only string patterns — both as scalars and as entries inside +arrays — before invoking knip's `main`. Non-string entries (regexes, +booleans, numbers) and intentionally-empty arrays are preserved. diff --git a/packages/react-doctor/src/utils/run-knip.ts b/packages/react-doctor/src/utils/run-knip.ts index c760fdc..856b849 100644 --- a/packages/react-doctor/src/utils/run-knip.ts +++ b/packages/react-doctor/src/utils/run-knip.ts @@ -10,6 +10,7 @@ import { findMonorepoRoot } from "./find-monorepo-root.js"; import { hasKnipConfig } from "./has-knip-config.js"; import { isFile } from "./is-file.js"; import { readPackageJson } from "./read-package-json.js"; +import { sanitizeKnipConfigPatterns } from "./sanitize-knip-config-patterns.js"; interface KnipIssueDescriptor { category: string; @@ -117,6 +118,7 @@ const runKnipWithOptions = async ( ); const parsedConfig = options.parsedConfig as Record; + sanitizeKnipConfigPatterns(parsedConfig); const disabledPlugins = new Set(); let lastKnipError: unknown; diff --git a/packages/react-doctor/src/utils/sanitize-knip-config-patterns.ts b/packages/react-doctor/src/utils/sanitize-knip-config-patterns.ts new file mode 100644 index 0000000..4693346 --- /dev/null +++ b/packages/react-doctor/src/utils/sanitize-knip-config-patterns.ts @@ -0,0 +1,36 @@ +import { isPlainObject } from "./is-plain-object.js"; + +const isMeaningfulPattern = (value: unknown): boolean => + typeof value !== "string" || value.trim().length > 0; + +const sanitizeStringArray = (values: unknown[]): unknown[] => + values.filter((entry) => (typeof entry === "string" ? entry.trim().length > 0 : true)); + +// HACK: knip funnels every pattern through picomatch which throws +// `Expected pattern to be a non-empty string` if any entry is `""`. +// Empty strings can sneak in via tsconfig/package.json fields, knip +// configs, or plugin shorthand resolution (issue #149). Walk the +// parsed config and strip empty/whitespace-only patterns so the bad +// entry doesn't take down the whole dead-code step. +export const sanitizeKnipConfigPatterns = (parsedConfig: Record): void => { + for (const [key, value] of Object.entries(parsedConfig)) { + if (typeof value === "string") { + if (!isMeaningfulPattern(value)) delete parsedConfig[key]; + continue; + } + if (Array.isArray(value)) { + if (value.length === 0) continue; + const sanitized = sanitizeStringArray(value); + if (sanitized.length === value.length) continue; + if (sanitized.length === 0) { + delete parsedConfig[key]; + } else { + parsedConfig[key] = sanitized; + } + continue; + } + if (isPlainObject(value)) { + sanitizeKnipConfigPatterns(value); + } + } +}; diff --git a/packages/react-doctor/tests/regressions/scan-resilience.test.ts b/packages/react-doctor/tests/regressions/scan-resilience.test.ts index baa715b..cc7d6cc 100644 --- a/packages/react-doctor/tests/regressions/scan-resilience.test.ts +++ b/packages/react-doctor/tests/regressions/scan-resilience.test.ts @@ -14,6 +14,10 @@ * #89 — `--offline` calculates the score locally (no network round trip) * #115 — `--staged` snapshots git INDEX content (not working tree) so * partially-staged hunks behave correctly + * #149 — empty / whitespace-only pattern strings reaching knip cause + * `picomatch` to throw `Expected pattern to be a non-empty + * string`, killing the whole dead-code step. The sanitizer + * strips them before `main()` runs. * #141 — REACT_COMPILER_RULES must not be enabled in the oxlint config * unless the `react-hooks-js` plugin (eslint-plugin-react-hooks, * an optional peer) actually resolved — otherwise oxlint errors @@ -38,6 +42,7 @@ import { batchIncludePaths } from "../../src/utils/batch-include-paths.js"; import { discoverProject } from "../../src/utils/discover-project.js"; import { extractFailedPluginName } from "../../src/utils/extract-failed-plugin-name.js"; import { getStagedSourceFiles, materializeStagedFiles } from "../../src/utils/get-staged-files.js"; +import { sanitizeKnipConfigPatterns } from "../../src/utils/sanitize-knip-config-patterns.js"; import { buildDiagnostic, initGitRepo, writeFile, writeJson } from "./_helpers.js"; const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "rd-scan-resilience-")); @@ -220,6 +225,31 @@ describe("issue #115: --staged uses git INDEX content, not working tree", () => }); }); +describe("issue #149: empty pattern strings cannot reach knip's picomatch matchers", () => { + it("strips empty/whitespace-only patterns from arrays, scalars, and nested plugin configs", () => { + const parsedConfig: Record = { + entry: ["src/index.ts", "", " "], + project: "", + ignore: ["", "node_modules/**"], + vite: { config: ["", "vite.config.ts"], entry: " " }, + workspaces: { + "packages/foo": { entry: ["", "src/index.ts"], ignore: "" }, + }, + }; + + sanitizeKnipConfigPatterns(parsedConfig); + + expect(parsedConfig).toEqual({ + entry: ["src/index.ts"], + ignore: ["node_modules/**"], + vite: { config: ["vite.config.ts"] }, + workspaces: { + "packages/foo": { entry: ["src/index.ts"] }, + }, + }); + }); +}); + describe("issue #141: oxlint config must not reference unloaded plugins", () => { // HACK: the bug only fires when eslint-plugin-react-hooks is missing // AND React Compiler is detected — so REACT_COMPILER_RULES (under the diff --git a/packages/react-doctor/tests/run-knip.test.ts b/packages/react-doctor/tests/run-knip.test.ts index 896dc2f..53268dd 100644 --- a/packages/react-doctor/tests/run-knip.test.ts +++ b/packages/react-doctor/tests/run-knip.test.ts @@ -220,6 +220,22 @@ describe("runKnip", () => { expect(mockKnipState.mainCallCount).toBe(1); }); + it("strips empty pattern strings from parsedConfig before calling knip (issue #149)", async () => { + mockKnipState.parsedConfig = { + entry: ["src/index.ts", "", "src/main.ts"], + ignore: "", + vite: { config: ["", "vite.config.ts"] }, + }; + + await runKnip(standaloneRoot); + + expect(mockKnipState.parsedConfig).toEqual({ + entry: ["src/index.ts", "src/main.ts"], + vite: { config: ["vite.config.ts"] }, + }); + expect(mockKnipState.mainCallCount).toBe(1); + }); + it("rethrows the most recent error after exhausting retries instead of `Unreachable`", async () => { const sequencedErrors = [ new Error("Error loading /repo/vite.config.ts"), diff --git a/packages/react-doctor/tests/sanitize-knip-config-patterns.test.ts b/packages/react-doctor/tests/sanitize-knip-config-patterns.test.ts new file mode 100644 index 0000000..d56da54 --- /dev/null +++ b/packages/react-doctor/tests/sanitize-knip-config-patterns.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from "vite-plus/test"; +import { sanitizeKnipConfigPatterns } from "../src/utils/sanitize-knip-config-patterns.js"; + +describe("sanitizeKnipConfigPatterns", () => { + it("removes empty string values at the top level", () => { + const parsedConfig: Record = { + entry: "", + project: "src/**/*.ts", + }; + sanitizeKnipConfigPatterns(parsedConfig); + expect(parsedConfig).toEqual({ project: "src/**/*.ts" }); + }); + + it("removes whitespace-only string values", () => { + const parsedConfig: Record = { + entry: " ", + ignore: "\n\t", + project: "src/**/*.ts", + }; + sanitizeKnipConfigPatterns(parsedConfig); + expect(parsedConfig).toEqual({ project: "src/**/*.ts" }); + }); + + it("filters empty strings out of arrays", () => { + const parsedConfig: Record = { + entry: ["src/index.ts", "", "src/main.ts"], + }; + sanitizeKnipConfigPatterns(parsedConfig); + expect(parsedConfig).toEqual({ entry: ["src/index.ts", "src/main.ts"] }); + }); + + it("removes arrays that become empty after filtering", () => { + const parsedConfig: Record = { + entry: ["", " "], + project: ["src/**/*.ts"], + }; + sanitizeKnipConfigPatterns(parsedConfig); + expect(parsedConfig).toEqual({ project: ["src/**/*.ts"] }); + }); + + it("recurses into nested plugin configs and workspaces", () => { + const parsedConfig: Record = { + vite: { + config: ["", "vite.config.ts"], + entry: "", + }, + workspaces: { + "packages/foo": { + entry: ["", "src/index.ts"], + ignore: "", + }, + }, + }; + sanitizeKnipConfigPatterns(parsedConfig); + expect(parsedConfig).toEqual({ + vite: { config: ["vite.config.ts"] }, + workspaces: { + "packages/foo": { entry: ["src/index.ts"] }, + }, + }); + }); + + it("preserves non-string entries inside arrays", () => { + const parsedConfig: Record = { + ignoreDependencies: [/regex/, "valid", ""], + }; + sanitizeKnipConfigPatterns(parsedConfig); + expect(parsedConfig).toEqual({ ignoreDependencies: [/regex/, "valid"] }); + }); + + it("leaves boolean and falsy non-string values untouched", () => { + const parsedConfig: Record = { + vite: false, + eslint: true, + tags: [], + includeEntryExports: false, + }; + sanitizeKnipConfigPatterns(parsedConfig); + expect(parsedConfig).toEqual({ + vite: false, + eslint: true, + tags: [], + includeEntryExports: false, + }); + }); +});