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>

internal/lsp/code_action.go

@@ -81,7 +81,7 @@ func organizeImports(ctx context.Context, v source.View, s span.Span) (*protocol
 	if err != nil {
 		return nil, err
 	}
-	protocolEdits, err := toProtocolEdits(m, edits)
+	protocolEdits, err := ToProtocolEdits(m, edits)
 	if err != nil {
 		return nil, err
 	}

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 @@ func shortestEditSequence(a, b []string) ([][]int, int) {
 	}
 	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
+}

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 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+
+	"golang.org/x/tools/internal/lsp/diff"
 )
 
 const (
@@ -26,22 +28,22 @@ var verifyDiff = flag.Bool("verify-diff", false, "Check that the unified diff ou
 func TestDiff(t *testing.T) {
 	for _, test := range []struct {
 		a, b       string
-		lines      []*Op
+		lines      []*diff.Op
-		operations []*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{
+			operations: []*diff.Op{
-				&Op{Kind: Delete, I1: 0, I2: 1, J1: 0},
+				&diff.Op{Kind: diff.Delete, I1: 0, I2: 1, J1: 0},
-				&Op{Kind: Insert, Content: []string{"B\n"}, I1: 1, 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 @@ func TestDiff(t *testing.T) {
 `[1:]}, {
 			a: "A",
 			b: "B",
-			operations: []*Op{
+			operations: []*diff.Op{
-				&Op{Kind: Delete, I1: 0, I2: 1, J1: 0},
+				&diff.Op{Kind: diff.Delete, I1: 0, I2: 1, J1: 0},
-				&Op{Kind: Insert, Content: []string{"B"}, I1: 1, 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 @@ func TestDiff(t *testing.T) {
 `[1:]}, {
 			a: "A\nB\nC\nA\nB\nB\nA\n",
 			b: "C\nB\nA\nB\nA\nC\n",
-			operations: []*Op{
+			operations: []*diff.Op{
-				&Op{Kind: Delete, I1: 0, I2: 1, J1: 0},
+				&diff.Op{Kind: diff.Delete, I1: 0, I2: 1, J1: 0},
-				&Op{Kind: Delete, I1: 1, I2: 2, J1: 0},
+				&diff.Op{Kind: diff.Delete, I1: 1, I2: 2, J1: 0},
-				&Op{Kind: Insert, Content: []string{"B\n"}, I1: 3, I2: 3, J1: 1},
+				&diff.Op{Kind: diff.Insert, Content: []string{"B\n"}, I1: 3, I2: 3, J1: 1},
-				&Op{Kind: Delete, I1: 5, I2: 6, J1: 4},
+				&diff.Op{Kind: diff.Delete, I1: 5, I2: 6, J1: 4},
-				&Op{Kind: Insert, Content: []string{"C\n"}, I1: 7, I2: 7, J1: 5},
+				&diff.Op{Kind: diff.Insert, Content: []string{"C\n"}, I1: 7, I2: 7, J1: 5},
 			},
 			unified: `
 @@ -1,7 +1,6 @@
@@ -87,10 +89,10 @@ func TestDiff(t *testing.T) {
 		{
 			a: "A\nB\n",
 			b: "A\nC\n\n",
-			operations: []*Op{
+			operations: []*diff.Op{
-				&Op{Kind: Delete, I1: 1, I2: 2, J1: 1},
+				&diff.Op{Kind: diff.Delete, I1: 1, I2: 2, J1: 1},
-				&Op{Kind: Insert, Content: []string{"C\n"}, I1: 2, I2: 2, J1: 1},
+				&diff.Op{Kind: diff.Insert, Content: []string{"C\n"}, I1: 2, I2: 2, J1: 1},
-				&Op{Kind: Insert, Content: []string{"\n"}, I1: 2, I2: 2, J1: 2},
+				&diff.Op{Kind: diff.Insert, Content: []string{"\n"}, I1: 2, I2: 2, J1: 2},
 			},
 			unified: `
 @@ -1,2 +1,3 @@
@@ -118,9 +120,9 @@ func TestDiff(t *testing.T) {
 +K
 `[1:]},
 	} {
-		a := strings.SplitAfter(test.a, "\n")
+		a := diff.SplitLines(test.a)
-		b := strings.SplitAfter(test.b, "\n")
+		b := diff.SplitLines(test.b)
-		ops := Operations(a, 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 @@ func TestDiff(t *testing.T) {
 				}
 			}
 		}
-		applied := ApplyEdits(a, ops)
+		applied := diff.ApplyEdits(a, ops)
 		for i, want := range applied {
 			got := b[i]
 			if got != want {
@@ -140,7 +142,7 @@ func TestDiff(t *testing.T) {
 			}
 		}
 		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)

internal/lsp/diff/unified.go

@@ -38,9 +38,6 @@ func ToUnified(from, to string, lines []string, ops []*Op) Unified {
 	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 {

internal/lsp/format.go

@@ -31,10 +31,10 @@ func formatRange(ctx context.Context, v source.View, s span.Span) ([]protocol.Te
 	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 @@ func toProtocolEdits(m *protocol.ColumnMapper, edits []source.TextEdit) ([]proto
 	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 {

internal/lsp/lsp_test.go

@@ -20,6 +20,7 @@ import (
 
 	"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 @@ func (f formats) test(t *testing.T, s *Server) {
 			}
 			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 TestBytesOffset(t *testing.T) {
 		}
 	}
 }
-
-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
-}

internal/lsp/source/format.go

@@ -10,7 +10,6 @@ import (
 	"context"
 	"fmt"
 	"go/format"
-	"strings"
 
 	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/go/packages"
@@ -62,18 +61,7 @@ func Imports(ctx context.Context, f File, rng span.Range) ([]TextEdit, error) {
 }
 
 func computeTextEdits(ctx context.Context, file File, formatted string) (edits []TextEdit) {
-	u := strings.SplitAfter(string(file.GetContent(ctx)), "\n")
+	u := diff.SplitLines(string(file.GetContent(ctx)))
-	f := strings.SplitAfter(formatted, "\n")
+	f := diff.SplitLines(formatted)
-	for _, op := range diff.Operations(u, f) {
+	return DiffToEdits(file.URI(), 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
 }

internal/lsp/source/view.go

@@ -9,9 +9,11 @@ import (
 	"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 @@ type TextEdit struct {
 	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
+}