From a0f5e6c5c28cd7ac82dc45318ee2465c103c6e9f Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Mon, 15 Jul 2019 17:02:40 -0400 Subject: [PATCH] internal/lsp: sort rename results We recommend that gopls integrators apply the []TextEdit responses in reverse order to get a correct resulting document. This strategy works when the response is already sorted. Have gopls return sorted []TextEdit for each file. Fixes golang/go#33123 Change-Id: Ib570881c9623695d2ae3194fa8a97b0a681a3250 Reviewed-on: https://go-review.googlesource.com/c/tools/+/186258 Run-TryBot: Suzy Mueller Reviewed-by: Rebecca Stambler --- internal/lsp/lsp_test.go | 10 ---------- internal/lsp/source/rename.go | 11 ++++++++++- internal/lsp/source/source_test.go | 10 ---------- internal/lsp/source/view.go | 8 ++++++++ 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 7c8fdf8415..f8a818c735 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -582,7 +582,6 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) { func applyEdits(contents string, edits []source.TextEdit) string { res := contents - sortSourceTextEdits(edits) // Apply the edits from the end of the file forward // to preserve the offsets @@ -596,15 +595,6 @@ func applyEdits(contents string, edits []source.TextEdit) string { return res } -func sortSourceTextEdits(d []source.TextEdit) { - sort.Slice(d, func(i int, j int) bool { - if r := span.Compare(d[i].Span, d[j].Span); r != 0 { - return r < 0 - } - return d[i].NewText < d[j].NewText - }) -} - func (r *runner) Symbol(t *testing.T, data tests.Symbols) { for uri, expectedSymbols := range data { params := &protocol.DocumentSymbolParams{ diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 54a5af9fe3..d8eb3a3536 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -85,7 +85,16 @@ func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.U return nil, fmt.Errorf(r.errors) } - return r.update() + changes, err := r.update() + if err != nil { + return nil, err + } + + // Sort edits for each file. + for _, edits := range changes { + sortTextEdits(edits) + } + return changes, nil } // Rename all references to the identifier. diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 7bd469e732..a73a0afbf0 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -547,7 +547,6 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) { func applyEdits(contents string, edits []source.TextEdit) string { res := contents - sortSourceTextEdits(edits) // Apply the edits from the end of the file forward // to preserve the offsets @@ -561,15 +560,6 @@ func applyEdits(contents string, edits []source.TextEdit) string { return res } -func sortSourceTextEdits(d []source.TextEdit) { - sort.Slice(d, func(i int, j int) bool { - if r := span.Compare(d[i].Span, d[j].Span); r != 0 { - return r < 0 - } - return d[i].NewText < d[j].NewText - }) -} - func (r *runner) Symbol(t *testing.T, data tests.Symbols) { ctx := r.ctx for uri, expectedSymbols := range data { diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index bc0ad76a25..4f7e782a68 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -9,6 +9,7 @@ import ( "go/ast" "go/token" "go/types" + "sort" "strings" "golang.org/x/tools/go/analysis" @@ -318,3 +319,10 @@ func EditsToDiff(edits []TextEdit) []*diff.Op { } return ops } + +func sortTextEdits(d []TextEdit) { + // Use a stable sort to maintain the order of edits inserted at the same position. + sort.SliceStable(d, func(i int, j int) bool { + return span.Compare(d[i].Span, d[j].Span) < 0 + }) +}