From 7a2a8a0471126cd16344d2af78aed517c2c2e076 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Wed, 4 Dec 2019 13:18:11 -0500 Subject: [PATCH] internal/lsp: propagate errors through source.DocumentSymbols We had previously been ignoring many errors in textDocument/documentSymbols, which led to errors appearing in the VS Code extension logs (see https://github.com/microsoft/vscode-go/issues/2919 for more context). We should return errors so that we can more easily debug these issues in gopls directly. Change-Id: Ieef7c9f0bc8296f7e12d8c84e60d8b978d311651 Reviewed-on: https://go-review.googlesource.com/c/tools/+/209858 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/source/symbols.go | 86 ++++++++++++++++++++++------------ internal/lsp/symbols.go | 9 +++- 2 files changed, 63 insertions(+), 32 deletions(-) diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go index 8acbdd4d46..a2fc1bedb0 100644 --- a/internal/lsp/source/symbols.go +++ b/internal/lsp/source/symbols.go @@ -37,9 +37,13 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, f File) ([]protocol switch decl := decl.(type) { case *ast.FuncDecl: if obj := info.ObjectOf(decl.Name); obj != nil { - if fs := funcSymbol(ctx, snapshot.View(), m, decl, obj, q); fs.Kind == protocol.Method { - // Store methods separately, as we want them to appear as children - // of the corresponding type (which we may not have seen yet). + fs, err := funcSymbol(ctx, snapshot.View(), m, decl, obj, q) + if err != nil { + return nil, err + } + // Store methods separately, as we want them to appear as children + // of the corresponding type (which we may not have seen yet). + if fs.Kind == protocol.Method { rtype := obj.Type().(*types.Signature).Recv().Type() methodsToReceiver[rtype] = append(methodsToReceiver[rtype], fs) } else { @@ -51,14 +55,21 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, f File) ([]protocol switch spec := spec.(type) { case *ast.TypeSpec: if obj := info.ObjectOf(spec.Name); obj != nil { - ts := typeSymbol(ctx, snapshot.View(), m, info, spec, obj, q) + ts, err := typeSymbol(ctx, snapshot.View(), m, info, spec, obj, q) + if err != nil { + return nil, err + } symbols = append(symbols, ts) symbolsToReceiver[obj.Type()] = len(symbols) - 1 } case *ast.ValueSpec: for _, name := range spec.Names { if obj := info.ObjectOf(name); obj != nil { - symbols = append(symbols, varSymbol(ctx, snapshot.View(), m, decl, name, obj, q)) + vs, err := varSymbol(ctx, snapshot.View(), m, decl, name, obj, q) + if err != nil { + return nil, err + } + symbols = append(symbols, vs) } } } @@ -82,16 +93,19 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, f File) ([]protocol return symbols, nil } -func funcSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, decl *ast.FuncDecl, obj types.Object, q types.Qualifier) protocol.DocumentSymbol { +func funcSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, decl *ast.FuncDecl, obj types.Object, q types.Qualifier) (protocol.DocumentSymbol, error) { s := protocol.DocumentSymbol{ Name: obj.Name(), Kind: protocol.Function, } - if span, err := nodeToProtocolRange(ctx, view, m, decl); err == nil { - s.Range = span + var err error + s.Range, err = nodeToProtocolRange(ctx, view, m, decl) + if err != nil { + return protocol.DocumentSymbol{}, err } - if span, err := nodeToProtocolRange(ctx, view, m, decl.Name); err == nil { - s.SelectionRange = span + s.SelectionRange, err = nodeToProtocolRange(ctx, view, m, decl.Name) + if err != nil { + return protocol.DocumentSymbol{}, err } sig, _ := obj.Type().(*types.Signature) if sig != nil { @@ -112,7 +126,7 @@ func funcSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, decl * } s.Detail += ")" } - return s + return s, nil } func setKind(s *protocol.DocumentSymbol, typ types.Type, q types.Qualifier) { @@ -143,18 +157,21 @@ func setKind(s *protocol.DocumentSymbol, typ types.Type, q types.Qualifier) { } } -func typeSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, info *types.Info, spec *ast.TypeSpec, obj types.Object, q types.Qualifier) protocol.DocumentSymbol { +func typeSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, info *types.Info, spec *ast.TypeSpec, obj types.Object, q types.Qualifier) (protocol.DocumentSymbol, error) { s := protocol.DocumentSymbol{ Name: obj.Name(), } s.Detail, _ = formatType(obj.Type(), q) setKind(&s, obj.Type(), q) - if span, err := nodeToProtocolRange(ctx, view, m, spec); err == nil { - s.Range = span + var err error + s.Range, err = nodeToProtocolRange(ctx, view, m, spec) + if err != nil { + return protocol.DocumentSymbol{}, err } - if span, err := nodeToProtocolRange(ctx, view, m, spec.Name); err == nil { - s.SelectionRange = span + s.SelectionRange, err = nodeToProtocolRange(ctx, view, m, spec.Name) + if err != nil { + return protocol.DocumentSymbol{}, err } t, objIsStruct := obj.Type().Underlying().(*types.Struct) st, specIsStruct := spec.Type.(*ast.StructType) @@ -198,11 +215,13 @@ func typeSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, info * } } } - if span, err := nodeToProtocolRange(ctx, view, m, spanNode); err == nil { - child.Range = span + child.Range, err = nodeToProtocolRange(ctx, view, m, spanNode) + if err != nil { + return protocol.DocumentSymbol{}, err } - if span, err := nodeToProtocolRange(ctx, view, m, selectionNode); err == nil { - child.SelectionRange = span + child.SelectionRange, err = nodeToProtocolRange(ctx, view, m, selectionNode) + if err != nil { + return protocol.DocumentSymbol{}, err } s.Children = append(s.Children, child) } @@ -230,16 +249,18 @@ func typeSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, info * break Embeddeds } } - if rng, err := nodeToProtocolRange(ctx, view, m, spanNode); err == nil { - child.Range = rng + child.Range, err = nodeToProtocolRange(ctx, view, m, spanNode) + if err != nil { + return protocol.DocumentSymbol{}, err } - if span, err := nodeToProtocolRange(ctx, view, m, selectionNode); err == nil { - child.SelectionRange = span + child.SelectionRange, err = nodeToProtocolRange(ctx, view, m, selectionNode) + if err != nil { + return protocol.DocumentSymbol{}, err } s.Children = append(s.Children, child) } } - return s + return s, nil } func nodesForStructField(i int, st *ast.StructType) (span, selection ast.Node) { @@ -262,7 +283,7 @@ func nodesForStructField(i int, st *ast.StructType) (span, selection ast.Node) { return nil, nil } -func varSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, decl ast.Node, name *ast.Ident, obj types.Object, q types.Qualifier) protocol.DocumentSymbol { +func varSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, decl ast.Node, name *ast.Ident, obj types.Object, q types.Qualifier) (protocol.DocumentSymbol, error) { s := protocol.DocumentSymbol{ Name: obj.Name(), Kind: protocol.Variable, @@ -270,12 +291,15 @@ func varSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, decl as if _, ok := obj.(*types.Const); ok { s.Kind = protocol.Constant } - if rng, err := nodeToProtocolRange(ctx, view, m, decl); err == nil { - s.Range = rng + var err error + s.Range, err = nodeToProtocolRange(ctx, view, m, decl) + if err != nil { + return protocol.DocumentSymbol{}, err } - if span, err := nodeToProtocolRange(ctx, view, m, name); err == nil { - s.SelectionRange = span + s.SelectionRange, err = nodeToProtocolRange(ctx, view, m, name) + if err != nil { + return protocol.DocumentSymbol{}, err } s.Detail = types.TypeString(obj.Type(), q) - return s + return s, nil } diff --git a/internal/lsp/symbols.go b/internal/lsp/symbols.go index 033c6ffbb2..00b441c92b 100644 --- a/internal/lsp/symbols.go +++ b/internal/lsp/symbols.go @@ -9,7 +9,9 @@ import ( "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/lsp/telemetry" "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/telemetry/log" "golang.org/x/tools/internal/telemetry/trace" ) @@ -27,5 +29,10 @@ func (s *Server) documentSymbol(ctx context.Context, params *protocol.DocumentSy if err != nil { return nil, err } - return source.DocumentSymbols(ctx, snapshot, f) + symbols, err := source.DocumentSymbols(ctx, snapshot, f) + if err != nil { + log.Error(ctx, "DocumentSymbols failed", err, telemetry.URI.Of(uri)) + return []protocol.DocumentSymbol{}, nil + } + return symbols, nil }