gopls/internal/golang: allow rename in doc comments

This CL enables the renaming of an identifier from within its
doc comment. It modifies the PrepareRename command to check
for renames inside doc comments and then changes the rename
target to be the identifier instead of the doc comment
so that all references to the identifier are updated.

Fixes golang/go#42301

Change-Id: I5110d805dbe499c3e4010a5bd313df2a51b9f296
Reviewed-on: https://go-review.googlesource.com/c/tools/+/686015
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Madeline Kalil <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>

Author: madelinekalil

gopls/internal/golang/rename.go

@@ -62,6 +62,7 @@ import (
 
 	"golang.org/x/mod/modfile"
 	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/types/objectpath"
 	"golang.org/x/tools/go/types/typeutil"
 	"golang.org/x/tools/gopls/internal/cache"
@@ -135,17 +136,34 @@ func PrepareRename(ctx context.Context, snapshot *cache.Snapshot, f file.Handle,
 		return nil, nil, err
 	}
 
+	cur, ok := pgf.Cursor.FindByPos(pos, pos)
+	if !ok {
+		return nil, nil, fmt.Errorf("can't find cursor for selection")
+	}
+
 	// Check if we're in a 'func' keyword. If so, we hijack the renaming to
 	// change the function signature.
-	if item, err := prepareRenameFuncSignature(pgf, pos); err != nil {
+	if item, err := prepareRenameFuncSignature(pgf, pos, cur); err != nil {
 		return nil, nil, err
 	} else if item != nil {
 		return item, nil, nil
 	}
 
 	targets, node, err := objectsAt(pkg.TypesInfo(), pgf.File, pos)
 	if err != nil {
-		return nil, nil, err
+		// Check if we are renaming an ident inside its doc comment. The call to
+		// objectsAt will have returned an error in this case.
+		id := docCommentPosToIdent(pgf, pos, cur)
+		if id == nil {
+			return nil, nil, err
+		}
+		obj := pkg.TypesInfo().Defs[id]
+		if obj == nil {
+			return nil, nil, fmt.Errorf("error fetching Object for ident %q", id.Name)
+		}
+		// Change rename target to the ident.
+		targets = map[types.Object]ast.Node{obj: id}
+		node = id
 	}
 	var obj types.Object
 	for obj = range targets {
@@ -209,8 +227,8 @@ func prepareRenamePackageName(ctx context.Context, snapshot *cache.Snapshot, pgf
 //
 // The resulting text is the signature of the function, which may be edited to
 // the new signature.
-func prepareRenameFuncSignature(pgf *parsego.File, pos token.Pos) (*PrepareItem, error) {
-	fdecl := funcKeywordDecl(pgf, pos)
+func prepareRenameFuncSignature(pgf *parsego.File, pos token.Pos, cursor inspector.Cursor) (*PrepareItem, error) {
+	fdecl := funcKeywordDecl(pos, cursor)
 	if fdecl == nil {
 		return nil, nil
 	}
@@ -264,17 +282,8 @@ func nameBlankParams(ftype *ast.FuncType) *ast.FuncType {
 
 // renameFuncSignature computes and applies the effective change signature
 // operation resulting from a 'renamed' (=rewritten) signature.
-func renameFuncSignature(ctx context.Context, snapshot *cache.Snapshot, f file.Handle, pp protocol.Position, newName string) (map[protocol.DocumentURI][]protocol.TextEdit, error) {
-	// Find the renamed signature.
-	pkg, pgf, err := NarrowestPackageForFile(ctx, snapshot, f.URI())
-	if err != nil {
-		return nil, err
-	}
-	pos, err := pgf.PositionPos(pp)
-	if err != nil {
-		return nil, err
-	}
-	fdecl := funcKeywordDecl(pgf, pos)
+func renameFuncSignature(ctx context.Context, pkg *cache.Package, pgf *parsego.File, pos token.Pos, snapshot *cache.Snapshot, cursor inspector.Cursor, f file.Handle, pp protocol.Position, newName string) (map[protocol.DocumentURI][]protocol.TextEdit, error) {
+	fdecl := funcKeywordDecl(pos, cursor)
 	if fdecl == nil {
 		return nil, nil
 	}
@@ -350,15 +359,12 @@ func renameFuncSignature(ctx context.Context, snapshot *cache.Snapshot, f file.H
 
 // funcKeywordDecl returns the FuncDecl for which pos is in the 'func' keyword,
 // if any.
-func funcKeywordDecl(pgf *parsego.File, pos token.Pos) *ast.FuncDecl {
-	path, _ := astutil.PathEnclosingInterval(pgf.File, pos, pos)
-	if len(path) < 1 {
-		return nil
-	}
-	fdecl, _ := path[0].(*ast.FuncDecl)
-	if fdecl == nil {
+func funcKeywordDecl(pos token.Pos, cursor inspector.Cursor) *ast.FuncDecl {
+	curDecl, ok := moreiters.First(cursor.Enclosing((*ast.FuncDecl)(nil)))
+	if !ok {
 		return nil
 	}
+	fdecl := curDecl.Node().(*ast.FuncDecl)
 	ftyp := fdecl.Type
 	if pos < ftyp.Func || pos > ftyp.Func+token.Pos(len("func")) { // tolerate renaming immediately after 'func'
 		return nil
@@ -396,7 +402,21 @@ func Rename(ctx context.Context, snapshot *cache.Snapshot, f file.Handle, pp pro
 	ctx, done := event.Start(ctx, "golang.Rename")
 	defer done()
 
-	if edits, err := renameFuncSignature(ctx, snapshot, f, pp, newName); err != nil {
+	pkg, pgf, err := NarrowestPackageForFile(ctx, snapshot, f.URI())
+	if err != nil {
+		return nil, false, err
+	}
+	pos, err := pgf.PositionPos(pp)
+	if err != nil {
+		return nil, false, err
+	}
+
+	cur, ok := pgf.Cursor.FindByPos(pos, pos)
+	if !ok {
+		return nil, false, fmt.Errorf("can't find cursor for selection")
+	}
+
+	if edits, err := renameFuncSignature(ctx, pkg, pgf, pos, snapshot, cur, f, pp, newName); err != nil {
 		return nil, false, err
 	} else if edits != nil {
 		return edits, false, nil
@@ -503,7 +523,18 @@ func renameOrdinary(ctx context.Context, snapshot *cache.Snapshot, uri protocol.
 	}
 	targets, node, err := objectsAt(pkg.TypesInfo(), pgf.File, pos)
 	if err != nil {
-		return nil, err
+		// Check if we are renaming an ident inside its doc comment. The call to
+		// objectsAt will have returned an error in this case.
+		id := docCommentPosToIdent(pgf, pos, cur)
+		if id == nil {
+			return nil, err
+		}
+		obj := pkg.TypesInfo().Defs[id]
+		if obj == nil {
+			return nil, fmt.Errorf("error fetching types.Object for ident %q", id.Name)
+		}
+		// Change rename target to the ident.
+		targets = map[types.Object]ast.Node{obj: id}
 	}
 
 	// Pick a representative object arbitrarily.
@@ -1633,6 +1664,41 @@ func docComment(pgf *parsego.File, id *ast.Ident) *ast.CommentGroup {
 	return nil
 }
 
+// docCommentPosToIdent returns the node whose doc comment contains pos, if any.
+// The pos must be within an occurrence of the identifier's name, otherwise it returns nil.
+func docCommentPosToIdent(pgf *parsego.File, pos token.Pos, cur inspector.Cursor) *ast.Ident {
+	for curId := range cur.Preorder((*ast.Ident)(nil)) {
+		id := curId.Node().(*ast.Ident)
+		if pos > id.Pos() {
+			continue // Doc comments are not located after an ident.
+		}
+		doc := docComment(pgf, id)
+		if doc == nil || !(doc.Pos() <= pos && pos < doc.End()) {
+			continue
+		}
+
+		docRegexp := regexp.MustCompile(`\b` + id.Name + `\b`)
+		for _, comment := range doc.List {
+			if isDirective(comment.Text) || !(comment.Pos() <= pos && pos < comment.End()) {
+				continue
+			}
+			start := comment.Pos()
+			text, err := pgf.NodeText(comment)
+			if err != nil {
+				return nil
+			}
+			for _, locs := range docRegexp.FindAllIndex(text, -1) {
+				matchStart := start + token.Pos(locs[0])
+				matchEnd := start + token.Pos(locs[1])
+				if matchStart <= pos && pos <= matchEnd {
+					return id
+				}
+			}
+		}
+	}
+	return nil
+}
+
 // updatePkgName returns the updates to rename a pkgName in the import spec by
 // only modifying the package name portion of the import declaration.
 func (r *renamer) updatePkgName(pgf *parsego.File, pkgName *types.PkgName) (diff.Edit, error) {

gopls/internal/test/marker/testdata/rename/issue42301.txt

@@ -0,0 +1,63 @@
+This test verifies the fix for golang/go#42301: renaming an ident inside its doc
+comment should also rename the ident.
+
+-- go.mod --
+module example.com
+
+go 1.21
+-- a/a.go --
+package a
+
+// Foo doesn't do anything, Foo is just an empty function. //@rename("Foo", "Bar", fooToBar), renameerr("anything", "Bar", "no identifier found")
+func Foo() {}
+
+func _() {
+	Foo()
+}
+
+-- b/b.go --
+package b
+
+import "example.com/a"
+
+func _() {
+	a.Foo()
+}
+
+-- c/c.go --
+package c
+
+// A is an empty struct. //@rename("A", "B", aToB)
+type A struct {}
+
+-- d/d.go --
+package d
+
+// Bar doesn't do anything, Bar is just an empty function. //@loc(Bar, re`^.*?\bBar\b.*?\b(Bar)\b.*`), rename(Bar, "Foo", barToFoo)
+func Bar() {}
+
+-- @aToB/c/c.go --
+@@ -3,2 +3,2 @@
+-// A is an empty struct. //@rename("A", "B", aToB)
+-type A struct {}
++// B is an empty struct. //@rename("B", "B", aToB)
++type B struct {}
+-- @barToFoo/d/d.go --
+@@ -3,2 +3,2 @@
+-// Bar doesn't do anything, Bar is just an empty function. //@loc(Bar, re`^.*?\bBar\b.*?\b(Bar)\b.*`), rename(Bar, "Foo", barToFoo)
+-func Bar() {}
++// Foo doesn't do anything, Foo is just an empty function. //@loc(Foo, re`^.*?\bBar\b.*?\b(Foo)\b.*`), rename(Foo, "Foo", barToFoo)
++func Foo() {}
+-- @fooToBar/a/a.go --
+@@ -3,2 +3,2 @@
+-// Foo doesn't do anything, Foo is just an empty function. //@rename("Foo", "Bar", fooToBar), renameerr("anything", "Bar", "no identifier found")
+-func Foo() {}
++// Bar doesn't do anything, Bar is just an empty function. //@rename("Bar", "Bar", fooToBar), renameerr("anything", "Bar", "no identifier found")
++func Bar() {}
+@@ -7 +7 @@
+-	Foo()
++	Bar()
+-- @fooToBar/b/b.go --
+@@ -6 +6 @@
+-	a.Foo()
++	a.Bar()