Reverse the extended headers in ReverseFileDiff

diff/reverse.go

@@ -4,17 +4,24 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"strings"
 )
 
-// ReverseFileDiff takes a diff.FileDiff, and returns the reverse operation.
-// This is a FileDiff that undoes the edit of the original.
+// ReverseFileDiff takes a diff.FileDiff and returns the reverse operation.
+// This is a FileDiff that undoes the edit of the original. Git copy diffs
+// cannot be reversed because they do not contain enough information to delete
+// the copied file.
 func ReverseFileDiff(fd *FileDiff) (*FileDiff, error) {
+	extended, err := reverseExtendedHeaders(fd.Extended)
+	if err != nil {
+		return nil, err
+	}
 	reverse := FileDiff{
 		OrigName: fd.NewName,
 		OrigTime: fd.NewTime,
 		NewName:  fd.OrigName,
 		NewTime:  fd.OrigTime,
-		Extended: fd.Extended,
+		Extended: extended,
 	}
 	for _, hunk := range fd.Hunks {
 		invHunk, err := reverseHunk(hunk)
@@ -26,6 +33,63 @@ func ReverseFileDiff(fd *FileDiff) (*FileDiff, error) {
 	return &reverse, nil
 }
 
+// reverseExtendedHeaders reverses the direction encoded in git's extended headers.
+func reverseExtendedHeaders(headers []string) ([]string, error) {
+	// handleEmpty gates on the same prefix when it reads the direction back out.
+	if len(headers) == 0 || !strings.HasPrefix(headers[0], "diff --git ") {
+		return headers, nil
+	}
+	reversed := make([]string, len(headers))
+	copy(reversed, headers)
+	for i, header := range reversed {
+		switch {
+		case strings.HasPrefix(header, "new file mode "):
+			reversed[i] = "deleted file mode " + header[len("new file mode "):]
+		case strings.HasPrefix(header, "deleted file mode "):
+			reversed[i] = "new file mode " + header[len("deleted file mode "):]
+		case strings.HasPrefix(header, "index "):
+			reversed[i] = reverseIndexHeader(header)
+		case strings.HasPrefix(header, "copy from "), strings.HasPrefix(header, "copy to "):
+			return nil, errors.New("cannot reverse a git copy diff")
+		}
+	}
+	swapHeaderValues(reversed, "old mode ", "new mode ")
+	swapHeaderValues(reversed, "rename from ", "rename to ")
+	return reversed, nil
+}
+
+// swapHeaderValues exchanges the values of the first "from" header and the
+// first "to" header, leaving both prefixes where they are.
+func swapHeaderValues(headers []string, fromPrefix, toPrefix string) {
+	from, to := -1, -1
+	for i, header := range headers {
+		if from < 0 && strings.HasPrefix(header, fromPrefix) {
+			from = i
+		}
+		if to < 0 && strings.HasPrefix(header, toPrefix) {
+			to = i
+		}
+	}
+	if from < 0 || to < 0 {
+		return
+	}
+	headers[from], headers[to] = fromPrefix+headers[to][len(toPrefix):], toPrefix+headers[from][len(fromPrefix):]
+}
+
+// reverseIndexHeader swaps the two blob hashes in an "index <old>..<new>[ <mode>]"
+// header, leaving the trailing mode (if any) alone.
+func reverseIndexHeader(header string) string {
+	const prefix = "index "
+	oldHash, newHash, ok := strings.Cut(header[len(prefix):], "..")
+	if !ok || strings.ContainsAny(oldHash, " \r") {
+		return header
+	}
+	if i := strings.IndexAny(newHash, " \r"); i >= 0 {
+		return prefix + newHash[:i] + ".." + oldHash + newHash[i:]
+	}
+	return prefix + newHash + ".." + oldHash
+}
+
 // ReverseMultiFileDiff reverses a series of FileDiffs.
 func ReverseMultiFileDiff(fds []*FileDiff) ([]*FileDiff, error) {
 	var reverse []*FileDiff

diff/reverse_test.go

@@ -169,6 +169,13 @@ func TestReverseRoundTripOnTestdata(t *testing.T) {
 			}
 
 			if fileDiffs, err := ParseMultiFileDiff(data); err == nil && len(fileDiffs) > 0 {
+				if name == "complicated_filenames.diff" {
+					if _, err := ReverseMultiFileDiff(fileDiffs); err == nil {
+						t.Fatal("reversing fixture with copy diffs succeeded")
+					}
+					return
+				}
+
 				reversed, err := ReverseMultiFileDiff(fileDiffs)
 				if err != nil {
 					t.Fatalf("first reverse: %s", err)
@@ -216,3 +223,103 @@ func TestReverseRoundTripOnTestdata(t *testing.T) {
 		})
 	}
 }
+
+func TestReverseFileDiffExtendedHeaders(t *testing.T) {
+	tests := []struct {
+		name  string
+		input []string
+		want  []string
+	}{
+		{
+			name:  "new file",
+			input: []string{"diff --git a/f b/f", "new file mode 100644", "index 0000000..587be6b"},
+			want:  []string{"diff --git a/f b/f", "deleted file mode 100644", "index 587be6b..0000000"},
+		},
+		{
+			name:  "deleted file",
+			input: []string{"diff --git a/f b/f", "deleted file mode 100644", "index 587be6b..0000000"},
+			want:  []string{"diff --git a/f b/f", "new file mode 100644", "index 0000000..587be6b"},
+		},
+		{
+			name:  "rename",
+			input: []string{"diff --git a/old b/new", "similarity index 70%", "rename from old", "rename to new", "index 94954ab..8b14c4f 100644"},
+			want:  []string{"diff --git a/old b/new", "similarity index 70%", "rename from new", "rename to old", "index 8b14c4f..94954ab 100644"},
+		},
+		{
+			name:  "mode change",
+			input: []string{"diff --git a/f b/f", "old mode 100644", "new mode 100755"},
+			want:  []string{"diff --git a/f b/f", "old mode 100755", "new mode 100644"},
+		},
+		{
+			name:  "CRLF index",
+			input: []string{"diff --git a/f b/f\r", "index 94954ab..8b14c4f 100644\r"},
+			want:  []string{"diff --git a/f b/f\r", "index 8b14c4f..94954ab 100644\r"},
+		},
+		{
+			name:  "no extended headers",
+			input: nil,
+			want:  nil,
+		},
+		{
+			// Only git emits extended headers, so leave anything else alone.
+			name:  "non-git header block",
+			input: []string{"diff --ruN a/f b/f", "old mode 0777", "new mode 0755"},
+			want:  []string{"diff --ruN a/f b/f", "old mode 0777", "new mode 0755"},
+		},
+	}
+	for _, test := range tests {
+		orig := append([]string(nil), test.input...)
+		fd := &FileDiff{OrigName: "a/f", NewName: "b/f", Extended: test.input}
+		reversed, err := ReverseFileDiff(fd)
+		if err != nil {
+			t.Errorf("%s: ReverseFileDiff: %s", test.name, err)
+			continue
+		}
+		if d := cmp.Diff(test.want, reversed.Extended); d != "" {
+			t.Errorf("%s: reversed extended headers differ (-want +got):\n%s", test.name, d)
+		}
+		if d := cmp.Diff(orig, fd.Extended); d != "" {
+			t.Errorf("%s: ReverseFileDiff mutated its input (-want +got):\n%s", test.name, d)
+		}
+	}
+}
+
+func TestReverseFileDiffRejectsCopy(t *testing.T) {
+	input := []byte("diff --git a/old b/new\nsimilarity index 100%\ncopy from old\ncopy to new\n")
+	fd, err := ParseFileDiff(input)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := ReverseFileDiff(fd); err == nil {
+		t.Fatal("ReverseFileDiff succeeded for a copy diff")
+	}
+}
+
+func TestReverseFileDiffEmptyNewFile(t *testing.T) {
+	input := []byte("diff --git a/empty.txt b/empty.txt\nnew file mode 100644\nindex 0000000..e69de29\n")
+	fd, err := ParseFileDiff(input)
+	if err != nil {
+		t.Fatal(err)
+	}
+	reversed, err := ReverseFileDiff(fd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	printed, err := PrintFileDiff(reversed)
+	if err != nil {
+		t.Fatal(err)
+	}
+	roundTrip, err := ParseFileDiff(printed)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// The forward diff creates the file, so it parses as OrigName=/dev/null.
+	// Reversing it must delete the file, i.e. NewName=/dev/null.
+	if fd.OrigName != "/dev/null" {
+		t.Fatalf("forward diff: got OrigName=%q, want /dev/null", fd.OrigName)
+	}
+	if roundTrip.NewName != "/dev/null" || roundTrip.OrigName == "/dev/null" {
+		t.Errorf("reversed empty new-file diff: got OrigName=%q NewName=%q, want NewName=/dev/null\nprinted:\n%s",
+			roundTrip.OrigName, roundTrip.NewName, printed)
+	}
+}