diff --git a/package.json b/package.json index 555617e..3b2d166 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "hardcode-replacer", - "version": "2.3.0", + "version": "2.3.1", "description": "CLI + programmatic library for finding hardcoded colors, Tailwind classes, and extractable patterns in codebases", "main": "src/cli.js", "bin": { diff --git a/src/commands/find-duplicate-literals.js b/src/commands/find-duplicate-literals.js index aa1df41..32b49dc 100644 --- a/src/commands/find-duplicate-literals.js +++ b/src/commands/find-duplicate-literals.js @@ -189,10 +189,15 @@ function buildFindings(map, minOccurrences, minFiles) { // --------------------------------------------------------------------------- function discoverFiles(searchPaths, options, includeTests) { + // filesOnly: we only need the SET of candidate files here, not per-match + // rows — using `-l`/`-rl` keeps discovery bounded on a large monorepo (the + // full `--json` output overflowed the child-process buffer and silently + // returned nothing, so the whole command scanned zero files). const results = search(LITERAL_CANDIDATE_MARKER, searchPaths, { include: options.include || DEFAULT_INCLUDE, exclude: options.exclude, caseSensitive: true, + filesOnly: true, }); const fileSet = new Set(); diff --git a/src/search.js b/src/search.js index d7799ba..ca11bfd 100644 --- a/src/search.js +++ b/src/search.js @@ -3,6 +3,37 @@ const { DEFAULT_FILE_TYPES } = require("./color-patterns"); const GREP_LINE_RE = /^(.+?):(\d+):(.*)$/; +// Child-process stdout buffer cap. ripgrep's `--json` output is verbose (one +// JSON object per submatch), so a whole-monorepo scan of a common marker can +// run to hundreds of MB. The old 50MB cap silently overflowed on large trees: +// execFileSync throws ERR_CHILD_PROCESS_STDIO_MAXBUFFER, whose error carries +// no `.status`, so it fell through the `err.status` checks to a `return []` — +// a scan that found everything reported as finding NOTHING (a hollow result; +// the exact trap that made `duplicate-literals` scan zero files on a large +// repo). Raised here as defense, and — crucially — overflow now THROWS loudly +// (see the catch blocks) instead of returning []. For pure file discovery, +// prefer `filesOnly` (below), which emits only filenames and never overflows. +const MAX_STDOUT_BUFFER = 256 * 1024 * 1024; + +/** + * True when `err` is the Node child-process stdout-overflow error. Such an + * error has no `.status` (it's not a non-zero exit), so it must be recognised + * explicitly — otherwise it masquerades as "no matches" and silently zeroes a + * scan. + */ +function isMaxBufferError(err) { + return err && (err.code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER" || /maxBuffer/i.test(err.message || "")); +} + +/** Newline-separated `rg -l` / `grep -rl` output → discovery rows (file only). */ +function parseFilesList(output) { + return output + .split("\n") + .map((f) => f.trim()) + .filter(Boolean) + .map((file) => ({ file, line: 0, column: 0, match: "", text: "" })); +} + // Check if ripgrep is available (memoized) let _hasRipgrep = null; function hasRipgrep() { @@ -27,8 +58,16 @@ function hasRipgrep() { function searchWithRipgrep(pattern, paths, options = {}) { const args = []; - // Output format: JSON for structured parsing - args.push("--json"); + // filesOnly: emit only the names of files that contain a match (`-l`), not + // per-match JSON. Discovery callers (which only need the file set) use this + // to stay bounded on huge trees; the output is one path per line. + const filesOnly = options.filesOnly === true; + if (filesOnly) { + args.push("-l"); + } else { + // Output format: JSON for structured parsing + args.push("--json"); + } // Case insensitive by default for color matching if (options.caseSensitive !== true) { @@ -79,21 +118,32 @@ function searchWithRipgrep(pattern, paths, options = {}) { try { const result = execFileSync("rg", args, { encoding: "utf-8", - maxBuffer: 50 * 1024 * 1024, // 50MB buffer + maxBuffer: MAX_STDOUT_BUFFER, stdio: ["pipe", "pipe", "pipe"], }); - return parseRipgrepJson(result); + return filesOnly ? parseFilesList(result) : parseRipgrepJson(result); } catch (err) { - // ripgrep exits with code 1 when no matches found + // ripgrep exits with code 1 when no matches found — a real empty result. if (err.status === 1) { return []; } - // Exit code 2 means error + // Exit code 2 means a ripgrep error (bad glob, unreadable path, …). if (err.status === 2) { throw new Error(`ripgrep error: ${err.stderr || err.message}`); } - return []; + // Output overflowed the buffer: DO NOT silently return [] (that would + // report a scan that found matches as finding none). Fail loudly with an + // actionable hint. + if (isMaxBufferError(err)) { + throw new Error( + `ripgrep output exceeded the ${Math.round(MAX_STDOUT_BUFFER / (1024 * 1024))}MB buffer for pattern ${JSON.stringify( + pattern, + )}. Narrow the search paths, or pass { filesOnly: true } for discovery.`, + ); + } + // Any other unexpected failure is surfaced, never swallowed. + throw new Error(`ripgrep failed: ${err.stderr || err.message}`); } } @@ -138,7 +188,10 @@ function parseRipgrepJson(output) { */ function searchWithGrep(pattern, paths, options = {}) { const searchPaths = paths.length > 0 ? paths : ["."]; - const args = ["-rn", "-E"]; + // filesOnly → `-rl` (recurse, list files only); otherwise `-rn` (with line + // numbers) for full match rows. + const filesOnly = options.filesOnly === true; + const args = filesOnly ? ["-rlE"] : ["-rnE"]; if (options.caseSensitive !== true) { args.push("-i"); @@ -165,15 +218,23 @@ function searchWithGrep(pattern, paths, options = {}) { try { const result = execFileSync("grep", args, { encoding: "utf-8", - maxBuffer: 50 * 1024 * 1024, + maxBuffer: MAX_STDOUT_BUFFER, stdio: ["pipe", "pipe", "pipe"], }); - return parseGrepOutput(result); + return filesOnly ? parseFilesList(result) : parseGrepOutput(result); } catch (err) { + // grep exits 1 when there are no matches — a real empty result. if (err.status === 1) { return []; } + if (isMaxBufferError(err)) { + throw new Error( + `grep output exceeded the ${Math.round(MAX_STDOUT_BUFFER / (1024 * 1024))}MB buffer for pattern ${JSON.stringify( + pattern, + )}. Narrow the search paths, or pass { filesOnly: true } for discovery.`, + ); + } throw new Error(`grep error: ${err.stderr || err.message}`); } } @@ -211,4 +272,4 @@ function search(pattern, paths = [], options = {}) { return searchWithGrep(pattern, paths, options); } -module.exports = { search }; +module.exports = { search, isMaxBufferError, parseFilesList, MAX_STDOUT_BUFFER }; diff --git a/test/search.test.js b/test/search.test.js index 8e64087..cb3229b 100644 --- a/test/search.test.js +++ b/test/search.test.js @@ -1,7 +1,7 @@ const path = require('node:path'); -const { search } = require('../src/search'); +const { search, isMaxBufferError, parseFilesList, MAX_STDOUT_BUFFER } = require('../src/search'); const FIXTURES = path.join(__dirname, 'fixtures'); @@ -75,4 +75,51 @@ describe('search', () => { // At minimum we should get results expect(results.length).toBeGreaterThanOrEqual(0); }); + + // Regression: on a large tree the verbose --json output overflowed the + // child-process buffer and the error (no `.status`) was swallowed into a + // `return []`, so a scan that found matches reported none — the bug that + // made `duplicate-literals` scan zero files on a monorepo. + describe('filesOnly discovery + overflow safety', () => { + test('filesOnly returns one row per matching file (no per-match duplication)', () => { + const dir = path.join(FIXTURES, 'dupes'); + const rows = search("['\"`/]", [dir], { filesOnly: true, caseSensitive: true }); + const files = new Set(rows.map((r) => r.file)); + // Every returned row is unique-per-file (files-with-matches), not per match. + expect(rows.length).toBe(files.size); + // The dupes fixture has multiple source files, all discovered. + expect(files.size).toBeGreaterThanOrEqual(2); + // filesOnly rows carry the file but no line/text payload. + for (const r of rows) { + expect(r).toHaveProperty('file'); + expect(r.line).toBe(0); + expect(r.text).toBe(''); + } + }); + + test('filesOnly still discovers the same file SET as a full match search', () => { + const dir = path.join(FIXTURES, 'dupes'); + const full = new Set(search("['\"`/]", [dir], { caseSensitive: true }).map((r) => r.file)); + const only = new Set(search("['\"`/]", [dir], { filesOnly: true, caseSensitive: true }).map((r) => r.file)); + expect(only).toEqual(full); + }); + + test('isMaxBufferError recognises a stdout-overflow error (never mistaken for "no matches")', () => { + expect(isMaxBufferError({ code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER' })).toBe(true); + expect(isMaxBufferError({ message: 'stdout maxBuffer length exceeded' })).toBe(true); + // A no-match exit (status 1) is NOT an overflow — it stays a legit empty result. + expect(isMaxBufferError({ status: 1 })).toBe(false); + expect(isMaxBufferError(null)).toBeFalsy(); + }); + + test('MAX_STDOUT_BUFFER is raised well above the old 50MB cap', () => { + expect(MAX_STDOUT_BUFFER).toBeGreaterThan(50 * 1024 * 1024); + }); + + test('parseFilesList trims, drops blanks, and shapes discovery rows', () => { + const rows = parseFilesList('a.js\n b/c.ts \n\n'); + expect(rows.map((r) => r.file)).toEqual(['a.js', 'b/c.ts']); + expect(rows[0]).toEqual({ file: 'a.js', line: 0, column: 0, match: '', text: '' }); + }); + }); });