mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp/source: improve logic for finding full syntax in hover 

When enriching identifier info with full syntax, it's cleaner to find
the enclosing decl. Use the full decl in hover if we were unable to find
a node in the original type-checked package.

Update the regtest to exercise hovering in a non-workspace package.

Updates golang/go#46158

Change-Id: Ic1772a38fb1758fb57a09da9483a8853cc5498f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333690
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Rob Findley 2021-07-09 15:47:49 -04:00 committed by Robert Findley
parent 4ad98e9670
commit f09387104b
3 changed files with 57 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
gopls/internal/regtest/misc/hover_test.go
View File

@ -26,6 +26,10 @@ type Mixed struct {
Exported int
unexported string
}
func printMixed(m Mixed) {
println(m)
}
`
const mod = `
-- go.mod --
@ -35,7 +39,7 @@ go 1.12
require golang.org/x/structs v1.0.0
-- go.sum --
golang.org/x/structs v1.0.0 h1:oxD5q25qV458xBbXf5+QX+Johgg71KFtwuJzt145c9A=
golang.org/x/structs v1.0.0 h1:3DlrFfd3OsEen7FnCHfqtnJvjBZ8ZFKmrD/+HjpdJj0=
golang.org/x/structs v1.0.0/go.mod h1:47gkSIdo5AaQaWJS0upVORsxfEr1LL1MWv9dmYF3iq4=
-- main.go --
package main
@ -51,9 +55,16 @@ func main() {
ProxyFiles(proxy),
).Run(t, mod, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
got, _ := env.Hover("main.go", env.RegexpSearch("main.go", "Mixed"))
mixedPos := env.RegexpSearch("main.go", "Mixed")
got, _ := env.Hover("main.go", mixedPos)
if !strings.Contains(got.Value, "unexported") {
t.Errorf("Hover: missing expected field 'unexported'. Got:\n%q", got.Value)
t.Errorf("Workspace hover: missing expected field 'unexported'. Got:\n%q", got.Value)
}
cacheFile, _ := env.GoToDefinition("main.go", mixedPos)
argPos := env.RegexpSearch(cacheFile, "printMixed.*(Mixed)")
got, _ = env.Hover(cacheFile, argPos)
if !strings.Contains(got.Value, "unexported") {
t.Errorf("Non-workspace hover: missing expected field 'unexported'. Got:\n%q", got.Value)
}
})
}

46
internal/lsp/source/hover.go
View File

