Skip to content

Commit 66f8f71

Browse files
findleyrfindleyr
committed
gopls/internal/lsp/source: use syntax alone in FormatVarType
FormatVarType re-formats a variable type using syntax, in order to get accurate presentation of aliases and ellipses. However, as a result it required typed syntax trees for the type declaration, which may exist in a distinct package from the current package. In the near future we may not have typed syntax trees for these packages. We could type-check on demand, but (1) that could be costly and (2) it may break qualification using the go/types qualifier. Instead, perform this operation using a qualifier based on syntax and metadata, so that we only need a fully parsed file rather than a fully type-checked package. The resulting expressions may be inaccurate due to built-ins, "." imported packages, or missing metadata, but that seems acceptable for the current use-cases of this function, which are in completion and signature help. While doing this, add a FormatNodeWithFile helper that allows formatting a node from a *token.File, rather than *token.FileSet. This can help us avoid relying on a global fileset. To facilitate this, move the GetLines helper from internal/gcimporter into a shared tokeninternal package. For golang/go#57987 Change-Id: I3b8a5256bc2261be8b5175ee360b9336228928ac Reviewed-on: https://go-review.googlesource.com/c/tools/+/464301 Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent 30f191f commit 66f8f71

File tree

14 files changed

+571
-217
lines changed

14 files changed

+571
-217
lines changed

