diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index 6ef2e7984f..ea67807f78 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -142,7 +142,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns } fileContents[file] = contents } - spn, err := span.NewRange(act.Pass.Fset, edit.Pos, edit.End).Span() + spn, err := span.NewRange(file, edit.Pos, edit.End).Span() if err != nil { t.Errorf("error converting edit to span %s: %v", file.Name(), err) } diff --git a/go/analysis/passes/tests/tests.go b/go/analysis/passes/tests/tests.go index 56b20ebd51..cab2fa20fa 100644 --- a/go/analysis/passes/tests/tests.go +++ b/go/analysis/passes/tests/tests.go @@ -475,10 +475,12 @@ func checkTest(pass *analysis.Pass, fn *ast.FuncDecl, prefix string) { if tparams := typeparams.ForFuncType(fn.Type); tparams != nil && len(tparams.List) > 0 { // Note: cmd/go/internal/load also errors about TestXXX and BenchmarkXXX functions with type parameters. // We have currently decided to also warn before compilation/package loading. This can help users in IDEs. + // TODO(adonovan): use ReportRangef(tparams). pass.Reportf(fn.Pos(), "%s has type parameters: it will not be run by go test as a %sXXX function", fn.Name.Name, prefix) } if !isTestSuffix(fn.Name.Name[len(prefix):]) { + // TODO(adonovan): use ReportRangef(fn.Name). pass.Reportf(fn.Pos(), "%s has malformed name: first letter after '%s' must not be lowercase", fn.Name.Name, prefix) } } diff --git a/go/packages/packagestest/expect.go b/go/packages/packagestest/expect.go index 430258681f..841099c0cd 100644 --- a/go/packages/packagestest/expect.go +++ b/go/packages/packagestest/expect.go @@ -409,6 +409,7 @@ func (e *Exported) buildConverter(pt reflect.Type) (converter, error) { } func (e *Exported) rangeConverter(n *expect.Note, args []interface{}) (span.Range, []interface{}, error) { + tokFile := e.ExpectFileSet.File(n.Pos) if len(args) < 1 { return span.Range{}, nil, fmt.Errorf("missing argument") } @@ -419,10 +420,9 @@ func (e *Exported) rangeConverter(n *expect.Note, args []interface{}) (span.Rang // handle the special identifiers switch arg { case eofIdentifier: - // end of file identifier, look up the current file - f := e.ExpectFileSet.File(n.Pos) - eof := f.Pos(f.Size()) - return span.NewRange(e.ExpectFileSet, eof, token.NoPos), args, nil + // end of file identifier + eof := tokFile.Pos(tokFile.Size()) + return span.NewRange(tokFile, eof, eof), args, nil default: // look up an marker by name mark, ok := e.markers[string(arg)] @@ -436,19 +436,19 @@ func (e *Exported) rangeConverter(n *expect.Note, args []interface{}) (span.Rang if err != nil { return span.Range{}, nil, err } - if start == token.NoPos { + if !start.IsValid() { return span.Range{}, nil, fmt.Errorf("%v: pattern %s did not match", e.ExpectFileSet.Position(n.Pos), arg) } - return span.NewRange(e.ExpectFileSet, start, end), args, nil + return span.NewRange(tokFile, start, end), args, nil case *regexp.Regexp: start, end, err := expect.MatchBefore(e.ExpectFileSet, e.FileContents, n.Pos, arg) if err != nil { return span.Range{}, nil, err } - if start == token.NoPos { + if !start.IsValid() { return span.Range{}, nil, fmt.Errorf("%v: pattern %s did not match", e.ExpectFileSet.Position(n.Pos), arg) } - return span.NewRange(e.ExpectFileSet, start, end), args, nil + return span.NewRange(tokFile, start, end), args, nil default: return span.Range{}, nil, fmt.Errorf("cannot convert %v to pos", arg) } diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 7ebc777d76..aae6de0eea 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -664,7 +664,7 @@ func (s *snapshot) depsErrors(ctx context.Context, pkg *pkg) ([]*source.Diagnost } for _, imp := range allImports[item] { - rng, err := source.NewMappedRange(s.FileSet(), imp.cgf.Mapper, imp.imp.Pos(), imp.imp.End()).Range() + rng, err := source.NewMappedRange(imp.cgf.Tok, imp.cgf.Mapper, imp.imp.Pos(), imp.imp.End()).Range() if err != nil { return nil, err } diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index 342f2bea5d..a1aecb35c7 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -16,6 +16,7 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/analysisinternal" + "golang.org/x/tools/internal/lsp/bug" "golang.org/x/tools/internal/lsp/command" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" @@ -82,7 +83,7 @@ func parseErrorDiagnostics(snapshot *snapshot, pkg *pkg, errList scanner.ErrorLi return nil, err } pos := pgf.Tok.Pos(e.Pos.Offset) - spn, err := span.NewRange(snapshot.FileSet(), pos, pos).Span() + spn, err := span.NewRange(pgf.Tok, pos, pos).Span() if err != nil { return nil, err } @@ -196,8 +197,15 @@ func analysisDiagnosticDiagnostics(snapshot *snapshot, pkg *pkg, a *analysis.Ana break } } - - spn, err := span.NewRange(snapshot.FileSet(), e.Pos, e.End).Span() + tokFile := snapshot.FileSet().File(e.Pos) + if tokFile == nil { + return nil, bug.Errorf("no file for position of %q diagnostic", e.Category) + } + end := e.End + if !end.IsValid() { + end = e.Pos + } + spn, err := span.NewRange(tokFile, e.Pos, end).Span() if err != nil { return nil, err } @@ -282,7 +290,11 @@ func suggestedAnalysisFixes(snapshot *snapshot, pkg *pkg, diag *analysis.Diagnos for _, fix := range diag.SuggestedFixes { edits := make(map[span.URI][]protocol.TextEdit) for _, e := range fix.TextEdits { - spn, err := span.NewRange(snapshot.FileSet(), e.Pos, e.End).Span() + tokFile := snapshot.FileSet().File(e.Pos) + if tokFile == nil { + return nil, bug.Errorf("no file for edit position") + } + spn, err := span.NewRange(tokFile, e.Pos, e.End).Span() if err != nil { return nil, err } @@ -310,7 +322,11 @@ func suggestedAnalysisFixes(snapshot *snapshot, pkg *pkg, diag *analysis.Diagnos func relatedInformation(pkg *pkg, fset *token.FileSet, diag *analysis.Diagnostic) ([]source.RelatedInformation, error) { var out []source.RelatedInformation for _, related := range diag.Related { - spn, err := span.NewRange(fset, related.Pos, related.End).Span() + tokFile := fset.File(related.Pos) + if tokFile == nil { + return nil, bug.Errorf("no file for %q diagnostic position", diag.Category) + } + spn, err := span.NewRange(tokFile, related.Pos, related.End).Span() if err != nil { return nil, err } @@ -397,7 +413,7 @@ func parseGoListImportCycleError(snapshot *snapshot, e packages.Error, pkg *pkg) // Search file imports for the import that is causing the import cycle. for _, imp := range cgf.File.Imports { if imp.Path.Value == circImp { - spn, err := span.NewRange(snapshot.FileSet(), imp.Pos(), imp.End()).Span() + spn, err := span.NewRange(cgf.Tok, imp.Pos(), imp.End()).Span() if err != nil { return msg, span.Span{}, false } diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index e4fd671987..08c88ab8e7 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -359,7 +359,7 @@ func (s *snapshot) applyCriticalErrorToFiles(ctx context.Context, msg string, fi switch s.view.FileKind(fh) { case source.Go: if pgf, err := s.ParseGo(ctx, fh, source.ParseHeader); err == nil { - pkgDecl := span.NewRange(s.FileSet(), pgf.File.Package, pgf.File.Name.End()) + pkgDecl := span.NewRange(pgf.Tok, pgf.File.Package, pgf.File.Name.End()) if spn, err := pkgDecl.Span(); err == nil { rng, _ = pgf.Mapper.Range(spn) } diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go index b81caabde5..9139465950 100644 --- a/internal/lsp/cache/mod_tidy.go +++ b/internal/lsp/cache/mod_tidy.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "go/ast" + "go/token" "io/ioutil" "os" "path/filepath" @@ -282,7 +283,7 @@ func modTidyDiagnostics(ctx context.Context, snapshot source.Snapshot, pm *sourc if !ok { return nil, fmt.Errorf("no missing module fix for %q (%q)", importPath, req.Mod.Path) } - srcErr, err := missingModuleForImport(snapshot, m, imp, req, fixes) + srcErr, err := missingModuleForImport(pgf.Tok, m, imp, req, fixes) if err != nil { return nil, err } @@ -445,11 +446,11 @@ func switchDirectness(req *modfile.Require, m *protocol.ColumnMapper, computeEdi // missingModuleForImport creates an error for a given import path that comes // from a missing module. -func missingModuleForImport(snapshot source.Snapshot, m *protocol.ColumnMapper, imp *ast.ImportSpec, req *modfile.Require, fixes []source.SuggestedFix) (*source.Diagnostic, error) { +func missingModuleForImport(file *token.File, m *protocol.ColumnMapper, imp *ast.ImportSpec, req *modfile.Require, fixes []source.SuggestedFix) (*source.Diagnostic, error) { if req.Syntax == nil { return nil, fmt.Errorf("no syntax for %v", req) } - spn, err := span.NewRange(snapshot.FileSet(), imp.Path.Pos(), imp.Path.End()).Span() + spn, err := span.NewRange(file, imp.Path.Pos(), imp.Path.End()).Span() if err != nil { return nil, err } diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index 9d78e3c9ac..4147e17ce6 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -294,7 +294,7 @@ func extractionFixes(ctx context.Context, snapshot source.Snapshot, pkg source.P } puri := protocol.URIFromSpanURI(uri) var commands []protocol.Command - if _, ok, methodOk, _ := source.CanExtractFunction(snapshot.FileSet(), srng, pgf.Src, pgf.File); ok { + if _, ok, methodOk, _ := source.CanExtractFunction(pgf.Tok, srng, pgf.Src, pgf.File); ok { cmd, err := command.NewApplyFixCommand("Extract function", command.ApplyFixArgs{ URI: puri, Fix: source.ExtractFunction, diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 9648921ef5..59fe1716b9 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -467,7 +467,7 @@ func (s *Server) checkForOrphanedFile(ctx context.Context, snapshot source.Snaps if !pgf.File.Name.Pos().IsValid() { return nil } - spn, err := span.NewRange(snapshot.FileSet(), pgf.File.Name.Pos(), pgf.File.Name.End()).Span() + spn, err := span.NewRange(pgf.Tok, pgf.File.Name.Pos(), pgf.File.Name.End()).Span() if err != nil { return nil } diff --git a/internal/lsp/link.go b/internal/lsp/link.go index 7bb09b4035..a2962b6659 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -68,7 +68,7 @@ func modLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandl // dependency within the require statement. start, end := token.Pos(s+i), token.Pos(s+i+len(dep)) target := source.BuildLink(snapshot.View().Options().LinkTarget, "mod/"+req.Mod.String(), "") - l, err := toProtocolLink(snapshot, pm.Mapper, target, start, end, source.Mod) + l, err := toProtocolLink(nil, pm.Mapper, target, start, end, source.Mod) if err != nil { return nil, err } @@ -78,6 +78,10 @@ func modLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandl if syntax := pm.File.Syntax; syntax == nil { return links, nil } + + // Create a throwaway token.File. + tokFile := token.NewFileSet().AddFile(fh.URI().Filename(), -1, len(pm.Mapper.Content)) + // Get all the links that are contained in the comments of the file. for _, expr := range pm.File.Syntax.Stmt { comments := expr.Comment() @@ -86,7 +90,8 @@ func modLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandl } for _, section := range [][]modfile.Comment{comments.Before, comments.Suffix, comments.After} { for _, comment := range section { - l, err := findLinksInString(ctx, snapshot, comment.Token, token.Pos(comment.Start.Byte), pm.Mapper, source.Mod) + start := tokFile.Pos(comment.Start.Byte) + l, err := findLinksInString(ctx, snapshot, comment.Token, start, tokFile, pm.Mapper, source.Mod) if err != nil { return nil, err } @@ -144,7 +149,7 @@ func goLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle start := imp.Path.Pos() + 1 end := imp.Path.End() - 1 target = source.BuildLink(view.Options().LinkTarget, target, "") - l, err := toProtocolLink(snapshot, pgf.Mapper, target, start, end, source.Go) + l, err := toProtocolLink(pgf.Tok, pgf.Mapper, target, start, end, source.Go) if err != nil { return nil, err } @@ -152,7 +157,7 @@ func goLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle } } for _, s := range str { - l, err := findLinksInString(ctx, snapshot, s.Value, s.Pos(), pgf.Mapper, source.Go) + l, err := findLinksInString(ctx, snapshot, s.Value, s.Pos(), pgf.Tok, pgf.Mapper, source.Go) if err != nil { return nil, err } @@ -160,7 +165,7 @@ func goLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle } for _, commentGroup := range pgf.File.Comments { for _, comment := range commentGroup.List { - l, err := findLinksInString(ctx, snapshot, comment.Text, comment.Pos(), pgf.Mapper, source.Go) + l, err := findLinksInString(ctx, snapshot, comment.Text, comment.Pos(), pgf.Tok, pgf.Mapper, source.Go) if err != nil { return nil, err } @@ -193,7 +198,8 @@ var acceptedSchemes = map[string]bool{ "https": true, } -func findLinksInString(ctx context.Context, snapshot source.Snapshot, src string, pos token.Pos, m *protocol.ColumnMapper, fileKind source.FileKind) ([]protocol.DocumentLink, error) { +// tokFile may be a throwaway File for non-Go files. +func findLinksInString(ctx context.Context, snapshot source.Snapshot, src string, pos token.Pos, tokFile *token.File, m *protocol.ColumnMapper, fileKind source.FileKind) ([]protocol.DocumentLink, error) { var links []protocol.DocumentLink for _, index := range snapshot.View().Options().URLRegexp.FindAllIndex([]byte(src), -1) { start, end := index[0], index[1] @@ -216,7 +222,7 @@ func findLinksInString(ctx context.Context, snapshot source.Snapshot, src string if !acceptedSchemes[linkURL.Scheme] { continue } - l, err := toProtocolLink(snapshot, m, linkURL.String(), startPos, endPos, fileKind) + l, err := toProtocolLink(tokFile, m, linkURL.String(), startPos, endPos, fileKind) if err != nil { return nil, err } @@ -234,7 +240,7 @@ func findLinksInString(ctx context.Context, snapshot source.Snapshot, src string } org, repo, number := matches[1], matches[2], matches[3] target := fmt.Sprintf("https://github.com/%s/%s/issues/%s", org, repo, number) - l, err := toProtocolLink(snapshot, m, target, startPos, endPos, fileKind) + l, err := toProtocolLink(tokFile, m, target, startPos, endPos, fileKind) if err != nil { return nil, err } @@ -255,11 +261,12 @@ var ( issueRegexp *regexp.Regexp ) -func toProtocolLink(snapshot source.Snapshot, m *protocol.ColumnMapper, target string, start, end token.Pos, fileKind source.FileKind) (protocol.DocumentLink, error) { +func toProtocolLink(tokFile *token.File, m *protocol.ColumnMapper, target string, start, end token.Pos, fileKind source.FileKind) (protocol.DocumentLink, error) { var rng protocol.Range switch fileKind { case source.Go: - spn, err := span.NewRange(snapshot.FileSet(), start, end).Span() + // TODO(adonovan): can we now use this logic for the Mod case too? + spn, err := span.NewRange(tokFile, start, end).Span() if err != nil { return protocol.DocumentLink{}, err } diff --git a/internal/lsp/semantic.go b/internal/lsp/semantic.go index 286d2fd160..429dc0660b 100644 --- a/internal/lsp/semantic.go +++ b/internal/lsp/semantic.go @@ -186,7 +186,7 @@ func (e *encoded) token(start token.Pos, leng int, typ tokenType, mods []string) } // want a line and column from start (in LSP coordinates) // [//line directives should be ignored] - rng := source.NewMappedRange(e.fset, e.pgf.Mapper, start, start+token.Pos(leng)) + rng := source.NewMappedRange(e.pgf.Tok, e.pgf.Mapper, start, start+token.Pos(leng)) lspRange, err := rng.Range() if err != nil { // possibly a //line directive. TODO(pjw): fix this somehow diff --git a/internal/lsp/source/call_hierarchy.go b/internal/lsp/source/call_hierarchy.go index c2c8a1866d..4e7daf0f9b 100644 --- a/internal/lsp/source/call_hierarchy.go +++ b/internal/lsp/source/call_hierarchy.go @@ -152,12 +152,12 @@ outer: kind = protocol.Function } - nameStart, nameEnd := nameIdent.NamePos, nameIdent.NamePos+token.Pos(len(nameIdent.Name)) + nameStart, nameEnd := nameIdent.Pos(), nameIdent.End() if funcLit != nil { nameStart, nameEnd = funcLit.Type.Func, funcLit.Type.Params.Pos() kind = protocol.Function } - rng, err := NewMappedRange(snapshot.FileSet(), pgf.Mapper, nameStart, nameEnd).Range() + rng, err := NewMappedRange(pgf.Tok, pgf.Mapper, nameStart, nameEnd).Range() if err != nil { return protocol.CallHierarchyItem{}, err } @@ -194,14 +194,22 @@ func OutgoingCalls(ctx context.Context, snapshot Snapshot, fh FileHandle, pos pr if _, ok := identifier.Declaration.obj.Type().Underlying().(*types.Signature); !ok { return nil, nil } - if identifier.Declaration.node == nil { + node := identifier.Declaration.node + if node == nil { return nil, nil } if len(identifier.Declaration.MappedRange) == 0 { return nil, nil } declMappedRange := identifier.Declaration.MappedRange[0] - callExprs, err := collectCallExpressions(snapshot.FileSet(), declMappedRange.m, identifier.Declaration.node) + // TODO(adonovan): avoid Fileset.File call by somehow getting at + // declMappedRange.spanRange.TokFile, or making Identifier retain the + // token.File of the identifier and its declaration, since it looks up both anyway. + tokFile := snapshot.FileSet().File(node.Pos()) + if tokFile == nil { + return nil, fmt.Errorf("no file for position") + } + callExprs, err := collectCallExpressions(tokFile, declMappedRange.m, node) if err != nil { return nil, err } @@ -210,7 +218,7 @@ func OutgoingCalls(ctx context.Context, snapshot Snapshot, fh FileHandle, pos pr } // collectCallExpressions collects call expression ranges inside a function. -func collectCallExpressions(fset *token.FileSet, mapper *protocol.ColumnMapper, node ast.Node) ([]protocol.Range, error) { +func collectCallExpressions(tokFile *token.File, mapper *protocol.ColumnMapper, node ast.Node) ([]protocol.Range, error) { type callPos struct { start, end token.Pos } @@ -240,7 +248,7 @@ func collectCallExpressions(fset *token.FileSet, mapper *protocol.ColumnMapper, callRanges := []protocol.Range{} for _, call := range callPositions { - callRange, err := NewMappedRange(fset, mapper, call.start, call.end).Range() + callRange, err := NewMappedRange(tokFile, mapper, call.start, call.end).Range() if err != nil { return nil, err } diff --git a/internal/lsp/source/code_lens.go b/internal/lsp/source/code_lens.go index 0ab857ac60..0e9453a662 100644 --- a/internal/lsp/source/code_lens.go +++ b/internal/lsp/source/code_lens.go @@ -67,7 +67,7 @@ func runTestCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]p return nil, err } // add a code lens to the top of the file which runs all benchmarks in the file - rng, err := NewMappedRange(snapshot.FileSet(), pgf.Mapper, pgf.File.Package, pgf.File.Package).Range() + rng, err := NewMappedRange(pgf.Tok, pgf.Mapper, pgf.File.Package, pgf.File.Package).Range() if err != nil { return nil, err } @@ -111,7 +111,7 @@ func TestsAndBenchmarks(ctx context.Context, snapshot Snapshot, fh FileHandle) ( continue } - rng, err := NewMappedRange(snapshot.FileSet(), pgf.Mapper, d.Pos(), fn.End()).Range() + rng, err := NewMappedRange(pgf.Tok, pgf.Mapper, fn.Pos(), fn.End()).Range() if err != nil { return out, err } @@ -177,7 +177,7 @@ func goGenerateCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ( if !strings.HasPrefix(l.Text, ggDirective) { continue } - rng, err := NewMappedRange(snapshot.FileSet(), pgf.Mapper, l.Pos(), l.Pos()+token.Pos(len(ggDirective))).Range() + rng, err := NewMappedRange(pgf.Tok, pgf.Mapper, l.Pos(), l.Pos()+token.Pos(len(ggDirective))).Range() if err != nil { return nil, err } @@ -214,7 +214,7 @@ func regenerateCgoLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ([ if c == nil { return nil, nil } - rng, err := NewMappedRange(snapshot.FileSet(), pgf.Mapper, c.Pos(), c.EndPos).Range() + rng, err := NewMappedRange(pgf.Tok, pgf.Mapper, c.Pos(), c.End()).Range() if err != nil { return nil, err } @@ -231,7 +231,7 @@ func toggleDetailsCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle if err != nil { return nil, err } - rng, err := NewMappedRange(snapshot.FileSet(), pgf.Mapper, pgf.File.Package, pgf.File.Package).Range() + rng, err := NewMappedRange(pgf.Tok, pgf.Mapper, pgf.File.Package, pgf.File.Package).Range() if err != nil { return nil, err } diff --git a/internal/lsp/source/completion/completion.go b/internal/lsp/source/completion/completion.go index a2dfae6984..be613d3e3e 100644 --- a/internal/lsp/source/completion/completion.go +++ b/internal/lsp/source/completion/completion.go @@ -173,8 +173,9 @@ type completer struct { // file is the AST of the file associated with this completion request. file *ast.File - // pos is the position at which the request was triggered. - pos token.Pos + // (tokFile, pos) is the position at which the request was triggered. + tokFile *token.File + pos token.Pos // path is the path of AST nodes enclosing the position. path []ast.Node @@ -325,7 +326,7 @@ func (c *completer) setSurrounding(ident *ast.Ident) { content: ident.Name, cursor: c.pos, // Overwrite the prefix only. - rng: span.NewRange(c.snapshot.FileSet(), ident.Pos(), ident.End()), + rng: span.NewRange(c.tokFile, ident.Pos(), ident.End()), } c.setMatcherFromPrefix(c.surrounding.Prefix()) @@ -347,7 +348,7 @@ func (c *completer) getSurrounding() *Selection { c.surrounding = &Selection{ content: "", cursor: c.pos, - rng: span.NewRange(c.snapshot.FileSet(), c.pos, c.pos), + rng: span.NewRange(c.tokFile, c.pos, c.pos), } } return c.surrounding @@ -486,7 +487,7 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan qual := types.RelativeTo(pkg.GetTypes()) objStr = types.ObjectString(obj, qual) } - ans, sel := definition(path, obj, snapshot.FileSet(), fh) + ans, sel := definition(path, obj, pgf.Tok, fh) if ans != nil { sort.Slice(ans, func(i, j int) bool { return ans[i].Score > ans[j].Score @@ -513,6 +514,7 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan }, fh: fh, filename: fh.URI().Filename(), + tokFile: pgf.Tok, file: pgf.File, path: path, pos: pos, @@ -798,7 +800,7 @@ func (c *completer) populateImportCompletions(ctx context.Context, searchImport c.surrounding = &Selection{ content: content, cursor: c.pos, - rng: span.NewRange(c.snapshot.FileSet(), start, end), + rng: span.NewRange(c.tokFile, start, end), } seenImports := make(map[string]struct{}) @@ -1018,7 +1020,7 @@ func (c *completer) setSurroundingForComment(comments *ast.CommentGroup) { c.surrounding = &Selection{ content: cursorComment.Text[start:end], cursor: c.pos, - rng: span.NewRange(c.snapshot.FileSet(), token.Pos(int(cursorComment.Slash)+start), token.Pos(int(cursorComment.Slash)+end)), + rng: span.NewRange(c.tokFile, token.Pos(int(cursorComment.Slash)+start), token.Pos(int(cursorComment.Slash)+end)), } c.setMatcherFromPrefix(c.surrounding.Prefix()) } diff --git a/internal/lsp/source/completion/definition.go b/internal/lsp/source/completion/definition.go index 44d5a33b2f..7644fc443d 100644 --- a/internal/lsp/source/completion/definition.go +++ b/internal/lsp/source/completion/definition.go @@ -23,7 +23,7 @@ import ( // BenchmarkFoo(b *testing.B), FuzzFoo(f *testing.F) // path[0] is known to be *ast.Ident -func definition(path []ast.Node, obj types.Object, fset *token.FileSet, fh source.FileHandle) ([]CompletionItem, *Selection) { +func definition(path []ast.Node, obj types.Object, tokFile *token.File, fh source.FileHandle) ([]CompletionItem, *Selection) { if _, ok := obj.(*types.Func); !ok { return nil, nil // not a function at all } @@ -40,7 +40,7 @@ func definition(path []ast.Node, obj types.Object, fset *token.FileSet, fh sourc sel := &Selection{ content: "", cursor: pos, - rng: span.NewRange(fset, pos, pos), + rng: span.NewRange(tokFile, pos, pos), } var ans []CompletionItem diff --git a/internal/lsp/source/completion/package.go b/internal/lsp/source/completion/package.go index 21244efb5e..566d8ee2a0 100644 --- a/internal/lsp/source/completion/package.go +++ b/internal/lsp/source/completion/package.go @@ -104,7 +104,7 @@ func packageCompletionSurrounding(fset *token.FileSet, pgf *source.ParsedGoFile, return &Selection{ content: name.Name, cursor: cursor, - rng: span.NewRange(fset, name.Pos(), name.End()), + rng: span.NewRange(tok, name.Pos(), name.End()), }, nil } } @@ -141,7 +141,7 @@ func packageCompletionSurrounding(fset *token.FileSet, pgf *source.ParsedGoFile, return &Selection{ content: content, cursor: cursor, - rng: span.NewRange(fset, start, end), + rng: span.NewRange(tok, start, end), }, nil } } @@ -154,7 +154,7 @@ func packageCompletionSurrounding(fset *token.FileSet, pgf *source.ParsedGoFile, } // If the cursor is in a comment, don't offer any completions. - if cursorInComment(fset, cursor, pgf.Src) { + if cursorInComment(fset.File(cursor), cursor, pgf.Src) { return nil, fmt.Errorf("cursor in comment") } @@ -168,13 +168,13 @@ func packageCompletionSurrounding(fset *token.FileSet, pgf *source.ParsedGoFile, return &Selection{ content: "", cursor: cursor, - rng: span.NewRange(fset, start, end), + rng: span.NewRange(tok, start, end), }, nil } -func cursorInComment(fset *token.FileSet, cursor token.Pos, src []byte) bool { +func cursorInComment(file *token.File, cursor token.Pos, src []byte) bool { var s scanner.Scanner - s.Init(fset.File(cursor), src, func(_ token.Position, _ string) {}, scanner.ScanComments) + s.Init(file, src, func(_ token.Position, _ string) {}, scanner.ScanComments) for { pos, tok, lit := s.Scan() if pos <= cursor && cursor <= token.Pos(int(pos)+len(lit)) { diff --git a/internal/lsp/source/completion/util.go b/internal/lsp/source/completion/util.go index e6d3bfd745..e0a264bef9 100644 --- a/internal/lsp/source/completion/util.go +++ b/internal/lsp/source/completion/util.go @@ -311,7 +311,7 @@ func isBasicKind(t types.Type, k types.BasicInfo) bool { } func (c *completer) editText(from, to token.Pos, newText string) ([]protocol.TextEdit, error) { - rng := source.NewMappedRange(c.snapshot.FileSet(), c.mapper, from, to) + rng := source.NewMappedRange(c.tokFile, c.mapper, from, to) spn, err := rng.Span() if err != nil { return nil, err diff --git a/internal/lsp/source/extract.go b/internal/lsp/source/extract.go index 90999d821a..a4e0a148ad 100644 --- a/internal/lsp/source/extract.go +++ b/internal/lsp/source/extract.go @@ -18,11 +18,13 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/internal/analysisinternal" + "golang.org/x/tools/internal/lsp/bug" "golang.org/x/tools/internal/lsp/safetoken" "golang.org/x/tools/internal/span" ) func extractVariable(fset *token.FileSet, rng span.Range, src []byte, file *ast.File, _ *types.Package, info *types.Info) (*analysis.SuggestedFix, error) { + tokFile := fset.File(file.Pos()) expr, path, ok, err := CanExtractVariable(rng, file) if !ok { return nil, fmt.Errorf("extractVariable: cannot extract %s: %v", fset.Position(rng.Start), err) @@ -60,11 +62,7 @@ func extractVariable(fset *token.FileSet, rng span.Range, src []byte, file *ast. if insertBeforeStmt == nil { return nil, fmt.Errorf("cannot find location to insert extraction") } - tok := fset.File(expr.Pos()) - if tok == nil { - return nil, fmt.Errorf("no file for pos %v", fset.Position(file.Pos())) - } - indent, err := calculateIndentation(src, tok, insertBeforeStmt) + indent, err := calculateIndentation(src, tokFile, insertBeforeStmt) if err != nil { return nil, err } @@ -217,7 +215,12 @@ func extractFunctionMethod(fset *token.FileSet, rng span.Range, src []byte, file if isMethod { errorPrefix = "extractMethod" } - p, ok, methodOk, err := CanExtractFunction(fset, rng, src, file) + + tok := fset.File(file.Pos()) + if tok == nil { + return nil, bug.Errorf("no file for position") + } + p, ok, methodOk, err := CanExtractFunction(tok, rng, src, file) if (!ok && !isMethod) || (!methodOk && isMethod) { return nil, fmt.Errorf("%s: cannot extract %s: %v", errorPrefix, fset.Position(rng.Start), err) @@ -344,7 +347,7 @@ func extractFunctionMethod(fset *token.FileSet, rng span.Range, src []byte, file if v.obj.Parent() == nil { return nil, fmt.Errorf("parent nil") } - isUsed, firstUseAfter := objUsed(info, span.NewRange(fset, rng.End, v.obj.Parent().End()), v.obj) + isUsed, firstUseAfter := objUsed(info, span.NewRange(tok, rng.End, v.obj.Parent().End()), v.obj) if v.assigned && isUsed && !varOverridden(info, firstUseAfter, v.obj, v.free, outer) { returnTypes = append(returnTypes, &ast.Field{Type: typ}) returns = append(returns, identifier) @@ -941,14 +944,10 @@ type fnExtractParams struct { // CanExtractFunction reports whether the code in the given range can be // extracted to a function. -func CanExtractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast.File) (*fnExtractParams, bool, bool, error) { +func CanExtractFunction(tok *token.File, rng span.Range, src []byte, file *ast.File) (*fnExtractParams, bool, bool, error) { if rng.Start == rng.End { return nil, false, false, fmt.Errorf("start and end are equal") } - tok := fset.File(file.Pos()) - if tok == nil { - return nil, false, false, fmt.Errorf("no file for pos %v", fset.Position(file.Pos())) - } var err error rng, err = adjustRangeForWhitespace(rng, tok, src) if err != nil { diff --git a/internal/lsp/source/fix.go b/internal/lsp/source/fix.go index 6a7f77dab3..dce279e201 100644 --- a/internal/lsp/source/fix.go +++ b/internal/lsp/source/fix.go @@ -14,6 +14,7 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/internal/lsp/analysis/fillstruct" "golang.org/x/tools/internal/lsp/analysis/undeclaredname" + "golang.org/x/tools/internal/lsp/bug" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" ) @@ -84,7 +85,15 @@ func ApplyFix(ctx context.Context, fix string, snapshot Snapshot, fh VersionedFi fset := snapshot.FileSet() editsPerFile := map[span.URI]*protocol.TextDocumentEdit{} for _, edit := range suggestion.TextEdits { - spn, err := span.NewRange(fset, edit.Pos, edit.End).Span() + tokFile := fset.File(edit.Pos) + if tokFile == nil { + return nil, bug.Errorf("no file for edit position") + } + end := edit.End + if !end.IsValid() { + end = edit.Pos + } + spn, err := span.NewRange(tokFile, edit.Pos, end).Span() if err != nil { return nil, err } diff --git a/internal/lsp/source/folding_range.go b/internal/lsp/source/folding_range.go index 576308f996..b70cb4decd 100644 --- a/internal/lsp/source/folding_range.go +++ b/internal/lsp/source/folding_range.go @@ -41,13 +41,11 @@ func FoldingRange(ctx context.Context, snapshot Snapshot, fh FileHandle, lineFol return nil, nil } - fset := snapshot.FileSet() - // Get folding ranges for comments separately as they are not walked by ast.Inspect. - ranges = append(ranges, commentsFoldingRange(fset, pgf.Mapper, pgf.File)...) + ranges = append(ranges, commentsFoldingRange(pgf.Tok, pgf.Mapper, pgf.File)...) visit := func(n ast.Node) bool { - rng := foldingRangeFunc(fset, pgf.Mapper, n, lineFoldingOnly) + rng := foldingRangeFunc(pgf.Tok, pgf.Mapper, n, lineFoldingOnly) if rng != nil { ranges = append(ranges, rng) } @@ -66,7 +64,7 @@ func FoldingRange(ctx context.Context, snapshot Snapshot, fh FileHandle, lineFol } // foldingRangeFunc calculates the line folding range for ast.Node n -func foldingRangeFunc(fset *token.FileSet, m *protocol.ColumnMapper, n ast.Node, lineFoldingOnly bool) *FoldingRangeInfo { +func foldingRangeFunc(tokFile *token.File, m *protocol.ColumnMapper, n ast.Node, lineFoldingOnly bool) *FoldingRangeInfo { // TODO(suzmue): include trailing empty lines before the closing // parenthesis/brace. var kind protocol.FoldingRangeKind @@ -78,7 +76,7 @@ func foldingRangeFunc(fset *token.FileSet, m *protocol.ColumnMapper, n ast.Node, if num := len(n.List); num != 0 { startList, endList = n.List[0].Pos(), n.List[num-1].End() } - start, end = validLineFoldingRange(fset, n.Lbrace, n.Rbrace, startList, endList, lineFoldingOnly) + start, end = validLineFoldingRange(tokFile, n.Lbrace, n.Rbrace, startList, endList, lineFoldingOnly) case *ast.CaseClause: // Fold from position of ":" to end. start, end = n.Colon+1, n.End() @@ -94,7 +92,7 @@ func foldingRangeFunc(fset *token.FileSet, m *protocol.ColumnMapper, n ast.Node, if num := len(n.List); num != 0 { startList, endList = n.List[0].Pos(), n.List[num-1].End() } - start, end = validLineFoldingRange(fset, n.Opening, n.Closing, startList, endList, lineFoldingOnly) + start, end = validLineFoldingRange(tokFile, n.Opening, n.Closing, startList, endList, lineFoldingOnly) case *ast.GenDecl: // If this is an import declaration, set the kind to be protocol.Imports. if n.Tok == token.IMPORT { @@ -105,7 +103,7 @@ func foldingRangeFunc(fset *token.FileSet, m *protocol.ColumnMapper, n ast.Node, if num := len(n.Specs); num != 0 { startSpecs, endSpecs = n.Specs[0].Pos(), n.Specs[num-1].End() } - start, end = validLineFoldingRange(fset, n.Lparen, n.Rparen, startSpecs, endSpecs, lineFoldingOnly) + start, end = validLineFoldingRange(tokFile, n.Lparen, n.Rparen, startSpecs, endSpecs, lineFoldingOnly) case *ast.BasicLit: // Fold raw string literals from position of "`" to position of "`". if n.Kind == token.STRING && len(n.Value) >= 2 && n.Value[0] == '`' && n.Value[len(n.Value)-1] == '`' { @@ -117,7 +115,7 @@ func foldingRangeFunc(fset *token.FileSet, m *protocol.ColumnMapper, n ast.Node, if num := len(n.Elts); num != 0 { startElts, endElts = n.Elts[0].Pos(), n.Elts[num-1].End() } - start, end = validLineFoldingRange(fset, n.Lbrace, n.Rbrace, startElts, endElts, lineFoldingOnly) + start, end = validLineFoldingRange(tokFile, n.Lbrace, n.Rbrace, startElts, endElts, lineFoldingOnly) } // Check that folding positions are valid. @@ -125,18 +123,18 @@ func foldingRangeFunc(fset *token.FileSet, m *protocol.ColumnMapper, n ast.Node, return nil } // in line folding mode, do not fold if the start and end lines are the same. - if lineFoldingOnly && fset.Position(start).Line == fset.Position(end).Line { + if lineFoldingOnly && tokFile.Line(start) == tokFile.Line(end) { return nil } return &FoldingRangeInfo{ - MappedRange: NewMappedRange(fset, m, start, end), + MappedRange: NewMappedRange(tokFile, m, start, end), Kind: kind, } } // validLineFoldingRange returns start and end token.Pos for folding range if the range is valid. // returns token.NoPos otherwise, which fails token.IsValid check -func validLineFoldingRange(fset *token.FileSet, open, close, start, end token.Pos, lineFoldingOnly bool) (token.Pos, token.Pos) { +func validLineFoldingRange(tokFile *token.File, open, close, start, end token.Pos, lineFoldingOnly bool) (token.Pos, token.Pos) { if lineFoldingOnly { if !open.IsValid() || !close.IsValid() { return token.NoPos, token.NoPos @@ -146,8 +144,8 @@ func validLineFoldingRange(fset *token.FileSet, open, close, start, end token.Po // as an example, the example below should *not* fold: // var x = [2]string{"d", // "e" } - if fset.Position(open).Line == fset.Position(start).Line || - fset.Position(close).Line == fset.Position(end).Line { + if tokFile.Line(open) == tokFile.Line(start) || + tokFile.Line(close) == tokFile.Line(end) { return token.NoPos, token.NoPos } @@ -159,25 +157,25 @@ func validLineFoldingRange(fset *token.FileSet, open, close, start, end token.Po // commentsFoldingRange returns the folding ranges for all comment blocks in file. // The folding range starts at the end of the first line of the comment block, and ends at the end of the // comment block and has kind protocol.Comment. -func commentsFoldingRange(fset *token.FileSet, m *protocol.ColumnMapper, file *ast.File) (comments []*FoldingRangeInfo) { +func commentsFoldingRange(tokFile *token.File, m *protocol.ColumnMapper, file *ast.File) (comments []*FoldingRangeInfo) { for _, commentGrp := range file.Comments { - startGrp, endGrp := fset.Position(commentGrp.Pos()), fset.Position(commentGrp.End()) - if startGrp.Line == endGrp.Line { + startGrpLine, endGrpLine := tokFile.Line(commentGrp.Pos()), tokFile.Line(commentGrp.End()) + if startGrpLine == endGrpLine { // Don't fold single line comments. continue } firstComment := commentGrp.List[0] startPos, endLinePos := firstComment.Pos(), firstComment.End() - startCmmnt, endCmmnt := fset.Position(startPos), fset.Position(endLinePos) - if startCmmnt.Line != endCmmnt.Line { + startCmmntLine, endCmmntLine := tokFile.Line(startPos), tokFile.Line(endLinePos) + if startCmmntLine != endCmmntLine { // If the first comment spans multiple lines, then we want to have the // folding range start at the end of the first line. endLinePos = token.Pos(int(startPos) + len(strings.Split(firstComment.Text, "\n")[0])) } comments = append(comments, &FoldingRangeInfo{ // Fold from the end of the first line comment to the end of the comment block. - MappedRange: NewMappedRange(fset, m, endLinePos, commentGrp.End()), + MappedRange: NewMappedRange(tokFile, m, endLinePos, commentGrp.End()), Kind: protocol.Comment, }) } diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index 40655e2077..c87725c485 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -226,7 +226,7 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa // The builtin package isn't in the dependency graph, so the usual // utilities won't work here. - rng := NewMappedRange(snapshot.FileSet(), builtin.Mapper, decl.Pos(), decl.Pos()+token.Pos(len(result.Name))) + rng := NewMappedRange(builtin.Tok, builtin.Mapper, decl.Pos(), decl.Pos()+token.Pos(len(result.Name))) result.Declaration.MappedRange = append(result.Declaration.MappedRange, rng) return result, nil } @@ -267,7 +267,7 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa } name := method.Names[0].Name result.Declaration.node = method - rng := NewMappedRange(snapshot.FileSet(), builtin.Mapper, method.Pos(), method.Pos()+token.Pos(len(name))) + rng := NewMappedRange(builtin.Tok, builtin.Mapper, method.Pos(), method.Pos()+token.Pos(len(name))) result.Declaration.MappedRange = append(result.Declaration.MappedRange, rng) return result, nil } diff --git a/internal/lsp/source/references.go b/internal/lsp/source/references.go index 85bf41a21b..a3d32a6d71 100644 --- a/internal/lsp/source/references.go +++ b/internal/lsp/source/references.go @@ -48,6 +48,7 @@ func References(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Posit return nil, err } + packageName := pgf.File.Name.Name // from package decl packageNameStart, err := safetoken.Offset(pgf.Tok, pgf.File.Name.Pos()) if err != nil { return nil, err @@ -75,8 +76,8 @@ func References(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Posit for _, imp := range f.File.Imports { if path, err := strconv.Unquote(imp.Path.Value); err == nil && path == renamingPkg.PkgPath() { refs = append(refs, &ReferenceInfo{ - Name: pgf.File.Name.Name, - MappedRange: NewMappedRange(s.FileSet(), f.Mapper, imp.Pos(), imp.End()), + Name: packageName, + MappedRange: NewMappedRange(f.Tok, f.Mapper, imp.Pos(), imp.End()), }) } } @@ -86,8 +87,8 @@ func References(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Posit // Find internal references to the package within the package itself for _, f := range renamingPkg.CompiledGoFiles() { refs = append(refs, &ReferenceInfo{ - Name: pgf.File.Name.Name, - MappedRange: NewMappedRange(s.FileSet(), f.Mapper, f.File.Name.Pos(), f.File.Name.End()), + Name: packageName, + MappedRange: NewMappedRange(f.Tok, f.Mapper, f.File.Name.Pos(), f.File.Name.End()), }) } diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 6312bcb129..503422aa90 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -238,15 +238,15 @@ func (r *renamer) update() (map[span.URI][]diff.TextEdit, error) { continue } lines := strings.Split(comment.Text, "\n") - tok := r.fset.File(comment.Pos()) - commentLine := tok.Position(comment.Pos()).Line + tokFile := r.fset.File(comment.Pos()) + commentLine := tokFile.Line(comment.Pos()) for i, line := range lines { lineStart := comment.Pos() if i > 0 { - lineStart = tok.LineStart(commentLine + i) + lineStart = tokFile.LineStart(commentLine + i) } for _, locs := range docRegexp.FindAllIndex([]byte(line), -1) { - rng := span.NewRange(r.fset, lineStart+token.Pos(locs[0]), lineStart+token.Pos(locs[1])) + rng := span.NewRange(tokFile, lineStart+token.Pos(locs[0]), lineStart+token.Pos(locs[1])) spn, err := rng.Span() if err != nil { return nil, err @@ -265,7 +265,7 @@ func (r *renamer) update() (map[span.URI][]diff.TextEdit, error) { // docComment returns the doc for an identifier. func (r *renamer) docComment(pkg Package, id *ast.Ident) *ast.CommentGroup { - _, nodes, _ := pathEnclosingInterval(r.fset, pkg, id.Pos(), id.End()) + _, tokFile, nodes, _ := pathEnclosingInterval(r.fset, pkg, id.Pos(), id.End()) for _, node := range nodes { switch decl := node.(type) { case *ast.FuncDecl: @@ -294,25 +294,14 @@ func (r *renamer) docComment(pkg Package, id *ast.Ident) *ast.CommentGroup { return nil } - var file *ast.File - for _, f := range pkg.GetSyntax() { - if f.Pos() <= id.Pos() && id.Pos() <= f.End() { - file = f - break - } - } - if file == nil { - return nil - } - - identLine := r.fset.Position(id.Pos()).Line - for _, comment := range file.Comments { + identLine := tokFile.Line(id.Pos()) + for _, comment := range nodes[len(nodes)-1].(*ast.File).Comments { if comment.Pos() > id.Pos() { // Comment is after the identifier. continue } - lastCommentLine := r.fset.Position(comment.End()).Line + lastCommentLine := tokFile.Line(comment.End()) if lastCommentLine+1 == identLine { return comment } @@ -328,7 +317,7 @@ func (r *renamer) docComment(pkg Package, id *ast.Ident) *ast.CommentGroup { func (r *renamer) updatePkgName(pkgName *types.PkgName) (*diff.TextEdit, error) { // Modify ImportSpec syntax to add or remove the Name as needed. pkg := r.packages[pkgName.Pkg()] - _, path, _ := pathEnclosingInterval(r.fset, pkg, pkgName.Pos(), pkgName.Pos()) + _, tokFile, path, _ := pathEnclosingInterval(r.fset, pkg, pkgName.Pos(), pkgName.Pos()) if len(path) < 2 { return nil, fmt.Errorf("no path enclosing interval for %s", pkgName.Name()) } @@ -350,7 +339,7 @@ func (r *renamer) updatePkgName(pkgName *types.PkgName) (*diff.TextEdit, error) EndPos: spec.EndPos, } - rng := span.NewRange(r.fset, spec.Pos(), spec.End()) + rng := span.NewRange(tokFile, spec.Pos(), spec.End()) spn, err := rng.Span() if err != nil { return nil, err diff --git a/internal/lsp/source/rename_check.go b/internal/lsp/source/rename_check.go index b17f9b8706..6fb7ddf9ba 100644 --- a/internal/lsp/source/rename_check.go +++ b/internal/lsp/source/rename_check.go @@ -372,7 +372,7 @@ func (r *renamer) checkStructField(from *types.Var) { if !ok { return } - pkg, path, _ := pathEnclosingInterval(r.fset, fromPkg, from.Pos(), from.Pos()) + pkg, _, path, _ := pathEnclosingInterval(r.fset, fromPkg, from.Pos(), from.Pos()) if pkg == nil || path == nil { return } @@ -821,13 +821,13 @@ func someUse(info *types.Info, obj types.Object) *ast.Ident { return nil } -// pathEnclosingInterval returns the Package and ast.Node that +// pathEnclosingInterval returns the Package, token.File, and ast.Node that // contain source interval [start, end), and all the node's ancestors // up to the AST root. It searches all ast.Files of all packages. // exact is defined as for astutil.PathEnclosingInterval. // // The zero value is returned if not found. -func pathEnclosingInterval(fset *token.FileSet, pkg Package, start, end token.Pos) (resPkg Package, path []ast.Node, exact bool) { +func pathEnclosingInterval(fset *token.FileSet, pkg Package, start, end token.Pos) (resPkg Package, tokFile *token.File, path []ast.Node, exact bool) { pkgs := []Package{pkg} for _, f := range pkg.GetSyntax() { for _, imp := range f.Imports { @@ -840,35 +840,36 @@ func pathEnclosingInterval(fset *token.FileSet, pkg Package, start, end token.Po } importPkg, err := pkg.GetImport(importPath) if err != nil { - return nil, nil, false + return nil, nil, nil, false } pkgs = append(pkgs, importPkg) } } for _, p := range pkgs { for _, f := range p.GetSyntax() { - if f.Pos() == token.NoPos { + if !f.Pos().IsValid() { // This can happen if the parser saw // too many errors and bailed out. // (Use parser.AllErrors to prevent that.) continue } - if !tokenFileContainsPos(fset.File(f.Pos()), start) { + tokFile := fset.File(f.Pos()) + if !tokenFileContainsPos(tokFile, start) { continue } if path, exact := astutil.PathEnclosingInterval(f, start, end); path != nil { - return pkg, path, exact + return pkg, tokFile, path, exact } } } - return nil, nil, false + return nil, nil, nil, false } // TODO(adonovan): make this a method: func (*token.File) Contains(token.Pos) func tokenFileContainsPos(tf *token.File, pos token.Pos) bool { p := int(pos) base := tf.Base() - return base <= p && p < base+tf.Size() + return base <= p && p <= base+tf.Size() } func objectKind(obj types.Object) string { diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index b8a7fc9135..1097038929 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -30,26 +30,22 @@ type MappedRange struct { m *protocol.ColumnMapper // a mapper of the edited source (package.GoFiles) } -// NewMappedRange returns a MappedRange for the given start and end token.Pos. +// NewMappedRange returns a MappedRange for the given file and valid start/end token.Pos. // // By convention, start and end are assumed to be positions in the compiled (== // type checked) source, whereas the column mapper m maps positions in the -// user-edited source. Note that these may not be the same, as when using CGo: +// user-edited source. Note that these may not be the same, as when using goyacc or CGo: // CompiledGoFiles contains generated files, whose positions (via // token.File.Position) point to locations in the edited file -- the file // containing `import "C"`. -func NewMappedRange(fset *token.FileSet, m *protocol.ColumnMapper, start, end token.Pos) MappedRange { - if tf := fset.File(start); tf == nil { - bug.Report("nil file", nil) - } else { - mapped := m.TokFile.Name() - adjusted := tf.PositionFor(start, true) // adjusted position - if adjusted.Filename != mapped { - bug.Reportf("mapped file %q does not match start position file %q", mapped, adjusted.Filename) - } +func NewMappedRange(file *token.File, m *protocol.ColumnMapper, start, end token.Pos) MappedRange { + mapped := m.TokFile.Name() + adjusted := file.PositionFor(start, true) // adjusted position + if adjusted.Filename != mapped { + bug.Reportf("mapped file %q does not match start position file %q", mapped, adjusted.Filename) } return MappedRange{ - spanRange: span.NewRange(fset, start, end), + spanRange: span.NewRange(file, start, end), m: m, } } @@ -134,7 +130,10 @@ func nodeToProtocolRange(snapshot Snapshot, pkg Package, n ast.Node) (protocol.R 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 { // An imported Go package has a package-local, unqualified name. // When the name matches the imported package name, there is no @@ -147,29 +146,35 @@ func objToMappedRange(snapshot Snapshot, pkg Package, obj types.Object) (MappedR // When the identifier does not appear in the source, have the range // of the object be the import path, including quotes. if pkgName.Imported().Name() == pkgName.Name() { - return posToMappedRange(snapshot, pkg, obj.Pos(), obj.Pos()+token.Pos(len(pkgName.Imported().Path())+2)) + nameLen = len(pkgName.Imported().Path()) + len(`""`) } } - return nameToMappedRange(snapshot, pkg, obj.Pos(), obj.Name()) -} - -func nameToMappedRange(snapshot Snapshot, pkg Package, pos token.Pos, name string) (MappedRange, error) { - return posToMappedRange(snapshot, pkg, pos, pos+token.Pos(len(name))) + return posToMappedRange(snapshot, pkg, obj.Pos(), obj.Pos()+token.Pos(nameLen)) } +// posToMappedRange returns the MappedRange for the given [start, end) span, +// which must be among the transitive dependencies of pkg. func posToMappedRange(snapshot Snapshot, pkg Package, pos, end token.Pos) (MappedRange, error) { - logicalFilename := snapshot.FileSet().File(pos).Position(pos).Filename + tokFile := snapshot.FileSet().File(pos) + // Subtle: it is not safe to simplify this to tokFile.Name + // because, due to //line directives, a Position within a + // token.File may have a different filename than the File itself. + logicalFilename := tokFile.Position(pos).Filename pgf, _, err := findFileInDeps(pkg, span.URIFromPath(logicalFilename)) if err != nil { return MappedRange{}, err } if !pos.IsValid() { - return MappedRange{}, fmt.Errorf("invalid position for %v", pos) + return MappedRange{}, fmt.Errorf("invalid start position") } if !end.IsValid() { - return MappedRange{}, fmt.Errorf("invalid position for %v", end) + return MappedRange{}, fmt.Errorf("invalid end position") } - return NewMappedRange(snapshot.FileSet(), pgf.Mapper, pos, end), nil + // It is fishy that pgf.Mapper (from the parsed Go file) is + // accompanied here not by pgf.Tok but by tokFile from the global + // FileSet, which is a distinct token.File that doesn't + // contain [pos,end). TODO(adonovan): clean this up. + return NewMappedRange(tokFile, pgf.Mapper, pos, end), nil } // Matches cgo generated comment as well as the proposed standard: diff --git a/internal/lsp/source/util_test.go b/internal/lsp/source/util_test.go index 5d4e98f151..fe505e4d06 100644 --- a/internal/lsp/source/util_test.go +++ b/internal/lsp/source/util_test.go @@ -41,7 +41,7 @@ const a𐐀b = 42`) start := cf.Pos(bytes.Index(compiled, []byte("a𐐀b"))) end := start + token.Pos(len("a𐐀b")) - mr := NewMappedRange(fset, mapper, start, end) + mr := NewMappedRange(cf, mapper, start, end) gotRange, err := mr.Range() if err != nil { t.Fatal(err) diff --git a/internal/span/token.go b/internal/span/token.go index af01d7b834..cae696db75 100644 --- a/internal/span/token.go +++ b/internal/span/token.go @@ -12,28 +12,40 @@ import ( ) // Range represents a source code range in token.Pos form. -// It also carries the FileSet that produced the positions, so that it is +// It also carries the token.File that produced the positions, so that it is // self contained. type Range struct { - Start token.Pos - End token.Pos - - // TokFile may be nil if Start or End is invalid. - // TODO: Eventually we should guarantee that it is non-nil. - TokFile *token.File + TokFile *token.File // non-nil + Start, End token.Pos // both IsValid() } -// NewRange creates a new Range from a FileSet and two positions. -// To represent a point pass a 0 as the end pos. -func NewRange(fset *token.FileSet, start, end token.Pos) Range { - tf := fset.File(start) - if tf == nil { - bug.Reportf("nil file") +// NewRange creates a new Range from a token.File and two valid positions within it. +// +// (If you only have a token.FileSet, use file = fset.File(start). But +// most callers know exactly which token.File they're dealing with and +// should pass it explicitly. Not only does this save a lookup, but it +// brings us a step closer to eliminating the global FileSet.) +func NewRange(file *token.File, start, end token.Pos) Range { + if file == nil { + panic("nil *token.File") } + if !start.IsValid() || !end.IsValid() { + panic("invalid start/end token.Pos") + } + + // TODO(adonovan): ideally we would make this stronger assertion: + // + // // Assert that file is non-nil and contains start and end. + // _ = file.Offset(start) + // _ = file.Offset(end) + // + // but some callers (e.g. packageCompletionSurrounding, + // posToMappedRange) don't ensure this precondition. + return Range{ + TokFile: file, Start: start, End: end, - TokFile: tf, } }