diff --git a/internal/repo/patch.go b/internal/repo/patch.go index 32e324b..6f2e3c6 100644 --- a/internal/repo/patch.go +++ b/internal/repo/patch.go @@ -665,9 +665,9 @@ func splitByAnchor(lines []string, current hunk) []hunk { return parts } -// locate finds the one place block occurs in lines. A short block that -// appears more than once is ambiguous, and picking one would risk editing -// the wrong part of the file, so it is refused instead. +// locate finds the one place block occurs in lines. A block that appears +// more than once is ambiguous, however long it is, and picking one would +// risk editing the wrong part of the file, so it is refused instead. func locate(lines, block []string) (int, *HunkError) { matches := findAll(lines, block, func(a, b string) bool { return a == b }) if len(matches) == 0 { @@ -720,7 +720,7 @@ func locate(lines, block []string) (int, *HunkError) { detail += fmt.Sprintf("\n\nthe closest match is at line %d, where it differs:\n expected: %q\n in file: %q", at, expected, found) } return 0, &HunkError{Reason: reason, Expected: expected, Found: found, Detail: detail, Block: block} - case len(matches) > 1 && len(block) < 3: + case len(matches) > 1: reason := fmt.Sprintf("this hunk's lines appear %d times, too ambiguous to place", len(matches)) return 0, &HunkError{Reason: reason, Detail: fmt.Sprintf("%s:\n%s%s", reason, preview(block), disambiguate(lines, matches, block))} default: diff --git a/internal/repo/patch_test.go b/internal/repo/patch_test.go index c080b66..da08c5d 100644 --- a/internal/repo/patch_test.go +++ b/internal/repo/patch_test.go @@ -147,6 +147,34 @@ func TestAmbiguousHunkErrorPointsAtEachMatch(t *testing.T) { } } +func TestApplyIsRefusedWhenALongerHunkIsAmbiguous(t *testing.T) { + // Length does not make a block unique. Go's three-line error check + // appears in almost every function, and a hunk made of exactly that + // could mean any of them: placing it at the first one edits a function + // nobody asked about and reports the patch as applied. + root := t.TempDir() + original := "func a() error {\n\tif err != nil {\n\t\treturn err\n\t}\n\treturn nil\n}\n\n" + + "func b() error {\n\tif err != nil {\n\t\treturn err\n\t}\n\treturn nil\n}\n" + writeFile(t, root, "a.go", original) + repository := &Repository{Root: root} + patch := "@a.go\n" + + " \tif err != nil {\n" + + "-\t\treturn err\n" + + "+\t\treturn fmt.Errorf(\"b: %w\", err)\n" + + " \t}\n" + err := repository.Apply(patch) + if err == nil { + t.Fatal("expected a hunk that matches two places to be refused") + } + if !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("err = %v, want it to explain the ambiguity", err) + } + content, _ := os.ReadFile(filepath.Join(root, "a.go")) + if string(content) != original { + t.Fatalf("file was modified despite the refusal: %q", content) + } +} + func TestApplyLeavesFilesAloneWhenAHunkCannotBePlaced(t *testing.T) { patch := "--- a/index.html\n+++ b/index.html\n@@ -1,1 +1,1 @@\n" + "-