From 60ddccae8574e69e18ab4c19979ee15bfdf95af6 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 5 Oct 2022 18:58:00 -0400 Subject: [PATCH] internal/diff: Apply: validate inputs Apply now checks that its edits are valid (not out of bounds or overlapping), and reports an error if not. It also sorts them, if necessary, using (start, end) as the key, to ensure that insertions (end=start) are ordered before deletions at the same point (but without changing the relative order of insertions). Two other implementations of the diff.Apply algorithm have been eliminated. (One of them failed to sort edits, requiring the protocol sender to do so; that burden is now gone.) Change-Id: Ia76e485e6869db4a165835c3312fd14bc7d43db2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/439278 Auto-Submit: Alan Donovan gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Robert Findley Run-TryBot: Alan Donovan --- go/analysis/analysistest/analysistest.go | 12 ++- gopls/internal/hooks/diff.go | 4 +- gopls/internal/lsp/cmd/format.go | 3 +- gopls/internal/lsp/cmd/imports.go | 4 +- gopls/internal/lsp/cmd/rename.go | 4 +- gopls/internal/lsp/cmd/suggested_fix.go | 3 +- gopls/internal/lsp/fake/edit.go | 115 +++++++++-------------- gopls/internal/lsp/fake/edit_test.go | 4 +- gopls/internal/lsp/fake/editor.go | 4 +- gopls/internal/lsp/lsp_test.go | 24 +---- gopls/internal/lsp/source/format.go | 15 +++ gopls/internal/lsp/source/rename.go | 1 - gopls/internal/lsp/source/source_test.go | 34 +------ internal/diff/diff.go | 92 +++++++++++------- internal/diff/diff_test.go | 42 +++++++-- internal/diff/difftest/difftest.go | 5 +- 16 files changed, 183 insertions(+), 183 deletions(-) diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index bc25b9f2b7..14140be14b 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -183,7 +183,11 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns for _, vf := range ar.Files { if vf.Name == sf { found = true - out := diff.Apply(string(orig), edits) + out, err := diff.Apply(string(orig), edits) + if err != nil { + t.Errorf("%s: error applying fixes: %v", file.Name(), err) + continue + } // the file may contain multiple trailing // newlines if the user places empty lines // between files in the archive. normalize @@ -213,7 +217,11 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns catchallEdits = append(catchallEdits, edits...) } - out := diff.Apply(string(orig), catchallEdits) + out, err := diff.Apply(string(orig), catchallEdits) + if err != nil { + t.Errorf("%s: error applying fixes: %v", file.Name(), err) + continue + } want := string(ar.Comment) formatted, err := format.Source([]byte(out)) diff --git a/gopls/internal/hooks/diff.go b/gopls/internal/hooks/diff.go index 3aa1f0b780..cac136ae19 100644 --- a/gopls/internal/hooks/diff.go +++ b/gopls/internal/hooks/diff.go @@ -162,8 +162,8 @@ func BothDiffs(before, after string) (edits []diff.Edit) { newedits := diff.Strings(before, after) stat.Newedits = len(newedits) stat.Newtime = time.Now().Sub(now) - got := diff.Apply(before, newedits) - if got != after { + got, err := diff.Apply(before, newedits) + if err != nil || got != after { stat.Msg += "FAIL" disaster(before, after) stat.save() diff --git a/gopls/internal/lsp/cmd/format.go b/gopls/internal/lsp/cmd/format.go index 17e3e6a501..56e39053c1 100644 --- a/gopls/internal/lsp/cmd/format.go +++ b/gopls/internal/lsp/cmd/format.go @@ -76,11 +76,10 @@ func (c *format) Run(ctx context.Context, args ...string) error { if err != nil { return fmt.Errorf("%v: %v", spn, err) } - sedits, err := source.FromProtocolEdits(file.mapper, edits) + formatted, sedits, err := source.ApplyProtocolEdits(file.mapper, edits) if err != nil { return fmt.Errorf("%v: %v", spn, err) } - formatted := diff.Apply(string(file.mapper.Content), sedits) printIt := true if c.List { printIt = false diff --git a/gopls/internal/lsp/cmd/imports.go b/gopls/internal/lsp/cmd/imports.go index 7fd548381f..4ee6fce4c9 100644 --- a/gopls/internal/lsp/cmd/imports.go +++ b/gopls/internal/lsp/cmd/imports.go @@ -81,12 +81,10 @@ func (t *imports) Run(ctx context.Context, args ...string) error { } } } - sedits, err := source.FromProtocolEdits(file.mapper, edits) + newContent, sedits, err := source.ApplyProtocolEdits(file.mapper, edits) if err != nil { return fmt.Errorf("%v: %v", edits, err) } - newContent := diff.Apply(string(file.mapper.Content), sedits) - filename := file.uri.Filename() switch { case t.Write: diff --git a/gopls/internal/lsp/cmd/rename.go b/gopls/internal/lsp/cmd/rename.go index e0ffa663bf..58fb07d7f1 100644 --- a/gopls/internal/lsp/cmd/rename.go +++ b/gopls/internal/lsp/cmd/rename.go @@ -95,12 +95,10 @@ func (r *rename) Run(ctx context.Context, args ...string) error { cmdFile := conn.AddFile(ctx, uri) filename := cmdFile.uri.Filename() - // convert LSP-style edits to []diff.TextEdit cuz Spans are handy - renameEdits, err := source.FromProtocolEdits(cmdFile.mapper, edits[uri]) + newContent, renameEdits, err := source.ApplyProtocolEdits(cmdFile.mapper, edits[uri]) if err != nil { return fmt.Errorf("%v: %v", edits, err) } - newContent := diff.Apply(string(cmdFile.mapper.Content), renameEdits) switch { case r.Write: diff --git a/gopls/internal/lsp/cmd/suggested_fix.go b/gopls/internal/lsp/cmd/suggested_fix.go index b6022a7ee5..9c103deff8 100644 --- a/gopls/internal/lsp/cmd/suggested_fix.go +++ b/gopls/internal/lsp/cmd/suggested_fix.go @@ -142,11 +142,10 @@ func (s *suggestedFix) Run(ctx context.Context, args ...string) error { } } - sedits, err := source.FromProtocolEdits(file.mapper, edits) + newContent, sedits, err := source.ApplyProtocolEdits(file.mapper, edits) if err != nil { return fmt.Errorf("%v: %v", edits, err) } - newContent := diff.Apply(string(file.mapper.Content), sedits) filename := file.uri.Filename() switch { diff --git a/gopls/internal/lsp/fake/edit.go b/gopls/internal/lsp/fake/edit.go index bb5fb80900..3eb13ea2f4 100644 --- a/gopls/internal/lsp/fake/edit.go +++ b/gopls/internal/lsp/fake/edit.go @@ -6,14 +6,16 @@ package fake import ( "fmt" - "sort" "strings" + "unicode/utf8" "golang.org/x/tools/gopls/internal/lsp/protocol" + "golang.org/x/tools/internal/diff" ) -// Pos represents a position in a text buffer. Both Line and Column are -// 0-indexed. +// Pos represents a position in a text buffer. +// Both Line and Column are 0-indexed. +// Column counts runes. type Pos struct { Line, Column int } @@ -105,78 +107,51 @@ func inText(p Pos, content []string) bool { return true } -// editContent implements a simplistic, inefficient algorithm for applying text -// edits to our buffer representation. It returns an error if the edit is -// invalid for the current content. -// -// TODO(rfindley): this function does not handle non-ascii text correctly. -// TODO(rfindley): replace this with diff.Apply: we should not be -// maintaining an additional representation of edits. -func editContent(content []string, edits []Edit) ([]string, error) { - newEdits := make([]Edit, len(edits)) - copy(newEdits, edits) - sort.SliceStable(newEdits, func(i, j int) bool { - ei := newEdits[i] - ej := newEdits[j] +// applyEdits applies the edits to a file with the specified lines, +// and returns a new slice containing the lines of the patched file. +// It is a wrapper around diff.Apply; see that function for preconditions. +func applyEdits(lines []string, edits []Edit) ([]string, error) { + src := strings.Join(lines, "\n") - // Sort by edit start position followed by end position. Given an edit - // 3:1-3:1 followed by an edit 3:1-3:15, we must process the empty edit - // first. - if cmp := comparePos(ei.Start, ej.Start); cmp != 0 { - return cmp < 0 - } + // Build a table of byte offset of start of each line. + lineOffset := make([]int, len(lines)+1) + offset := 0 + for i, line := range lines { + lineOffset[i] = offset + offset += len(line) + len("\n") + } + lineOffset[len(lines)] = offset // EOF - return comparePos(ei.End, ej.End) < 0 - }) + var badCol error + posToOffset := func(pos Pos) int { + offset := lineOffset[pos.Line] + // Convert pos.Column (runes) to a UTF-8 byte offset. + if pos.Line < len(lines) { + for i := 0; i < pos.Column; i++ { + r, sz := utf8.DecodeRuneInString(src[offset:]) + if r == '\n' && badCol == nil { + badCol = fmt.Errorf("bad column") + } + offset += sz + } + } + return offset + } - // Validate edits. - for _, edit := range newEdits { - if edit.End.Line < edit.Start.Line || (edit.End.Line == edit.Start.Line && edit.End.Column < edit.Start.Column) { - return nil, fmt.Errorf("invalid edit: end %v before start %v", edit.End, edit.Start) - } - if !inText(edit.Start, content) { - return nil, fmt.Errorf("start position %v is out of bounds", edit.Start) - } - if !inText(edit.End, content) { - return nil, fmt.Errorf("end position %v is out of bounds", edit.End) + // Convert fake.Edits to diff.Edits + diffEdits := make([]diff.Edit, len(edits)) + for i, edit := range edits { + diffEdits[i] = diff.Edit{ + Start: posToOffset(edit.Start), + End: posToOffset(edit.End), + New: edit.Text, } } - var ( - b strings.Builder - line, column int - ) - advance := func(toLine, toColumn int) { - for ; line < toLine; line++ { - b.WriteString(string([]rune(content[line])[column:]) + "\n") - column = 0 - } - b.WriteString(string([]rune(content[line])[column:toColumn])) - column = toColumn + patched, err := diff.Apply(src, diffEdits) + if err != nil { + return nil, err } - for _, edit := range newEdits { - advance(edit.Start.Line, edit.Start.Column) - b.WriteString(edit.Text) - line = edit.End.Line - column = edit.End.Column - } - advance(len(content)-1, len([]rune(content[len(content)-1]))) - return strings.Split(b.String(), "\n"), nil -} - -// comparePos returns -1 if left < right, 0 if left == right, and 1 if left > right. -func comparePos(left, right Pos) int { - if left.Line < right.Line { - return -1 - } - if left.Line > right.Line { - return 1 - } - if left.Column < right.Column { - return -1 - } - if left.Column > right.Column { - return 1 - } - return 0 + + return strings.Split(patched, "\n"), badCol } diff --git a/gopls/internal/lsp/fake/edit_test.go b/gopls/internal/lsp/fake/edit_test.go index 4fa23bdb74..f87d921033 100644 --- a/gopls/internal/lsp/fake/edit_test.go +++ b/gopls/internal/lsp/fake/edit_test.go @@ -9,7 +9,7 @@ import ( "testing" ) -func TestApplyEdit(t *testing.T) { +func TestApplyEdits(t *testing.T) { tests := []struct { label string content string @@ -82,7 +82,7 @@ func TestApplyEdit(t *testing.T) { test := test t.Run(test.label, func(t *testing.T) { lines := strings.Split(test.content, "\n") - newLines, err := editContent(lines, test.edits) + newLines, err := applyEdits(lines, test.edits) if (err != nil) != test.wantErr { t.Errorf("got err %v, want error: %t", err, test.wantErr) } diff --git a/gopls/internal/lsp/fake/editor.go b/gopls/internal/lsp/fake/editor.go index 17bec286b7..e65db938ad 100644 --- a/gopls/internal/lsp/fake/editor.go +++ b/gopls/internal/lsp/fake/editor.go @@ -710,9 +710,7 @@ func (e *Editor) editBufferLocked(ctx context.Context, path string, edits []Edit if !ok { return fmt.Errorf("unknown buffer %q", path) } - content := make([]string, len(buf.lines)) - copy(content, buf.lines) - content, err := editContent(content, edits) + content, err := applyEdits(buf.lines, edits) if err != nil { return err } diff --git a/gopls/internal/lsp/lsp_test.go b/gopls/internal/lsp/lsp_test.go index 78032f0436..4cd009d2f0 100644 --- a/gopls/internal/lsp/lsp_test.go +++ b/gopls/internal/lsp/lsp_test.go @@ -24,7 +24,6 @@ import ( "golang.org/x/tools/gopls/internal/lsp/tests" "golang.org/x/tools/gopls/internal/lsp/tests/compare" "golang.org/x/tools/internal/bug" - "golang.org/x/tools/internal/diff" "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/testenv" @@ -409,11 +408,10 @@ func (r *runner) Format(t *testing.T, spn span.Span) { if err != nil { t.Fatal(err) } - sedits, err := source.FromProtocolEdits(m, edits) + got, _, err := source.ApplyProtocolEdits(m, edits) if err != nil { t.Error(err) } - got := diff.Apply(string(m.Content), sedits) if diff := compare.Text(gofmted, got); diff != "" { t.Errorf("format failed for %s (-want +got):\n%s", filename, diff) } @@ -975,11 +973,10 @@ func (r *runner) InlayHints(t *testing.T, spn span.Span) { if err != nil { t.Fatal(err) } - sedits, err := source.FromProtocolEdits(m, edits) + got, _, err := source.ApplyProtocolEdits(m, edits) if err != nil { t.Error(err) } - got := diff.Apply(string(m.Content), sedits) withinlayHints := string(r.data.Golden(t, "inlayHint", filename, func() ([]byte, error) { return []byte(got), nil @@ -1115,29 +1112,16 @@ func applyTextDocumentEdits(r *runner, edits []protocol.DocumentChanges) (map[sp return nil, err } } - res[uri] = string(m.Content) - sedits, err := source.FromProtocolEdits(m, docEdits.TextDocumentEdit.Edits) + patched, _, err := source.ApplyProtocolEdits(m, docEdits.TextDocumentEdit.Edits) if err != nil { return nil, err } - res[uri] = applyEdits(res[uri], sedits) + res[uri] = patched } } return res, nil } -func applyEdits(contents string, edits []diff.Edit) string { - res := contents - - // Apply the edits from the end of the file forward - // to preserve the offsets - for i := len(edits) - 1; i >= 0; i-- { - edit := edits[i] - res = res[:edit.Start] + edit.New + res[edit.End:] - } - return res -} - func (r *runner) Symbols(t *testing.T, uri span.URI, expectedSymbols []protocol.DocumentSymbol) { params := &protocol.DocumentSymbolParams{ TextDocument: protocol.TextDocumentIdentifier{ diff --git a/gopls/internal/lsp/source/format.go b/gopls/internal/lsp/source/format.go index dc7445a025..c6e70efce6 100644 --- a/gopls/internal/lsp/source/format.go +++ b/gopls/internal/lsp/source/format.go @@ -337,6 +337,8 @@ func protocolEditsFromSource(src []byte, edits []diff.Edit, tf *token.File) ([]p return result, nil } +// ToProtocolEdits converts diff.Edits to LSP TextEdits. +// See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textEditArray func ToProtocolEdits(m *protocol.ColumnMapper, edits []diff.Edit) ([]protocol.TextEdit, error) { result := make([]protocol.TextEdit, len(edits)) for i, edit := range edits { @@ -352,6 +354,8 @@ func ToProtocolEdits(m *protocol.ColumnMapper, edits []diff.Edit) ([]protocol.Te return result, nil } +// ToProtocolEdits converts LSP TextEdits to diff.Edits. +// See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textEditArray func FromProtocolEdits(m *protocol.ColumnMapper, edits []protocol.TextEdit) ([]diff.Edit, error) { if edits == nil { return nil, nil @@ -370,3 +374,14 @@ func FromProtocolEdits(m *protocol.ColumnMapper, edits []protocol.TextEdit) ([]d } return result, nil } + +// ApplyProtocolEdits applies the patch (edits) to m.Content and returns the result. +// It also returns the edits converted to diff-package form. +func ApplyProtocolEdits(m *protocol.ColumnMapper, edits []protocol.TextEdit) (string, []diff.Edit, error) { + diffEdits, err := FromProtocolEdits(m, edits) + if err != nil { + return "", nil, err + } + out, err := diff.Apply(string(m.Content), diffEdits) + return out, diffEdits, err +} diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go index 842b1a9850..c5af1ca896 100644 --- a/gopls/internal/lsp/source/rename.go +++ b/gopls/internal/lsp/source/rename.go @@ -446,7 +446,6 @@ func renameObj(ctx context.Context, s Snapshot, newName string, qos []qualifiedO return nil, err } m := protocol.NewColumnMapper(uri, data) - diff.SortEdits(edits) protocolEdits, err := ToProtocolEdits(m, edits) if err != nil { return nil, err diff --git a/gopls/internal/lsp/source/source_test.go b/gopls/internal/lsp/source/source_test.go index 0a4e70cb6f..d81bdb7aea 100644 --- a/gopls/internal/lsp/source/source_test.go +++ b/gopls/internal/lsp/source/source_test.go @@ -22,7 +22,6 @@ import ( "golang.org/x/tools/gopls/internal/lsp/tests" "golang.org/x/tools/gopls/internal/lsp/tests/compare" "golang.org/x/tools/internal/bug" - "golang.org/x/tools/internal/diff" "golang.org/x/tools/internal/fuzzy" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/testenv" @@ -477,19 +476,14 @@ func (r *runner) Format(t *testing.T, spn span.Span) { } return } - data, err := fh.Read() - if err != nil { - t.Fatal(err) - } m, err := r.data.Mapper(spn.URI()) if err != nil { t.Fatal(err) } - diffEdits, err := source.FromProtocolEdits(m, edits) + got, _, err := source.ApplyProtocolEdits(m, edits) if err != nil { t.Error(err) } - got := diff.Apply(string(data), diffEdits) if gofmted != got { t.Errorf("format failed for %s, expected:\n%v\ngot:\n%v", spn.URI().Filename(), gofmted, got) } @@ -508,19 +502,14 @@ func (r *runner) Import(t *testing.T, spn span.Span) { if err != nil { t.Error(err) } - data, err := fh.Read() - if err != nil { - t.Fatal(err) - } m, err := r.data.Mapper(fh.URI()) if err != nil { t.Fatal(err) } - diffEdits, err := source.FromProtocolEdits(m, edits) + got, _, err := source.ApplyProtocolEdits(m, edits) if err != nil { t.Error(err) } - got := diff.Apply(string(data), diffEdits) want := string(r.data.Golden(t, "goimports", spn.URI().Filename(), func() ([]byte, error) { return []byte(got), nil })) @@ -781,19 +770,14 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { if err != nil { t.Fatal(err) } - data, err := fh.Read() - if err != nil { - t.Fatal(err) - } m, err := r.data.Mapper(fh.URI()) if err != nil { t.Fatal(err) } - diffEdits, err := source.FromProtocolEdits(m, edits) + contents, _, err := source.ApplyProtocolEdits(m, edits) if err != nil { t.Fatal(err) } - contents := applyEdits(string(data), diffEdits) if len(changes) > 1 { filename := filepath.Base(editURI.Filename()) contents = fmt.Sprintf("%s:\n%s", filename, contents) @@ -821,18 +805,6 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { } } -func applyEdits(contents string, edits []diff.Edit) string { - res := contents - - // Apply the edits from the end of the file forward - // to preserve the offsets - for i := len(edits) - 1; i >= 0; i-- { - edit := edits[i] - res = res[:edit.Start] + edit.New + res[edit.End:] - } - return res -} - func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.PrepareItem) { // Removed in favor of just using the lsp_test implementation. See ../lsp_test.go } diff --git a/internal/diff/diff.go b/internal/diff/diff.go index e7f8469d13..a75026dcaa 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -6,6 +6,7 @@ package diff import ( + "fmt" "sort" "strings" ) @@ -22,46 +23,71 @@ type Edit struct { New string // the replacement } -// SortEdits orders edits by their start offset. The sort is stable -// so that edits with the same start offset will not be reordered. -func SortEdits(edits []Edit) { - sort.SliceStable(edits, func(i int, j int) bool { - return edits[i].Start < edits[j].Start - }) -} - -// Apply applies a sequence of edits to the src buffer and -// returns the result. It may panic or produce garbage if the edits -// are overlapping, out of bounds of src, or out of order. +// Apply applies a sequence of edits to the src buffer and returns the +// result. Edits are applied in order of start offset; edits with the +// same start offset are applied in they order they were provided. // -// TODO(adonovan): this function must not panic if the edits aren't -// consistent with src, or with each other---especially when fed -// information from an untrusted source. It should probably be -// defensive against bad input and report an error in any of the above -// situations. -func Apply(src string, edits []Edit) string { - SortEdits(edits) // TODO(adonovan): move to caller? What's the contract? Don't mutate arguments. +// Apply returns an error if any edit is out of bounds, +// or if any pair of edits is overlapping. +func Apply(src string, edits []Edit) (string, error) { + if !sort.IsSorted(editsSort(edits)) { + edits = append([]Edit(nil), edits...) + sortEdits(edits) + } - var out strings.Builder - // TODO(adonovan): opt: preallocate correct final size - // by scanning the list of edits. (This can be done - // in the same pass as detecting inconsistent edits.) - last := 0 + // Check validity of edits and compute final size. + size := len(src) + lastEnd := 0 for _, edit := range edits { - start := edit.Start - if start > last { - out.WriteString(src[last:start]) - last = start + if !(0 <= edit.Start && edit.Start <= edit.End && edit.End <= len(src)) { + return "", fmt.Errorf("diff has out-of-bounds edits") } - out.WriteString(edit.New) - last = edit.End + if edit.Start < lastEnd { + return "", fmt.Errorf("diff has overlapping edits") + } + size += len(edit.New) + edit.Start - edit.End + lastEnd = edit.End } - if last < len(src) { - out.WriteString(src[last:]) + + // Apply edits. + out := make([]byte, 0, size) + lastEnd = 0 + for _, edit := range edits { + if lastEnd < edit.Start { + out = append(out, src[lastEnd:edit.Start]...) + } + out = append(out, edit.New...) + lastEnd = edit.End } - return out.String() + out = append(out, src[lastEnd:]...) + + if len(out) != size { + panic("wrong size") + } + + return string(out), nil } +// sortEdits orders edits by (start, end) offset. +// This ordering puts insertions (end=start) before deletions +// (end>start) at the same point, but uses a stable sort to preserve +// the order of multiple insertions at the same point. +// (Apply detects multiple deletions at the same point as an error.) +func sortEdits(edits editsSort) { + sort.Stable(edits) +} + +type editsSort []Edit + +func (a editsSort) Len() int { return len(a) } +func (a editsSort) Less(i, j int) bool { + if cmp := a[i].Start - a[j].Start; cmp != 0 { + return cmp < 0 + } + return a[i].End < a[j].End +} +func (a editsSort) Swap(i, j int) { a[i], a[j] = a[j], a[i] } + // LineEdits expands and merges a sequence of edits so that each // resulting edit replaces one or more complete lines. // @@ -71,7 +97,7 @@ func Apply(src string, edits []Edit) string { // We could hide this from the API so that we can enforce // the precondition... but it seems like a reasonable feature. func LineEdits(src string, edits []Edit) []Edit { - SortEdits(edits) // TODO(adonovan): is this necessary? Move burden to caller? + sortEdits(edits) // TODO(adonovan): is this necessary? Move burden to caller? // Do all edits begin and end at the start of a line? // TODO(adonovan): opt: is this fast path necessary? diff --git a/internal/diff/diff_test.go b/internal/diff/diff_test.go index 8b8b6489e0..d4c3746184 100644 --- a/internal/diff/diff_test.go +++ b/internal/diff/diff_test.go @@ -18,11 +18,19 @@ import ( func TestApply(t *testing.T) { for _, tc := range difftest.TestCases { t.Run(tc.Name, func(t *testing.T) { - if got := diff.Apply(tc.In, tc.Edits); got != tc.Out { + got, err := diff.Apply(tc.In, tc.Edits) + if err != nil { + t.Fatalf("Apply(Edits) failed: %v", err) + } + if got != tc.Out { t.Errorf("Apply(Edits): got %q, want %q", got, tc.Out) } if tc.LineEdits != nil { - if got := diff.Apply(tc.In, tc.LineEdits); got != tc.Out { + got, err := diff.Apply(tc.In, tc.LineEdits) + if err != nil { + t.Fatalf("Apply(LineEdits) failed: %v", err) + } + if got != tc.Out { t.Errorf("Apply(LineEdits): got %q, want %q", got, tc.Out) } } @@ -33,7 +41,10 @@ func TestApply(t *testing.T) { func TestNEdits(t *testing.T) { for _, tc := range difftest.TestCases { edits := diff.Strings(tc.In, tc.Out) - got := diff.Apply(tc.In, edits) + got, err := diff.Apply(tc.In, edits) + if err != nil { + t.Fatalf("Apply failed: %v", err) + } if got != tc.Out { t.Fatalf("%s: got %q wanted %q", tc.Name, got, tc.Out) } @@ -49,7 +60,10 @@ func TestNRandom(t *testing.T) { a := randstr("abω", 16) b := randstr("abωc", 16) edits := diff.Strings(a, b) - got := diff.Apply(a, edits) + got, err := diff.Apply(a, edits) + if err != nil { + t.Fatalf("Apply failed: %v", err) + } if got != b { t.Fatalf("%d: got %q, wanted %q, starting with %q", i, got, b, a) } @@ -63,7 +77,10 @@ func FuzzRoundTrip(f *testing.F) { return // inputs must be text } edits := diff.Strings(a, b) - got := diff.Apply(a, edits) + got, err := diff.Apply(a, edits) + if err != nil { + t.Fatalf("Apply failed: %v", err) + } if got != b { t.Fatalf("applying diff(%q, %q) gives %q; edits=%v", a, b, got, edits) } @@ -90,7 +107,10 @@ func TestNLinesRandom(t *testing.T) { } a, b := strings.SplitAfter(x, "\n"), strings.SplitAfter(y, "\n") edits := diff.Lines(a, b) - got := diff.Apply(x, edits) + got, err := diff.Apply(x, edits) + if err != nil { + t.Fatalf("Apply failed: %v", err) + } if got != y { t.Fatalf("%d: got\n%q, wanted\n%q, starting with %q", i, got, y, a) } @@ -134,7 +154,10 @@ func TestRegressionOld001(t *testing.T) { b := "// Copyright 2019 The Go Authors. All rights reserved.\n// Use of this source code is governed by a BSD-style\n// license that can be found in the LICENSE file.\n\npackage diff_test\n\nimport (\n\t\"fmt\"\n\t\"math/rand\"\n\t\"strings\"\n\t\"testing\"\n\n\t\"github.com/google/safehtml/template\"\n\t\"golang.org/x/tools/gopls/internal/lsp/diff\"\n\t\"golang.org/x/tools/internal/diff/difftest\"\n\t\"golang.org/x/tools/internal/span\"\n)\n" diffs := diff.Strings(a, b) - got := diff.Apply(a, diffs) + got, err := diff.Apply(a, diffs) + if err != nil { + t.Fatalf("Apply failed: %v", err) + } if got != b { i := 0 for ; i < len(a) && i < len(b) && got[i] == b[i]; i++ { @@ -148,7 +171,10 @@ func TestRegressionOld002(t *testing.T) { a := "n\"\n)\n" b := "n\"\n\t\"golang.org/x//nnal/stack\"\n)\n" diffs := diff.Strings(a, b) - got := diff.Apply(a, diffs) + got, err := diff.Apply(a, diffs) + if err != nil { + t.Fatalf("Apply failed: %v", err) + } if got != b { i := 0 for ; i < len(a) && i < len(b) && got[i] == b[i]; i++ { diff --git a/internal/diff/difftest/difftest.go b/internal/diff/difftest/difftest.go index 998a90f109..5c0a741367 100644 --- a/internal/diff/difftest/difftest.go +++ b/internal/diff/difftest/difftest.go @@ -245,7 +245,10 @@ func DiffTest(t *testing.T, compute func(before, after string) []diff.Edit) { for _, test := range TestCases { t.Run(test.Name, func(t *testing.T) { edits := compute(test.In, test.Out) - got := diff.Apply(test.In, edits) + got, err := diff.Apply(test.In, edits) + if err != nil { + t.Fatalf("Apply failed: %v", err) + } unified := diff.Unified(FileA, FileB, test.In, edits) if got != test.Out { t.Errorf("Apply: got patched:\n%v\nfrom diff:\n%v\nexpected:\n%v", got, unified, test.Out)