internal/lsp/diff: fix bug that adds extra line to files on format

Small changes to handle the last line in the diff library, LSP tests,
and diff to text edits conversion.

Fixes golang/go#30137

Change-Id: Iff0e53a04c2dabf6f54eb7c738b4c0837f16efba
Reviewed-on: https://go-review.googlesource.com/c/162217
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/diff/diff.go

@@ -5,7 +5,9 @@
 // Package diff implements the Myers diff algorithm.
 package diff
 
-import "strings"
+import (
+	"strings"
+)
 
 // Sources:
 // https://blog.jcoglan.com/2017/02/17/the-myers-diff-algorithm-part-3/
@@ -70,6 +72,8 @@ func Operations(a, b []string) []*Op {
 	trace, offset := shortestEditSequence(a, b)
 	snakes := backtrack(trace, len(a), len(b), offset)
 
+	M, N := len(a), len(b)
+
 	var i int
 	solution := make([]*Op, len(a)+len(b))
 
@@ -101,7 +105,7 @@ func Operations(a, b []string) []*Op {
 				}
 			}
 			x++
-			if x == len(a) {
+			if x == M {
 				break
 			}
 		}
@@ -125,58 +129,7 @@ func Operations(a, b []string) []*Op {
 			x++
 			y++
 		}
