Skip to content

Commit eb5f3de

Browse files
committed
Improve conflict error detection
Conflict errors are now represented by an exported type that is compatible with errors.Is instead of using the method defined on ApplyError. This makes the tests slightly cleaner and should be more idiomatic for clients.
1 parent 1b341e0 commit eb5f3de

File tree

2 files changed

+39
-24
lines changed

2 files changed

+39
-24
lines changed

gitdiff/apply.go

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,31 @@ import (
55
"io"
66
)
77

8-
type conflictError string
8+
// Conflict indicates an apply failed due to a conflict between the patch and
9+
// the source content.
10+
//
11+
// Users can test if an error was caused by a conflict by using errors.Is with
12+
// an empty Conflict:
13+
//
14+
// if errors.Is(err, &Conflict{}) {
15+
// // handle conflict
16+
// }
17+
//
18+
type Conflict struct {
19+
msg string
20+
}
921

10-
func (e conflictError) Error() string {
11-
return "conflict: " + string(e)
22+
func (c *Conflict) Error() string {
23+
return "conflict: " + c.msg
24+
}
25+
26+
// Is implements error matching for Conflict. Passing an empty instance of
27+
// Conflict always returns true.
28+
func (c *Conflict) Is(other error) bool {
29+
if other, ok := other.(*Conflict); ok {
30+
return other.msg == "" || other.msg == c.msg
31+
}
32+
return false
1233
}
1334

1435
// ApplyError wraps an error that occurs during patch application with
@@ -29,13 +50,6 @@ func (e *ApplyError) Unwrap() error {
2950
return e.err
3051
}
3152

32-
// Conflict returns true if the error is due to a conflict between the fragment
33-
// and the source data.
34-
func (e *ApplyError) Conflict() bool {
35-
_, ok := e.err.(conflictError)
36-
return ok
37-
}
38-
3953
func (e *ApplyError) Error() string {
4054
return fmt.Sprintf("%v", e.err)
4155
}
@@ -152,7 +166,7 @@ func applyTextLine(dst io.Writer, src string, line Line) (err error) {
152166
switch line.Op {
153167
case OpContext, OpDelete:
154168
if src != line.Line {
155-
return conflictError("fragment line does not match src line")
169+
return &Conflict{"fragment line does not match src line"}
156170
}
157171
}
158172
switch line.Op {
@@ -176,9 +190,9 @@ func copyLines(dst io.Writer, src LineReader, limit int64) (string, int64, error
176190
return line, n, err
177191
case n > limit:
178192
if limit < 0 {
179-
return "", n, conflictError("cannot create new file from non-empty src")
193+
return "", n, &Conflict{"cannot create new file from non-empty src"}
180194
}
181-
return "", n, conflictError("fragment overlaps with an applied fragment")
195+
return "", n, &Conflict{"fragment overlaps with an applied fragment"}
182196
case err != nil:
183197
if err == io.EOF {
184198
err = io.ErrUnexpectedEOF

gitdiff/apply_test.go

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ package gitdiff
22

33
import (
44
"bytes"
5+
"errors"
6+
"io"
57
"io/ioutil"
68
"path/filepath"
7-
"strings"
89
"testing"
910
)
1011

@@ -15,7 +16,7 @@ func TestTextFragmentApplyStrict(t *testing.T) {
1516
PatchFile string
1617
DstFile string
1718

18-
Err string
19+
Err error
1920
}{
2021
"createFile": {File: "text_fragment_new"},
2122
"deleteFile": {File: "text_fragment_delete_all"},
@@ -34,27 +35,27 @@ func TestTextFragmentApplyStrict(t *testing.T) {
3435
"errorShortSrcBefore": {
3536
SrcFile: "text_fragment_error",
3637
PatchFile: "text_fragment_error_short_src_before",
37-
Err: "unexpected EOF",
38+
Err: io.ErrUnexpectedEOF,
3839
},
3940
"errorShortSrc": {
4041
SrcFile: "text_fragment_error",
4142
PatchFile: "text_fragment_error_short_src",
42-
Err: "unexpected EOF",
43+
Err: io.ErrUnexpectedEOF,
4344
},
4445
"errorContextConflict": {
4546
SrcFile: "text_fragment_error",
4647
PatchFile: "text_fragment_error_context_conflict",
47-
Err: "conflict",
48+
Err: &Conflict{},
4849
},
4950
"errorDeleteConflict": {
5051
SrcFile: "text_fragment_error",
5152
PatchFile: "text_fragment_error_delete_conflict",
52-
Err: "conflict",
53+
Err: &Conflict{},
5354
},
5455
"errorNewFile": {
5556
SrcFile: "text_fragment_error",
5657
PatchFile: "text_fragment_error_new_file",
57-
Err: "conflict",
58+
Err: &Conflict{},
5859
},
5960
}
6061

@@ -75,7 +76,7 @@ func TestTextFragmentApplyStrict(t *testing.T) {
7576
patch := loadFile(test.PatchFile, test.File, "patch")
7677

7778
var result []byte
78-
if test.Err == "" {
79+
if test.Err == nil {
7980
result = loadFile(test.DstFile, test.File, "dst")
8081
}
8182

@@ -88,12 +89,12 @@ func TestTextFragmentApplyStrict(t *testing.T) {
8889

8990
var dst bytes.Buffer
9091
err = frag.ApplyStrict(&dst, NewLineReader(bytes.NewReader(src), 0))
91-
if test.Err != "" {
92+
if test.Err != nil {
9293
if err == nil {
9394
t.Fatalf("expected error applying fragment, but got nil")
9495
}
95-
if !strings.Contains(err.Error(), test.Err) {
96-
t.Fatalf("incorrect apply error: expected %q, actual %q", test.Err, err.Error())
96+
if !errors.Is(err, test.Err) {
97+
t.Fatalf("incorrect apply error: expected: %T (%v), actual: %T (%v)", test.Err, test.Err, err, err)
9798
}
9899
return
99100
}

0 commit comments

Comments
 (0)