internal/lsp: expand edits to whole lines in ToUnified

We don't want to support sub line edits or non line context in the unified diff
printing.

Change-Id: I4267b11b50f12d817dac0fbbae7e1db44ca3dad0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199739
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/diff/diff.go

@@ -44,19 +44,17 @@ func ApplyEdits(before string, edits []TextEdit) string {
 	if len(edits) == 0 {
 		return before
 	}
-	edits = prepareEdits(edits)
-	c := span.NewContentConverter("", []byte(before))
+	_, edits, _ = prepareEdits(before, edits)
 	after := strings.Builder{}
 	last := 0
 	for _, edit := range edits {
-		spn, _ := edit.Span.WithAll(c)
-		start := spn.Start().Offset()
+		start := edit.Span.Start().Offset()
 		if start > last {
 			after.WriteString(before[last:start])
 			last = start
 		}
 		after.WriteString(edit.NewText)
-		last = spn.End().Offset()
+		last = edit.Span.End().Offset()
 	}
 	if last < len(before) {
 		after.WriteString(before[last:])
@@ -64,10 +62,83 @@ func ApplyEdits(before string, edits []TextEdit) string {
 	return after.String()
 }
 
-// prepareEdits returns a sorted copy of the edits
-func prepareEdits(edits []TextEdit) []TextEdit {
-	copied := make([]TextEdit, len(edits))
-	copy(copied, edits)
-	SortTextEdits(copied)
-	return copied
+// LineEdits takes a set of edits and expands and merges them as necessary
+// to ensure that there are only full line edits left when it is done.
+func LineEdits(before string, edits []TextEdit) []TextEdit {
+	if len(edits) == 0 {
+		return nil
+	}
+	c, edits, partial := prepareEdits(before, edits)
+	if partial {
+		edits = lineEdits(before, c, edits)
+	}
+	return edits
+}
+
+// prepareEdits returns a sorted copy of the edits
+func prepareEdits(before string, edits []TextEdit) (*span.TokenConverter, []TextEdit, bool) {
+	partial := false
+	c := span.NewContentConverter("", []byte(before))
+	copied := make([]TextEdit, len(edits))
+	for i, edit := range edits {
+		edit.Span, _ = edit.Span.WithAll(c)
+		copied[i] = edit
+		partial = partial || edit.Span.Start().Column() > 1 || edit.Span.End().Column() > 1
+	}
+	SortTextEdits(copied)
+	return c, copied, partial
+}
+
+// lineEdits rewrites the edits to always be full line edits
+func lineEdits(before string, c *span.TokenConverter, edits []TextEdit) []TextEdit {
+	adjusted := make([]TextEdit, 0, len(edits))
+	current := TextEdit{Span: span.Invalid}
+	for _, edit := range edits {
+		if current.Span.IsValid() && edit.Span.Start().Line() <= current.Span.End().Line() {
+			// overlaps with the current edit, need to combine
+			// first get the gap from the previous edit
+			gap := before[current.Span.End().Offset():edit.Span.Start().Offset()]
+			// now add the text of this edit
+			current.NewText += gap + edit.NewText
+			// and then adjust the end position
+			current.Span = span.New(current.Span.URI(), current.Span.Start(), edit.Span.End())
+		} else {
+			// does not overlap, add previous run (if there is one)
+			adjusted = addEdit(before, adjusted, current)
+			// and then remember this edit as the start of the next run
+			current = edit
+		}
+	}
+	// add the current pending run if there is one
+	return addEdit(before, adjusted, current)
+}
+
+func addEdit(before string, edits []TextEdit, edit TextEdit) []TextEdit {
+	if !edit.Span.IsValid() {
+		return edits
+	}
+	// if edit is partial, expand it to full line now
+	start := edit.Span.Start()
+	end := edit.Span.End()
+	if start.Column() > 1 {
+		// prepend the text and adjust to start of line
+		delta := start.Column() - 1
+		start = span.NewPoint(start.Line(), 1, start.Offset()-delta)
+		edit.Span = span.New(edit.Span.URI(), start, end)
+		edit.NewText = before[start.Offset():start.Offset()+delta] + edit.NewText
+	}
+	if end.Column() > 1 {
+		remains := before[end.Offset():]
+		eol := strings.IndexRune(remains, '\n')
+		if eol < 0 {
+			eol = len(remains)
+		} else {
+			eol++
+		}
+		end = span.NewPoint(end.Line()+1, 1, end.Offset()+eol)
+		edit.Span = span.New(edit.Span.URI(), start, end)
+		edit.NewText = edit.NewText + remains[:eol]
+	}
+	edits = append(edits, edit)
+	return edits
 }

internal/lsp/diff/diff_test.go

@@ -1,10 +1,12 @@
 package diff_test
 
 import (
+	"fmt"
 	"testing"
 
 	"golang.org/x/tools/internal/lsp/diff"
 	"golang.org/x/tools/internal/lsp/diff/difftest"
+	"golang.org/x/tools/internal/span"
 )
 
 func TestApplyEdits(t *testing.T) {
@@ -14,6 +16,61 @@ func TestApplyEdits(t *testing.T) {
 			if got := diff.ApplyEdits(tc.In, tc.Edits); got != tc.Out {
 				t.Errorf("ApplyEdits edits got %q, want %q", got, tc.Out)
 			}
+			if tc.LineEdits != nil {
+				if got := diff.ApplyEdits(tc.In, tc.LineEdits); got != tc.Out {
+					t.Errorf("ApplyEdits lineEdits got %q, want %q", got, tc.Out)
+				}
+			}
 		})
 	}
 }
+
+func TestLineEdits(t *testing.T) {
+	for _, tc := range difftest.TestCases {
+		t.Run(tc.Name, func(t *testing.T) {
+			t.Helper()
+			// if line edits not specified, it is the same as edits
+			edits := tc.LineEdits
+			if edits == nil {
+				edits = tc.Edits
+			}
+			if got := diff.LineEdits(tc.In, tc.Edits); diffEdits(got, edits) {
+				t.Errorf("LineEdits got %q, want %q", got, edits)
+			}
+		})
+	}
+}
+
+func TestUnified(t *testing.T) {
+	for _, tc := range difftest.TestCases {
+		t.Run(tc.Name, func(t *testing.T) {
+			t.Helper()
+			unified := fmt.Sprint(diff.ToUnified(difftest.FileA, difftest.FileB, tc.In, tc.Edits))
+			if unified != tc.Unified {
+				t.Errorf("edits got diff:\n%v\nexpected:\n%v", unified, tc.Unified)
+			}
+			if tc.LineEdits != nil {
+				unified := fmt.Sprint(diff.ToUnified(difftest.FileA, difftest.FileB, tc.In, tc.LineEdits))
+				if unified != tc.Unified {
+					t.Errorf("lineEdits got diff:\n%v\nexpected:\n%v", unified, tc.Unified)
+				}
+			}
+		})
+	}
+}
+
+func diffEdits(got, want []diff.TextEdit) bool {
+	if len(got) != len(want) {
+		return true
+	}
+	for i, w := range want {
+		g := got[i]
+		if span.Compare(w.Span, g.Span) != 0 {
+			return true
+		}
+		if w.NewText != g.NewText {
+			return true
+		}
+	}
+	return false
+}

internal/lsp/diff/difftest/difftest.go

@@ -23,7 +23,7 @@ const (
 
 var TestCases = []struct {
 	Name, In, Out, Unified string
-	Edits                  []diff.TextEdit
+	Edits, LineEdits       []diff.TextEdit
 	NoDiff                 bool
 }{{
 	Name: "empty",
@@ -42,7 +42,8 @@ var TestCases = []struct {
 -fruit
 +cheese
 `[1:],
-	Edits: []diff.TextEdit{{Span: newSpan(0, 5), NewText: "cheese"}},
+	Edits:     []diff.TextEdit{{Span: newSpan(0, 5), NewText: "cheese"}},
+	LineEdits: []diff.TextEdit{{Span: newSpan(0, 6), NewText: "cheese\n"}},
 }, {
 	Name: "insert_rune",
 	In:   "gord\n",
@@ -52,7 +53,8 @@ var TestCases = []struct {
 -gord
 +gourd
 `[1:],
-	Edits: []diff.TextEdit{{Span: newSpan(2, 2), NewText: "u"}},
+	Edits:     []diff.TextEdit{{Span: newSpan(2, 2), NewText: "u"}},
+	LineEdits: []diff.TextEdit{{Span: newSpan(0, 5), NewText: "gourd\n"}},
 }, {
 	Name: "delete_rune",
 	In:   "groat\n",
@@ -62,7 +64,8 @@ var TestCases = []struct {
 -groat
 +goat
 `[1:],
-	Edits: []diff.TextEdit{{Span: newSpan(1, 2), NewText: ""}},
+	Edits:     []diff.TextEdit{{Span: newSpan(1, 2), NewText: ""}},
+	LineEdits: []diff.TextEdit{{Span: newSpan(0, 6), NewText: "goat\n"}},
 }, {
 	Name: "replace_rune",
 	In:   "loud\n",
@@ -72,7 +75,8 @@ var TestCases = []struct {
 -loud
 +lord
 `[1:],
-	Edits: []diff.TextEdit{{Span: newSpan(2, 3), NewText: "r"}},
+	Edits:     []diff.TextEdit{{Span: newSpan(2, 3), NewText: "r"}},
+	LineEdits: []diff.TextEdit{{Span: newSpan(0, 5), NewText: "lord\n"}},
 }, {
 	Name: "replace_partials",
 	In:   "blanket\n",
@@ -86,6 +90,7 @@ var TestCases = []struct {
 		{Span: newSpan(1, 3), NewText: "u"},
 		{Span: newSpan(6, 7), NewText: "r"},
 	},
+	LineEdits: []diff.TextEdit{{Span: newSpan(0, 8), NewText: "bunker\n"}},
 }, {
 	Name: "insert_line",
 	In:   "one\nthree\n",
@@ -144,7 +149,8 @@ var TestCases = []struct {
 +C
 +
 `[1:],
-		Edits: []diff.TextEdit{{Span: newSpan(2, 3), NewText: "C\n"}},
+		Edits:     []diff.TextEdit{{Span: newSpan(2, 3), NewText: "C\n"}},
+		LineEdits: []diff.TextEdit{{Span: newSpan(2, 4), NewText: "C\n\n"}},
 	},
 	{
 		Name: "mulitple_replace",
@@ -179,6 +185,9 @@ func init() {
 		for i := range tc.Edits {
 			tc.Edits[i].Span, _ = tc.Edits[i].Span.WithAll(c)
 		}
+		for i := range tc.LineEdits {
+			tc.LineEdits[i].Span, _ = tc.LineEdits[i].Span.WithAll(c)
+		}
 	}
 }
 

internal/lsp/diff/unified.go

@@ -7,8 +7,6 @@ package diff
 import (
 	"fmt"
 	"strings"
-
-	"golang.org/x/tools/internal/span"
 )
 
 // Unified represents a set of edits as a unified diff.
@@ -85,21 +83,18 @@ func ToUnified(from, to string, content string, edits []TextEdit) Unified {
 	if len(edits) == 0 {
 		return u
 	}
+	c, edits, partial := prepareEdits(content, edits)
+	if partial {
+		edits = lineEdits(content, c, edits)
+	}
 	lines := splitLines(content)
 	var h *Hunk
 	last := 0
-	c := span.NewContentConverter(from, []byte(content))
 	toLine := 0
 	for _, edit := range edits {
-		spn, _ := edit.Span.WithAll(c)
-		start := spn.Start().Line() - 1
-		end := spn.End().Line() - 1
-		if spn.Start().Column() > 1 || spn.End().Column() > 1 {
-			panic("cannot convert partial line edits to unified diff")
-		}
+		start := edit.Span.Start().Line() - 1
+		end := edit.Span.End().Line() - 1
 		switch {
-		case start < last:
-			panic("cannot convert unsorted edits to unified diff")
 		case h != nil && start == last:
 			//direct extension
 		case h != nil && start <= last+gap:
@@ -123,12 +118,11 @@ func ToUnified(from, to string, content string, edits []TextEdit) Unified {
 			h.ToLine -= delta
 		}
 		last = start
-		if edit.NewText == "" {
-			for i := start; i < end; i++ {
-				h.Lines = append(h.Lines, Line{Kind: Delete, Content: lines[i]})
-				last++
-			}
-		} else {
+		for i := start; i < end; i++ {
+			h.Lines = append(h.Lines, Line{Kind: Delete, Content: lines[i]})
+			last++
+		}
+		if edit.NewText != "" {
 			for _, line := range splitLines(edit.NewText) {
 				h.Lines = append(h.Lines, Line{Kind: Insert, Content: line})
 				toLine++