@ -98,7 +98,7 @@ func HoverIdentifier(ctx context.Context, i *IdentifierInfo) (*HoverInformation,
defer done()
fset := i.Snapshot.FileSet()
h, err := HoverInfo(ctx, i.Snapshot, i.pkg, i.Declaration.obj, i.Declaration.node, i.Declaration.fullSpec)
h, err := HoverInfo(ctx, i.Snapshot, i.pkg, i.Declaration.obj, i.Declaration.node, i.Declaration.fullDecl)
if err != nil {
return nil, err
}
@ -284,15 +284,28 @@ func objectString(obj types.Object, qf types.Qualifier, inferred *types.Signatur
}
// HoverInfo returns a HoverInformation struct for an ast node and its type
// object.
func HoverInfo(ctx context.Context, s Snapshot, pkg Package, obj types.Object, node ast.Node, spec ast.Spec) (*HoverInformation, error) {
// object. node should be the actual node used in type checking, while fullNode
// could be a separate node with more complete syntactic information.
func HoverInfo(ctx context.Context, s Snapshot, pkg Package, obj types.Object, pkgNode ast.Node, fullDecl ast.Decl) (*HoverInformation, error) {
var info *HoverInformation
// This is problematic for a number of reasons. We really need to have a more
// general mechanism to validate the coherency of AST with type information,
// but absent that we must do our best to ensure that we don't use fullNode
// when we actually need the node that was type checked.
//
// pkgNode may be nil, if it was eliminated from the type-checked syntax. In
// that case, use fullDecl if available.
node := pkgNode
if node == nil && fullDecl != nil {
node = fullDecl
}
switch node := node.(type) {
case *ast.Ident:
// The package declaration.
for _, f := range pkg.GetSyntax() {
if f.Name == node {
if f.Name == pkgNode {
info = &HoverInformation{comment: f.Doc}
}
}
@ -316,6 +329,23 @@ func HoverInfo(ctx context.Context, s Snapshot, pkg Package, obj types.Object, n
case *ast.GenDecl:
switch obj := obj.(type) {
case *types.TypeName, *types.Var, *types.Const, *types.Func:
// Always use the full declaration here if we have it, because the
// dependent code doesn't rely on pointer identity. This is fragile.
if d, _ := fullDecl.(*ast.GenDecl); d != nil {
node = d
}
// obj may not have been produced by type checking the AST containing
// node, so we need to be careful about using token.Pos.
tok := s.FileSet().File(obj.Pos())
offset := tok.Offset(obj.Pos())
tok2 := s.FileSet().File(node.Pos())
var spec ast.Spec
for _, s := range node.Specs {
if tok2.Offset(s.Pos()) <= offset && offset <= tok2.Offset(s.End()) {
spec = s
break
}
}
var err error
info, err = formatGenDecl(node, spec, obj, obj.Type())
if err != nil {
@ -396,14 +426,6 @@ func formatGenDecl(node *ast.GenDecl, spec ast.Spec, obj types.Object, typ types
return formatGenDecl(node, spec, obj, typ.Underlying())
}
}
if spec == nil {
for _, s := range node.Specs {
if s.Pos() <= obj.Pos() && obj.Pos() <= s.End() {
spec = s
break
}
}
}
if spec == nil {
return nil, errors.Errorf("no spec for node %v at position %v", node, obj.Pos())
}

18
internal/lsp/source/identifier.go
View File

@ -57,10 +57,11 @@ type Declaration struct {
// The typechecked node.
node ast.Node
// Optional: the fully parsed spec, to be used for formatting in cases where
// Optional: the fully parsed node, to be used for formatting in cases where
// node has missing information. This could be the case when node was parsed
// in ParseExported mode.
fullSpec ast.Spec
fullDecl ast.Decl
// The typechecked object.
obj types.Object
@ -290,8 +291,7 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa
}
// Ensure that we have the full declaration, in case the declaration was
// parsed in ParseExported and therefore could be missing information.
result.Declaration.fullSpec, err = fullSpec(snapshot, result.Declaration.obj, declPkg)
if err != nil {
if result.Declaration.fullDecl, err = fullNode(snapshot, result.Declaration.obj, declPkg); err != nil {
return nil, err
}
typ := pkg.GetTypesInfo().TypeOf(result.ident)
@ -314,10 +314,10 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa
return result, nil
}
// fullSpec tries to extract the full spec corresponding to obj's declaration.
// fullNode tries to extract the full spec corresponding to obj's declaration.
// If the package was not parsed in full, the declaration file will be
// re-parsed to ensure it has complete syntax.
func fullSpec(snapshot Snapshot, obj types.Object, pkg Package) (ast.Spec, error) {
func fullNode(snapshot Snapshot, obj types.Object, pkg Package) (ast.Decl, error) {
// declaration in a different package... make sure we have full AST information.
tok := snapshot.FileSet().File(obj.Pos())
uri := span.URIFromPath(tok.Name())
@ -338,9 +338,9 @@ func fullSpec(snapshot Snapshot, obj types.Object, pkg Package) (ast.Spec, error
}
}
path, _ := astutil.PathEnclosingInterval(file, pos, pos)
if len(path) > 1 {
if spec, _ := path[1].(*ast.TypeSpec); spec != nil {
return spec, nil
for _, n := range path {
if decl, ok := n.(ast.Decl); ok {
return decl, nil
}
}
return nil, nil