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
microsoft/vscode-go#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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>

Author: stamblerre

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,20 +283,23 @@ 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,
 	}
 	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
 }

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
 }