From 7025dca8beb3e7331201265a1bb833461f6a8091 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Thu, 3 Oct 2019 14:56:48 -0400 Subject: [PATCH] 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 Reviewed-by: Rebecca Stambler --- internal/lsp/diff/diff.go | 93 +++++++++++++++++++++++--- internal/lsp/diff/diff_test.go | 57 ++++++++++++++++ internal/lsp/diff/difftest/difftest.go | 21 ++++-- internal/lsp/diff/unified.go | 28 +++----- 4 files changed, 165 insertions(+), 34 deletions(-) diff --git a/internal/lsp/diff/diff.go b/internal/lsp/diff/diff.go index a3560e886e..2aad5138ce 100644 --- a/internal/lsp/diff/diff.go +++ b/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 } diff --git a/internal/lsp/diff/diff_test.go b/internal/lsp/diff/diff_test.go index 68ab1e8420..8c93c10283 100644 --- a/internal/lsp/diff/diff_test.go +++ b/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 +} diff --git a/internal/lsp/diff/difftest/difftest.go b/internal/lsp/diff/difftest/difftest.go index da82704285..22edd91992 100644 --- a/internal/lsp/diff/difftest/difftest.go +++ b/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) + } } } diff --git a/internal/lsp/diff/unified.go b/internal/lsp/diff/unified.go index 5cc93865c4..b2e630effe 100644 --- a/internal/lsp/diff/unified.go +++ b/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++