From f80f67146e584e84e6c179fdb72a38f367331d6e Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Thu, 27 Jun 2019 14:01:56 -0400 Subject: [PATCH] internal/lsp: support renaming import specs Support the renaming of the imported name of a package within a file. This case needs to be special cased because the ident may be added or removed. Change-Id: I333bc2b2ca5ce81c4a2afb8b10035f525dfad464 Reviewed-on: https://go-review.googlesource.com/c/tools/+/184199 Run-TryBot: Suzy Mueller TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/source/rename.go | 70 ++++++- .../lsp/testdata/rename/a/random.go.golden | 196 ++++++++++++++---- internal/lsp/testdata/rename/a/random.go.in | 12 +- internal/lsp/tests/tests.go | 2 +- 4 files changed, 230 insertions(+), 50 deletions(-) diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 59b61712..41668a80 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -5,9 +5,11 @@ package source import ( + "bytes" "context" "fmt" "go/ast" + "go/format" "go/token" "go/types" "regexp" @@ -49,11 +51,6 @@ func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.U return nil, fmt.Errorf("failed to rename because %q is declared in package %q", i.Name, i.decl.obj.Pkg().Name()) } - // TODO(suzmue): Support renaming of imported packages. - if _, ok := i.decl.obj.(*types.PkgName); ok { - return nil, fmt.Errorf("renaming imported package %s not supported", i.Name) - } - refs, err := i.References(ctx) if err != nil { return nil, err @@ -96,6 +93,18 @@ func (r *renamer) update() (map[span.URI][]TextEdit, error) { return nil, err } + // Renaming a types.PkgName may result in the addition or removal of an identifier, + // so we deal with this separately. + if pkgName, ok := ref.obj.(*types.PkgName); ok && ref.isDeclaration { + edit, err := r.updatePkgName(pkgName) + if err != nil { + return nil, err + } + result[refSpan.URI()] = append(result[refSpan.URI()], *edit) + continue + } + + // Replace the identifier with r.to. edit := TextEdit{ Span: refSpan, NewText: r.to, @@ -103,16 +112,16 @@ func (r *renamer) update() (map[span.URI][]TextEdit, error) { result[refSpan.URI()] = append(result[refSpan.URI()], edit) - if !ref.isDeclaration || ref.ident == nil { // done if it it is a use or does not have an identifier + if !ref.isDeclaration || ref.ident == nil { // uses do not have doc comments to update. continue } doc := r.docComment(r.pkg, ref.ident) - if doc == nil { // no doc comment + if doc == nil { continue } - // Perform the rename in doc comments declared in the original package + // Perform the rename in doc comments declared in the original package. for _, comment := range doc.List { for _, locs := range docRegexp.FindAllStringIndex(comment.Text, -1) { rng := span.NewRange(r.fset, comment.Pos()+token.Pos(locs[0]), comment.Pos()+token.Pos(locs[1])) @@ -120,7 +129,7 @@ func (r *renamer) update() (map[span.URI][]TextEdit, error) { if err != nil { return nil, err } - result[refSpan.URI()] = append(result[refSpan.URI()], TextEdit{ + result[spn.URI()] = append(result[spn.URI()], TextEdit{ Span: spn, NewText: r.to, }) @@ -159,3 +168,46 @@ func (r *renamer) docComment(pkg Package, id *ast.Ident) *ast.CommentGroup { } return nil } + +// updatePkgName returns the updates to rename a pkgName in the import spec +func (r *renamer) updatePkgName(pkgName *types.PkgName) (*TextEdit, error) { + // Modify ImportSpec syntax to add or remove the Name as needed. + pkg := r.packages[pkgName.Pkg()] + _, path, _ := pathEnclosingInterval(r.ctx, r.fset, pkg, pkgName.Pos(), pkgName.Pos()) + + if len(path) < 2 { + return nil, fmt.Errorf("failed to update PkgName for %s", pkgName.Name()) + } + spec, ok := path[1].(*ast.ImportSpec) + if !ok { + return nil, fmt.Errorf("failed to update PkgName for %s", pkgName.Name()) + } + + var astIdent *ast.Ident // will be nil if ident is removed + if pkgName.Imported().Name() != r.to { + // ImportSpec.Name needed + astIdent = &ast.Ident{NamePos: spec.Path.Pos(), Name: r.to} + } + + // Make a copy of the ident that just has the name and path. + updated := &ast.ImportSpec{ + Name: astIdent, + Path: spec.Path, + EndPos: spec.EndPos, + } + + rng := span.NewRange(r.fset, spec.Pos(), spec.End()) + spn, err := rng.Span() + if err != nil { + return nil, err + } + + var buf bytes.Buffer + format.Node(&buf, r.fset, updated) + newText := buf.String() + + return &TextEdit{ + Span: spn, + NewText: newText, + }, nil +} diff --git a/internal/lsp/testdata/rename/a/random.go.golden b/internal/lsp/testdata/rename/a/random.go.golden index bd07e970..427b7d2d 100644 --- a/internal/lsp/testdata/rename/a/random.go.golden +++ b/internal/lsp/testdata/rename/a/random.go.golden @@ -1,7 +1,11 @@ -- GetSum-rename -- package a -import "fmt" +import ( + lg "log" + "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -30,18 +34,22 @@ func sw() { switch y := x.(type) { //@rename("y", "y0") case int: - fmt.Printf("%d", y) //@rename("y", "y1") + fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format") case string: - fmt.Printf("%s", y) //@rename("y", "y2") + lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log") default: - fmt.Printf("%v", y) //@rename("y", "y3") + f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2") } } -- myX-rename -- package a -import "fmt" +import ( + lg "log" + "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -70,18 +78,22 @@ func sw() { switch y := x.(type) { //@rename("y", "y0") case int: - fmt.Printf("%d", y) //@rename("y", "y1") + fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format") case string: - fmt.Printf("%s", y) //@rename("y", "y2") + lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log") default: - fmt.Printf("%v", y) //@rename("y", "y3") + f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2") } } -- pos-rename -- package a -import "fmt" +import ( + lg "log" + "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -110,18 +122,22 @@ func sw() { switch y := x.(type) { //@rename("y", "y0") case int: - fmt.Printf("%d", y) //@rename("y", "y1") + fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format") case string: - fmt.Printf("%s", y) //@rename("y", "y2") + lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log") default: - fmt.Printf("%v", y) //@rename("y", "y3") + f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2") } } -- z-rename -- package a -import "fmt" +import ( + lg "log" + "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -150,18 +166,22 @@ func sw() { switch y := x.(type) { //@rename("y", "y0") case int: - fmt.Printf("%d", y) //@rename("y", "y1") + fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format") case string: - fmt.Printf("%s", y) //@rename("y", "y2") + lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log") default: - fmt.Printf("%v", y) //@rename("y", "y3") + f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2") } } -- y0-rename -- package a -import "fmt" +import ( + lg "log" + "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -190,18 +210,22 @@ func sw() { switch y0 := x.(type) { //@rename("y", "y0") case int: - fmt.Printf("%d", y0) //@rename("y", "y1") + fmt.Printf("%d", y0) //@rename("y", "y1"),rename("fmt", "format") case string: - fmt.Printf("%s", y0) //@rename("y", "y2") + lg.Printf("%s", y0) //@rename("y", "y2"),rename("lg","log") default: - fmt.Printf("%v", y0) //@rename("y", "y3") + f2.Printf("%v", y0) //@rename("y", "y3"),rename("f2","fmt2") } } -- y1-rename -- package a -import "fmt" +import ( + lg "log" + "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -230,18 +254,22 @@ func sw() { switch y1 := x.(type) { //@rename("y", "y0") case int: - fmt.Printf("%d", y1) //@rename("y", "y1") + fmt.Printf("%d", y1) //@rename("y", "y1"),rename("fmt", "format") case string: - fmt.Printf("%s", y1) //@rename("y", "y2") + lg.Printf("%s", y1) //@rename("y", "y2"),rename("lg","log") default: - fmt.Printf("%v", y1) //@rename("y", "y3") + f2.Printf("%v", y1) //@rename("y", "y3"),rename("f2","fmt2") } } -- y2-rename -- package a -import "fmt" +import ( + lg "log" + "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -270,18 +298,22 @@ func sw() { switch y2 := x.(type) { //@rename("y", "y0") case int: - fmt.Printf("%d", y2) //@rename("y", "y1") + fmt.Printf("%d", y2) //@rename("y", "y1"),rename("fmt", "format") case string: - fmt.Printf("%s", y2) //@rename("y", "y2") + lg.Printf("%s", y2) //@rename("y", "y2"),rename("lg","log") default: - fmt.Printf("%v", y2) //@rename("y", "y3") + f2.Printf("%v", y2) //@rename("y", "y3"),rename("f2","fmt2") } } -- y3-rename -- package a -import "fmt" +import ( + lg "log" + "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -310,18 +342,22 @@ func sw() { switch y3 := x.(type) { //@rename("y", "y0") case int: - fmt.Printf("%d", y3) //@rename("y", "y1") + fmt.Printf("%d", y3) //@rename("y", "y1"),rename("fmt", "format") case string: - fmt.Printf("%s", y3) //@rename("y", "y2") + lg.Printf("%s", y3) //@rename("y", "y2"),rename("lg","log") default: - fmt.Printf("%v", y3) //@rename("y", "y3") + f2.Printf("%v", y3) //@rename("y", "y3"),rename("f2","fmt2") } } -- format-rename -- package a -import format "fmt" +import ( + lg "log" + format "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -350,11 +386,99 @@ func sw() { switch y := x.(type) { //@rename("y", "y0") case int: - format.Printf("%d", y) //@rename("y", "y1") + format.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format") case string: - format.Printf("%s", y) //@rename("y", "y2") + lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log") default: - format.Printf("%v", y) //@rename("y", "y3") + f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2") + } +} + +-- log-rename -- +package a + +import ( + "log" + "fmt" + f2 "fmt" +) + +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") +} + +func sw() { + var x interface{} + + switch y := x.(type) { //@rename("y", "y0") + case int: + fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format") + case string: + log.Printf("%s", y) //@rename("y", "y2"),rename("lg","log") + default: + f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2") + } +} + +-- fmt2-rename -- +package a + +import ( + lg "log" + "fmt" + fmt2 "fmt" +) + +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") +} + +func sw() { + var x interface{} + + switch y := x.(type) { //@rename("y", "y0") + case int: + fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format") + case string: + lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log") + default: + fmt2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2") } } diff --git a/internal/lsp/testdata/rename/a/random.go.in b/internal/lsp/testdata/rename/a/random.go.in index c1c0fc52..c2cf0202 100644 --- a/internal/lsp/testdata/rename/a/random.go.in +++ b/internal/lsp/testdata/rename/a/random.go.in @@ -1,6 +1,10 @@ package a -import "fmt" +import ( + lg "log" + "fmt" + f2 "fmt" +) func Random() int { y := 6 + 7 @@ -29,10 +33,10 @@ func sw() { switch y := x.(type) { //@rename("y", "y0") case int: - fmt.Printf("%d", y) //@rename("y", "y1") + fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format") case string: - fmt.Printf("%s", y) //@rename("y", "y2") + lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log") default: - fmt.Printf("%v", y) //@rename("y", "y3") + f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2") } } diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 6d659a5c..86bd9228 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -34,7 +34,7 @@ const ( ExpectedTypeDefinitionsCount = 2 ExpectedHighlightsCount = 2 ExpectedReferencesCount = 4 - ExpectedRenamesCount = 8 + ExpectedRenamesCount = 11 ExpectedSymbolsCount = 1 ExpectedSignaturesCount = 20 ExpectedLinksCount = 2