Add a well-formedness check for a parsed patch

Parse is lenient: it takes any text and reads a diff out of it, which is
what we want when we hand it a diff, but it has no way to say "this
isn't one". We're about to parse the *rendered* contents of a diff view,
which a diff renderer is free to restructure — putting the line numbers
in a gutter, say, shifts the +/- marker off the start of each body line,
so every line reads as context and the parse silently lies about which
lines are changes.

Comparing each hunk's body against the lengths its header declares
catches exactly that, without teaching us anything about any particular
renderer's layout: a faithful unified diff agrees with its headers, a
restructured one doesn't.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller
2026-10-04 19:01:01 +02:00
co-authored by Claude Opus 5
parent 43a4ba3666
commit db6a04b8a5
4 changed files with 171 additions and 11 deletions
+5
View File
@@ -16,6 +16,11 @@ type Hunk struct {
newStart int
// the context at the end of the header line (' func (f *CommitFile) Description() string {' in the above example)
headerContext string
// the lengths declared in the header line ('2' and '3' in the above example),
// kept so that we can check the parsed body against them (see
// Patch.IsWellFormed). Only set by Parse.
declaredOldLength int
declaredNewLength int
// the body of the hunk, excluding the header line
bodyLines []*PatchLine
}
+26 -11
View File
@@ -7,7 +7,9 @@ import (
"github.com/jesseduffield/lazygit/pkg/utils"
)
var hunkHeaderRegexp = regexp.MustCompile(`(?m)^@@ -(\d+)[^\+]+\+(\d+)[^@]+@@(.*)$`)
// Captures, in order: the old start, the old length (omitted by git when it is
// 1), the new start, the new length (likewise), and the trailing context.
var hunkHeaderRegexp = regexp.MustCompile(`(?m)^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)$`)
func Parse(patchStr string) *Patch {
// ignore trailing newline.
@@ -19,13 +21,15 @@ func Parse(patchStr string) *Patch {
var currentHunk *Hunk
for _, line := range lines {
if strings.HasPrefix(line, "@@") {
oldStart, newStart, headerContext := headerInfo(line)
oldStart, oldLength, newStart, newLength, headerContext := headerInfo(line)
currentHunk = &Hunk{
oldStart: oldStart,
newStart: newStart,
headerContext: headerContext,
bodyLines: []*PatchLine{},
oldStart: oldStart,
newStart: newStart,
declaredOldLength: oldLength,
declaredNewLength: newLength,
headerContext: headerContext,
bodyLines: []*PatchLine{},
}
hunks = append(hunks, currentHunk)
} else if currentHunk != nil {
@@ -41,14 +45,25 @@ func Parse(patchStr string) *Patch {
}
}
func headerInfo(header string) (int, int, string) {
func headerInfo(header string) (oldStart int, oldLength int, newStart int, newLength int, headerContext string) {
match := hunkHeaderRegexp.FindStringSubmatch(header)
oldStart := utils.MustConvertToInt(match[1])
newStart := utils.MustConvertToInt(match[2])
headerContext := match[3]
oldStart = utils.MustConvertToInt(match[1])
oldLength = declaredLength(match[2])
newStart = utils.MustConvertToInt(match[3])
newLength = declaredLength(match[4])
headerContext = match[5]
return oldStart, newStart, headerContext
return oldStart, oldLength, newStart, newLength, headerContext
}
// declaredLength parses a length capture of a hunk header, which git omits when
// it is 1 (e.g. "@@ -0,0 +1 @@").
func declaredLength(match string) int {
if match == "" {
return 1
}
return utils.MustConvertToInt(match)
}
func newHunkLine(line string) *PatchLine {
+38
View File
@@ -79,6 +79,44 @@ func (self *Patch) HunkEndIdx(hunkIndex int) int {
return self.HunkStartIdx(hunkIndex) + self.hunks[hunkIndex].lineCount() - 1
}
// IsWellFormed reports whether every hunk's body matches the lengths declared in
// its header. A faithful unified diff always satisfies this; a rendering that
// restructured the diff body does not — a diff renderer that puts line numbers in
// a gutter, say, shifts the +/- marker off the start of each line, so every body
// line reads as context and the computed lengths no longer match the header. That
// makes this the test for whether a rendered diff can be parsed as a unified diff
// at all, rather than trusting a mis-parse. Only meaningful for patches produced
// by Parse, which is where the declared lengths come from.
func (self *Patch) IsWellFormed() bool {
return self.isWellFormed(false)
}
// IsWellFormedSoFar is IsWellFormed for a patch parsed from a diff we have only the
// beginning of. Its last hunk holds the first lines of a body that hasn't all arrived,
// so every hunk but the last has to match its header exactly, as before, while the last
// one only has to fit within what its header declares.
//
// The check exists to tell a faithful rendering from a restructured one, and it still
// does that. A rendering that moves the +/- marker off the start of the line makes us
// read a change as context, and a context line counts towards both lengths, so such a
// hunk comes out longer than its header declares rather than shorter.
func (self *Patch) IsWellFormedSoFar() bool {
return self.isWellFormed(true)
}
func (self *Patch) isWellFormed(lastHunkMayBeIncomplete bool) bool {
for i, hunk := range self.hunks {
if lastHunkMayBeIncomplete && i == len(self.hunks)-1 {
return hunk.oldLength() <= hunk.declaredOldLength &&
hunk.newLength() <= hunk.declaredNewLength
}
if hunk.oldLength() != hunk.declaredOldLength || hunk.newLength() != hunk.declaredNewLength {
return false
}
}
return true
}
func (self *Patch) ContainsChanges() bool {
return lo.SomeBy(self.hunks, func(hunk *Hunk) bool {
return hunk.containsChanges()
+102
View File
@@ -696,6 +696,108 @@ func TestLineNumberOfLine(t *testing.T) {
}
}
func TestIsWellFormed(t *testing.T) {
// The body of a diff as rendered with the +/- markers moved out of the text
// and into a gutter: every body line now reads as context, so the lengths no
// longer match the header.
const gutterMangled = `diff --git a/filename b/filename
index 9320895..6d79956 100644
--- a/filename
+++ b/filename
@@ -1,4 +1,2 @@
apple
grape
pear
lemon
`
scenarios := []struct {
testName string
patchStr string
expected bool
}{
{"simpleDiff", simpleDiff, true},
{"renameWithModificationDiff", renameWithModificationDiff, true},
{"addNewlineToEndOfFile", addNewlineToEndOfFile, true},
{"twoHunks", twoHunks, true},
{"consecutiveDeletions", consecutiveDeletions, true},
{"newFile", newFile, true},
{"deletedFile", deletedFile, true},
{"addNewlineToPreviouslyEmptyFile", addNewlineToPreviouslyEmptyFile, true},
{"exampleHunk", exampleHunk, true},
{"gutterMangled", gutterMangled, false},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
assert.Equal(t, s.expected, Parse(s.patchStr).IsWellFormed())
})
}
}
func TestIsWellFormedSoFar(t *testing.T) {
// A diff read only as far as the middle of its second hunk.
const cutShort = `diff --git a/filename b/filename
index e48a11c..b2ab81b 100644
--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-grape
+orange
...
...
...
@@ -8,6 +8,8 @@ grape
...
...
`
// The same diff cut short in its first hunk, so that the second is missing
// entirely rather than short.
const cutShortInTheFirstHunk = `diff --git a/filename b/filename
index e48a11c..b2ab81b 100644
--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-grape
`
// A rendering with the +/- markers moved into a gutter, cut short: reading the
// changes as context makes the hunk longer than its header declares, not shorter,
// so it doesn't pass for a diff we only have the beginning of.
const gutterMangledAndCutShort = `diff --git a/filename b/filename
index 9320895..6d79956 100644
--- a/filename
+++ b/filename
@@ -1,4 +1,2 @@
apple
grape
pear
lemon
melon
`
scenarios := []struct {
testName string
patchStr string
expected bool
}{
{"simpleDiff", simpleDiff, true},
{"twoHunks", twoHunks, true},
{"cutShort", cutShort, true},
{"cutShortInTheFirstHunk", cutShortInTheFirstHunk, true},
{"gutterMangledAndCutShort", gutterMangledAndCutShort, false},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
assert.Equal(t, s.expected, Parse(s.patchStr).IsWellFormedSoFar())
})
}
}
func TestOldLineNumberOfLine(t *testing.T) {
type scenario struct {
testName string