Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions review-enrichment/src/analyzers/dependency-scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
} from "../types.js";
import type { AnalysisContext } from "../analysis-context.js";
import { boundedFetchJson } from "../external-fetch.js";
import { isDiffFileHeaderLine } from "./diff-lines.js";

export interface DepChange {
ecosystem: string;
Expand Down Expand Up @@ -107,11 +108,7 @@ export function extractDependencyChanges(
if (manifestFiles > maxManifestFiles) break;
for (const line of file.patch.split("\n", maxPatchLinesPerFile)) {
const sign = line[0];
if (
(sign !== "+" && sign !== "-") ||
line.startsWith("+++ ") ||
line.startsWith("---")
)
if ((sign !== "+" && sign !== "-") || isDiffFileHeaderLine(line))
continue;
const parsed = parseLine(manifest, line.slice(1).trim());
if (!parsed) continue;
Expand Down
3 changes: 2 additions & 1 deletion review-enrichment/src/analyzers/lockfile-drift.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
import type { AnalysisContext } from "../analysis-context.js";
import { extractDependencyChanges } from "./dependency-scan.js";
import { boundedFetchJson } from "../external-fetch.js";
import { isDiffFileHeaderLine } from "./diff-lines.js";
import { isParseableLockfile, lockfileBasename } from "../lockfile-path.js";

interface LockfileChange {
Expand Down Expand Up @@ -126,7 +127,7 @@ function* patchLines(
for (const raw of patch.split("\n")) {
seen += 1;
if (seen > maxLines) break;
if (raw.startsWith("+++ ") || raw.startsWith("---")) continue;
if (isDiffFileHeaderLine(raw)) continue;
const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(raw);
if (hunk) {
newLine = Number(hunk[1]);
Expand Down
28 changes: 28 additions & 0 deletions review-enrichment/test/dependency-scan.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { test } from "node:test";
import assert from "node:assert/strict";

import { extractDependencyChanges } from "../dist/analyzers/dependency-scan.js";

test("extractDependencyChanges skips real file headers via the shared discriminator, not spurious deps", () => {
// The patch carries the unified-diff file headers (`--- a/…`, `+++ b/…`) ahead of the hunk. They must be
// skipped as headers — never parsed as dependency lines — while the real version bump is extracted. This
// pins the behavior after swapping the anchored `startsWith("+++ ")`/`startsWith("---")` guard for the
// shared isDiffFileHeaderLine helper (which only matches `+++ a/`/`b/`/`/dev/null` headers).
const changes = extractDependencyChanges([
{
path: "package.json",
patch: [
"--- a/package.json",
"+++ b/package.json",
"@@ -5,3 +5,3 @@",
' "dependencies": {',
'- "lodash": "4.17.20",',
'+ "lodash": "4.17.21",',
].join("\n"),
},
]);

assert.deepEqual(changes, [
{ ecosystem: "npm", package: "lodash", from: "4.17.20", to: "4.17.21" },
]);
});
29 changes: 29 additions & 0 deletions review-enrichment/test/lockfile-drift.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,35 @@ test("extractLockfileChanges matches lockfile basenames case-insensitively", ()
]);
});

test("extractLockfileChanges keeps new-file line numbers correct across ++-content added lines", () => {
// An added line whose CONTENT begins with `++ ` renders in the diff as `+++ …`. The old anchored
// `startsWith("+++ ")` guard mistook it for a `+++ b/file` header and `continue`d WITHOUT advancing the
// new-file line counter, so every finding AFTER it was reported one line too low. The shared
// isDiffFileHeaderLine helper only skips real `+++ a/`/`b/`/`/dev/null` headers, so the counter stays true.
const changes = extractLockfileChanges([
{
path: "package-lock.json",
patch: [
"@@ -9,0 +10,3 @@",
'+ "node_modules/lodash": {', // new-file line 10
"+++ not a header — added content whose text begins with ++", // new-file line 11 (must be counted)
'+ "version": "4.17.21"', // new-file line 12
].join("\n"),
},
]);

assert.deepEqual(changes, [
{
file: "package-lock.json",
line: 12, // 12, not 11 — the intervening ++-content line is counted, not swallowed as a header
ecosystem: "npm",
package: "lodash",
from: null,
to: "4.17.21",
},
]);
});

test("extractLockfileChanges does not let unparsed lockfiles consume the scan budget", () => {
const yarnPatch = [
"@@ -1,0 +1,2 @@",
Expand Down
Loading