From 7f3b2cbe882a466ccfd287c23b0d769dd2d78975 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Fri, 3 Jul 2026 16:23:12 -0600 Subject: [PATCH 1/3] fix(check): scope signature-change detection to exported symbols + fix hunk-context false positive checkNoSignatureChanges flagged any function/method/class whose declaration line fell inside a diff hunk's line range, without distinguishing: (1) unchanged declarations swept in purely because unified diffs include 3 context lines around a real change elsewhere in the same hunk, and (2) private, file-local helpers whose deletion/rename can never break an external caller (every call site lives in the same file, in the same diff). parseDiffOutput now walks each hunk body and tracks only the lines actually marked '-', producing precise oldRanges instead of trusting the raw hunk header span. checkNoSignatureChanges is now scoped to exported=1 symbols, matching the check's actual purpose (protect callers outside the diff). Discovered while adopting the leiden typed-array helpers extracted in a prior refactor: removing a private duplicate fget/iget pair from two sibling files (never exported, zero external callers) was blocked by both issues. docs check acknowledged: internal bug fix to codegraph's own check.ts gate predicate, no user-facing feature/language/architecture-table changes. Closes #1760 Impact: 6 functions changed, 7 affected --- src/cli/commands/check.ts | 2 +- src/features/check.ts | 87 +++++++++++++++++++++++------ tests/integration/check.test.ts | 99 ++++++++++++++++++++++++++++++--- 3 files changed, 162 insertions(+), 26 deletions(-) diff --git a/src/cli/commands/check.ts b/src/cli/commands/check.ts index fed4d1fcb..a00cec970 100644 --- a/src/cli/commands/check.ts +++ b/src/cli/commands/check.ts @@ -30,7 +30,7 @@ export const command: CommandDefinition = { ['--rules', 'Also run manifesto rules alongside diff predicates'], ['--cycles', 'Assert no dependency cycles involve changed files'], ['--blast-radius ', 'Assert no function exceeds N transitive callers'], - ['--signatures', 'Assert no function declaration lines were modified'], + ['--signatures', 'Assert no exported function/method/class declaration lines were modified'], ['--boundaries', 'Assert no cross-owner boundary violations'], ['--depth ', 'Max BFS depth for blast radius (default: 3)'], ['-f, --file ', 'Scope to file (partial match, repeatable, manifesto mode)', collectFile], diff --git a/src/features/check.ts b/src/features/check.ts index 471e7492e..ec3338b19 100644 --- a/src/features/check.ts +++ b/src/features/check.ts @@ -41,23 +41,49 @@ function extractNewFileName(line: string): string | null { return m ? m[1]! : null; } -function pushHunkRanges( - line: string, - currentFile: string, - changedRanges: Map, - oldRanges: Map, -): void { - const hunkMatch = line.match(HUNK_RE); - if (!hunkMatch) return; - const oldStart = parseInt(hunkMatch[1]!, 10); - const oldCount = parseInt(hunkMatch[2] || '1', 10); - if (oldCount > 0) { - oldRanges.get(currentFile)!.push({ start: oldStart, end: oldStart + oldCount - 1 }); +/** + * Tracks the old-side line cursor and the current run of contiguously + * removed lines while walking a hunk body, so `parseDiffOutput` can emit + * precise `oldRanges` (only lines actually deleted/replaced) instead of the + * raw hunk header span (which always includes 3 lines of unchanged context + * on each side per the unified diff format). + */ +class RemovedLineTracker { + private oldLineCursor = 0; + private runStart: number | null = null; + private runEnd: number | null = null; + + startHunk(oldStart: number): void { + this.oldLineCursor = oldStart; + } + + /** + * Consumes one hunk body line, advancing the old-side cursor as needed. + * Lines beginning with `--- ` (source-file headers) are already filtered + * out by the caller before a line ever reaches here, so a leading `-` + * unambiguously marks a removed line — even one whose own content starts + * with dashes (e.g. removing a line of literal text `-- foo`). + */ + consume(line: string, file: string, oldRanges: Map): void { + if (line.startsWith('-')) { + if (this.runStart === null) this.runStart = this.oldLineCursor; + this.runEnd = this.oldLineCursor; + this.oldLineCursor++; + return; + } + // Any non-removed line (context, addition, or a "\ No newline" marker) + // ends the current run of removed lines. + this.flush(file, oldRanges); + if (line.startsWith(' ')) this.oldLineCursor++; } - const newStart = parseInt(hunkMatch[3]!, 10); - const newCount = parseInt(hunkMatch[4] || '1', 10); - if (newCount > 0) { - changedRanges.get(currentFile)!.push({ start: newStart, end: newStart + newCount - 1 }); + + /** Closes out the current removed-line run, if any, into `oldRanges`. */ + flush(file: string, oldRanges: Map): void { + if (this.runStart !== null) { + oldRanges.get(file)!.push({ start: this.runStart, end: this.runEnd! }); + this.runStart = null; + this.runEnd = null; + } } } @@ -67,6 +93,7 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { const newFiles = new Set(); let currentFile: string | null = null; let prevIsDevNull = false; + const removedTracker = new RemovedLineTracker(); for (const line of diffOutput.split('\n')) { if (isDevNullSourceLine(line)) { @@ -79,6 +106,7 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { } const newFile = extractNewFileName(line); if (newFile) { + if (currentFile) removedTracker.flush(currentFile, oldRanges); currentFile = newFile; if (!changedRanges.has(currentFile)) changedRanges.set(currentFile, []); if (!oldRanges.has(currentFile)) oldRanges.set(currentFile, []); @@ -86,8 +114,24 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { prevIsDevNull = false; continue; } - if (currentFile) pushHunkRanges(line, currentFile, changedRanges, oldRanges); + if (!currentFile) continue; + + const hunkMatch = line.match(HUNK_RE); + if (hunkMatch) { + removedTracker.flush(currentFile, oldRanges); + removedTracker.startHunk(parseInt(hunkMatch[1]!, 10)); + const newStart = parseInt(hunkMatch[3]!, 10); + const newCount = parseInt(hunkMatch[4] || '1', 10); + if (newCount > 0) { + changedRanges.get(currentFile)!.push({ start: newStart, end: newStart + newCount - 1 }); + } + continue; + } + + removedTracker.consume(line, currentFile, oldRanges); } + if (currentFile) removedTracker.flush(currentFile, oldRanges); + return { changedRanges, oldRanges, newFiles }; } @@ -209,9 +253,16 @@ export function checkNoSignatureChanges( if (ranges.length === 0) continue; if (noTests && isTestFile(file)) continue; + // Scoped to `exported = 1`: only a symbol reachable from outside its own + // file can have callers this diff doesn't already account for. A + // private, file-local helper's declaration can be freely deleted, + // renamed, or reshaped — every call site lives in the same file and is + // necessarily part of the same diff. Without this filter, any adoption + // of a shared helper that removes a file-local duplicate (the exact + // pattern the grind workflow performs) would always trip this check. const defs = db .prepare( - `SELECT name, kind, file, line FROM nodes WHERE file = ? AND kind IN ('function', 'method', 'class') ORDER BY line`, + `SELECT name, kind, file, line FROM nodes WHERE file = ? AND kind IN ('function', 'method', 'class') AND exported = 1 ORDER BY line`, ) .all(file) as SignatureViolation[]; diff --git a/tests/integration/check.test.ts b/tests/integration/check.test.ts index 9866f20e0..6c4bbd36b 100644 --- a/tests/integration/check.test.ts +++ b/tests/integration/check.test.ts @@ -22,10 +22,12 @@ import { // ─── Helpers ─────────────────────────────────────────────────────────── -function insertNode(db, name, kind, file, line, endLine = null) { +function insertNode(db, name, kind, file, line, endLine = null, exported = 0) { return db - .prepare('INSERT INTO nodes (name, kind, file, line, end_line) VALUES (?, ?, ?, ?, ?)') - .run(name, kind, file, line, endLine).lastInsertRowid; + .prepare( + 'INSERT INTO nodes (name, kind, file, line, end_line, exported) VALUES (?, ?, ?, ?, ?, ?)', + ) + .run(name, kind, file, line, endLine, exported).lastInsertRowid; } function insertEdge(db, sourceId, targetId, kind) { @@ -48,9 +50,11 @@ beforeAll(() => { initSchema(db); // --- Functions --- - // src/math.js: add (line 1-5), multiply (line 7-12) - const add = insertNode(db, 'add', 'function', 'src/math.js', 1, 5); - const multiply = insertNode(db, 'multiply', 'function', 'src/math.js', 7, 12); + // src/math.js: add (line 1-5, exported), multiply (line 7-12, exported), + // roundHalfEven (line 14-16, private — not part of the module's public API) + const add = insertNode(db, 'add', 'function', 'src/math.js', 1, 5, 1); + const multiply = insertNode(db, 'multiply', 'function', 'src/math.js', 7, 12, 1); + insertNode(db, 'roundHalfEven', 'function', 'src/math.js', 14, 16, 0); // src/utils.js: formatResult (line 1-10), parseInput (line 12-20) const formatResult = insertNode(db, 'formatResult', 'function', 'src/utils.js', 1, 10); @@ -101,9 +105,13 @@ describe('parseDiffOutput', () => { '--- a/src/math.js', '+++ b/src/math.js', '@@ -1,3 +1,4 @@', - '-old line', + '-old line 1', + '-old line 2', + '-old line 3', '+new line 1', '+new line 2', + '+new line 3', + '+new line 4', ].join('\n'); const { changedRanges, oldRanges, newFiles } = parseDiffOutput(diff); @@ -113,6 +121,29 @@ describe('parseDiffOutput', () => { expect(newFiles.size).toBe(0); }); + test('old-side ranges exclude unchanged context lines around a removal', () => { + // Simulates deleting a block that sits between two untouched declarations — + // the hunk header span (2..9) must not swallow the untouched line 9. + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -2,8 +2,2 @@', + ' function add() {}', // context, old line 2 — untouched + '-function removedHelper() {', + '- return 1;', + '-}', + '-function alsoRemoved() {', + '- return 2;', + '-}', + ' function multiply() {}', // context, old line 9 — untouched + ].join('\n'); + + const { oldRanges } = parseDiffOutput(diff); + // Only the 6 actually-removed lines (3-8) should appear, not the + // untouched context lines immediately before (2) and after (9). + expect(oldRanges.get('src/math.js')).toEqual([{ start: 3, end: 8 }]); + }); + test('detects new files', () => { const diff = ['--- /dev/null', '+++ b/src/new-file.js', '@@ -0,0 +1,5 @@', '+line1'].join('\n'); @@ -212,6 +243,60 @@ describe('checkNoSignatureChanges', () => { const result = checkNoSignatureChanges(db, oldRanges, true); expect(result.passed).toBe(true); }); + + test('does not flag a private (non-exported) symbol whose declaration was removed', () => { + // roundHalfEven is at line 14, exported=0. Deleting it entirely — the + // exact "adopt shared helper, drop the file-local duplicate" pattern + // grind performs — must not trip this check: every caller of a + // private helper lives in the same file and is already part of the diff. + const oldRanges = new Map([['src/math.js', [{ start: 14, end: 16 }]]]); + const result = checkNoSignatureChanges(db, oldRanges, false); + expect(result.passed).toBe(true); + }); + + test('still flags an exported symbol even when a private symbol shares the file', () => { + // Sanity check that the exported-only filter doesn't accidentally + // suppress real violations on exported declarations in the same file. + const oldRanges = new Map([ + [ + 'src/math.js', + [ + { start: 1, end: 2 }, // add's declaration (exported) + { start: 14, end: 16 }, // roundHalfEven's declaration (private) + ], + ], + ]); + const result = checkNoSignatureChanges(db, oldRanges, false); + expect(result.passed).toBe(false); + expect(result.violations.map((v) => v.name)).toEqual(['add']); + }); + + test('regression: deleting a line near a declaration does not flag it (issue #1760)', () => { + // Fixture DB: `add` spans lines 1-5, `multiply`'s declaration is at + // line 7. A real `git diff` carries 3 lines of context around a + // change, so deleting the single stale line 6 produces a hunk whose + // *header span* (3..9) includes multiply's declaration line (7) even + // though multiply's own text is untouched. parseDiffOutput must only + // record line 6 as removed, not the surrounding context. + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -3,7 +3,6 @@', + ' return 1;', // context, old line 3 (inside add) + ' }', // context, old line 4 + ' ', // context, old line 5 + '-// stale comment', // removed, old line 6 + ' function multiply() {', // context, old line 7 — multiply's declaration + ' return 2;', // context, old line 8 + ' }', // context, old line 9 + ].join('\n'); + + const { oldRanges } = parseDiffOutput(diff); + expect(oldRanges.get('src/math.js')).toEqual([{ start: 6, end: 6 }]); + + const result = checkNoSignatureChanges(db, oldRanges, false); + expect(result.passed).toBe(true); + }); }); // ─── checkNoBoundaryViolations ──────────────────────────────────────── From 2a5853174acc8e89bba9579535bdd8ff12143042 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Sat, 4 Jul 2026 13:40:37 -0600 Subject: [PATCH 2/3] fix(check): track new-side diff lines symmetrically with old-side (#1792) changedRanges was still derived from the raw +start,+count hunk header while oldRanges was precisely line-tracked, so a context-carrying diff passed to the public parseDiffOutput export could silently sweep up untouched lines on the new side. Extend the line tracker to walk both sides of the hunk body symmetrically. docs check acknowledged: internal parsing fix only, no README/CLAUDE.md/ROADMAP.md changes needed. Impact: 7 functions changed, 7 affected --- src/features/check.ts | 114 +++++++++++++++++++++----------- tests/integration/check.test.ts | 26 ++++++++ 2 files changed, 103 insertions(+), 37 deletions(-) diff --git a/src/features/check.ts b/src/features/check.ts index ec3338b19..87c2e7f90 100644 --- a/src/features/check.ts +++ b/src/features/check.ts @@ -42,49 +42,94 @@ function extractNewFileName(line: string): string | null { } /** - * Tracks the old-side line cursor and the current run of contiguously - * removed lines while walking a hunk body, so `parseDiffOutput` can emit - * precise `oldRanges` (only lines actually deleted/replaced) instead of the - * raw hunk header span (which always includes 3 lines of unchanged context - * on each side per the unified diff format). + * Tracks the old-side and new-side line cursors and the current runs of + * contiguously removed/added lines while walking a hunk body, so + * `parseDiffOutput` can emit precise `oldRanges` and `changedRanges` (only + * lines actually deleted/replaced or added) instead of the raw hunk header + * span (which always includes unchanged context lines on each side per the + * unified diff format whenever the diff was produced with non-zero context). + * `getGitDiff` always passes `--unified=0`, so in practice the header span + * and the tracked lines coincide today — but `parseDiffOutput` is a public + * export and this keeps it correct for any diff input, not just this one + * caller's. */ -class RemovedLineTracker { +class DiffLineTracker { private oldLineCursor = 0; - private runStart: number | null = null; - private runEnd: number | null = null; + private newLineCursor = 0; + private removedRunStart: number | null = null; + private removedRunEnd: number | null = null; + private addedRunStart: number | null = null; + private addedRunEnd: number | null = null; - startHunk(oldStart: number): void { + startHunk(oldStart: number, newStart: number): void { this.oldLineCursor = oldStart; + this.newLineCursor = newStart; } /** - * Consumes one hunk body line, advancing the old-side cursor as needed. - * Lines beginning with `--- ` (source-file headers) are already filtered - * out by the caller before a line ever reaches here, so a leading `-` - * unambiguously marks a removed line — even one whose own content starts - * with dashes (e.g. removing a line of literal text `-- foo`). + * Consumes one hunk body line, advancing the old- and new-side cursors as + * needed. Lines beginning with `--- `/`+++ ` (source-file headers) are + * already filtered out by the caller before a line ever reaches here, so a + * leading `-`/`+` unambiguously marks a removed/added line — even one + * whose own content starts with dashes or pluses (e.g. removing a line of + * literal text `-- foo`). */ - consume(line: string, file: string, oldRanges: Map): void { + consume( + line: string, + file: string, + oldRanges: Map, + changedRanges: Map, + ): void { if (line.startsWith('-')) { - if (this.runStart === null) this.runStart = this.oldLineCursor; - this.runEnd = this.oldLineCursor; + this.flushAdded(file, changedRanges); + if (this.removedRunStart === null) this.removedRunStart = this.oldLineCursor; + this.removedRunEnd = this.oldLineCursor; this.oldLineCursor++; return; } - // Any non-removed line (context, addition, or a "\ No newline" marker) - // ends the current run of removed lines. - this.flush(file, oldRanges); - if (line.startsWith(' ')) this.oldLineCursor++; + if (line.startsWith('+')) { + this.flushRemoved(file, oldRanges); + if (this.addedRunStart === null) this.addedRunStart = this.newLineCursor; + this.addedRunEnd = this.newLineCursor; + this.newLineCursor++; + return; + } + // A context line or a "\ No newline" marker ends both runs. + this.flushRemoved(file, oldRanges); + this.flushAdded(file, changedRanges); + if (line.startsWith(' ')) { + this.oldLineCursor++; + this.newLineCursor++; + } } /** Closes out the current removed-line run, if any, into `oldRanges`. */ - flush(file: string, oldRanges: Map): void { - if (this.runStart !== null) { - oldRanges.get(file)!.push({ start: this.runStart, end: this.runEnd! }); - this.runStart = null; - this.runEnd = null; + flushRemoved(file: string, oldRanges: Map): void { + if (this.removedRunStart !== null) { + oldRanges.get(file)!.push({ start: this.removedRunStart, end: this.removedRunEnd! }); + this.removedRunStart = null; + this.removedRunEnd = null; + } + } + + /** Closes out the current added-line run, if any, into `changedRanges`. */ + flushAdded(file: string, changedRanges: Map): void { + if (this.addedRunStart !== null) { + changedRanges.get(file)!.push({ start: this.addedRunStart, end: this.addedRunEnd! }); + this.addedRunStart = null; + this.addedRunEnd = null; } } + + /** Closes out both the removed- and added-line runs, if any. */ + flush( + file: string, + oldRanges: Map, + changedRanges: Map, + ): void { + this.flushRemoved(file, oldRanges); + this.flushAdded(file, changedRanges); + } } export function parseDiffOutput(diffOutput: string): ParsedDiff { @@ -93,7 +138,7 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { const newFiles = new Set(); let currentFile: string | null = null; let prevIsDevNull = false; - const removedTracker = new RemovedLineTracker(); + const tracker = new DiffLineTracker(); for (const line of diffOutput.split('\n')) { if (isDevNullSourceLine(line)) { @@ -106,7 +151,7 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { } const newFile = extractNewFileName(line); if (newFile) { - if (currentFile) removedTracker.flush(currentFile, oldRanges); + if (currentFile) tracker.flush(currentFile, oldRanges, changedRanges); currentFile = newFile; if (!changedRanges.has(currentFile)) changedRanges.set(currentFile, []); if (!oldRanges.has(currentFile)) oldRanges.set(currentFile, []); @@ -118,19 +163,14 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { const hunkMatch = line.match(HUNK_RE); if (hunkMatch) { - removedTracker.flush(currentFile, oldRanges); - removedTracker.startHunk(parseInt(hunkMatch[1]!, 10)); - const newStart = parseInt(hunkMatch[3]!, 10); - const newCount = parseInt(hunkMatch[4] || '1', 10); - if (newCount > 0) { - changedRanges.get(currentFile)!.push({ start: newStart, end: newStart + newCount - 1 }); - } + tracker.flush(currentFile, oldRanges, changedRanges); + tracker.startHunk(parseInt(hunkMatch[1]!, 10), parseInt(hunkMatch[3]!, 10)); continue; } - removedTracker.consume(line, currentFile, oldRanges); + tracker.consume(line, currentFile, oldRanges, changedRanges); } - if (currentFile) removedTracker.flush(currentFile, oldRanges); + if (currentFile) tracker.flush(currentFile, oldRanges, changedRanges); return { changedRanges, oldRanges, newFiles }; } diff --git a/tests/integration/check.test.ts b/tests/integration/check.test.ts index 6c4bbd36b..8113f2f38 100644 --- a/tests/integration/check.test.ts +++ b/tests/integration/check.test.ts @@ -144,6 +144,32 @@ describe('parseDiffOutput', () => { expect(oldRanges.get('src/math.js')).toEqual([{ start: 3, end: 8 }]); }); + test('new-side ranges exclude unchanged context lines around an addition', () => { + // Symmetric counterpart of the old-side test above: `getGitDiff` always + // uses `--unified=0` so this never happens in practice, but + // `parseDiffOutput` is a public export and must stay correct for any + // unified diff, including ones with non-zero context. + const diff = [ + '--- a/src/math.js', + '+++ b/src/math.js', + '@@ -2,2 +2,8 @@', + ' function add() {}', // context, line 2 — untouched + '+function addedHelper() {', + '+ return 1;', + '+}', + '+function alsoAdded() {', + '+ return 2;', + '+}', + ' function multiply() {}', // context, line 9 — untouched + ].join('\n'); + + const { changedRanges } = parseDiffOutput(diff); + // Only the 6 actually-added lines (3-8) should appear, not the untouched + // context lines immediately before (2) and after (9) — even though the + // raw hunk header span for the new side is 2..9. + expect(changedRanges.get('src/math.js')).toEqual([{ start: 3, end: 8 }]); + }); + test('detects new files', () => { const diff = ['--- /dev/null', '+++ b/src/new-file.js', '@@ -0,0 +1,5 @@', '+line1'].join('\n'); From c4f8cea9c995f24a55907065dec43e87a3a617e8 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Sun, 5 Jul 2026 01:40:15 -0600 Subject: [PATCH 3/3] fix: guard +++ /dev/null deletion header in parseDiffOutput (#1792) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DiffLineTracker refactor actively consumes any +/--prefixed hunk body line, but a `+++ /dev/null` deletion header isn't `b/`-prefixed so it fell through every existing guard and was misread as an added line under whichever file preceded it in the diff — corrupting that file's changedRanges and then misattributing the deleted file's removed lines to it via the next hunk header. Add an explicit guard that flushes and clears the file context on `+++ /dev/null`, and a regression test covering a deleted file following a modified one. docs check acknowledged: internal bug fix in check.ts diff parsing, no README/CLAUDE.md/ROADMAP changes needed. Impact: 2 functions changed, 5 affected --- src/features/check.ts | 18 ++++++++++++++++++ tests/integration/check.test.ts | 31 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/features/check.ts b/src/features/check.ts index 87c2e7f90..2e9fc9df6 100644 --- a/src/features/check.ts +++ b/src/features/check.ts @@ -41,6 +41,11 @@ function extractNewFileName(line: string): string | null { return m ? m[1]! : null; } +/** Returns true if the diff line marks the new file as /dev/null (file deletion). */ +function isDevNullTargetLine(line: string): boolean { + return line.startsWith('+++ /dev/null'); +} + /** * Tracks the old-side and new-side line cursors and the current runs of * contiguously removed/added lines while walking a hunk body, so @@ -159,6 +164,19 @@ export function parseDiffOutput(diffOutput: string): ParsedDiff { prevIsDevNull = false; continue; } + if (isDevNullTargetLine(line)) { + // `+++ /dev/null` (file deletion) is not `b/`-prefixed, so + // extractNewFileName returned null above and this line would otherwise + // fall through to tracker.consume and be misread as an added source + // line under whichever file preceded this one in the diff. Flush and + // clear the file context instead — the deleted file's hunk body that + // follows has no corresponding DB entry to check against anyway (its + // nodes are purged from the graph), so there is nothing to track here. + if (currentFile) tracker.flush(currentFile, oldRanges, changedRanges); + currentFile = null; + prevIsDevNull = false; + continue; + } if (!currentFile) continue; const hunkMatch = line.match(HUNK_RE); diff --git a/tests/integration/check.test.ts b/tests/integration/check.test.ts index 8113f2f38..3fc78fbbc 100644 --- a/tests/integration/check.test.ts +++ b/tests/integration/check.test.ts @@ -179,6 +179,37 @@ describe('parseDiffOutput', () => { expect(oldRanges.get('src/new-file.js')).toEqual([]); }); + test('a deleted file does not corrupt the ranges of the preceding file', () => { + // Regression test: `+++ /dev/null` (file deletion) is not `b/`-prefixed, + // so it was previously missed by every guard and fell through to + // tracker.consume, which recorded it as an added line under whichever + // file preceded it in the diff — corrupting that file's changedRanges + // and then misattributing the deleted file's removed lines to it too. + const diff = [ + '--- a/src/kept.js', + '+++ b/src/kept.js', + '@@ -1,1 +1,1 @@', + '-old kept line', + '+new kept line', + '--- a/src/removed.js', + '+++ /dev/null', + '@@ -1,2 +0,0 @@', + '-deleted line 1', + '-deleted line 2', + ].join('\n'); + + const { changedRanges, oldRanges } = parseDiffOutput(diff); + // kept.js's ranges must reflect only its own hunk, not the deleted + // file's `+++ /dev/null` header or removed body lines. + expect(changedRanges.get('src/kept.js')).toEqual([{ start: 1, end: 1 }]); + expect(oldRanges.get('src/kept.js')).toEqual([{ start: 1, end: 1 }]); + // The deleted file itself is not tracked — its nodes are purged from the + // DB during the rebuild, so there is nothing for the check predicates to + // match against. + expect(changedRanges.has('src/removed.js')).toBe(false); + expect(oldRanges.has('src/removed.js')).toBe(false); + }); + test('handles multiple files', () => { const diff = [ '--- a/src/a.js',