diff --git a/packages/core/test/patch.test.ts b/packages/core/test/patch.test.ts index d3ff40b7869..18dd3695ce0 100644 --- a/packages/core/test/patch.test.ts +++ b/packages/core/test/patch.test.ts @@ -22,6 +22,30 @@ describe("Patch", () => { ]) }) + test("parses an empty patch", () => { + expect(parse("*** Begin Patch\n*** End Patch")).toEqual([]) + }) + + test("ignores a Codex environment preamble", () => { + expect( + parse("*** Begin Patch\n*** Environment ID: remote\n*** Add File: file.txt\n+content\n*** End Patch"), + ).toEqual([{ type: "add", path: "file.txt", contents: "content" }]) + }) + + test("parses an update followed by an add", () => { + expect( + parse("*** Begin Patch\n*** Update File: update.txt\n@@\n+line\n*** Add File: add.txt\n+content\n*** End Patch"), + ).toEqual([ + { + type: "update", + path: "update.txt", + movePath: undefined, + chunks: [{ oldLines: [], newLines: ["line"], changeContext: undefined }], + }, + { type: "add", path: "add.txt", contents: "content" }, + ]) + }) + test("parses a file move", () => { expect( parse("*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-old\n+new\n*** End Patch"), @@ -66,6 +90,20 @@ describe("Patch", () => { ]) }) + test("strips quoted heredoc wrappers", () => { + const patch = "*** Begin Patch\n*** Add File: add.txt\n+added\n*** End Patch" + expect(parse(`<<'EOF'\n${patch}\nEOF`)).toEqual([{ type: "add", path: "add.txt", contents: "added" }]) + expect(parse(`<<\"EOF\"\n${patch}\nEOF`)).toEqual([{ type: "add", path: "add.txt", contents: "added" }]) + }) + + test("rejects malformed heredoc wrappers", () => { + const patch = "*** Begin Patch\n*** Add File: add.txt\n+added\n*** End Patch" + expect(() => parse(`<<\"EOF'\n${patch}\nEOF`)).toThrow("The first line of the patch must be '*** Begin Patch'") + expect(() => parse("< { expect(parse("*** Begin Patch\n *** Update File: foo.txt\n@@\n-old\n+new\n*** End Patch")).toEqual([ { @@ -99,6 +137,23 @@ describe("Patch", () => { ]) }) + test("parses relative and absolute hunk paths", () => { + expect( + parse( + "*** Begin Patch\n*** Add File: relative.txt\n+content\n*** Delete File: /tmp/delete.txt\n*** Update File: /tmp/update.txt\n@@\n-old\n+new\n*** End Patch", + ), + ).toEqual([ + { type: "add", path: "relative.txt", contents: "content" }, + { type: "delete", path: "/tmp/delete.txt" }, + { + type: "update", + path: "/tmp/update.txt", + movePath: undefined, + chunks: [{ oldLines: ["old"], newLines: ["new"], changeContext: undefined }], + }, + ]) + }) + test("strips one carriage return from CRLF patch lines", () => { expect(parse("*** Begin Patch\r\n*** Update File: file.txt\r\n@@\r\n-old\r\n+new\r\n*** End Patch\r\n")).toEqual([ { @@ -136,6 +191,42 @@ describe("Patch", () => { ]) }) + test("allows an end-of-file marker before an explicit chunk", () => { + expect( + parse("*** Begin Patch\n*** Update File: file.txt\n*** End of File\n@@\n-old\n+new\n*** End Patch"), + ).toEqual([ + { + type: "update", + path: "file.txt", + movePath: undefined, + chunks: [{ oldLines: ["old"], newLines: ["new"], changeContext: undefined }], + }, + ]) + }) + + test("allows an end-of-file marker before an implicit chunk and move", () => { + expect(parse("*** Begin Patch\n*** Update File: file.txt\n*** End of File\n-old\n+new\n*** End Patch")).toEqual([ + { + type: "update", + path: "file.txt", + movePath: undefined, + chunks: [{ oldLines: ["old"], newLines: ["new"] }], + }, + ]) + expect( + parse( + "*** Begin Patch\n*** Update File: old.txt\n*** End of File\n*** Move to: new.txt\n@@\n-old\n+new\n*** End Patch", + ), + ).toEqual([ + { + type: "update", + path: "old.txt", + movePath: "new.txt", + chunks: [{ oldLines: ["old"], newLines: ["new"], changeContext: undefined }], + }, + ]) + }) + test("derives fuzzy line updates while preserving BOM", () => { const update = Patch.derive("update.txt", [{ oldLines: [" old "], newLines: ["new"] }], "\uFEFFold\n") expect(update).toEqual({ content: "new\n", bom: true }) @@ -236,15 +327,157 @@ describe("Patch", () => { ).toThrow("Failed to find expected lines") }) - test("matches V1 lenient parsing of malformed hunk bodies", () => { - expect(parse("*** Begin Patch\n*** Add File: add.txt\nmissing plus\n*** End Patch")).toEqual([ - { type: "add", path: "add.txt", contents: "" }, - ]) - expect(parse("*** Begin Patch\n*** Update File: update.txt\n*** End Patch")).toEqual([ - { type: "update", path: "update.txt", movePath: undefined, chunks: [] }, - ]) - expect(parse("*** Begin Patch\n*** Delete File: delete.txt\nunexpected body\n*** End Patch")).toEqual([ - { type: "delete", path: "delete.txt" }, + test("parses an update without an explicit first chunk header", () => { + expect(parse("*** Begin Patch\n*** Update File: file.txt\n import foo\n+bar\n*** End Patch")).toEqual([ + { + type: "update", + path: "file.txt", + movePath: undefined, + chunks: [{ oldLines: ["import foo"], newLines: ["import foo", "bar"] }], + }, ]) }) + + test("keeps indented update markers as context lines", () => { + expect( + parse( + "*** Begin Patch\n*** Update File: a.txt\n@@\n-old a\n+new a\n *** Update File: b.txt\n@@\n-old b\n+new b\n*** End Patch", + ), + ).toEqual([ + { + type: "update", + path: "a.txt", + movePath: undefined, + chunks: [ + { + oldLines: ["old a", "*** Update File: b.txt"], + newLines: ["new a", "*** Update File: b.txt"], + changeContext: undefined, + }, + { oldLines: ["old b"], newLines: ["new b"], changeContext: undefined }, + ], + }, + ]) + }) + + test("keeps indented move and EOF markers as context lines", () => { + expect( + parse( + "*** Begin Patch\n*** Update File: file.txt\n@@\n before\n *** Move to: moved.txt\n *** End of File\n*** End Patch", + ), + ).toEqual([ + { + type: "update", + path: "file.txt", + movePath: undefined, + chunks: [ + { + oldLines: ["before", "*** Move to: moved.txt", "*** End of File"], + newLines: ["before", "*** Move to: moved.txt", "*** End of File"], + changeContext: undefined, + }, + ], + }, + ]) + }) + + test("preserves update context indentation", () => { + expect(parse("*** Begin Patch\n*** Update File: file.txt\n@@ section\n-old\n+new\n*** End Patch")).toEqual([ + { + type: "update", + path: "file.txt", + movePath: undefined, + chunks: [{ oldLines: ["old"], newLines: ["new"], changeContext: " section" }], + }, + ]) + }) + + test("preserves bare empty update lines as context", () => { + expect( + parse("*** Begin Patch\n*** Update File: file.txt\n@@\n context before\n\n context after\n*** End Patch"), + ).toEqual([ + { + type: "update", + path: "file.txt", + movePath: undefined, + chunks: [ + { + oldLines: ["context before", "", "context after"], + newLines: ["context before", "", "context after"], + changeContext: undefined, + }, + ], + }, + ]) + }) + + test("rejects invalid add and delete lines", () => { + expect(() => parse("*** Begin Patch\n*** Add File: file.txt\nbad\n*** End Patch")).toThrow( + "Invalid hunk at line 3: 'bad' is not a valid hunk header", + ) + expect(() => parse("*** Begin Patch\n*** Delete File: file.txt\nbad\n*** End Patch")).toThrow( + "Invalid hunk at line 3: 'bad' is not a valid hunk header", + ) + }) + + test("rejects an empty update hunk", () => { + expect(() => parse("*** Begin Patch\n*** Update File: file.txt\n*** End Patch")).toThrow( + "Invalid hunk at line 2: Update file hunk for path 'file.txt' is empty", + ) + expect(() => + parse( + "*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n*** Delete File: other.txt\n*** End Patch", + ), + ).toThrow("Invalid hunk at line 2: Update file hunk for path 'old.txt' is empty") + expect(() => parse("*** Begin Patch\n*** Update File: file.txt\n*** End of File\n*** End Patch")).toThrow( + "Invalid hunk at line 2: Update file hunk for path 'file.txt' is empty", + ) + }) + + test("rejects an empty update chunk", () => { + expect(() => parse("*** Begin Patch\n*** Update File: file.txt\n@@\n*** End Patch")).toThrow( + "Invalid hunk at line 4: Update hunk does not contain any lines", + ) + expect(() => parse("*** Begin Patch\n*** Update File: file.txt\n@@\n*** End of File\n*** End Patch")).toThrow( + "Invalid hunk at line 4: Update hunk does not contain any lines", + ) + expect(() => parse("*** Begin Patch\n*** Update File: file.txt\n@@\n@@\n-old\n+new\n*** End Patch")).toThrow( + "Invalid hunk at line 4: Unexpected line found in update hunk: '@@'", + ) + expect(() => + parse("*** Begin Patch\n*** Update File: file.txt\n@@\n*** Update File: other.txt\n@@\n-old\n+new\n*** End Patch"), + ).toThrow("Invalid hunk at line 4: Unexpected line found in update hunk: '*** Update File: other.txt'") + expect(() => parse("*** Begin Patch\n*** Update File: file.txt\n@@\nbad\n*** End Patch")).toThrow( + "Invalid hunk at line 4: Unexpected line found in update hunk: 'bad'", + ) + }) + + test("rejects an invalid update line", () => { + expect(() => parse("*** Begin Patch\n*** Update File: file.txt\n@@\n-old\nbad\n*** End Patch")).toThrow( + "Invalid hunk at line 5: Expected update hunk to start with a @@ context marker, got: 'bad'", + ) + expect(() => parse("*** Begin Patch\n*** Update File: file.txt\n@@foo\n*** End Patch")).toThrow( + "Invalid hunk at line 3: Unexpected line found in update hunk: '@@foo'", + ) + expect(() => parse("*** Begin Patch\n*** Update File: file.txt\n@@\n-old\n*** Frobnicate File: foo\n*** End Patch")).toThrow( + "Invalid hunk at line 5: Expected update hunk to start with a @@ context marker, got: '*** Frobnicate File: foo'", + ) + }) + + test("rejects invalid and pathless hunk headers", () => { + expect(() => parse("*** Begin Patch\n*** Frobnicate File: foo\n*** End Patch")).toThrow( + "Invalid hunk at line 2: '*** Frobnicate File: foo' is not a valid hunk header", + ) + expect(() => parse("*** Begin Patch\n*** Add File:\n*** End Patch")).toThrow( + "Invalid hunk at line 2: '*** Add File:' is not a valid hunk header", + ) + for (const header of ["*** Add File: ", "*** Delete File: ", "*** Update File: "]) { + expect(() => parse(`*** Begin Patch\n${header}\n*** End Patch`)).toThrow( + `Invalid hunk at line 2: '${header.trim()}' is not a valid hunk header`, + ) + } + expect(() => + parse("*** Begin Patch\n*** Update File: old.txt\n*** Move to: \n@@\n-old\n+new\n*** End Patch"), + ).toThrow("Invalid hunk at line 3: '*** Move to:' is not a valid hunk header") + }) }) diff --git a/packages/util/src/patch.ts b/packages/util/src/patch.ts index d3aeb73fd91..843e14efa14 100644 --- a/packages/util/src/patch.ts +++ b/packages/util/src/patch.ts @@ -13,8 +13,10 @@ export class BoundaryError extends Schema.TaggedErrorClass()("Pat export class InvalidHunkError extends Schema.TaggedErrorClass()("Patch.InvalidHunkError", { line: Schema.String, lineNumber: Schema.Number, + reason: Schema.optional(Schema.String), }) { override get message() { + if (this.reason) return `Invalid hunk at line ${this.lineNumber}: ${this.reason}` return `Invalid hunk at line ${this.lineNumber}: '${this.line}' is not a valid hunk header. Valid hunk headers: '*** Add File: {path}', '*** Delete File: {path}', '*** Update File: {path}'` } } @@ -57,51 +59,48 @@ export function parse(patchText: string): Result.Result, Par while (index < end) { const line = lines[index]! const header = line.trim() - if (header.startsWith("*** Add File:")) { - const path = header.slice("*** Add File:".length).trim() - if (!path) { - index++ - continue - } + if ( + index === begin + 1 && + header.startsWith("*** Environment ID:") && + header.slice("*** Environment ID:".length).trim() + ) { + index++ + continue + } + if (header.startsWith("*** Add File: ")) { + const path = header.slice("*** Add File: ".length).trim() const parsed = parseAdd(lines, index + 1, end) + if ("error" in parsed) return Result.fail(parsed.error) hunks.push({ type: "add", path, contents: parsed.content }) index = parsed.next continue } - if (header.startsWith("*** Delete File:")) { - const path = header.slice("*** Delete File:".length).trim() - if (!path) { - index++ - continue - } + if (header.startsWith("*** Delete File: ")) { + const path = header.slice("*** Delete File: ".length).trim() hunks.push({ type: "delete", path }) index++ continue } - if (header.startsWith("*** Update File:")) { - const path = header.slice("*** Update File:".length).trim() - if (!path) { - index++ - continue - } + if (header.startsWith("*** Update File: ")) { + const path = header.slice("*** Update File: ".length).trim() let next = index + 1 let movePath: string | undefined - if (lines[next]?.startsWith("*** Move to:")) { - movePath = lines[next]!.slice("*** Move to:".length).trim() + while (lines[next]?.trimEnd() === "*** End of File") next++ + const move = lines[next]?.trimEnd() + if (move === "*** Move to:" || move?.startsWith("*** Move to: ")) { + movePath = move.slice("*** Move to: ".length).trim() + if (!movePath) { + return Result.fail(new InvalidHunkError({ line: lines[next]!.trim(), lineNumber: next + 1 })) + } next++ } - const parsed = parseUpdate(lines, next, end) + const parsed = parseUpdate(lines, next, end, path, index) + if ("error" in parsed) return Result.fail(parsed.error) hunks.push({ type: "update", path, movePath, chunks: parsed.chunks }) index = parsed.next continue } - index++ - } - if (hunks.length === 0) { - const invalid = lines.findIndex((line, index) => index > begin && index < end && line.trim() !== "") - if (invalid !== -1) { - return Result.fail(new InvalidHunkError({ line: lines[invalid]!.trim(), lineNumber: invalid + 1 })) - } + return Result.fail(new InvalidHunkError({ line: header, lineNumber: index + 1 })) } return Result.succeed(hunks) } @@ -123,47 +122,166 @@ export function joinBom(text: string, bom: boolean) { return bom ? `\uFEFF${stripped}` : stripped } -function parseAdd(lines: ReadonlyArray, start: number, end: number) { +function parseAdd( + lines: ReadonlyArray, + start: number, + end: number, +): { content: string; next: number } | { error: InvalidHunkError } { const content: string[] = [] let index = start - while (index < end && !lines[index]!.startsWith("***")) { - if (lines[index]!.startsWith("+")) content.push(lines[index]!.slice(1)) + while (index < end && !isBoundary(lines[index]!.trim())) { + if (!lines[index]!.startsWith("+")) { + return { error: new InvalidHunkError({ line: lines[index]!.trim(), lineNumber: index + 1 }) } + } + content.push(lines[index]!.slice(1)) index++ } return { content: content.join("\n"), next: index } } -function parseUpdate(lines: ReadonlyArray, start: number, end: number) { - const chunks: UpdateFileChunk[] = [] +function parseUpdate( + lines: ReadonlyArray, + start: number, + end: number, + path: string, + hunk: number, +): { chunks: ReadonlyArray; next: number } | { error: InvalidHunkError } { + const chunks: Array<{ + oldLines: string[] + newLines: string[] + changeContext?: string + endOfFile?: boolean + }> = [] let index = start - while (index < end && !lines[index]!.startsWith("***")) { - if (!lines[index]!.startsWith("@@")) { + let afterEndOfFile = false + while (index < end) { + const line = lines[index]! + const updateLine = line.trimEnd() + if (afterEndOfFile) { + if (updateLine === "") { + index++ + continue + } + if (updateLine === "@@" || updateLine.startsWith("@@ ")) afterEndOfFile = false + else if (isBoundary(updateLine)) break + else { + return { + error: new InvalidHunkError({ + line, + lineNumber: index + 1, + reason: `Expected update hunk to start with a @@ context marker, got: '${line}'`, + }), + } + } + } + if (updateLine === "*** End of File") { + const chunk = chunks.at(-1) + if (chunk && chunk.oldLines.length === 0 && chunk.newLines.length === 0) { + return { + error: new InvalidHunkError({ + line: updateLine, + lineNumber: index + 1, + reason: "Update hunk does not contain any lines", + }), + } + } + if (chunk) { + chunk.endOfFile = true + afterEndOfFile = true + } index++ continue } - const changeContext = lines[index]!.slice(2).trim() || undefined - const oldLines: string[] = [] - const newLines: string[] = [] - let endOfFile = false - index++ - while (index < end && !lines[index]!.startsWith("@@") && !lines[index]!.startsWith("***")) { - const line = lines[index]! - if (line.startsWith(" ")) { - oldLines.push(line.slice(1)) - newLines.push(line.slice(1)) - } else if (line.startsWith("-")) oldLines.push(line.slice(1)) - else if (line.startsWith("+")) newLines.push(line.slice(1)) + if (isBoundary(updateLine)) break + if (updateLine === "@@" || updateLine.startsWith("@@ ")) { + const previous = chunks.at(-1) + if (previous && previous.oldLines.length === 0 && previous.newLines.length === 0) { + return { + error: new InvalidHunkError({ + line, + lineNumber: index + 1, + reason: `Unexpected line found in update hunk: '${line}'. Every line should start with ' ' (context line), '+' (added line), or '-' (removed line)`, + }), + } + } + chunks.push({ + oldLines: [], + newLines: [], + changeContext: updateLine === "@@" ? undefined : updateLine.slice("@@ ".length), + }) index++ + continue } - if (lines[index]?.trim() === "*** End of File") { - endOfFile = true + if (chunks.length === 0) chunks.push({ oldLines: [], newLines: [] }) + const chunk = chunks.at(-1)! + if (line === "") { + chunk.oldLines.push("") + chunk.newLines.push("") index++ + continue + } + if (line.startsWith(" ")) { + chunk.oldLines.push(line.slice(1)) + chunk.newLines.push(line.slice(1)) + index++ + continue + } + if (line.startsWith("-")) { + chunk.oldLines.push(line.slice(1)) + index++ + continue + } + if (line.startsWith("+")) { + chunk.newLines.push(line.slice(1)) + index++ + continue + } + const populated = chunk.oldLines.length > 0 || chunk.newLines.length > 0 + return { + error: new InvalidHunkError({ + line, + lineNumber: index + 1, + reason: populated + ? `Expected update hunk to start with a @@ context marker, got: '${line}'` + : `Unexpected line found in update hunk: '${line}'. Every line should start with ' ' (context line), '+' (added line), or '-' (removed line)`, + }), + } + } + if (chunks.length === 0) { + return { + error: new InvalidHunkError({ + line: lines[hunk]!.trim(), + lineNumber: hunk + 1, + reason: `Update file hunk for path '${path}' is empty`, + }), + } + } + const last = chunks.at(-1)! + if (last.oldLines.length === 0 && last.newLines.length === 0) { + const line = lines[index]!.trim() + return { + error: new InvalidHunkError({ + line, + lineNumber: index + 1, + reason: + line === "*** End Patch" + ? "Update hunk does not contain any lines" + : `Unexpected line found in update hunk: '${line}'. Every line should start with ' ' (context line), '+' (added line), or '-' (removed line)`, + }), } - chunks.push({ oldLines, newLines, changeContext, endOfFile: endOfFile || undefined }) } return { chunks, next: index } } +function isBoundary(line: string) { + return ( + line === "*** End Patch" || + line.startsWith("*** Add File: ") || + line.startsWith("*** Delete File: ") || + line.startsWith("*** Update File: ") + ) +} + function computeReplacements(lines: ReadonlyArray, path: string, chunks: ReadonlyArray) { const replacements: Array]> = [] let lineIndex = 0 @@ -231,5 +349,4 @@ const normalize = (value: string) => .replace(/[\u00A0\u2002-\u200A\u202F\u205F\u3000]/g, " ") const splitBom = (text: string) => text.startsWith("\uFEFF") ? { bom: true, text: text.slice(1) } : { bom: false, text } -const stripHeredoc = (input: string) => - input.match(/^(?:cat\s+)?<<['"]?(\w+)['"]?\s*\n([\s\S]*?)\n\1\s*$/)?.[2] ?? input +const stripHeredoc = (input: string) => input.match(/^(?:cat\s+)?<<(['"]?)(\w+)\1\s*\n([\s\S]*?)\n\2\s*$/)?.[3] ?? input