internal/lsp: normalise and make public diff<->edit conversions
This allows us to use the diff.ApplyEdits in tests, saving us from a different
implementation.
It also prepares for command lines that need to use diff features based on the
results of a protocol message.
Splitting content into lines is too easy to get wrong, and needs to be done
correctly or the diff results make no sense. This adds the SplitLines function
to the diff pacakge to do it right and then uses it everwhere we we already
doing it wrong.
It also makes all the diff tests external black box tests.
Change-Id: I698227d5769a2bfbfd22a64ea42906b1df9268d9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/171027
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index e95d167..177c145 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -81,7 +81,7 @@
if err != nil {
return nil, err
}
- protocolEdits, err := toProtocolEdits(m, edits)
+ protocolEdits, err := ToProtocolEdits(m, edits)
if err != nil {
return nil, err
}
diff --git a/internal/lsp/diff/diff.go b/internal/lsp/diff/diff.go
index 5e06ec4..f894b86 100644
--- a/internal/lsp/diff/diff.go
+++ b/internal/lsp/diff/diff.go
@@ -5,6 +5,8 @@
// Package diff implements the Myers diff algorithm.
package diff
+import "strings"
+
// Sources:
// https://blog.jcoglan.com/2017/02/17/the-myers-diff-algorithm-part-3/
// https://www.codeproject.com/Articles/42279/%2FArticles%2F42279%2FInvestigating-Myers-diff-algorithm-Part-1-of-2
@@ -208,3 +210,11 @@
}
return nil, 0
}
+
+func SplitLines(text string) []string {
+ lines := strings.SplitAfter(text, "\n")
+ if lines[len(lines)-1] == "" {
+ lines = lines[:len(lines)-1]
+ }
+ return lines
+}
diff --git a/internal/lsp/diff/diff_test.go b/internal/lsp/diff/diff_test.go
index 1eb32f4..1fe4292 100644
--- a/internal/lsp/diff/diff_test.go
+++ b/internal/lsp/diff/diff_test.go
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-package diff
+package diff_test
import (
"flag"
@@ -13,6 +13,8 @@
"reflect"
"strings"
"testing"
+
+ "golang.org/x/tools/internal/lsp/diff"
)
const (
@@ -26,22 +28,22 @@
func TestDiff(t *testing.T) {
for _, test := range []struct {
a, b string
- lines []*Op
- operations []*Op
+ lines []*diff.Op
+ operations []*diff.Op
unified string
nodiff bool
}{
{
a: "A\nB\nC\n",
b: "A\nB\nC\n",
- operations: []*Op{},
+ operations: []*diff.Op{},
unified: `
`[1:]}, {
a: "A\n",
b: "B\n",
- operations: []*Op{
- &Op{Kind: Delete, I1: 0, I2: 1, J1: 0},
- &Op{Kind: Insert, Content: []string{"B\n"}, I1: 1, I2: 1, J1: 0},
+ operations: []*diff.Op{
+ &diff.Op{Kind: diff.Delete, I1: 0, I2: 1, J1: 0},
+ &diff.Op{Kind: diff.Insert, Content: []string{"B\n"}, I1: 1, I2: 1, J1: 0},
},
unified: `
@@ -1 +1 @@
@@ -50,9 +52,9 @@
`[1:]}, {
a: "A",
b: "B",
- operations: []*Op{
- &Op{Kind: Delete, I1: 0, I2: 1, J1: 0},
- &Op{Kind: Insert, Content: []string{"B"}, I1: 1, I2: 1, J1: 0},
+ operations: []*diff.Op{
+ &diff.Op{Kind: diff.Delete, I1: 0, I2: 1, J1: 0},
+ &diff.Op{Kind: diff.Insert, Content: []string{"B"}, I1: 1, I2: 1, J1: 0},
},
unified: `
@@ -1 +1 @@
@@ -63,12 +65,12 @@
`[1:]}, {
a: "A\nB\nC\nA\nB\nB\nA\n",
b: "C\nB\nA\nB\nA\nC\n",
- operations: []*Op{
- &Op{Kind: Delete, I1: 0, I2: 1, J1: 0},
- &Op{Kind: Delete, I1: 1, I2: 2, J1: 0},
- &Op{Kind: Insert, Content: []string{"B\n"}, I1: 3, I2: 3, J1: 1},
- &Op{Kind: Delete, I1: 5, I2: 6, J1: 4},
- &Op{Kind: Insert, Content: []string{"C\n"}, I1: 7, I2: 7, J1: 5},
+ operations: []*diff.Op{
+ &diff.Op{Kind: diff.Delete, I1: 0, I2: 1, J1: 0},
+ &diff.Op{Kind: diff.Delete, I1: 1, I2: 2, J1: 0},
+ &diff.Op{Kind: diff.Insert, Content: []string{"B\n"}, I1: 3, I2: 3, J1: 1},
+ &diff.Op{Kind: diff.Delete, I1: 5, I2: 6, J1: 4},
+ &diff.Op{Kind: diff.Insert, Content: []string{"C\n"}, I1: 7, I2: 7, J1: 5},
},
unified: `
@@ -1,7 +1,6 @@
@@ -87,10 +89,10 @@
{
a: "A\nB\n",
b: "A\nC\n\n",
- operations: []*Op{
- &Op{Kind: Delete, I1: 1, I2: 2, J1: 1},
- &Op{Kind: Insert, Content: []string{"C\n"}, I1: 2, I2: 2, J1: 1},
- &Op{Kind: Insert, Content: []string{"\n"}, I1: 2, I2: 2, J1: 2},
+ operations: []*diff.Op{
+ &diff.Op{Kind: diff.Delete, I1: 1, I2: 2, J1: 1},
+ &diff.Op{Kind: diff.Insert, Content: []string{"C\n"}, I1: 2, I2: 2, J1: 1},
+ &diff.Op{Kind: diff.Insert, Content: []string{"\n"}, I1: 2, I2: 2, J1: 2},
},
unified: `
@@ -1,2 +1,3 @@
@@ -118,9 +120,9 @@
+K
`[1:]},
} {
- a := strings.SplitAfter(test.a, "\n")
- b := strings.SplitAfter(test.b, "\n")
- ops := Operations(a, b)
+ a := diff.SplitLines(test.a)
+ b := diff.SplitLines(test.b)
+ ops := diff.Operations(a, b)
if test.operations != nil {
if len(ops) != len(test.operations) {
t.Fatalf("expected %v operations, got %v", len(test.operations), len(ops))
@@ -132,7 +134,7 @@
}
}
}
- applied := ApplyEdits(a, ops)
+ applied := diff.ApplyEdits(a, ops)
for i, want := range applied {
got := b[i]
if got != want {
@@ -140,7 +142,7 @@
}
}
if test.unified != "" {
- diff := ToUnified(fileA, fileB, a, ops)
+ diff := diff.ToUnified(fileA, fileB, a, ops)
got := fmt.Sprint(diff)
if !strings.HasPrefix(got, unifiedPrefix) {
t.Errorf("expected prefix:\n%s\ngot:\n%s", unifiedPrefix, got)
diff --git a/internal/lsp/diff/unified.go b/internal/lsp/diff/unified.go
index ab014aa..427a871 100644
--- a/internal/lsp/diff/unified.go
+++ b/internal/lsp/diff/unified.go
@@ -38,9 +38,6 @@
if len(ops) == 0 {
return u
}
- if lines[len(lines)-1] == "" {
- lines = lines[:len(lines)-1]
- }
var h *Hunk
last := -(gap + 2)
for _, op := range ops {
diff --git a/internal/lsp/format.go b/internal/lsp/format.go
index 5145e2a..0277f5c 100644
--- a/internal/lsp/format.go
+++ b/internal/lsp/format.go
@@ -31,10 +31,10 @@
if err != nil {
return nil, err
}
- return toProtocolEdits(m, edits)
+ return ToProtocolEdits(m, edits)
}
-func toProtocolEdits(m *protocol.ColumnMapper, edits []source.TextEdit) ([]protocol.TextEdit, error) {
+func ToProtocolEdits(m *protocol.ColumnMapper, edits []source.TextEdit) ([]protocol.TextEdit, error) {
if edits == nil {
return nil, nil
}
@@ -52,6 +52,24 @@
return result, nil
}
+func FromProtocolEdits(m *protocol.ColumnMapper, edits []protocol.TextEdit) ([]source.TextEdit, error) {
+ if edits == nil {
+ return nil, nil
+ }
+ result := make([]source.TextEdit, len(edits))
+ for i, edit := range edits {
+ spn, err := m.RangeSpan(edit.Range)
+ if err != nil {
+ return nil, err
+ }
+ result[i] = source.TextEdit{
+ Span: spn,
+ NewText: edit.NewText,
+ }
+ }
+ return result, nil
+}
+
func newColumnMap(ctx context.Context, v source.View, uri span.URI) (source.File, *protocol.ColumnMapper, error) {
f, err := v.GetFile(ctx, uri)
if err != nil {
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 60ff67f..22a502b 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -20,6 +20,7 @@
"golang.org/x/tools/go/packages/packagestest"
"golang.org/x/tools/internal/lsp/cache"
+ "golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/lsp/xlog"
@@ -413,17 +414,18 @@
}
continue
}
- f, m, err := newColumnMap(ctx, s.findView(ctx, uri), uri)
+ _, m, err := newColumnMap(ctx, s.findView(ctx, uri), uri)
if err != nil {
t.Error(err)
}
- buf, err := applyEdits(m, f.GetContent(context.Background()), edits)
+ sedits, err := FromProtocolEdits(m, edits)
if err != nil {
t.Error(err)
}
- got := string(buf)
+ ops := source.EditsToDiff(sedits)
+ got := strings.Join(diff.ApplyEdits(diff.SplitLines(string(m.Content)), ops), "")
if gofmted != got {
- t.Errorf("format failed for %s: expected '%v', got '%v'", filename, gofmted, got)
+ t.Errorf("format failed for %s, expected:\n%v\ngot:\n%v", filename, gofmted, got)
}
}
}
@@ -660,26 +662,3 @@
}
}
}
-
-func applyEdits(m *protocol.ColumnMapper, content []byte, edits []protocol.TextEdit) ([]byte, error) {
- prev := 0
- result := make([]byte, 0, len(content))
- for _, edit := range edits {
- spn, err := m.RangeSpan(edit.Range)
- if err != nil {
- return nil, err
- }
- offset := spn.Start().Offset()
- if offset > prev {
- result = append(result, content[prev:offset]...)
- }
- if len(edit.NewText) > 0 {
- result = append(result, []byte(edit.NewText)...)
- }
- prev = spn.End().Offset()
- }
- if prev < len(content) {
- result = append(result, content[prev:]...)
- }
- return result, nil
-}
diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go
index cc7d72a..bcc0d7b 100644
--- a/internal/lsp/source/format.go
+++ b/internal/lsp/source/format.go
@@ -10,7 +10,6 @@
"context"
"fmt"
"go/format"
- "strings"
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/packages"
@@ -62,18 +61,7 @@
}
func computeTextEdits(ctx context.Context, file File, formatted string) (edits []TextEdit) {
- u := strings.SplitAfter(string(file.GetContent(ctx)), "\n")
- f := strings.SplitAfter(formatted, "\n")
- for _, op := range diff.Operations(u, f) {
- s := span.New(file.URI(), span.NewPoint(op.I1+1, 1, 0), span.NewPoint(op.I2+1, 1, 0))
- switch op.Kind {
- case diff.Delete:
- // Delete: unformatted[i1:i2] is deleted.
- edits = append(edits, TextEdit{Span: s})
- case diff.Insert:
- // Insert: formatted[j1:j2] is inserted at unformatted[i1:i1].
- edits = append(edits, TextEdit{Span: s, NewText: strings.Join(op.Content, "")})
- }
- }
- return edits
+ u := diff.SplitLines(string(file.GetContent(ctx)))
+ f := diff.SplitLines(formatted)
+ return DiffToEdits(file.URI(), diff.Operations(u, f))
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 243857f..896ac8e 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -9,9 +9,11 @@
"go/ast"
"go/token"
"go/types"
+ "strings"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"
+ "golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/xlog"
"golang.org/x/tools/internal/span"
)
@@ -58,3 +60,49 @@
Span span.Span
NewText string
}
+
+// DiffToEdits converts from a sequence of diff operations to a sequence of
+// source.TextEdit
+func DiffToEdits(uri span.URI, ops []*diff.Op) []TextEdit {
+ edits := make([]TextEdit, 0, len(ops))
+ for _, op := range ops {
+ s := span.New(uri, span.NewPoint(op.I1+1, 1, 0), span.NewPoint(op.I2+1, 1, 0))
+ switch op.Kind {
+ case diff.Delete:
+ // Delete: unformatted[i1:i2] is deleted.
+ edits = append(edits, TextEdit{Span: s})
+ case diff.Insert:
+ // Insert: formatted[j1:j2] is inserted at unformatted[i1:i1].
+ if content := strings.Join(op.Content, ""); content != "" {
+ edits = append(edits, TextEdit{Span: s, NewText: content})
+ }
+ }
+ }
+ return edits
+}
+
+func EditsToDiff(edits []TextEdit) []*diff.Op {
+ iToJ := 0
+ ops := make([]*diff.Op, len(edits))
+ for i, edit := range edits {
+ i1 := edit.Span.Start().Line() - 1
+ i2 := edit.Span.End().Line() - 1
+ kind := diff.Insert
+ if edit.NewText == "" {
+ kind = diff.Delete
+ }
+ ops[i] = &diff.Op{
+ Kind: kind,
+ Content: diff.SplitLines(edit.NewText),
+ I1: i1,
+ I2: i2,
+ J1: i1 + iToJ,
+ }
+ if kind == diff.Insert {
+ iToJ += len(ops[i].Content)
+ } else {
+ iToJ -= i2 - i1
+ }
+ }
+ return ops
+}
