From 7042ee646e79c720e7af67701faf6462ce0e0938 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Wed, 15 Jan 2020 19:55:03 -0500 Subject: [PATCH] internal/lsp/source: always look up mapper when building ranges Any file could have //line directives in it, which means that we should never trust a mapper that was looked up for a whole file. Remove the range conversion helpers that accepted a mapper and look it up on the spot. Change-Id: Ic518891fcc1a682b31cbc6d1d4e1e1af1ef21962 Reviewed-on: https://go-review.googlesource.com/c/tools/+/214949 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/source/highlight.go | 26 +++++++++++----------- internal/lsp/source/symbols.go | 38 ++++++++++++++++---------------- internal/lsp/source/util.go | 14 +++--------- 3 files changed, 35 insertions(+), 43 deletions(-) diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go index af9c6dab0b..b453157c32 100644 --- a/internal/lsp/source/highlight.go +++ b/internal/lsp/source/highlight.go @@ -55,17 +55,17 @@ func Highlight(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protoc switch path[0].(type) { case *ast.ReturnStmt, *ast.FuncDecl, *ast.FuncType, *ast.BasicLit: - return highlightFuncControlFlow(ctx, snapshot, m, path) + return highlightFuncControlFlow(ctx, snapshot, pkg, path) case *ast.Ident: - return highlightIdentifiers(ctx, snapshot, m, path, pkg) + return highlightIdentifiers(ctx, snapshot, pkg, path) case *ast.BranchStmt, *ast.ForStmt, *ast.RangeStmt: - return highlightLoopControlFlow(ctx, snapshot, m, path) + return highlightLoopControlFlow(ctx, snapshot, pkg, path) } // If the cursor is in an unidentified area, return empty results. return nil, nil } -func highlightFuncControlFlow(ctx context.Context, snapshot Snapshot, m *protocol.ColumnMapper, path []ast.Node) ([]protocol.Range, error) { +func highlightFuncControlFlow(ctx context.Context, snapshot Snapshot, pkg Package, path []ast.Node) ([]protocol.Range, error) { var enclosingFunc ast.Node var returnStmt *ast.ReturnStmt var resultsList *ast.FieldList @@ -137,7 +137,7 @@ Outer: result := make(map[protocol.Range]bool) // Highlight the correct argument in the function declaration return types. if resultsList != nil && -1 < index && index < len(resultsList.List) { - rng, err := nodeToProtocolRange(snapshot.View(), m, resultsList.List[index]) + rng, err := nodeToProtocolRange(snapshot.View(), pkg, resultsList.List[index]) if err != nil { log.Error(ctx, "Error getting range for node", err) } else { @@ -146,7 +146,7 @@ Outer: } // Add the "func" part of the func declaration. if highlightAllReturnsAndFunc { - funcStmt, err := posToRange(snapshot.View(), m, enclosingFunc.Pos(), enclosingFunc.Pos()+token.Pos(len("func"))) + funcStmt, err := posToMappedRange(snapshot.View(), pkg, enclosingFunc.Pos(), enclosingFunc.Pos()+token.Pos(len("func"))) if err != nil { return nil, err } @@ -174,7 +174,7 @@ Outer: toAdd = n.Results[index] } if toAdd != nil { - rng, err := nodeToProtocolRange(snapshot.View(), m, toAdd) + rng, err := nodeToProtocolRange(snapshot.View(), pkg, toAdd) if err != nil { log.Error(ctx, "Error getting range for node", err) } else { @@ -188,7 +188,7 @@ Outer: return rangeMapToSlice(result), nil } -func highlightLoopControlFlow(ctx context.Context, snapshot Snapshot, m *protocol.ColumnMapper, path []ast.Node) ([]protocol.Range, error) { +func highlightLoopControlFlow(ctx context.Context, snapshot Snapshot, pkg Package, path []ast.Node) ([]protocol.Range, error) { var loop ast.Node Outer: // Reverse walk the path till we get to the for loop. @@ -205,7 +205,7 @@ Outer: } result := make(map[protocol.Range]bool) // Add the for statement. - forStmt, err := posToRange(snapshot.View(), m, loop.Pos(), loop.Pos()+token.Pos(len("for"))) + forStmt, err := posToMappedRange(snapshot.View(), pkg, loop.Pos(), loop.Pos()+token.Pos(len("for"))) if err != nil { return nil, err } @@ -223,7 +223,7 @@ Outer: } // Add all branch statements in same scope as the identified one. if n, ok := n.(*ast.BranchStmt); ok { - rng, err := nodeToProtocolRange(snapshot.View(), m, n) + rng, err := nodeToProtocolRange(snapshot.View(), pkg, n) if err != nil { log.Error(ctx, "Error getting range for node", err) return false @@ -235,14 +235,14 @@ Outer: return rangeMapToSlice(result), nil } -func highlightIdentifiers(ctx context.Context, snapshot Snapshot, m *protocol.ColumnMapper, path []ast.Node, pkg Package) ([]protocol.Range, error) { +func highlightIdentifiers(ctx context.Context, snapshot Snapshot, pkg Package, path []ast.Node) ([]protocol.Range, error) { result := make(map[protocol.Range]bool) id, ok := path[0].(*ast.Ident) if !ok { return nil, errors.Errorf("highlightIdentifiers called with an ast.Node of type %T", id) } // Check if ident is inside return or func decl. - if toAdd, err := highlightFuncControlFlow(ctx, snapshot, m, path); toAdd != nil && err == nil { + if toAdd, err := highlightFuncControlFlow(ctx, snapshot, pkg, path); toAdd != nil && err == nil { for _, r := range toAdd { result[r] = true } @@ -262,7 +262,7 @@ func highlightIdentifiers(ctx context.Context, snapshot Snapshot, m *protocol.Co if nObj := pkg.GetTypesInfo().ObjectOf(n); nObj != idObj { return false } - if rng, err := nodeToProtocolRange(snapshot.View(), m, n); err == nil { + if rng, err := nodeToProtocolRange(snapshot.View(), pkg, n); err == nil { result[rng] = true } else { log.Error(ctx, "Error getting range for node", err) diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go index 0627d03470..989492cc73 100644 --- a/internal/lsp/source/symbols.go +++ b/internal/lsp/source/symbols.go @@ -22,7 +22,7 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p if err != nil { return nil, fmt.Errorf("getting file for DocumentSymbols: %v", err) } - file, m, _, err := pgh.Cached() + file, _, _, err := pgh.Cached() if err != nil { return nil, err } @@ -37,7 +37,7 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p switch decl := decl.(type) { case *ast.FuncDecl: if obj := info.ObjectOf(decl.Name); obj != nil { - fs, err := funcSymbol(ctx, snapshot.View(), m, decl, obj, q) + fs, err := funcSymbol(ctx, snapshot.View(), pkg, decl, obj, q) if err != nil { return nil, err } @@ -55,7 +55,7 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p switch spec := spec.(type) { case *ast.TypeSpec: if obj := info.ObjectOf(spec.Name); obj != nil { - ts, err := typeSymbol(ctx, snapshot.View(), m, info, spec, obj, q) + ts, err := typeSymbol(ctx, snapshot.View(), pkg, info, spec, obj, q) if err != nil { return nil, err } @@ -65,7 +65,7 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p case *ast.ValueSpec: for _, name := range spec.Names { if obj := info.ObjectOf(name); obj != nil { - vs, err := varSymbol(ctx, snapshot.View(), m, decl, name, obj, q) + vs, err := varSymbol(ctx, snapshot.View(), pkg, decl, name, obj, q) if err != nil { return nil, err } @@ -93,17 +93,17 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p return symbols, nil } -func funcSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, decl *ast.FuncDecl, obj types.Object, q types.Qualifier) (protocol.DocumentSymbol, error) { +func funcSymbol(ctx context.Context, view View, pkg Package, decl *ast.FuncDecl, obj types.Object, q types.Qualifier) (protocol.DocumentSymbol, error) { s := protocol.DocumentSymbol{ Name: obj.Name(), Kind: protocol.Function, } var err error - s.Range, err = nodeToProtocolRange(view, m, decl) + s.Range, err = nodeToProtocolRange(view, pkg, decl) if err != nil { return protocol.DocumentSymbol{}, err } - s.SelectionRange, err = nodeToProtocolRange(view, m, decl.Name) + s.SelectionRange, err = nodeToProtocolRange(view, pkg, decl.Name) if err != nil { return protocol.DocumentSymbol{}, err } @@ -157,7 +157,7 @@ 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, error) { +func typeSymbol(ctx context.Context, view View, pkg Package, info *types.Info, spec *ast.TypeSpec, obj types.Object, q types.Qualifier) (protocol.DocumentSymbol, error) { s := protocol.DocumentSymbol{ Name: obj.Name(), } @@ -165,11 +165,11 @@ func typeSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, info * setKind(&s, obj.Type(), q) var err error - s.Range, err = nodeToProtocolRange(view, m, spec) + s.Range, err = nodeToProtocolRange(view, pkg, spec) if err != nil { return protocol.DocumentSymbol{}, err } - s.SelectionRange, err = nodeToProtocolRange(view, m, spec.Name) + s.SelectionRange, err = nodeToProtocolRange(view, pkg, spec.Name) if err != nil { return protocol.DocumentSymbol{}, err } @@ -185,10 +185,10 @@ func typeSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, info * child.Detail, _ = formatType(f.Type(), q) spanNode, selectionNode := nodesForStructField(i, st) - if span, err := nodeToProtocolRange(view, m, spanNode); err == nil { + if span, err := nodeToProtocolRange(view, pkg, spanNode); err == nil { child.Range = span } - if span, err := nodeToProtocolRange(view, m, selectionNode); err == nil { + if span, err := nodeToProtocolRange(view, pkg, selectionNode); err == nil { child.SelectionRange = span } s.Children = append(s.Children, child) @@ -215,11 +215,11 @@ func typeSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, info * } } } - child.Range, err = nodeToProtocolRange(view, m, spanNode) + child.Range, err = nodeToProtocolRange(view, pkg, spanNode) if err != nil { return protocol.DocumentSymbol{}, err } - child.SelectionRange, err = nodeToProtocolRange(view, m, selectionNode) + child.SelectionRange, err = nodeToProtocolRange(view, pkg, selectionNode) if err != nil { return protocol.DocumentSymbol{}, err } @@ -249,11 +249,11 @@ func typeSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, info * break Embeddeds } } - child.Range, err = nodeToProtocolRange(view, m, spanNode) + child.Range, err = nodeToProtocolRange(view, pkg, spanNode) if err != nil { return protocol.DocumentSymbol{}, err } - child.SelectionRange, err = nodeToProtocolRange(view, m, selectionNode) + child.SelectionRange, err = nodeToProtocolRange(view, pkg, selectionNode) if err != nil { return protocol.DocumentSymbol{}, err } @@ -283,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, error) { +func varSymbol(ctx context.Context, view View, pkg Package, decl ast.Node, name *ast.Ident, obj types.Object, q types.Qualifier) (protocol.DocumentSymbol, error) { s := protocol.DocumentSymbol{ Name: obj.Name(), Kind: protocol.Variable, @@ -292,11 +292,11 @@ func varSymbol(ctx context.Context, view View, m *protocol.ColumnMapper, decl as s.Kind = protocol.Constant } var err error - s.Range, err = nodeToProtocolRange(view, m, decl) + s.Range, err = nodeToProtocolRange(view, pkg, decl) if err != nil { return protocol.DocumentSymbol{}, err } - s.SelectionRange, err = nodeToProtocolRange(view, m, name) + s.SelectionRange, err = nodeToProtocolRange(view, pkg, name) if err != nil { return protocol.DocumentSymbol{}, err } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 7039fb899f..4c9854a826 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -168,8 +168,8 @@ func IsGenerated(ctx context.Context, snapshot Snapshot, uri span.URI) bool { return false } -func nodeToProtocolRange(view View, m *protocol.ColumnMapper, n ast.Node) (protocol.Range, error) { - mrng, err := nodeToMappedRange(view, m, n) +func nodeToProtocolRange(view View, pkg Package, n ast.Node) (protocol.Range, error) { + mrng, err := posToMappedRange(view, pkg, n.Pos(), n.End()) if err != nil { return protocol.Range{}, err } @@ -199,27 +199,19 @@ func nameToMappedRange(v View, pkg Package, pos token.Pos, name string) (mappedR return posToMappedRange(v, pkg, pos, pos+token.Pos(len(name))) } -func nodeToMappedRange(view View, m *protocol.ColumnMapper, n ast.Node) (mappedRange, error) { - return posToRange(view, m, n.Pos(), n.End()) -} - func posToMappedRange(v View, pkg Package, pos, end token.Pos) (mappedRange, error) { logicalFilename := v.Session().Cache().FileSet().File(pos).Position(pos).Filename m, err := findMapperInPackage(v, pkg, span.FileURI(logicalFilename)) if err != nil { return mappedRange{}, err } - return posToRange(v, m, pos, end) -} - -func posToRange(view View, m *protocol.ColumnMapper, pos, end token.Pos) (mappedRange, error) { if !pos.IsValid() { return mappedRange{}, errors.Errorf("invalid position for %v", pos) } if !end.IsValid() { return mappedRange{}, errors.Errorf("invalid position for %v", end) } - return newMappedRange(view.Session().Cache().FileSet(), m, pos, end), nil + return newMappedRange(v.Session().Cache().FileSet(), m, pos, end), nil } // Matches cgo generated comment as well as the proposed standard: