Skip to content

Commit 1943c1e

Browse files
committed
internal/diff: fix LineEdits bug in fast path
The fast-path "optimization" that skips the main algorithm when the input is already line-aligned failed to check that the replacement text consisted of complete lines. (Scare quotes because removing the "optimization" causes tests to fail. See CL 499377 next in stack for why.) Thanks to pjw for diagnosing the root cause and providing the test case in CL 498975. Fixes golang/go#60379 Change-Id: I2ff92de4550754691442362b8a8932ee42971461 Reviewed-on: https://go-review.googlesource.com/c/tools/+/499376 gopls-CI: kokoro <[email protected]> Run-TryBot: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Peter Weinberger <[email protected]>
1 parent c6d86c4 commit 1943c1e

File tree

3 files changed

+29
-2
lines changed

3 files changed

+29
-2
lines changed

internal/diff/diff.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,15 @@ func lineEdits(src string, edits []Edit) ([]Edit, error) {
114114
return nil, err
115115
}
116116

117-
// Do all edits begin and end at the start of a line?
117+
// Do all deletions begin and end at the start of a line,
118+
// and all insertions end with a newline?
118119
// TODO(adonovan, pjw): why does omitting this 'optimization'
119120
// cause tests to fail? (TestDiff/insert-line,extra_newline)
120121
for _, edit := range edits {
121122
if edit.Start >= len(src) || // insertion at EOF
122123
edit.Start > 0 && src[edit.Start-1] != '\n' || // not at line start
123-
edit.End > 0 && src[edit.End-1] != '\n' { // not at line start
124+
edit.End > 0 && src[edit.End-1] != '\n' || // not at line start
125+
edit.New != "" && edit.New[len(edit.New)-1] != '\n' { // partial insert
124126
goto expand
125127
}
126128
}

internal/diff/diff_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,14 @@ func TestLineEdits(t *testing.T) {
107107
if !reflect.DeepEqual(got, edits) {
108108
t.Errorf("LineEdits got\n%q, want\n%q\n%#v", got, edits, tc)
109109
}
110+
// make sure that applying the edits gives the expected result
111+
fixed, err := diff.Apply(tc.In, got)
112+
if err != nil {
113+
t.Error(err)
114+
}
115+
if fixed != tc.Out {
116+
t.Errorf("Apply(LineEdits): got %q, want %q", fixed, tc.Out)
117+
}
110118
})
111119
}
112120
}

internal/diff/difftest/difftest.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,23 @@ var TestCases = []struct {
279279
Edits: []diff.Edit{{Start: 3, End: 3, New: "\nbbb"}},
280280
LineEdits: []diff.Edit{{Start: 0, End: 4, New: "aaa\nbbb\n"}},
281281
Unified: UnifiedPrefix + "@@ -1,2 +1,3 @@\n aaa\n+bbb\n ccc\n",
282+
}, {
283+
Name: "60379",
284+
In: `package a
285+
286+
type S struct {
287+
s fmt.Stringer
288+
}
289+
`,
290+
Out: `package a
291+
292+
type S struct {
293+
s fmt.Stringer
294+
}
295+
`,
296+
Edits: []diff.Edit{{Start: 27, End: 27, New: "\t"}},
297+
LineEdits: []diff.Edit{{Start: 27, End: 42, New: "\ts fmt.Stringer\n"}},
298+
Unified: UnifiedPrefix + "@@ -1,5 +1,5 @@\n package a\n \n type S struct {\n-s fmt.Stringer\n+\ts fmt.Stringer\n }\n",
282299
},
283300
}
284301

0 commit comments

Comments
 (0)