Skip to content

Commit

Permalink
gopls/internal/golang: Definitions: support renaming imports in doc l…
Browse files Browse the repository at this point in the history
…inks

This CL adds support for jumping to the definition of a doc link when
the import is renamed. Before, the doc link had to use the local
(renamed) name, which is unnatural; now, it can use either the local
name or the package's declared name.

+ test

Updates golang/go#61677

Change-Id: Ibbe18ab1527800c41900d42781677ad892b55cd4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/612045
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Commit-Queue: Alan Donovan <[email protected]>
  • Loading branch information
adonovan committed Sep 11, 2024
1 parent b0f680c commit 2884375
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 8 deletions.
40 changes: 32 additions & 8 deletions gopls/internal/golang/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func parseDocLink(pkg *cache.Package, pgf *parsego.File, pos token.Pos) (types.O

for _, idx := range docLinkRegex.FindAllStringSubmatchIndex(text, -1) {
// The [idx[2], idx[3]) identifies the first submatch,
// which is the reference name in the doc link.
// which is the reference name in the doc link (sans '*').
// e.g. The "[fmt.Println]" reference name is "fmt.Println".
if !(idx[2] <= lineOffset && lineOffset < idx[3]) {
continue
Expand All @@ -126,7 +126,7 @@ func parseDocLink(pkg *cache.Package, pgf *parsego.File, pos token.Pos) (types.O
name = name[:i]
i = strings.LastIndexByte(name, '.')
}
obj := lookupObjectByName(pkg, pgf, name)
obj := lookupDocLinkSymbol(pkg, pgf, name)
if obj == nil {
return nil, protocol.Range{}, errNoCommentReference
}
Expand All @@ -141,19 +141,42 @@ func parseDocLink(pkg *cache.Package, pgf *parsego.File, pos token.Pos) (types.O
return nil, protocol.Range{}, errNoCommentReference
}

func lookupObjectByName(pkg *cache.Package, pgf *parsego.File, name string) types.Object {
// lookupDocLinkSymbol returns the symbol denoted by a doc link such
// as "fmt.Println" or "bytes.Buffer.Write" in the specified file.
func lookupDocLinkSymbol(pkg *cache.Package, pgf *parsego.File, name string) types.Object {
scope := pkg.Types().Scope()

prefix, suffix, _ := strings.Cut(name, ".")

// Try treating the prefix as a package name,
// allowing for non-renaming and renaming imports.
fileScope := pkg.TypesInfo().Scopes[pgf.File]
pkgName, suffix, _ := strings.Cut(name, ".")
obj, ok := fileScope.Lookup(pkgName).(*types.PkgName)
if ok {
scope = obj.Imported().Scope()
pkgname, ok := fileScope.Lookup(prefix).(*types.PkgName) // ok => prefix is imported name
if !ok {
// Handle renaming import, e.g.
// [path.Join] after import pathpkg "path".
// (Should we look at all files of the package?)
for _, imp := range pgf.File.Imports {
pkgname2 := pkg.TypesInfo().PkgNameOf(imp)
if pkgname2 != nil && pkgname2.Imported().Name() == prefix {
pkgname = pkgname2
break
}
}
}
if pkgname != nil {
scope = pkgname.Imported().Scope()
if suffix == "" {
return obj
return pkgname // not really a valid doc link
}
name = suffix
}

// TODO(adonovan): try searching the forward closure for packages
// that define the symbol but are not directly imported;
// see https://github.com/golang/go/issues/61677

// Type.Method?
recv, method, ok := strings.Cut(name, ".")
if ok {
obj, ok := scope.Lookup(recv).(*types.TypeName)
Expand All @@ -173,5 +196,6 @@ func lookupObjectByName(pkg *cache.Package, pgf *parsego.File, name string) type
return nil
}

// package-level symbol
return scope.Lookup(name)
}
13 changes: 13 additions & 0 deletions gopls/internal/test/marker/testdata/definition/comment.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,16 @@ module mod.com

go 1.19

-- path/path.go --
package path

func Join() //@loc(Join, "Join")

-- a.go --
package p

import "strconv" //@loc(strconv, `"strconv"`)
import pathpkg "mod.com/path"

const NumberBase = 10 //@loc(NumberBase, "NumberBase")

Expand All @@ -19,3 +25,10 @@ func Conv(s string) int { //@loc(Conv, "Conv")
i, _ := strconv.ParseInt(s, NumberBase, 64)
return int(i)
}

// The declared and imported names of the package both work:
// [path.Join] //@ def("Join", Join)
// [pathpkg.Join] //@ def("Join", Join)
func _() {
pathpkg.Join()
}

0 comments on commit 2884375

Please sign in to comment.