‎gopls/internal/lsp/semantic.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,7 @@ type encoded struct {
220220

221221
ctx context.Context
222222
// metadataSource is used to resolve imports
223-
metadataSource interface {
224-
Metadata(source.PackageID) *source.Metadata
225-
}
223+
metadataSource source.MetadataSource
226224
tokTypes, tokMods []string
227225
pgf *source.ParsedGoFile
228226
rng *protocol.Range

‎gopls/internal/lsp/source/call_hierarchy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func OutgoingCalls(ctx context.Context, snapshot Snapshot, fh FileHandle, pp pro
231231
return nil, err
232232
}
233233

234-
declNode, _ := FindDeclAndField([]*ast.File{declPGF.File}, declPos)
234+
declNode, _, _ := FindDeclInfo([]*ast.File{declPGF.File}, declPos)
235235
if declNode == nil {
236236
// TODO(rfindley): why don't we return an error here, or even bug.Errorf?
237237
return nil, nil

‎gopls/internal/lsp/source/completion/completion.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ func (ipm insensitivePrefixMatcher) Score(candidateLabel string) float32 {
156156
type completer struct {
157157
snapshot source.Snapshot
158158
pkg source.Package
159-
qf types.Qualifier
159+
qf types.Qualifier // for qualifying typed expressions
160+
mq source.MetadataQualifier // for syntactic qualifying
160161
opts *completionOptions
161162

162163
// completionContext contains information about the trigger for this
@@ -509,6 +510,7 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan
509510
pkg: pkg,
510511
snapshot: snapshot,
511512
qf: source.Qualifier(pgf.File, pkg.GetTypes(), pkg.GetTypesInfo()),
513+
mq: source.MetadataQualifierForFile(snapshot, pgf.File, pkg.Metadata()),
512514
completionContext: completionContext{
513515
triggerCharacter: protoContext.TriggerCharacter,
514516
triggerKind: protoContext.TriggerKind,

‎gopls/internal/lsp/source/completion/format.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,11 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
8181
if _, ok := obj.Type().(*types.Struct); ok {
8282
detail = "struct{...}" // for anonymous structs
8383
} else if obj.IsField() {
84-
detail = source.FormatVarType(ctx, c.snapshot, c.pkg, obj, c.qf)
84+
var err error
85+
detail, err = source.FormatVarType(ctx, c.snapshot, c.pkg, c.file, obj, c.qf, c.mq)
86+
if err != nil {
87+
return CompletionItem{}, err
88+
}
8589
}
8690
if obj.IsField() {
8791
kind = protocol.FieldCompletion
@@ -130,7 +134,10 @@ Suffixes:
130134
switch mod {
131135
case invoke:
132136
if sig, ok := funcType.Underlying().(*types.Signature); ok {
133-
s := source.NewSignature(ctx, c.snapshot, c.pkg, sig, nil, c.qf)
137+
s, err := source.NewSignature(ctx, c.snapshot, c.pkg, c.file, sig, nil, c.qf, c.mq)
138+
if err != nil {
139+
return CompletionItem{}, err
140+
}
134141
c.functionCallSnippet("", s.TypeParams(), s.Params(), &snip)
135142
if sig.Results().Len() == 1 {
136143
funcType = sig.Results().At(0).Type()
@@ -243,7 +250,7 @@ Suffixes:
243250
return item, nil
244251
}
245252

246-
decl, _ := source.FindDeclAndField(pkg.GetSyntax(), obj.Pos()) // may be nil
253+
decl, _, _ := source.FindDeclInfo(pkg.GetSyntax(), obj.Pos()) // may be nil
247254
hover, err := source.FindHoverContext(ctx, c.snapshot, pkg, obj, decl, nil)
248255
if err != nil {
249256
event.Error(ctx, "failed to find Hover", err, tag.URI.Of(uri))

‎gopls/internal/lsp/source/completion/literal.go

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,10 +202,18 @@ func (c *completer) functionLiteral(ctx context.Context, sig *types.Signature, m
202202
// If the param has no name in the signature, guess a name based
203203
// on the type. Use an empty qualifier to ignore the package.
204204
// For example, we want to name "http.Request" "r", not "hr".
205-
name = source.FormatVarType(ctx, c.snapshot, c.pkg, p, func(p *types.Package) string {
206-
return ""
207-
})
208-
name = abbreviateTypeName(name)
205+
typeName, err := source.FormatVarType(ctx, c.snapshot, c.pkg, c.file, p,
206+
func(p *types.Package) string { return "" },
207+
func(source.PackageName, source.ImportPath, source.PackagePath) string { return "" })
208+
if err != nil {
209+
// In general, the only error we should encounter while formatting is
210+
// context cancellation.
211+
if ctx.Err() == nil {
212+
event.Error(ctx, "formatting var type", err)
213+
}
214+
return
215+
}
216+
name = abbreviateTypeName(typeName)
209217
}
210218
paramNames[i] = name
211219
if name != "_" {
@@ -264,7 +272,15 @@ func (c *completer) functionLiteral(ctx context.Context, sig *types.Signature, m
264272
// of "i int, j int".
265273
if i == sig.Params().Len()-1 || !types.Identical(p.Type(), sig.Params().At(i+1).Type()) {
266274
snip.WriteText(" ")
267-
typeStr := source.FormatVarType(ctx, c.snapshot, c.pkg, p, c.qf)
275+
typeStr, err := source.FormatVarType(ctx, c.snapshot, c.pkg, c.file, p, c.qf, c.mq)
276+
if err != nil {
277+
// In general, the only error we should encounter while formatting is
278+
// context cancellation.
279+
if ctx.Err() == nil {
280+
event.Error(ctx, "formatting var type", err)
281+
}
282+
return
283+
}
268284
if sig.Variadic() && i == sig.Params().Len()-1 {
269285
typeStr = strings.Replace(typeStr, "[]", "...", 1)
270286
}
@@ -314,7 +330,15 @@ func (c *completer) functionLiteral(ctx context.Context, sig *types.Signature, m
314330
snip.WriteText(name + " ")
315331
}
316332

317-
text := source.FormatVarType(ctx, c.snapshot, c.pkg, r, c.qf)
333+
text, err := source.FormatVarType(ctx, c.snapshot, c.pkg, c.file, r, c.qf, c.mq)
334+
if err != nil {
335+
// In general, the only error we should encounter while formatting is
336+
// context cancellation.
337+
if ctx.Err() == nil {
338+
event.Error(ctx, "formatting var type", err)
339+
}
340+
return
341+
}
318342
if tp, _ := r.Type().(*typeparams.TypeParam); tp != nil && !c.typeParamInScope(tp) {
319343
snip.WritePlaceholder(func(snip *snippet.Builder) {
320344
snip.WriteText(text)

‎gopls/internal/lsp/source/hover.go

Lines changed: 85 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"golang.org/x/tools/go/types/typeutil"
2525
"golang.org/x/tools/gopls/internal/lsp/protocol"
2626
"golang.org/x/tools/gopls/internal/lsp/safetoken"
27+
"golang.org/x/tools/gopls/internal/span"
2728
"golang.org/x/tools/internal/bug"
2829
"golang.org/x/tools/internal/event"
2930
"golang.org/x/tools/internal/typeparams"
@@ -498,6 +499,41 @@ func objectString(obj types.Object, qf types.Qualifier, inferred *types.Signatur
498499
return str
499500
}
500501

502+
// parseFull fully parses the file corresponding to position pos, referenced
503+
// from the given srcpkg.
504+
//
505+
// It returns the resulting ParsedGoFile as well as new pos contained in the
506+
// parsed file.
507+
func parseFull(ctx context.Context, snapshot Snapshot, srcpkg Package, pos token.Pos) (*ParsedGoFile, token.Pos, error) {
508+
f := srcpkg.FileSet().File(pos)
509+
if f == nil {
510+
return nil, 0, bug.Errorf("internal error: no file for position %d in %s", pos, srcpkg.Metadata().ID)
511+
}
512+
513+
uri := span.URIFromPath(f.Name())
514+
fh, err := snapshot.GetFile(ctx, uri)
515+
if err != nil {
516+
return nil, 0, err
517+
}
518+
519+
pgf, err := snapshot.ParseGo(ctx, fh, ParseFull)
520+
if err != nil {
521+
return nil, 0, err
522+
}
523+
524+
offset, err := safetoken.Offset(f, pos)
525+
if err != nil {
526+
return nil, 0, bug.Errorf("offset out of bounds in %q", uri)
527+
}
528+
529+
fullPos, err := safetoken.Pos(pgf.Tok, offset)
530+
if err != nil {
531+
return nil, 0, err
532+
}
533+
534+
return pgf, fullPos, nil
535+
}
536+
501537
// FindHoverContext returns a HoverContext struct for an AST node and its
502538
// declaration object. node should be the actual node used in type checking,
503539
// while fullNode could be a separate node with more complete syntactic
@@ -637,7 +673,7 @@ func FindHoverContext(ctx context.Context, s Snapshot, pkg Package, obj types.Ob
637673
break
638674
}
639675

640-
_, field := FindDeclAndField(pkg.GetSyntax(), obj.Pos())
676+
_, _, field := FindDeclInfo(pkg.GetSyntax(), obj.Pos())
641677
if field != nil {
642678
comment := field.Doc
643679
if comment.Text() == "" {
@@ -893,16 +929,19 @@ func anyNonEmpty(x []string) bool {
893929
return false
894930
}
895931

896-
// FindDeclAndField returns the var/func/type/const Decl that declares
897-
// the identifier at pos, searching the given list of file syntax
898-
// trees. If pos is the position of an ast.Field or one of its Names
899-
// or Ellipsis.Elt, the field is returned, along with the innermost
900-
// enclosing Decl, which could be only loosely related---consider:
932+
// FindDeclInfo returns the syntax nodes involved in the declaration of the
933+
// types.Object with position pos, searching the given list of file syntax
934+
// trees.
935+
//
936+
// Pos may be the position of the name-defining identifier in a FuncDecl,
937+
// ValueSpec, TypeSpec, Field, or as a special case the position of
938+
// Ellipsis.Elt in an ellipsis field.
901939
//
902-
// var decl = f( func(field int) {} )
940+
// If found, the resulting decl, spec, and field will be the inner-most
941+
// instance of each node type surrounding pos.
903942
//
904-
// It returns (nil, nil) if no Field or Decl is found at pos.
905-
func FindDeclAndField(files []*ast.File, pos token.Pos) (decl ast.Decl, field *ast.Field) {
943+
// It returns a nil decl if no object-defining node is found at pos.
944+
func FindDeclInfo(files []*ast.File, pos token.Pos) (decl ast.Decl, spec ast.Spec, field *ast.Field) {
906945
// panic(found{}) breaks off the traversal and
907946
// causes the function to return normally.
908947
type found struct{}
@@ -933,33 +972,45 @@ func FindDeclAndField(files []*ast.File, pos token.Pos) (decl ast.Decl, field *a
933972

934973
switch n := n.(type) {
935974
case *ast.Field:
936-
checkField := func(f ast.Node) {
937-
if f.Pos() == pos {
938-
field = n
939-
for i := len(stack) - 1; i >= 0; i-- {
940-
if d, ok := stack[i].(ast.Decl); ok {
941-
decl = d // innermost enclosing decl
942-
break
943-
}
975+
findEnclosingDeclAndSpec := func() {
976+
for i := len(stack) - 1; i >= 0; i-- {
977+
switch n := stack[i].(type) {
978+
case ast.Spec:
979+
spec = n
980+
case ast.Decl:
981+
decl = n
982+
return
944983
}
984+
}
985+
}
986+
987+
// Check each field name since you can have
988+
// multiple names for the same type expression.
989+
for _, id := range n.Names {
990+
if id.Pos() == pos {
991+
field = n
992+
findEnclosingDeclAndSpec()
945993
panic(found{})
946994
}
947995
}
948996

949997
// Check *ast.Field itself. This handles embedded
950998
// fields which have no associated *ast.Ident name.
951-
checkField(n)
952-
953-
// Check each field name since you can have
954-
// multiple names for the same type expression.
955-
for _, name := range n.Names {
956-
checkField(name)
999+
if n.Pos() == pos {
1000+
field = n
1001+
findEnclosingDeclAndSpec()
1002+
panic(found{})
9571003
}
9581004

959-
// Also check "X" in "...X". This makes it easy
960-
// to format variadic signature params properly.
961-
if ell, ok := n.Type.(*ast.Ellipsis); ok && ell.Elt != nil {
962-
checkField(ell.Elt)
1005+
// Also check "X" in "...X". This makes it easy to format variadic
1006+
// signature params properly.
1007+
//
1008+
// TODO(rfindley): I don't understand this comment. How does finding the
1009+
// field in this case make it easier to format variadic signature params?
1010+
if ell, ok := n.Type.(*ast.Ellipsis); ok && ell.Elt != nil && ell.Elt.Pos() == pos {
1011+
field = n
1012+
findEnclosingDeclAndSpec()
1013+
panic(found{})
9631014
}
9641015

9651016
case *ast.FuncDecl:
@@ -969,17 +1020,19 @@ func FindDeclAndField(files []*ast.File, pos token.Pos) (decl ast.Decl, field *a
9691020
}
9701021

9711022
case *ast.GenDecl:
972-
for _, spec := range n.Specs {
973-
switch spec := spec.(type) {
1023+
for _, s := range n.Specs {
1024+
switch s := s.(type) {
9741025
case *ast.TypeSpec:
975-
if spec.Name.Pos() == pos {
1026+
if s.Name.Pos() == pos {
9761027
decl = n
1028+
spec = s
9771029
panic(found{})
9781030
}
9791031
case *ast.ValueSpec:
980-
for _, id := range spec.Names {
1032+
for _, id := range s.Names {
9811033
if id.Pos() == pos {
9821034
decl = n
1035+
spec = s
9831036
panic(found{})
9841037
}
9851038
}
@@ -992,5 +1045,5 @@ func FindDeclAndField(files []*ast.File, pos token.Pos) (decl ast.Decl, field *a
9921045
ast.Inspect(file, f)
9931046
}
9941047

995-
return nil, nil
1048+
return nil, nil, nil
9961049
}

‎gopls/internal/lsp/source/identifier.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,7 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa
274274
if err != nil {
275275
return nil, err
276276
}
277-
// TODO(adonovan): there's no need to inspect the entire GetSyntax() slice:
278-
// we already know it's declFile.File.
279-
result.Declaration.node, _ = FindDeclAndField(declPkg.GetSyntax(), declPos) // may be nil
277+
result.Declaration.node, _, _ = FindDeclInfo([]*ast.File{declFile.File}, declPos) // may be nil
280278
result.Declaration.nodeFile = declFile
281279

282280
// Ensure that we have the full declaration, in case the declaration was

‎gopls/internal/lsp/source/signature_help.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ FindCall:
101101
if err != nil {
102102
return nil, 0, err
103103
}
104-
node, _ := FindDeclAndField(declPkg.GetSyntax(), obj.Pos()) // may be nil
104+
node, _, _ := FindDeclInfo(declPkg.GetSyntax(), obj.Pos()) // may be nil
105105
d, err := FindHoverContext(ctx, snapshot, pkg, obj, node, nil)
106106
if err != nil {
107107
return nil, 0, err
@@ -111,7 +111,11 @@ FindCall:
111111
} else {
112112
name = "func"
113113
}
114-
s := NewSignature(ctx, snapshot, pkg, sig, comment, qf)
114+
mq := MetadataQualifierForFile(snapshot, pgf.File, pkg.Metadata())
115+
s, err := NewSignature(ctx, snapshot, pkg, pgf.File, sig, comment, qf, mq)
116+
if err != nil {
117+
return nil, 0, err
118+
}
115119
paramInfo := make([]protocol.ParameterInformation, 0, len(s.params))
116120
for _, p := range s.params {
117121
paramInfo = append(paramInfo, protocol.ParameterInformation{Label: p})

0 commit comments

Comments
 (0)