internal/span: make NewRange accept File, not FileSet
span.NewRange now accepts a *token.File and two token.Pos.
It is the caller's responsibility to look up the File
in the FileSet, if necessary (it usually isn't), and to
ensure the Pos values are valid. Ditto NewMappedRange.
This reduces the creep of Snapshot into functions that
have no need to know about it. Also the bug.Report call
in NewRange has been pushed up into the caller and
logically eliminated in all but one case.
I think we should aim for the invariant that functions that
operate on a single file should accept a *token.File, not
a FileSet; only functions that operate on sets of files
(e.g. type checking, analysis) should use a FileSet.
This is not always possible: some public functions
accept a FileSet only to re-lookup a single file already
known to the caller; if necessary we could provide token.File
variants of these.
This may ultimately allow us to create a new FileSet per
call to the parser, so that Files and FileSets are in
1:1 correspondance and there is no global FileSet.
(It currently grows without bound, on the order of kilobytes
per keystroke.) FileSets containing multiple files,
needed when we interact with the type checker and analysis,
may be temporarily synthesized on demand from a set of Files,
analogous to mmap'ing a few files into a blank address space.
Also:
- replace File.Position(pos).Line by File.Line(pos)
- replace pos == token.NoPos by pos.IsValid()
- avoid fishy token.Pos conversions in link.go
- other minor simplifications
Change-Id: Ia3119e0ac7e193801fbafa81c8f48acfa14e9ae4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/409935
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go
index 6312bcb..503422a 100644
--- a/internal/lsp/source/rename.go
+++ b/internal/lsp/source/rename.go
@@ -238,15 +238,15 @@
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 @@
// 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 @@
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) 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 @@
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