-		if x >= len(a) && y >= len(b) {
+		if x >= M && y >= N {
-			break
-		}
-	}
-	return solution[:i]
-}
-
-// Lines returns a list of per-line operations to convert a into b.
-func Lines(a, b []string) []*Op {
-	trace, offset := shortestEditSequence(a, b)
-	snakes := backtrack(trace, len(a), len(b), offset)
-
-	var i int
-	solution := make([]*Op, len(a)+len(b))
-
-	x, y := 0, 0
-	for _, snake := range snakes {
-		if len(snake) < 2 {
-			continue
-		}
-		// horizontal
-		for snake[0]-snake[1] > x-y {
-			solution[i] = &Op{
-				Kind:    Delete,
-				Content: a[x],
-			}
-			i++
-			x++
-			if x == len(a) {
-				break
-			}
-		}
-		// vertical
-		for snake[0]-snake[1] < x-y {
-			solution[i] = &Op{
-				Kind:    Insert,
-				Content: b[y],
-			}
-			i++
-			y++
-		}
-		// diagonal
-		for x < snake[0] {
-			solution[i] = &Op{
-				Kind:    Equal,
-				Content: a[x],
-			}
-			i++
-			x++
-			y++
-		}
-		if x >= len(a) && y >= len(b) {
 			break
 		}
 	}
@@ -208,7 +161,6 @@ func backtrack(trace [][]int, x, y, offset int) [][]int {
 		x = V[kPrev+offset]
 		y = x - kPrev
 	}
-	// this feels questionable
 	if x < 0 || y < 0 {
 		return snakes
 	}
@@ -254,7 +206,7 @@ func shortestEditSequence(a, b []string) ([][]int, int) {
 			trace[d] = copyV
 
 			// Return if we've exceeded the maximum values.
-			if x >= M-1 && y >= N-1 {
+			if x == M && y == N {
 				return trace, offset
 			}
 		}

internal/lsp/diff/diff_test.go

@@ -18,17 +18,6 @@ func TestDiff(t *testing.T) {
 		{
 			a: []string{"A", "B", "C", "A", "B", "B", "A"},
 			b: []string{"C", "B", "A", "B", "A", "C"},
-			lines: []*Op{
-				&Op{Kind: Delete, Content: "A"},
-				&Op{Kind: Delete, Content: "B"},
-				&Op{Kind: Equal, Content: "C"},
-				&Op{Kind: Insert, Content: "B"},
-				&Op{Kind: Equal, Content: "A"},
-				&Op{Kind: Equal, Content: "B"},
-				&Op{Kind: Delete, Content: "B"},
-				&Op{Kind: Equal, Content: "A"},
-				&Op{Kind: Insert, Content: "C"},
-			},
 			operations: []*Op{
 				&Op{Kind: Delete, I1: 0, I2: 1, J1: 0, J2: 0},
 				&Op{Kind: Delete, I1: 1, I2: 2, J1: 0, J2: 0},
@@ -37,27 +26,27 @@ func TestDiff(t *testing.T) {
 				&Op{Kind: Insert, Content: "C", I1: 7, I2: 7, J1: 5, J2: 6},
 			},
 		},
+		{
+			a: []string{"A", "B"},
+			b: []string{"A", "C", ""},
+			operations: []*Op{
+				&Op{Kind: Delete, I1: 1, I2: 2, J1: 1, J2: 1},
+				&Op{Kind: Insert, Content: "C", I1: 2, I2: 2, J1: 1, J2: 2},
+				&Op{Kind: Insert, Content: "", I1: 2, I2: 2, J1: 2, J2: 3},
+			},
+		},
 	} {
-		for i, got := range Lines(tt.a, tt.b) {
+		ops := Operations(tt.a, tt.b)
-			want := tt.lines[i]
+		if len(ops) != len(tt.operations) {
-			if !reflect.DeepEqual(want, got) {
+			t.Fatalf("expected %v operations, got %v", len(tt.operations), len(ops))
-				t.Errorf("expected %v, got %v", want, got)
-			}
 		}
-		b := ApplyEdits(tt.a, tt.lines)
+		for i, got := range ops {
-		for i, want := range tt.b {
-			got := b[i]
-			if got != want {
-				t.Errorf("expected %v got %v", want, got)
-			}
-		}
-		for i, got := range Operations(tt.a, tt.b) {
 			want := tt.operations[i]
 			if !reflect.DeepEqual(want, got) {
 				t.Errorf("expected %v, got %v", want, got)
 			}
 		}
-		b = ApplyEdits(tt.a, tt.operations)
+		b := ApplyEdits(tt.a, tt.operations)
 		for i, want := range tt.b {
 			got := b[i]
 			if got != want {

internal/lsp/lsp_test.go

@@ -38,7 +38,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	// are being executed. If a test is added, this number must be changed.
 	const expectedCompletionsCount = 63
 	const expectedDiagnosticsCount = 17
-	const expectedFormatCount = 3
+	const expectedFormatCount = 4
 	const expectedDefinitionsCount = 16
 	const expectedTypeDefinitionsCount = 2
 
@@ -380,25 +380,30 @@ func (f formats) test(t *testing.T, s *server) {
 		}
 		var ops []*diff.Op
 		for _, edit := range edits {
+			start := int(edit.Range.Start.Line)
+			end := int(edit.Range.End.Line)
+			if start == end && edit.Range.End.Character > 1 {
+				end++
+			}
 			if edit.NewText == "" { // deletion
 				ops = append(ops, &diff.Op{
 					Kind: diff.Delete,
-					I1:   int(edit.Range.Start.Line),
+					I1:   start,
-					I2:   int(edit.Range.End.Line),
+					I2:   end,
 				})
 			} else if edit.Range.Start == edit.Range.End { // insertion
 				ops = append(ops, &diff.Op{
 					Kind:    diff.Insert,
 					Content: edit.NewText,
-					I1:      int(edit.Range.Start.Line),
+					I1:      start,
-					I2:      int(edit.Range.End.Line),
+					I2:      end,
 				})
 			}
 		}
 		split := strings.SplitAfter(string(original), "\n")
 		got := strings.Join(diff.ApplyEdits(split, ops), "")
 		if gofmted != got {
-			t.Errorf("format failed for %s: expected %v, got %v", filename, gofmted, got)
+			t.Errorf("format failed for %s: expected '%v', got '%v'", filename, gofmted, got)
 		}
 	}
 }

internal/lsp/source/format.go

@@ -82,21 +82,29 @@ func computeTextEdits(rng Range, tok *token.File, unformatted, formatted string)
 	u := strings.SplitAfter(unformatted, "\n")
 	f := strings.SplitAfter(formatted, "\n")
 	for _, op := range diff.Operations(u, f) {
+		start := lineStart(tok, op.I1+1)
+		if start == token.NoPos && op.I1 == len(u) {
+			start = tok.Pos(tok.Size())
+		}
+		end := lineStart(tok, op.I2+1)
+		if end == token.NoPos && op.I2 == len(u) {
+			end = tok.Pos(tok.Size())
+		}
 		switch op.Kind {
 		case diff.Delete:
 			// Delete: unformatted[i1:i2] is deleted.
 			edits = append(edits, TextEdit{
 				Range: Range{
-					Start: lineStart(tok, op.I1+1),
+					Start: start,
-					End:   lineStart(tok, op.I2+1),
+					End:   end,
 				},
 			})
 		case diff.Insert:
 			// Insert: formatted[j1:j2] is inserted at unformatted[i1:i1].
 			edits = append(edits, TextEdit{
 				Range: Range{
-					Start: lineStart(tok, op.I1+1),
+					Start: start,
-					End:   lineStart(tok, op.I1+1),
+					End:   start,
 				},
 				NewText: op.Content,
 			})

internal/lsp/testdata/format/newline_format.go.in

@@ -0,0 +1,2 @@
+package format //@format("package")
+func _() {}