[tools] internal/lsp/source: derive document symbols from syntax alone
0 views
Skip to first unread message
Robert Findley (Gerrit)
Sep 23, 2022, 9:51:13 AM9/23/22
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Hyang-Ah Hana Kim, Gopher Robot, kokoro, Alan Donovan, golang-co...@googlegroups.com
Robert Findley submitted this change.
10 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: gopls/internal/lsp/testdata/symbols/main.go.golden
Insertions: 34, Deletions: 34.
@@ -1,36 +1,36 @@
-- symbols --
-x Variable 12:5-12:6
-nested Variable 14:5-14:11
- nestedField Field 15:2-15:13
-y Constant 20:7-20:8
-Number Class 22:6-22:12
-Alias Class 24:6-24:11
-NumberAlias Class 26:6-26:17
-Boolean Class 29:2-29:9
-BoolAlias Class 30:2-30:11
-Foo Struct 33:6-33:9
- Bar Field 36:2-36:5
- Quux Field 34:2-34:6
- W Field 35:2-35:3
- baz Field 37:2-37:5
- funcField Field 38:2-38:11
-Quux Struct 41:6-41:10
- X Field 42:2-42:3
- Y Field 42:5-42:6
-EmptyStruct Struct 45:6-45:17
-(Foo).Baz Method 47:14-47:17
-(*Quux).Do Method 53:16-53:18
-main Function 55:6-55:10
-Stringer Interface 58:6-58:14
- String Method 59:2-59:8
-ABer Interface 62:6-62:10
- A Method 64:2-64:3
- B Method 63:2-63:3
-WithEmbeddeds Interface 67:6-67:19
- ABer Field 69:2-69:6
- Do Method 68:2-68:4
- Writer Field 70:5-70:11
-EmptyInterface Interface 73:6-73:20
-Dunk Function 75:6-75:10
-dunk Function 77:6-77:10
+x Variable 26:5-26:6
+nested Variable 28:5-28:11
+ nestedField Field 29:2-29:13
+y Constant 34:7-34:8
+Number Class 36:6-36:12
+Alias Class 38:6-38:11
+NumberAlias Class 40:6-40:17
+Boolean Class 43:2-43:9
+BoolAlias Class 44:2-44:11
+Foo Struct 47:6-47:9
+ Bar Field 50:2-50:5
+ Quux Field 48:2-48:6
+ W Field 49:2-49:3
+ baz Field 51:2-51:5
+ funcField Field 52:2-52:11
+Quux Struct 55:6-55:10
+ X Field 56:2-56:3
+ Y Field 56:5-56:6
+EmptyStruct Struct 59:6-59:17
+(Foo).Baz Method 61:14-61:17
+(*Quux).Do Method 67:16-67:18
+main Function 69:6-69:10
+Stringer Interface 72:6-72:14
+ String Method 73:2-73:8
+ABer Interface 76:6-76:10
+ A Method 78:2-78:3
+ B Method 77:2-77:3
+WithEmbeddeds Interface 81:6-81:19
+ ABer Field 83:2-83:6
+ Do Method 82:2-82:4
+ Writer Field 84:5-84:11
+EmptyInterface Interface 87:6-87:20
+Dunk Function 89:6-89:10
+dunk Function 91:6-91:10
```
Approvals:
Robert Findley: Run TryBots
Alan Donovan: Looks good to me, approved
Gopher Robot: TryBots succeeded
Hyang-Ah Hana Kim: Looks good to me, but someone else must approve
kokoro: gopls CI succeeded
internal/lsp/source: derive document symbols from syntax alone
The documentSymbols handler joined syntax information with type
information, meaning that it was only able to satisfy requests for files
in valid workspace packages. However, the value added by type
information was limited, and in many cases could be derived from syntax
alone. For example, while generating symbols for a const declaration, we
don't need the type checker to tell us that the symbol kind is const.
Refactor the documentSymbols handler to derive symbols from syntax
alone. This leads to some simplifications from the code, in addition to
eliminating the dependency on package data. Also, simplify symbol
details to just use types.ExprString, which includes some missing
information such as function return values. Also, update handling to
support Go 1.18 type embedding in interfaces.
Notably, this reverts decisions like golang/go#31202, in which we went to
effort to make the symbol kind more accurate. In my opinion (and the
opinion expressed in golang/go#52797), the cost of requiring type
information is not worth the minor improvement in accuracy of the symbol
kind, which (as far as I know) is only used for UI elements.
To facilitate testing (and start to clean up the test framework), make
several simplifications / improvements to the marker tests:
- simplify the collection of symbol data
- eliminate unused marks
- just use cmp.Diff for comparing results
- allow for arbitrary nesting of symbols.
- remove unnecessary @symbol annotations from workspace_symbol tests --
their data is not used by workspace_symbol handlers
- remove Symbol and WorkspaceSymbol handlers from source_test.go. On
inspection, these handlers were redundant with lsp_test.go.
Notably, the collection and assembly of @symbol annotations is still way
too complicated. It would be much simpler to just have a single golden
file summarizing the entire output, rather than weaving it together from
annotations. However, I realized this too late, and so it will have to
wait for a separate CL.
Fixes golang/go#52797
Fixes golang/vscode-go#2242
Updates golang/go#54845
Change-Id: I3a2c2d39f59f9d045a6cedf8023ff0c80a69d974
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405254
gopls-CI: kokoro <noreply...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hya...@gmail.com>
Run-TryBot: Robert Findley <rfin...@google.com>
Reviewed-by: Alan Donovan <adon...@google.com>
---
M gopls/internal/lsp/cache/parse.go
M gopls/internal/lsp/cmd/test/symbols.go
M gopls/internal/lsp/lsp_test.go
M gopls/internal/lsp/lsppos/token.go
M gopls/internal/lsp/protocol/span.go
M gopls/internal/lsp/source/source_test.go
M gopls/internal/lsp/source/symbols.go
M gopls/internal/lsp/source/util.go
M gopls/internal/lsp/testdata/summary.txt.golden
M gopls/internal/lsp/testdata/summary_go1.18.txt.golden
A gopls/internal/lsp/testdata/symbols/go1.18.go
A gopls/internal/lsp/testdata/symbols/go1.18.go.golden
M gopls/internal/lsp/testdata/symbols/main.go
M gopls/internal/lsp/testdata/symbols/main.go.golden
M gopls/internal/lsp/testdata/workspacesymbol/a/a.go
D gopls/internal/lsp/testdata/workspacesymbol/a/a.go.golden
M gopls/internal/lsp/testdata/workspacesymbol/a/a_test.go
D gopls/internal/lsp/testdata/workspacesymbol/a/a_test.go.golden
M gopls/internal/lsp/testdata/workspacesymbol/a/a_x_test.go
D gopls/internal/lsp/testdata/workspacesymbol/a/a_x_test.go.golden
M gopls/internal/lsp/testdata/workspacesymbol/b/b.go
D gopls/internal/lsp/testdata/workspacesymbol/b/b.go.golden
M gopls/internal/lsp/tests/tests.go
M gopls/internal/lsp/tests/util.go
24 files changed, 470 insertions(+), 415 deletions(-)
diff --git a/gopls/internal/lsp/cache/parse.go b/gopls/internal/lsp/cache/parse.go
index 5cb5ef7..981e698 100644
--- a/gopls/internal/lsp/cache/parse.go
+++ b/gopls/internal/lsp/cache/parse.go
@@ -18,13 +18,13 @@
"strconv"
"strings"
- "golang.org/x/tools/internal/event"
- "golang.org/x/tools/internal/event/tag"
- "golang.org/x/tools/internal/diff"
- "golang.org/x/tools/internal/diff/myers"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/safetoken"
"golang.org/x/tools/gopls/internal/lsp/source"
+ "golang.org/x/tools/internal/diff"
+ "golang.org/x/tools/internal/diff/myers"
+ "golang.org/x/tools/internal/event"
+ "golang.org/x/tools/internal/event/tag"
"golang.org/x/tools/internal/memoize"
)
@@ -237,7 +237,7 @@
func (f *unexportedFilter) filterDecl(decl ast.Decl) bool {
switch decl := decl.(type) {
case *ast.FuncDecl:
- if ident := recvIdent(decl); ident != nil && !f.keep(ident) {
+ if ident := source.RecvIdent(decl.Recv); ident != nil && !f.keep(ident) {
return false
}
return f.keep(decl.Name)
@@ -312,7 +312,7 @@
switch decl := decl.(type) {
case *ast.FuncDecl:
// Ignore methods on dropped types.
- if ident := recvIdent(decl); ident != nil && !f.keep(ident) {
+ if ident := source.RecvIdent(decl.Recv); ident != nil && !f.keep(ident) {
break
}
// Ignore functions with dropped names.
@@ -356,21 +356,6 @@
}
}
-// recvIdent returns the identifier of a method receiver, e.g. *int.
-func recvIdent(decl *ast.FuncDecl) *ast.Ident {
- if decl.Recv == nil || len(decl.Recv.List) == 0 {
- return nil
- }
- x := decl.Recv.List[0].Type
- if star, ok := x.(*ast.StarExpr); ok {
- x = star.X
- }
- if ident, ok := x.(*ast.Ident); ok {
- return ident
- }
- return nil
-}
-
// recordIdents records unexported identifiers in an Expr in uses.
// These may be types, e.g. in map[key]value, function names, e.g. in foo(),
// or simple variable references. References that will be discarded, such
diff --git a/gopls/internal/lsp/cmd/test/symbols.go b/gopls/internal/lsp/cmd/test/symbols.go
index fbaf679..3bd2fc0 100644
--- a/gopls/internal/lsp/cmd/test/symbols.go
+++ b/gopls/internal/lsp/cmd/test/symbols.go
@@ -8,6 +8,7 @@
"testing"
"golang.org/x/tools/gopls/internal/lsp/protocol"
+ "golang.org/x/tools/gopls/internal/lsp/tests/compare"
"golang.org/x/tools/internal/span"
)
@@ -17,7 +18,7 @@
expect := string(r.data.Golden(t, "symbols", filename, func() ([]byte, error) {
return []byte(got), nil
}))
- if expect != got {
- t.Errorf("symbols failed for %s expected:\n%s\ngot:\n%s", filename, expect, got)
+ if diff := compare.Text(expect, got); diff != "" {
+ t.Errorf("symbols differ from expected:\n%s", diff)
}
}
diff --git a/gopls/internal/lsp/lsp_test.go b/gopls/internal/lsp/lsp_test.go
index 1cb5b3a..02b5727 100644
--- a/gopls/internal/lsp/lsp_test.go
+++ b/gopls/internal/lsp/lsp_test.go
@@ -15,6 +15,8 @@
"strings"
"testing"
+ "github.com/google/go-cmp/cmp"
+ "github.com/google/go-cmp/cmp/cmpopts"
"golang.org/x/tools/gopls/internal/lsp/cache"
"golang.org/x/tools/gopls/internal/lsp/command"
"golang.org/x/tools/gopls/internal/lsp/protocol"
@@ -1147,10 +1149,7 @@
if err != nil {
t.Fatal(err)
}
- if len(got) != len(expectedSymbols) {
- t.Errorf("want %d top-level symbols in %v, got %d", len(expectedSymbols), uri, len(got))
- return
- }
+
symbols := make([]protocol.DocumentSymbol, len(got))
for i, s := range got {
s, ok := s.(protocol.DocumentSymbol)
@@ -1159,18 +1158,25 @@
}
symbols[i] = s
}
- if diff := tests.DiffSymbols(t, uri, expectedSymbols, symbols); diff != "" {
- t.Error(diff)
+
+ // Sort by position to make it easier to find errors.
+ sortSymbols := func(s []protocol.DocumentSymbol) {
+ sort.Slice(s, func(i, j int) bool {
+ return protocol.CompareRange(s[i].SelectionRange, s[j].SelectionRange) < 0
+ })
+ }
+ sortSymbols(expectedSymbols)
+ sortSymbols(symbols)
+
+ // Ignore 'Range' here as it is difficult (impossible?) to express
+ // multi-line ranges in the packagestest framework.
+ ignoreRange := cmpopts.IgnoreFields(protocol.DocumentSymbol{}, "Range")
+ if diff := cmp.Diff(expectedSymbols, symbols, ignoreRange); diff != "" {
+ t.Errorf("mismatching symbols (-want +got)\n%s", diff)
}
}
func (r *runner) WorkspaceSymbols(t *testing.T, uri span.URI, query string, typ tests.WorkspaceSymbolsTestType) {
- r.callWorkspaceSymbols(t, uri, query, typ)
-}
-
-func (r *runner) callWorkspaceSymbols(t *testing.T, uri span.URI, query string, typ tests.WorkspaceSymbolsTestType) {
- t.Helper()
-
matcher := tests.WorkspaceSymbolsTestTypeToMatcher(typ)
original := r.server.session.Options()
diff --git a/gopls/internal/lsp/lsppos/token.go b/gopls/internal/lsp/lsppos/token.go
index 2a16ba2..c1b726f 100644
--- a/gopls/internal/lsp/lsppos/token.go
+++ b/gopls/internal/lsp/lsppos/token.go
@@ -6,6 +6,7 @@
import (
"errors"
+ "go/ast"
"go/token"
"golang.org/x/tools/gopls/internal/lsp/protocol"
@@ -58,3 +59,9 @@
return protocol.Range{Start: startPos, End: endPos}, nil
}
+
+// NodeRange returns the protocol range corresponding to the span of the given
+// node.
+func (m *TokenMapper) NodeRange(n ast.Node) (protocol.Range, error) {
+ return m.Range(n.Pos(), n.End())
+}
diff --git a/gopls/internal/lsp/protocol/span.go b/gopls/internal/lsp/protocol/span.go
index 744746d..23721ee 100644
--- a/gopls/internal/lsp/protocol/span.go
+++ b/gopls/internal/lsp/protocol/span.go
@@ -148,6 +148,11 @@
return r.Start.Line == r.End.Line && r.Start.Character == r.End.Character
}
+// CompareRange returns -1 if a is before b, 0 if a == b, and 1 if a is after
+// b.
+//
+// A range a is defined to be 'before' b if a.Start is before b.Start, or
+// a.Start == b.Start and a.End is before b.End.
func CompareRange(a, b Range) int {
if r := ComparePosition(a.Start, b.Start); r != 0 {
return r
@@ -155,6 +160,8 @@
return ComparePosition(a.End, b.End)
}
+// ComparePosition returns -1 if a is before b, 0 if a == b, and 1 if a is
+// after b.
func ComparePosition(a, b Position) int {
if a.Line < b.Line {
return -1
diff --git a/gopls/internal/lsp/source/source_test.go b/gopls/internal/lsp/source/source_test.go
index ce2cc0a..ad2ca6b 100644
--- a/gopls/internal/lsp/source/source_test.go
+++ b/gopls/internal/lsp/source/source_test.go
@@ -879,46 +879,11 @@
}
func (r *runner) Symbols(t *testing.T, uri span.URI, expectedSymbols []protocol.DocumentSymbol) {
- fh, err := r.snapshot.GetFile(r.ctx, uri)
- if err != nil {
- t.Fatal(err)
- }
- symbols, err := source.DocumentSymbols(r.ctx, r.snapshot, fh)
- if err != nil {
- t.Errorf("symbols failed for %s: %v", uri, err)
- }
- if len(symbols) != len(expectedSymbols) {
- t.Errorf("want %d top-level symbols in %v, got %d", len(expectedSymbols), uri, len(symbols))
- return
- }
- if diff := tests.DiffSymbols(t, uri, expectedSymbols, symbols); diff != "" {
- t.Error(diff)
- }
+ // Removed in favor of just using the lsp_test implementation. See ../lsp_test.go
}
func (r *runner) WorkspaceSymbols(t *testing.T, uri span.URI, query string, typ tests.WorkspaceSymbolsTestType) {
- r.callWorkspaceSymbols(t, uri, query, typ)
-}
-
-func (r *runner) callWorkspaceSymbols(t *testing.T, uri span.URI, query string, typ tests.WorkspaceSymbolsTestType) {
- t.Helper()
-
- matcher := tests.WorkspaceSymbolsTestTypeToMatcher(typ)
- gotSymbols, err := source.WorkspaceSymbols(r.ctx, matcher, r.view.Options().SymbolStyle, []source.View{r.view}, query)
- if err != nil {
- t.Fatal(err)
- }
- got, err := tests.WorkspaceSymbolsString(r.ctx, r.data, uri, gotSymbols)
- if err != nil {
- t.Fatal(err)
- }
- got = filepath.ToSlash(tests.Normalize(got, r.normalizers))
- want := string(r.data.Golden(t, fmt.Sprintf("workspace_symbol-%s-%s", strings.ToLower(string(matcher)), query), uri.Filename(), func() ([]byte, error) {
- return []byte(got), nil
- }))
- if d := compare.Text(want, got); d != "" {
- t.Error(d)
- }
+ // Removed in favor of just using the lsp_test implementation. See ../lsp_test.go
}
func (r *runner) SignatureHelp(t *testing.T, spn span.Span, want *protocol.SignatureHelp) {
diff --git a/gopls/internal/lsp/source/symbols.go b/gopls/internal/lsp/source/symbols.go
index 802de37..2bab241 100644
--- a/gopls/internal/lsp/source/symbols.go
+++ b/gopls/internal/lsp/source/symbols.go
@@ -8,25 +8,34 @@
"context"
"fmt"
"go/ast"
+ "go/token"
"go/types"
- "golang.org/x/tools/internal/event"
+ "golang.org/x/tools/gopls/internal/lsp/lsppos"
"golang.org/x/tools/gopls/internal/lsp/protocol"
+ "golang.org/x/tools/internal/event"
)
func DocumentSymbols(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.DocumentSymbol, error) {
ctx, done := event.Start(ctx, "source.DocumentSymbols")
defer done()
- pkg, pgf, err := GetParsedFile(ctx, snapshot, fh, NarrowestPackage)
+ content, err := fh.Read()
+ if err != nil {
+ return nil, err
+ }
+
+ pgf, err := snapshot.ParseGo(ctx, fh, ParseFull)
if err != nil {
return nil, fmt.Errorf("getting file for DocumentSymbols: %w", err)
}
- info := pkg.GetTypesInfo()
- q := Qualifier(pgf.File, pkg.GetTypes(), info)
+ m := lsppos.NewTokenMapper(content, pgf.Tok)
- symbolsToReceiver := make(map[types.Type]int)
+ // Build symbols for file declarations. When encountering a declaration with
+ // errors (typically because positions are invalid), we skip the declaration
+ // entirely. VS Code fails to show any symbols if one of the top-level
+ // symbols is missing position information.
var symbols []protocol.DocumentSymbol
for _, decl := range pgf.File.Decls {
switch decl := decl.(type) {
@@ -34,15 +43,11 @@
if decl.Name.Name == "_" {
continue
}
- if obj := info.ObjectOf(decl.Name); obj != nil {
- fs, err := funcSymbol(snapshot, pkg, decl, obj, q)
- if err != nil {
- return nil, err
- }
+ fs, err := funcSymbol(m, decl)
+ if err == nil {
// If function is a method, prepend the type of the method.
- if fs.Kind == protocol.Method {
- rtype := obj.Type().(*types.Signature).Recv().Type()
- fs.Name = fmt.Sprintf("(%s).%s", types.TypeString(rtype, q), fs.Name)
+ if decl.Recv != nil && len(decl.Recv.List) > 0 {
+ fs.Name = fmt.Sprintf("(%s).%s", types.ExprString(decl.Recv.List[0].Type), fs.Name)
}
symbols = append(symbols, fs)
}
@@ -53,24 +58,17 @@
if spec.Name.Name == "_" {
continue
}
- if obj := info.ObjectOf(spec.Name); obj != nil {
- ts, err := typeSymbol(snapshot, pkg, info, spec, obj, q)
- if err != nil {
- return nil, err
- }
+ ts, err := typeSymbol(m, spec)
+ if err == nil {
symbols = append(symbols, ts)
- symbolsToReceiver[obj.Type()] = len(symbols) - 1
}
case *ast.ValueSpec:
for _, name := range spec.Names {
if name.Name == "_" {
continue
}
- if obj := info.ObjectOf(name); obj != nil {
- vs, err := varSymbol(snapshot, pkg, decl, name, obj, q)
- if err != nil {
- return nil, err
- }
+ vs, err := varSymbol(m, spec, name, decl.Tok == token.CONST)
+ if err == nil {
symbols = append(symbols, vs)
}
}
@@ -81,185 +79,157 @@
return symbols, nil
}
-func funcSymbol(snapshot Snapshot, pkg Package, decl *ast.FuncDecl, obj types.Object, q types.Qualifier) (protocol.DocumentSymbol, error) {
+func funcSymbol(m *lsppos.TokenMapper, decl *ast.FuncDecl) (protocol.DocumentSymbol, error) {
s := protocol.DocumentSymbol{
- Name: obj.Name(),
+ Name: decl.Name.Name,
Kind: protocol.Function,
}
+ if decl.Recv != nil {
+ s.Kind = protocol.Method
+ }
var err error
- s.Range, err = nodeToProtocolRange(snapshot, pkg, decl)
+ s.Range, err = m.Range(decl.Pos(), decl.End())
if err != nil {
return protocol.DocumentSymbol{}, err
}
- s.SelectionRange, err = nodeToProtocolRange(snapshot, pkg, decl.Name)
+ s.SelectionRange, err = m.Range(decl.Name.Pos(), decl.Name.End())
if err != nil {
return protocol.DocumentSymbol{}, err
}
- sig, _ := obj.Type().(*types.Signature)
- if sig != nil {
- if sig.Recv() != nil {
- s.Kind = protocol.Method
- }
- s.Detail += "("
- for i := 0; i < sig.Params().Len(); i++ {
- if i > 0 {
- s.Detail += ", "
- }
- param := sig.Params().At(i)
- label := types.TypeString(param.Type(), q)
- if param.Name() != "" {
- label = fmt.Sprintf("%s %s", param.Name(), label)
- }
- s.Detail += label
- }
- s.Detail += ")"
- }
+ s.Detail = types.ExprString(decl.Type)
return s, nil
}
-func typeSymbol(snapshot Snapshot, pkg Package, info *types.Info, spec *ast.TypeSpec, obj types.Object, qf types.Qualifier) (protocol.DocumentSymbol, error) {
+func typeSymbol(m *lsppos.TokenMapper, spec *ast.TypeSpec) (protocol.DocumentSymbol, error) {
s := protocol.DocumentSymbol{
- Name: obj.Name(),
+ Name: spec.Name.Name,
}
- s.Detail, _ = FormatType(obj.Type(), qf)
- s.Kind = typeToKind(obj.Type())
-
var err error
- s.Range, err = nodeToProtocolRange(snapshot, pkg, spec)
+ s.Range, err = m.NodeRange(spec)
if err != nil {
return protocol.DocumentSymbol{}, err
}
- s.SelectionRange, err = nodeToProtocolRange(snapshot, pkg, spec.Name)
+ s.SelectionRange, err = m.NodeRange(spec.Name)
if err != nil {
return protocol.DocumentSymbol{}, err
}
- t, objIsStruct := obj.Type().Underlying().(*types.Struct)
- st, specIsStruct := spec.Type.(*ast.StructType)
- if objIsStruct && specIsStruct {
- for i := 0; i < t.NumFields(); i++ {
- f := t.Field(i)
- child := protocol.DocumentSymbol{
- Name: f.Name(),
- Kind: protocol.Field,
- }
- child.Detail, _ = FormatType(f.Type(), qf)
-
- spanNode, selectionNode := nodesForStructField(i, st)
- if span, err := nodeToProtocolRange(snapshot, pkg, spanNode); err == nil {
- child.Range = span
- }
- if span, err := nodeToProtocolRange(snapshot, pkg, selectionNode); err == nil {
- child.SelectionRange = span
- }
- s.Children = append(s.Children, child)
- }
- }
-
- ti, objIsInterface := obj.Type().Underlying().(*types.Interface)
- ai, specIsInterface := spec.Type.(*ast.InterfaceType)
- if objIsInterface && specIsInterface {
- for i := 0; i < ti.NumExplicitMethods(); i++ {
- method := ti.ExplicitMethod(i)
- child := protocol.DocumentSymbol{
- Name: method.Name(),
- Kind: protocol.Method,
- }
-
- var spanNode, selectionNode ast.Node
- Methods:
- for _, f := range ai.Methods.List {
- for _, id := range f.Names {
- if id.Name == method.Name() {
- spanNode, selectionNode = f, id
- break Methods
- }
- }
- }
- child.Range, err = nodeToProtocolRange(snapshot, pkg, spanNode)
- if err != nil {
- return protocol.DocumentSymbol{}, err
- }
- child.SelectionRange, err = nodeToProtocolRange(snapshot, pkg, selectionNode)
- if err != nil {
- return protocol.DocumentSymbol{}, err
- }
- s.Children = append(s.Children, child)
- }
-
- for i := 0; i < ti.NumEmbeddeds(); i++ {
- embedded := ti.EmbeddedType(i)
- nt, isNamed := embedded.(*types.Named)
- if !isNamed {
- continue
- }
-
- child := protocol.DocumentSymbol{
- Name: types.TypeString(embedded, qf),
- }
- child.Kind = typeToKind(embedded)
- var spanNode, selectionNode ast.Node
- Embeddeds:
- for _, f := range ai.Methods.List {
- if len(f.Names) > 0 {
- continue
- }
-
- if t := info.TypeOf(f.Type); types.Identical(nt, t) {
- spanNode, selectionNode = f, f.Type
- break Embeddeds
- }
- }
- child.Range, err = nodeToProtocolRange(snapshot, pkg, spanNode)
- if err != nil {
- return protocol.DocumentSymbol{}, err
- }
- child.SelectionRange, err = nodeToProtocolRange(snapshot, pkg, selectionNode)
- if err != nil {
- return protocol.DocumentSymbol{}, err
- }
- s.Children = append(s.Children, child)
- }
- }
+ s.Kind, s.Detail, s.Children = typeDetails(m, spec.Type)
return s, nil
}
-func nodesForStructField(i int, st *ast.StructType) (span, selection ast.Node) {
- j := 0
- for _, field := range st.Fields.List {
- if len(field.Names) == 0 {
- if i == j {
- return field, field.Type
- }
- j++
- continue
+func typeDetails(m *lsppos.TokenMapper, typExpr ast.Expr) (kind protocol.SymbolKind, detail string, children []protocol.DocumentSymbol) {
+ switch typExpr := typExpr.(type) {
+ case *ast.StructType:
+ kind = protocol.Struct
+ children = fieldListSymbols(m, typExpr.Fields, protocol.Field)
+ if len(children) > 0 {
+ detail = "struct{...}"
+ } else {
+ detail = "struct{}"
}
- for _, name := range field.Names {
- if i == j {
- return field, name
- }
- j++
+
+ // Find interface methods and embedded types.
+ case *ast.InterfaceType:
+ kind = protocol.Interface
+ children = fieldListSymbols(m, typExpr.Methods, protocol.Method)
+ if len(children) > 0 {
+ detail = "interface{...}"
+ } else {
+ detail = "interface{}"
}
+
+ case *ast.FuncType:
+ kind = protocol.Function
+ detail = types.ExprString(typExpr)
+
+ default:
+ kind = protocol.Class // catch-all, for cases where we don't know the kind syntactically
+ detail = types.ExprString(typExpr)
}
- return nil, nil
+ return
}
-func varSymbol(snapshot Snapshot, pkg Package, decl ast.Node, name *ast.Ident, obj types.Object, q types.Qualifier) (protocol.DocumentSymbol, error) {
+func fieldListSymbols(m *lsppos.TokenMapper, fields *ast.FieldList, fieldKind protocol.SymbolKind) []protocol.DocumentSymbol {
+ if fields == nil {
+ return nil
+ }
+
+ var symbols []protocol.DocumentSymbol
+ for _, field := range fields.List {
+ detail, children := "", []protocol.DocumentSymbol(nil)
+ if field.Type != nil {
+ _, detail, children = typeDetails(m, field.Type)
+ }
+ if len(field.Names) == 0 { // embedded interface or struct field
+ // By default, use the formatted type details as the name of this field.
+ // This handles potentially invalid syntax, as well as type embeddings in
+ // interfaces.
+ child := protocol.DocumentSymbol{
+ Name: detail,
+ Kind: protocol.Field, // consider all embeddings to be fields
+ Children: children,
+ }
+
+ // If the field is a valid embedding, promote the type name to field
+ // name.
+ selection := field.Type
+ if id := embeddedIdent(field.Type); id != nil {
+ child.Name = id.Name
+ child.Detail = detail
+ selection = id
+ }
+
+ if rng, err := m.NodeRange(field.Type); err == nil {
+ child.Range = rng
+ }
+ if rng, err := m.NodeRange(selection); err == nil {
+ child.SelectionRange = rng
+ }
+
+ symbols = append(symbols, child)
+ } else {
+ for _, name := range field.Names {
+ child := protocol.DocumentSymbol{
+ Name: name.Name,
+ Kind: fieldKind,
+ Detail: detail,
+ Children: children,
+ }
+
+ if rng, err := m.NodeRange(field); err == nil {
+ child.Range = rng
+ }
+ if rng, err := m.NodeRange(name); err == nil {
+ child.SelectionRange = rng
+ }
+
+ symbols = append(symbols, child)
+ }
+ }
+
+ }
+ return symbols
+}
+
+func varSymbol(m *lsppos.TokenMapper, spec *ast.ValueSpec, name *ast.Ident, isConst bool) (protocol.DocumentSymbol, error) {
s := protocol.DocumentSymbol{
- Name: obj.Name(),
+ Name: name.Name,
Kind: protocol.Variable,
}
- if _, ok := obj.(*types.Const); ok {
+ if isConst {
s.Kind = protocol.Constant
}
var err error
- s.Range, err = nodeToProtocolRange(snapshot, pkg, decl)
+ s.Range, err = m.NodeRange(spec)
if err != nil {
return protocol.DocumentSymbol{}, err
}
- s.SelectionRange, err = nodeToProtocolRange(snapshot, pkg, name)
+ s.SelectionRange, err = m.NodeRange(name)
if err != nil {
return protocol.DocumentSymbol{}, err
}
- s.Detail = types.TypeString(obj.Type(), q)
+ if spec.Type != nil { // type may be missing from the syntax
+ _, s.Detail, s.Children = typeDetails(m, spec.Type)
+ }
return s, nil
}
diff --git a/gopls/internal/lsp/source/util.go b/gopls/internal/lsp/source/util.go
index da9c3b5..d1c90b6 100644
--- a/gopls/internal/lsp/source/util.go
+++ b/gopls/internal/lsp/source/util.go
@@ -18,9 +18,10 @@
"strings"
"golang.org/x/mod/modfile"
- "golang.org/x/tools/internal/bug"
"golang.org/x/tools/gopls/internal/lsp/protocol"
+ "golang.org/x/tools/internal/bug"
"golang.org/x/tools/internal/span"
+ "golang.org/x/tools/internal/typeparams"
)
// MappedRange provides mapped protocol.Range for a span.Range, accounting for
@@ -122,16 +123,6 @@
return false
}
-func nodeToProtocolRange(snapshot Snapshot, pkg Package, n ast.Node) (protocol.Range, error) {
- mrng, err := posToMappedRange(snapshot, pkg, n.Pos(), n.End())
- if err != nil {
- return protocol.Range{}, err
- }
- return mrng.Range()
-}
-
-// objToMappedRange returns the MappedRange for the object's declaring
-// identifier (or string literal, for an import).
func objToMappedRange(snapshot Snapshot, pkg Package, obj types.Object) (MappedRange, error) {
nameLen := len(obj.Name())
if pkgName, ok := obj.(*types.PkgName); ok {
@@ -589,3 +580,51 @@
e := span.NewPoint(line, col, end)
return m.Range(span.New(uri, s, e))
}
+
+// RecvIdent returns the type identifier of a method receiver.
+// e.g. A for all of A, *A, A[T], *A[T], etc.
+func RecvIdent(recv *ast.FieldList) *ast.Ident {
+ if recv == nil || len(recv.List) == 0 {
+ return nil
+ }
+ x := recv.List[0].Type
+ if star, ok := x.(*ast.StarExpr); ok {
+ x = star.X
+ }
+ switch ix := x.(type) { // check for instantiated receivers
+ case *ast.IndexExpr:
+ x = ix.X
+ case *typeparams.IndexListExpr:
+ x = ix.X
+ }
+ if ident, ok := x.(*ast.Ident); ok {
+ return ident
+ }
+ return nil
+}
+
+// embeddedIdent returns the type name identifier for an embedding x, if x in a
+// valid embedding. Otherwise, it returns nil.
+//
+// Spec: An embedded field must be specified as a type name T or as a pointer
+// to a non-interface type name *T
+func embeddedIdent(x ast.Expr) *ast.Ident {
+ if star, ok := x.(*ast.StarExpr); ok {
+ x = star.X
+ }
+ switch ix := x.(type) { // check for instantiated receivers
+ case *ast.IndexExpr:
+ x = ix.X
+ case *typeparams.IndexListExpr:
+ x = ix.X
+ }
+ switch x := x.(type) {
+ case *ast.Ident:
+ return x
+ case *ast.SelectorExpr:
+ if _, ok := x.X.(*ast.Ident); ok {
+ return x.Sel
+ }
+ }
+ return nil
+}
diff --git a/gopls/internal/lsp/testdata/summary.txt.golden b/gopls/internal/lsp/testdata/summary.txt.golden
index c8e10d1..107e900 100644
--- a/gopls/internal/lsp/testdata/summary.txt.golden
+++ b/gopls/internal/lsp/testdata/summary.txt.golden
@@ -23,7 +23,7 @@
ReferencesCount = 27
RenamesCount = 41
PrepareRenamesCount = 7
-SymbolsCount = 5
+SymbolsCount = 1
WorkspaceSymbolsCount = 20
SignaturesCount = 33
LinksCount = 7
diff --git a/gopls/internal/lsp/testdata/summary_go1.18.txt.golden b/gopls/internal/lsp/testdata/summary_go1.18.txt.golden
index c3ac008..93d9c63 100644
--- a/gopls/internal/lsp/testdata/summary_go1.18.txt.golden
+++ b/gopls/internal/lsp/testdata/summary_go1.18.txt.golden
@@ -23,7 +23,7 @@
ReferencesCount = 27
RenamesCount = 48
PrepareRenamesCount = 7
-SymbolsCount = 5
+SymbolsCount = 2
WorkspaceSymbolsCount = 20
SignaturesCount = 33
LinksCount = 7
diff --git a/gopls/internal/lsp/testdata/symbols/go1.18.go b/gopls/internal/lsp/testdata/symbols/go1.18.go
new file mode 100644
index 0000000..cdf99dc
--- /dev/null
+++ b/gopls/internal/lsp/testdata/symbols/go1.18.go
@@ -0,0 +1,16 @@
+//go:build go1.18
+// +build go1.18
+
+package main
+
+type T[P any] struct { //@symbol("T", "T", "Struct", "struct{...}", "T", "")
+ F P //@symbol("F", "F", "Field", "P", "", "T")
+}
+
+type Constraint interface { //@symbol("Constraint", "Constraint", "Interface", "interface{...}", "Constraint", "")
+ ~int | struct{ int } //@symbol("~int | struct{int}", "~int | struct{ int }", "Field", "", "", "Constraint")
+
+ // TODO(rfindley): the selection range below is the entire interface field.
+ // Can we reduce it?
+ interface{ M() } //@symbol("interface{...}", "interface{ M() }", "Field", "", "iFaceField", "Constraint"), symbol("M", "M", "Method", "func()", "", "iFaceField")
+}
diff --git a/gopls/internal/lsp/testdata/symbols/go1.18.go.golden b/gopls/internal/lsp/testdata/symbols/go1.18.go.golden
new file mode 100644
index 0000000..5a0c1a9
--- /dev/null
+++ b/gopls/internal/lsp/testdata/symbols/go1.18.go.golden
@@ -0,0 +1,7 @@
+-- symbols --
+T Struct 6:6-6:7
+ F Field 7:2-7:3
+Constraint Interface 10:6-10:16
+ interface{...} Field 15:2-15:18
+ ~int | struct{int} Field 11:2-11:22
+
diff --git a/gopls/internal/lsp/testdata/symbols/main.go b/gopls/internal/lsp/testdata/symbols/main.go
index 8111250..65e0869 100644
--- a/gopls/internal/lsp/testdata/symbols/main.go
+++ b/gopls/internal/lsp/testdata/symbols/main.go
@@ -4,61 +4,88 @@
"io"
)
+// Each symbol marker in this file defines the following information:
+// symbol(name, selectionSpan, kind, detail, id, parentID)
+// - name: DocumentSymbol.Name
+// - selectionSpan: DocumentSymbol.SelectionRange
+// - kind: DocumentSymbol.Kind
+// - detail: DocumentSymbol.Detail
+// - id: if non-empty, a unique identifier for this symbol
+// - parentID: if non-empty, the id of the parent of this symbol
+//
+// This data in aggregate defines a set of document symbols and their
+// parent-child relationships, which is compared against the DocummentSymbols
+// response from gopls for the current file.
+//
+// TODO(rfindley): the symbol annotations here are complicated and difficult to
+// maintain. It would be simpler to just write out the full expected response
+// in the golden file, perhaps as raw JSON.
+
var _ = 1
-var x = 42 //@mark(symbolsx, "x"), symbol("x", "x", "Variable", "", "main.x")
+var x = 42 //@symbol("x", "x", "Variable", "", "", "")
-const y = 43 //@symbol("y", "y", "Constant", "", "main.y")
+var nested struct { //@symbol("nested", "nested", "Variable", "struct{...}", "nested", "")
+ nestedField struct { //@symbol("nestedField", "nestedField", "Field", "struct{...}", "nestedField", "nested")
+ f int //@symbol("f", "f", "Field", "int", "", "nestedField")
+ }
+}
-type Number int //@symbol("Number", "Number", "Number", "", "main.Number")
+const y = 43 //@symbol("y", "y", "Constant", "", "", "")
-type Alias = string //@symbol("Alias", "Alias", "String", "", "main.Alias")
+type Number int //@symbol("Number", "Number", "Class", "int", "", "")
-type NumberAlias = Number //@symbol("NumberAlias", "NumberAlias", "Number", "", "main.NumberAlias")
+type Alias = string //@symbol("Alias", "Alias", "Class", "string", "", "")
+
+type NumberAlias = Number //@symbol("NumberAlias", "NumberAlias", "Class", "Number", "", "")
type (
- Boolean bool //@symbol("Boolean", "Boolean", "Boolean", "", "main.Boolean")
- BoolAlias = bool //@symbol("BoolAlias", "BoolAlias", "Boolean", "", "main.BoolAlias")
+ Boolean bool //@symbol("Boolean", "Boolean", "Class", "bool", "", "")
+ BoolAlias = bool //@symbol("BoolAlias", "BoolAlias", "Class", "bool", "", "")
)
-type Foo struct { //@mark(symbolsFoo, "Foo"), symbol("Foo", "Foo", "Struct", "", "main.Foo")
- Quux //@mark(fQuux, "Quux"), symbol("Quux", "Quux", "Field", "Foo", "main.Foo.Quux")
- W io.Writer //@symbol("W" , "W", "Field", "Foo", "main.Foo.W")
- Bar int //@mark(fBar, "Bar"), symbol("Bar", "Bar", "Field", "Foo", "main.Foo.Bar")
- baz string //@symbol("baz", "baz", "Field", "Foo", "main.Foo.baz")
+type Foo struct { //@symbol("Foo", "Foo", "Struct", "struct{...}", "Foo", "")
+ Quux //@symbol("Quux", "Quux", "Field", "Quux", "", "Foo")
+ W io.Writer //@symbol("W", "W", "Field", "io.Writer", "", "Foo")
+ Bar int //@symbol("Bar", "Bar", "Field", "int", "", "Foo")
+ baz string //@symbol("baz", "baz", "Field", "string", "", "Foo")
+ funcField func(int) int //@symbol("funcField", "funcField", "Field", "func(int) int", "", "Foo")
}
-type Quux struct { //@symbol("Quux", "Quux", "Struct", "", "main.Quux")
- X, Y float64 //@mark(qX, "X"), symbol("X", "X", "Field", "Quux", "main.X"), symbol("Y", "Y", "Field", "Quux", "main.Y")
+type Quux struct { //@symbol("Quux", "Quux", "Struct", "struct{...}", "Quux", "")
+ X, Y float64 //@symbol("X", "X", "Field", "float64", "", "Quux"), symbol("Y", "Y", "Field", "float64", "", "Quux")
}
-func (f Foo) Baz() string { //@symbol("(Foo).Baz", "Baz", "Method", "", "main.Foo.Baz")
+type EmptyStruct struct{} //@symbol("EmptyStruct", "EmptyStruct", "Struct", "struct{}", "", "")
+
+func (f Foo) Baz() string { //@symbol("(Foo).Baz", "Baz", "Method", "func() string", "", "")
return f.baz
}
func _() {}
-func (q *Quux) Do() {} //@mark(qDo, "Do"), symbol("(*Quux).Do", "Do", "Method", "", "main.Quux.Do")
+func (q *Quux) Do() {} //@symbol("(*Quux).Do", "Do", "Method", "func()", "", "")
-func main() { //@symbol("main", "main", "Function", "", "main.main")
-
+func main() { //@symbol("main", "main", "Function", "func()", "", "")
}
-type Stringer interface { //@symbol("Stringer", "Stringer", "Interface", "", "main.Stringer")
- String() string //@symbol("String", "String", "Method", "Stringer", "main.Stringer.String")
+type Stringer interface { //@symbol("Stringer", "Stringer", "Interface", "interface{...}", "Stringer", "")
+ String() string //@symbol("String", "String", "Method", "func() string", "", "Stringer")
}
-type ABer interface { //@mark(ABerInterface, "ABer"), symbol("ABer", "ABer", "Interface", "", "main.ABer")
- B() //@symbol("B", "B", "Method", "ABer", "main.ABer.B")
- A() string //@mark(ABerA, "A"), symbol("A", "A", "Method", "ABer", "main.ABer.A")
+type ABer interface { //@symbol("ABer", "ABer", "Interface", "interface{...}", "ABer", "")
+ B() //@symbol("B", "B", "Method", "func()", "", "ABer")
+ A() string //@symbol("A", "A", "Method", "func() string", "", "ABer")
}
-type WithEmbeddeds interface { //@symbol("WithEmbeddeds", "WithEmbeddeds", "Interface", "", "main.WithEmbeddeds")
- Do() //@symbol("Do", "Do", "Method", "WithEmbeddeds", "main.WithEmbeddeds.Do")
- ABer //@symbol("ABer", "ABer", "Interface", "WithEmbeddeds", "main.WithEmbeddeds.ABer")
- io.Writer //@mark(ioWriter, "io.Writer"), symbol("io.Writer", "io.Writer", "Interface", "WithEmbeddeds", "main.WithEmbeddeds.Writer")
+type WithEmbeddeds interface { //@symbol("WithEmbeddeds", "WithEmbeddeds", "Interface", "interface{...}", "WithEmbeddeds", "")
+ Do() //@symbol("Do", "Do", "Method", "func()", "", "WithEmbeddeds")
+ ABer //@symbol("ABer", "ABer", "Field", "ABer", "", "WithEmbeddeds")
+ io.Writer //@symbol("Writer", "Writer", "Field", "io.Writer", "", "WithEmbeddeds")
}
-func Dunk() int { return 0 } //@symbol("Dunk", "Dunk", "Function", "", "main.Dunk")
+type EmptyInterface interface{} //@symbol("EmptyInterface", "EmptyInterface", "Interface", "interface{}", "", "")
-func dunk() {} //@symbol("dunk", "dunk", "Function", "", "main.dunk")
+func Dunk() int { return 0 } //@symbol("Dunk", "Dunk", "Function", "func() int", "", "")
+
+func dunk() {} //@symbol("dunk", "dunk", "Function", "func()", "", "")
diff --git a/gopls/internal/lsp/testdata/symbols/main.go.golden b/gopls/internal/lsp/testdata/symbols/main.go.golden
index ebb6a8a..98009b0 100644
--- a/gopls/internal/lsp/testdata/symbols/main.go.golden
+++ b/gopls/internal/lsp/testdata/symbols/main.go.golden
@@ -1,31 +1,36 @@
-- symbols --
-x Variable 9:5-9:6
-y Constant 11:7-11:8
-Number Number 13:6-13:12
-Alias String 15:6-15:11
-NumberAlias Number 17:6-17:17
-Boolean Boolean 20:2-20:9
-BoolAlias Boolean 21:2-21:11
-Foo Struct 24:6-24:9
- Bar Field 27:2-27:5
- Quux Field 25:2-25:6
- W Field 26:2-26:3
- baz Field 28:2-28:5
-Quux Struct 31:6-31:10
- X Field 32:2-32:3
- Y Field 32:5-32:6
-(Foo).Baz Method 35:14-35:17
-(*Quux).Do Method 41:16-41:18
-main Function 43:6-43:10
-Stringer Interface 47:6-47:14
- String Method 48:2-48:8
-ABer Interface 51:6-51:10
- A Method 53:2-53:3
- B Method 52:2-52:3
-WithEmbeddeds Interface 56:6-56:19
- ABer Interface 58:2-58:6
- Do Method 57:2-57:4
- io.Writer Interface 59:2-59:11
-Dunk Function 62:6-62:10
-dunk Function 64:6-64:10
+x Variable 26:5-26:6
+nested Variable 28:5-28:11
+ nestedField Field 29:2-29:13
+y Constant 34:7-34:8
+Number Class 36:6-36:12
+Alias Class 38:6-38:11
+NumberAlias Class 40:6-40:17
+Boolean Class 43:2-43:9
+BoolAlias Class 44:2-44:11
+Foo Struct 47:6-47:9
+ Bar Field 50:2-50:5
+ Quux Field 48:2-48:6
+ W Field 49:2-49:3
+ baz Field 51:2-51:5
+ funcField Field 52:2-52:11
+Quux Struct 55:6-55:10
+ X Field 56:2-56:3
+ Y Field 56:5-56:6
+EmptyStruct Struct 59:6-59:17
+(Foo).Baz Method 61:14-61:17
+(*Quux).Do Method 67:16-67:18
+main Function 69:6-69:10
+Stringer Interface 72:6-72:14
+ String Method 73:2-73:8
+ABer Interface 76:6-76:10
+ A Method 78:2-78:3
+ B Method 77:2-77:3
+WithEmbeddeds Interface 81:6-81:19
+ ABer Field 83:2-83:6
+ Do Method 82:2-82:4
+ Writer Field 84:5-84:11
+EmptyInterface Interface 87:6-87:20
+Dunk Function 89:6-89:10
+dunk Function 91:6-91:10
diff --git a/gopls/internal/lsp/testdata/workspacesymbol/a/a.go b/gopls/internal/lsp/testdata/workspacesymbol/a/a.go
index 6e5a68b..4ae9997 100644
--- a/gopls/internal/lsp/testdata/workspacesymbol/a/a.go
+++ b/gopls/internal/lsp/testdata/workspacesymbol/a/a.go
@@ -1,9 +1,9 @@
package a
-var RandomGopherVariableA = "a" //@symbol("RandomGopherVariableA", "RandomGopherVariableA", "Variable", "", "a.RandomGopherVariableA")
+var RandomGopherVariableA = "a"
-const RandomGopherConstantA = "a" //@symbol("RandomGopherConstantA", "RandomGopherConstantA", "Constant", "", "a.RandomGopherConstantA")
+const RandomGopherConstantA = "a"
const (
- randomgopherinvariable = iota //@symbol("randomgopherinvariable", "randomgopherinvariable", "Constant", "", "a.randomgopherinvariable")
+ randomgopherinvariable = iota
)
diff --git a/gopls/internal/lsp/testdata/workspacesymbol/a/a.go.golden b/gopls/internal/lsp/testdata/workspacesymbol/a/a.go.golden
deleted file mode 100644
index c3f0885..0000000
--- a/gopls/internal/lsp/testdata/workspacesymbol/a/a.go.golden
+++ /dev/null
@@ -1,5 +0,0 @@
--- symbols --
-RandomGopherVariableA Variable 3:5-3:26
-RandomGopherConstantA Constant 5:7-5:28
-randomgopherinvariable Constant 8:2-8:24
-
diff --git a/gopls/internal/lsp/testdata/workspacesymbol/a/a_test.go b/gopls/internal/lsp/testdata/workspacesymbol/a/a_test.go
index 30d5340..0d97c50 100644
--- a/gopls/internal/lsp/testdata/workspacesymbol/a/a_test.go
+++ b/gopls/internal/lsp/testdata/workspacesymbol/a/a_test.go
@@ -1,3 +1,3 @@
package a
-var RandomGopherTestVariableA = "a" //@symbol("RandomGopherTestVariableA", "RandomGopherTestVariableA", "Variable", "", "a.RandomGopherTestVariableA")
+var RandomGopherTestVariableA = "a"
diff --git a/gopls/internal/lsp/testdata/workspacesymbol/a/a_test.go.golden b/gopls/internal/lsp/testdata/workspacesymbol/a/a_test.go.golden
deleted file mode 100644
index af74619..0000000
--- a/gopls/internal/lsp/testdata/workspacesymbol/a/a_test.go.golden
+++ /dev/null
@@ -1,3 +0,0 @@
--- symbols --
-RandomGopherTestVariableA Variable 3:5-3:30
-
diff --git a/gopls/internal/lsp/testdata/workspacesymbol/a/a_x_test.go b/gopls/internal/lsp/testdata/workspacesymbol/a/a_x_test.go
index 76eb848..747cd17 100644
--- a/gopls/internal/lsp/testdata/workspacesymbol/a/a_x_test.go
+++ b/gopls/internal/lsp/testdata/workspacesymbol/a/a_x_test.go
@@ -1,3 +1,3 @@
package a_test
-var RandomGopherXTestVariableA = "a" //@symbol("RandomGopherXTestVariableA", "RandomGopherXTestVariableA", "Variable", "", "a_test.RandomGopherXTestVariableA")
+var RandomGopherXTestVariableA = "a"
diff --git a/gopls/internal/lsp/testdata/workspacesymbol/a/a_x_test.go.golden b/gopls/internal/lsp/testdata/workspacesymbol/a/a_x_test.go.golden
deleted file mode 100644
index dfd02a5..0000000
--- a/gopls/internal/lsp/testdata/workspacesymbol/a/a_x_test.go.golden
+++ /dev/null
@@ -1,3 +0,0 @@
--- symbols --
-RandomGopherXTestVariableA Variable 3:5-3:31
-
diff --git a/gopls/internal/lsp/testdata/workspacesymbol/b/b.go b/gopls/internal/lsp/testdata/workspacesymbol/b/b.go
index 89ce0d9..b2e2092 100644
--- a/gopls/internal/lsp/testdata/workspacesymbol/b/b.go
+++ b/gopls/internal/lsp/testdata/workspacesymbol/b/b.go
@@ -1,7 +1,7 @@
package b
-var RandomGopherVariableB = "b" //@symbol("RandomGopherVariableB", "RandomGopherVariableB", "Variable", "", "b.RandomGopherVariableB")
+var RandomGopherVariableB = "b"
-type RandomGopherStructB struct { //@symbol("RandomGopherStructB", "RandomGopherStructB", "Struct", "", "b.RandomGopherStructB")
- Bar int //@mark(bBar, "Bar"), symbol("Bar", "Bar", "Field", "RandomGopherStructB", "b.RandomGopherStructB.Bar")
+type RandomGopherStructB struct {
+ Bar int
}
diff --git a/gopls/internal/lsp/testdata/workspacesymbol/b/b.go.golden b/gopls/internal/lsp/testdata/workspacesymbol/b/b.go.golden
deleted file mode 100644
index 4711c9d..0000000
--- a/gopls/internal/lsp/testdata/workspacesymbol/b/b.go.golden
+++ /dev/null
@@ -1,5 +0,0 @@
--- symbols --
-RandomGopherVariableB Variable 3:5-3:26
-RandomGopherStructB Struct 5:6-5:25
- Bar Field 6:2-6:5
-
diff --git a/gopls/internal/lsp/tests/tests.go b/gopls/internal/lsp/tests/tests.go
index cfce843..f953a8f 100644
--- a/gopls/internal/lsp/tests/tests.go
+++ b/gopls/internal/lsp/tests/tests.go
@@ -86,8 +86,7 @@
type References = map[span.Span][]span.Span
type Renames = map[span.Span]string
type PrepareRenames = map[span.Span]*source.PrepareItem
-type Symbols = map[span.URI][]protocol.DocumentSymbol
-type SymbolsChildren = map[string][]protocol.DocumentSymbol
+type Symbols = map[span.URI][]*symbol
type SymbolInformation = map[span.Span]protocol.SymbolInformation
type InlayHints = []span.Span
type WorkspaceSymbols = map[WorkspaceSymbolsTestType]map[span.URI][]string
@@ -125,8 +124,6 @@
InlayHints InlayHints
PrepareRenames PrepareRenames
Symbols Symbols
- symbolsChildren SymbolsChildren
- symbolInformation SymbolInformation
WorkspaceSymbols WorkspaceSymbols
Signatures Signatures
Links Links
@@ -250,6 +247,12 @@
ActionKind, Title string
}
+// A symbol holds a DocumentSymbol along with its parent-child edge.
+type symbol struct {
+ pSymbol protocol.DocumentSymbol
+ id, parentID string
+}
+
type Golden struct {
Filename string
Archive *txtar.Archive
@@ -328,8 +331,6 @@
FunctionExtractions: make(FunctionExtractions),
MethodExtractions: make(MethodExtractions),
Symbols: make(Symbols),
- symbolsChildren: make(SymbolsChildren),
- symbolInformation: make(SymbolInformation),
WorkspaceSymbols: make(WorkspaceSymbols),
Signatures: make(Signatures),
Links: make(Links),
@@ -504,12 +505,7 @@
}); err != nil {
t.Fatal(err)
}
- for _, symbols := range datum.Symbols {
- for i := range symbols {
- children := datum.symbolsChildren[symbols[i].Name]
- symbols[i].Children = children
- }
- }
+
// Collect names for the entries that require golden files.
if err := datum.Exported.Expect(map[string]interface{}{
"godef": datum.collectDefinitionNames,
@@ -847,10 +843,44 @@
t.Run("Symbols", func(t *testing.T) {
t.Helper()
- for uri, expectedSymbols := range data.Symbols {
+ for uri, allSymbols := range data.Symbols {
+ byParent := make(map[string][]*symbol)
+ for _, sym := range allSymbols {
+ if sym.parentID != "" {
+ byParent[sym.parentID] = append(byParent[sym.parentID], sym)
+ }
+ }
+
+ // collectChildren does a depth-first traversal of the symbol tree,
+ // computing children of child nodes before returning to their parent.
+ // This is necessary as the Children field is slice of non-pointer types,
+ // and therefore we need to be careful to mutate children first before
+ // assigning them to their parent.
+ var collectChildren func(id string) []protocol.DocumentSymbol
+ collectChildren = func(id string) []protocol.DocumentSymbol {
+ children := byParent[id]
+ // delete from byParent before recursing, to ensure that
+ // collectChildren terminates even in the presence of cycles.
+ delete(byParent, id)
+ var result []protocol.DocumentSymbol
+ for _, child := range children {
+ child.pSymbol.Children = collectChildren(child.id)
+ result = append(result, child.pSymbol)
+ }
+ return result
+ }
+
+ var topLevel []protocol.DocumentSymbol
+ for _, sym := range allSymbols {
+ if sym.parentID == "" {
+ sym.pSymbol.Children = collectChildren(sym.id)
+ topLevel = append(topLevel, sym.pSymbol)
+ }
+ }
+
t.Run(uriName(uri), func(t *testing.T) {
t.Helper()
- tests.Symbols(t, uri, expectedSymbols)
+ tests.Symbols(t, uri, topLevel)
})
}
})
@@ -1356,36 +1386,32 @@
}
// collectSymbols is responsible for collecting @symbol annotations.
-func (data *Data) collectSymbols(name string, spn span.Span, kind string, parentName string, siName string) {
+func (data *Data) collectSymbols(name string, selectionRng span.Span, kind, detail, id, parentID string) {
+ // We don't set 'Range' here as it is difficult (impossible?) to express
+ // multi-line ranges in the packagestest framework.
+ uri := selectionRng.URI()
+ data.Symbols[uri] = append(data.Symbols[uri], &symbol{
+ pSymbol: protocol.DocumentSymbol{
+ Name: name,
+ Kind: protocol.ParseSymbolKind(kind),
+ SelectionRange: data.mustRange(selectionRng),
+ Detail: detail,
+ },
+ id: id,
+ parentID: parentID,
+ })
+}
+
+// mustRange converts spn into a protocol.Range, calling t.Fatal on any error.
+func (data *Data) mustRange(spn span.Span) protocol.Range {
m, err := data.Mapper(spn.URI())
- if err != nil {
- data.t.Fatal(err)
- }
rng, err := m.Range(spn)
if err != nil {
+ // TODO(rfindley): this can probably just be a panic, at which point we
+ // don't need to close over t.
data.t.Fatal(err)
}
- sym := protocol.DocumentSymbol{
- Name: name,
- Kind: protocol.ParseSymbolKind(kind),
- SelectionRange: rng,
- }
- if parentName == "" {
- data.Symbols[spn.URI()] = append(data.Symbols[spn.URI()], sym)
- } else {
- data.symbolsChildren[parentName] = append(data.symbolsChildren[parentName], sym)
- }
-
- // Reuse @symbol in the workspace symbols tests.
- si := protocol.SymbolInformation{
- Name: siName,
- Kind: sym.Kind,
- Location: protocol.Location{
- URI: protocol.URIFromSpanURI(spn.URI()),
- Range: sym.SelectionRange,
- },
- }
- data.symbolInformation[spn] = si
+ return rng
}
func (data *Data) collectWorkspaceSymbols(typ WorkspaceSymbolsTestType) func(*expect.Note, string) {
diff --git a/gopls/internal/lsp/tests/util.go b/gopls/internal/lsp/tests/util.go
index d697f18..8a08b04 100644
--- a/gopls/internal/lsp/tests/util.go
+++ b/gopls/internal/lsp/tests/util.go
@@ -14,13 +14,12 @@
"sort"
"strconv"
"strings"
- "testing"
- "golang.org/x/tools/internal/diff"
- "golang.org/x/tools/internal/diff/myers"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/lsp/source/completion"
+ "golang.org/x/tools/internal/diff"
+ "golang.org/x/tools/internal/diff/myers"
"golang.org/x/tools/internal/span"
)
@@ -66,50 +65,6 @@
return ""
}
-// DiffSymbols prints the diff between expected and actual symbols test results.
-func DiffSymbols(t *testing.T, uri span.URI, want, got []protocol.DocumentSymbol) string {
- sort.Slice(want, func(i, j int) bool { return want[i].Name < want[j].Name })
- sort.Slice(got, func(i, j int) bool { return got[i].Name < got[j].Name })
- if len(got) != len(want) {
- return summarizeSymbols(-1, want, got, "different lengths got %v want %v", len(got), len(want))
- }
- for i, w := range want {
- g := got[i]
- if w.Name != g.Name {
- return summarizeSymbols(i, want, got, "incorrect name got %v want %v", g.Name, w.Name)
- }
- if w.Kind != g.Kind {
- return summarizeSymbols(i, want, got, "incorrect kind got %v want %v", g.Kind, w.Kind)
- }
- if protocol.CompareRange(w.SelectionRange, g.SelectionRange) != 0 {
- return summarizeSymbols(i, want, got, "incorrect span got %v want %v", g.SelectionRange, w.SelectionRange)
- }
- if msg := DiffSymbols(t, uri, w.Children, g.Children); msg != "" {
- return fmt.Sprintf("children of %s: %s", w.Name, msg)
- }
- }
- return ""
-}
-
-func summarizeSymbols(i int, want, got []protocol.DocumentSymbol, reason string, args ...interface{}) string {
- msg := &bytes.Buffer{}
- fmt.Fprint(msg, "document symbols failed")
- if i >= 0 {
- fmt.Fprintf(msg, " at %d", i)
- }
- fmt.Fprint(msg, " because of ")
- fmt.Fprintf(msg, reason, args...)
- fmt.Fprint(msg, ":\nexpected:\n")
- for _, s := range want {
- fmt.Fprintf(msg, " %v %v %v\n", s.Name, s.Kind, s.SelectionRange)
- }
- fmt.Fprintf(msg, "got:\n")
- for _, s := range got {
- fmt.Fprintf(msg, " %v %v %v\n", s.Name, s.Kind, s.SelectionRange)
- }
- return msg.String()
-}
-
// DiffDiagnostics prints the diff between expected and actual diagnostics test
// results. If the sole expectation is "no_diagnostics", the check is suppressed.
// The Message field of each want element must be a regular expression.
To view, visit change 405254. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages