From 4adf7a708c2de4c9ea24a1f351c2e1c9b82fbde8 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Tue, 18 Jun 2019 10:23:37 -0400 Subject: [PATCH] internal/lsp: add identifier renaming This change provides support to rename identifiers within a single package. The renaming is performed by finding all references to an identifier, and then creating text edits to replace the existing text with the new identifier. Editing an import spec is not supported. Fixes #27571 Change-Id: I0881b65a1b3c72d7c53d7d6ab1ea386160dc00fb Reviewed-on: https://go-review.googlesource.com/c/tools/+/182585 Run-TryBot: Suzy Mueller Reviewed-by: Rebecca Stambler --- internal/lsp/cmd/cmd_test.go | 4 + internal/lsp/general.go | 3 + internal/lsp/lsp_test.go | 85 +++++++++++++++ internal/lsp/rename.go | 46 ++++++++ internal/lsp/server.go | 4 +- internal/lsp/source/rename.go | 59 +++++++++++ internal/lsp/source/source_test.go | 73 +++++++++++++ .../lsp/testdata/rename/a/random.go.golden | 100 ++++++++++++++++++ internal/lsp/testdata/rename/a/random.go.in | 23 ++++ internal/lsp/tests/tests.go | 18 ++++ 10 files changed, 413 insertions(+), 2 deletions(-) create mode 100644 internal/lsp/rename.go create mode 100644 internal/lsp/source/rename.go create mode 100644 internal/lsp/testdata/rename/a/random.go.golden create mode 100644 internal/lsp/testdata/rename/a/random.go.in diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go index c1a882ff..a9b173e7 100644 --- a/internal/lsp/cmd/cmd_test.go +++ b/internal/lsp/cmd/cmd_test.go @@ -54,6 +54,10 @@ func (r *runner) Reference(t *testing.T, data tests.References) { //TODO: add command line references tests when it works } +func (r *runner) Rename(t *testing.T, data tests.Renames) { + //TODO: add command line rename tests when it works +} + func (r *runner) Symbol(t *testing.T, data tests.Symbols) { //TODO: add command line symbol tests when it works } diff --git a/internal/lsp/general.go b/internal/lsp/general.go index dbc881c6..cba3e209 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -70,6 +70,9 @@ func (s *Server) initialize(ctx context.Context, params *protocol.InitializePara DocumentHighlightProvider: true, DocumentLinkProvider: &protocol.DocumentLinkOptions{}, ReferencesProvider: true, + RenameProvider: &protocol.RenameOptions{ + PrepareProvider: false, + }, SignatureHelpProvider: &protocol.SignatureHelpOptions{ TriggerCharacters: []string{"(", ","}, }, diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index ee6e02c8..db8e26e9 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -492,6 +492,91 @@ func (r *runner) Reference(t *testing.T, data tests.References) { } } +func (r *runner) Rename(t *testing.T, data tests.Renames) { + ctx := context.Background() + for spn, newText := range data { + uri := spn.URI() + filename := uri.Filename() + sm, err := r.mapper(uri) + if err != nil { + t.Fatal(err) + } + loc, err := sm.Location(spn) + if err != nil { + t.Fatalf("failed for %v: %v", spn, err) + } + + workspaceEdits, err := r.server.Rename(ctx, &protocol.RenameParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: protocol.NewURI(uri), + }, + Position: loc.Range.Start, + NewName: newText, + }) + if err != nil { + t.Error(err) + continue + } + + _, m, err := getSourceFile(ctx, r.server.session.ViewOf(uri), uri) + if err != nil { + t.Error(err) + } + + changes := *workspaceEdits.Changes + if len(changes) != 1 { // Renames must only affect a single file in these tests. + t.Errorf("rename failed for %s, edited %d files, wanted 1 file", newText, len(*workspaceEdits.Changes)) + continue + } + + edits := changes[string(uri)] + if edits == nil { + t.Errorf("rename failed for %s, did not edit %s", newText, filename) + continue + } + sedits, err := FromProtocolEdits(m, edits) + if err != nil { + t.Error(err) + } + + got := applyEdits(string(m.Content), sedits) + + tag := fmt.Sprintf("%s-rename", newText) + gorenamed := string(r.data.Golden(tag, filename, func() ([]byte, error) { + return []byte(got), nil + })) + + if gorenamed != got { + t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, gorenamed, got) + } + } +} + +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 + for i := len(edits) - 1; i >= 0; i-- { + edit := edits[i] + start := edit.Span.Start().Offset() + end := edit.Span.End().Offset() + tmp := res[0:start] + edit.NewText + res = tmp + res[end:] + } + 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/rename.go b/internal/lsp/rename.go new file mode 100644 index 00000000..3a2de760 --- /dev/null +++ b/internal/lsp/rename.go @@ -0,0 +1,46 @@ +package lsp + +import ( + "context" + + "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/span" +) + +func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*protocol.WorkspaceEdit, error) { + uri := span.NewURI(params.TextDocument.URI) + view := s.session.ViewOf(uri) + f, m, err := getGoFile(ctx, view, uri) + if err != nil { + return nil, err + } + spn, err := m.PointSpan(params.Position) + if err != nil { + return nil, err + } + rng, err := spn.Range(m.Converter) + if err != nil { + return nil, err + } + + edits, err := source.Rename(ctx, view, f, rng.Start, params.NewName) + if err != nil { + return nil, err + } + + changes := make(map[string][]protocol.TextEdit) + for uri, textEdits := range edits { + _, m, err := getGoFile(ctx, view, uri) + if err != nil { + return nil, err + } + protocolEdits, err := ToProtocolEdits(m, textEdits) + if err != nil { + return nil, err + } + changes[string(uri)] = protocolEdits + } + + return &protocol.WorkspaceEdit{Changes: &changes}, nil +} diff --git a/internal/lsp/server.go b/internal/lsp/server.go index ed04bde4..771e14a3 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -236,8 +236,8 @@ func (s *Server) OnTypeFormatting(context.Context, *protocol.DocumentOnTypeForma return nil, notImplemented("OnTypeFormatting") } -func (s *Server) Rename(context.Context, *protocol.RenameParams) (*protocol.WorkspaceEdit, error) { - return nil, notImplemented("Rename") +func (s *Server) Rename(ctx context.Context, params *protocol.RenameParams) (*protocol.WorkspaceEdit, error) { + return s.rename(ctx, params) } func (s *Server) Declaration(context.Context, *protocol.TextDocumentPositionParams) ([]protocol.DeclarationLink, error) { diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go new file mode 100644 index 00000000..903020df --- /dev/null +++ b/internal/lsp/source/rename.go @@ -0,0 +1,59 @@ +package source + +import ( + "context" + "fmt" + "go/token" + "go/types" + + "golang.org/x/tools/internal/span" +) + +// Rename returns a map of TextEdits for each file modified when renaming a given identifier within a package. +func Rename(ctx context.Context, view View, f GoFile, pos token.Pos, newName string) (map[span.URI][]TextEdit, error) { + pkg := f.GetPackage(ctx) + if pkg == nil || pkg.IsIllTyped() { + return nil, fmt.Errorf("package for %s is ill typed", f.URI()) + } + + // Get the identifier to rename. + ident, err := Identifier(ctx, view, f, pos) + if err != nil { + return nil, err + } + if ident.Name == newName { + return nil, fmt.Errorf("old and new names are the same: %s", newName) + } + + // Do not rename identifiers declared in another package. + if pkg.GetTypes() != ident.decl.obj.Pkg() { + return nil, fmt.Errorf("failed to rename because %q is declared in package %q", ident.Name, ident.decl.obj.Pkg().Name()) + } + + // TODO(suzmue): Support renaming of imported packages. + if _, ok := ident.decl.obj.(*types.PkgName); ok { + return nil, fmt.Errorf("renaming imported package %s not supported", ident.Name) + } + + // TODO(suzmue): Check that renaming ident is ok. + refs, err := ident.References(ctx) + if err != nil { + return nil, err + } + + changes := make(map[span.URI][]TextEdit) + for _, ref := range refs { + refSpan, err := ref.Range.Span() + if err != nil { + return nil, err + } + + edit := TextEdit{ + Span: refSpan, + NewText: newName, + } + changes[refSpan.URI()] = append(changes[refSpan.URI()], edit) + } + + return changes, nil +} diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index f071fb84..953c74b3 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -447,6 +447,79 @@ func (r *runner) Reference(t *testing.T, data tests.References) { } } +func (r *runner) Rename(t *testing.T, data tests.Renames) { + ctx := context.Background() + for spn, newText := range data { + uri := spn.URI() + filename := uri.Filename() + + f, err := r.view.GetFile(ctx, spn.URI()) + if err != nil { + t.Fatalf("failed for %v: %v", spn, err) + } + + tok := f.GetToken(ctx) + pos := tok.Pos(spn.Start().Offset()) + + changes, err := source.Rename(context.Background(), r.view, f.(source.GoFile), pos, newText) + if err != nil { + t.Error(err) + continue + } + + if len(changes) != 1 { // Renames must only affect a single file in these tests. + t.Errorf("rename failed for %s, edited %d files, wanted 1 file", newText, len(changes)) + continue + } + + edits := changes[uri] + if edits == nil { + t.Errorf("rename failed for %s, did not edit %s", newText, filename) + continue + } + data, _, err := f.Handle(ctx).Read(ctx) + if err != nil { + t.Error(err) + continue + } + + got := applyEdits(string(data), edits) + tag := fmt.Sprintf("%s-rename", newText) + gorenamed := string(r.data.Golden(tag, filename, func() ([]byte, error) { + return []byte(got), nil + })) + + if gorenamed != got { + t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, gorenamed, got) + } + } +} + +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 + for i := len(edits) - 1; i >= 0; i-- { + edit := edits[i] + start := edit.Span.Start().Offset() + end := edit.Span.End().Offset() + tmp := res[0:start] + edit.NewText + res = tmp + res[end:] + } + 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 := context.Background() for uri, expectedSymbols := range data { diff --git a/internal/lsp/testdata/rename/a/random.go.golden b/internal/lsp/testdata/rename/a/random.go.golden new file mode 100644 index 00000000..806b4d7f --- /dev/null +++ b/internal/lsp/testdata/rename/a/random.go.golden @@ -0,0 +1,100 @@ +-- GetSum-rename -- +package a + +func Random() int { + y := 6 + 7 + return y +} + +func Random2(y int) int { //@rename("y", "z") + return y +} + +type Pos struct { + x, y int +} + +func (p *Pos) GetSum() int { + return p.x + p.y //@rename("x", "myX") +} + +func _() { + var p Pos //@rename("p", "pos") + _ = p.GetSum() //@rename("Sum", "GetSum") +} + +-- myX-rename -- +package a + +func Random() int { + y := 6 + 7 + return y +} + +func Random2(y int) int { //@rename("y", "z") + return y +} + +type Pos struct { + myX, y int +} + +func (p *Pos) Sum() int { + return p.myX + p.y //@rename("x", "myX") +} + +func _() { + var p Pos //@rename("p", "pos") + _ = p.Sum() //@rename("Sum", "GetSum") +} + +-- pos-rename -- +package a + +func Random() int { + y := 6 + 7 + return y +} + +func Random2(y int) int { //@rename("y", "z") + return y +} + +type Pos struct { + x, y int +} + +func (p *Pos) Sum() int { + return p.x + p.y //@rename("x", "myX") +} + +func _() { + var pos Pos //@rename("p", "pos") + _ = pos.Sum() //@rename("Sum", "GetSum") +} + +-- z-rename -- +package a + +func Random() int { + y := 6 + 7 + return y +} + +func Random2(z int) int { //@rename("y", "z") + return z +} + +type Pos struct { + x, y int +} + +func (p *Pos) Sum() int { + return p.x + p.y //@rename("x", "myX") +} + +func _() { + var p Pos //@rename("p", "pos") + _ = p.Sum() //@rename("Sum", "GetSum") +} + diff --git a/internal/lsp/testdata/rename/a/random.go.in b/internal/lsp/testdata/rename/a/random.go.in new file mode 100644 index 00000000..d4347f87 --- /dev/null +++ b/internal/lsp/testdata/rename/a/random.go.in @@ -0,0 +1,23 @@ +package a + +func Random() int { + y := 6 + 7 + return y +} + +func Random2(y int) int { //@rename("y", "z") + return y +} + +type Pos struct { + x, y int +} + +func (p *Pos) Sum() int { + return p.x + p.y //@rename("x", "myX") +} + +func _() { + var p Pos //@rename("p", "pos") + _ = p.Sum() //@rename("Sum", "GetSum") +} diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index cfbb0615..ceda9f7b 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -34,6 +34,7 @@ const ( ExpectedTypeDefinitionsCount = 2 ExpectedHighlightsCount = 2 ExpectedReferencesCount = 2 + ExpectedRenamesCount = 4 ExpectedSymbolsCount = 1 ExpectedSignaturesCount = 20 ExpectedLinksCount = 2 @@ -57,6 +58,7 @@ type Imports []span.Span type Definitions map[span.Span]Definition type Highlights map[string][]span.Span type References map[span.Span][]span.Span +type Renames map[span.Span]string type Symbols map[span.URI][]source.Symbol type SymbolsChildren map[string][]source.Symbol type Signatures map[span.Span]source.SignatureInformation @@ -74,6 +76,7 @@ type Data struct { Definitions Definitions Highlights Highlights References References + Renames Renames Symbols Symbols symbolsChildren SymbolsChildren Signatures Signatures @@ -93,6 +96,7 @@ type Tests interface { Definition(*testing.T, Definitions) Highlight(*testing.T, Highlights) Reference(*testing.T, References) + Rename(*testing.T, Renames) Symbol(*testing.T, Symbols) SignatureHelp(*testing.T, Signatures) Link(*testing.T, Links) @@ -134,6 +138,7 @@ func Load(t testing.TB, exporter packagestest.Exporter, dir string) *Data { Definitions: make(Definitions), Highlights: make(Highlights), References: make(References), + Renames: make(Renames), Symbols: make(Symbols), symbolsChildren: make(SymbolsChildren), Signatures: make(Signatures), @@ -214,6 +219,7 @@ func Load(t testing.TB, exporter packagestest.Exporter, dir string) *Data { "hover": data.collectHoverDefinitions, "highlight": data.collectHighlights, "refs": data.collectReferences, + "rename": data.collectRenames, "symbol": data.collectSymbols, "signature": data.collectSignatures, "snippet": data.collectCompletionSnippets, @@ -302,6 +308,14 @@ func Run(t *testing.T, tests Tests, data *Data) { tests.Reference(t, data.References) }) + t.Run("Renames", func(t *testing.T) { + t.Helper() + if len(data.Renames) != ExpectedRenamesCount { + t.Errorf("got %v renames expected %v", len(data.Renames), ExpectedRenamesCount) + } + tests.Rename(t, data.Renames) + }) + t.Run("Symbols", func(t *testing.T) { t.Helper() if len(data.Symbols) != ExpectedSymbolsCount { @@ -473,6 +487,10 @@ func (data *Data) collectReferences(src span.Span, expected []span.Span) { data.References[src] = expected } +func (data *Data) collectRenames(src span.Span, newText string) { + data.Renames[src] = newText +} + func (data *Data) collectSymbols(name string, spn span.Span, kind string, parentName string) { sym := source.Symbol{ Name: name,