internal/lsp: look up files in packages by position instead of URI

This change makes sure that we only return files that contain the given
position. There are a few instances of needing to look up files by URI
in the internal/lsp/cache package, so use an unexported package for
that. This allows us to remove some code in the implementations code.

Change-Id: Ifa7a62c67271826e6c632e4c88667d60f8b760c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206880
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This commit is contained in:
Rebecca Stambler 2019-11-12 16:33:11 -05:00
parent e33b02e766
commit bc1376d635
11 changed files with 83 additions and 107 deletions

View File

@ -163,7 +163,7 @@ func typeErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, pos toke
return span.Span{}, err
}
posn := fset.Position(pos)
ph, _, err := findFileInPackage(ctx, span.FileURI(posn.Filename), pkg)
ph, _, err := findFileInPackage(pkg, span.FileURI(posn.Filename))
if err != nil {
return spn, nil // ignore errors
}
@ -190,7 +190,7 @@ func typeErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, pos toke
}
func scannerErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, posn token.Position) (span.Span, error) {
ph, _, err := findFileInPackage(ctx, span.FileURI(posn.Filename), pkg)
ph, _, err := findFileInPackage(pkg, span.FileURI(posn.Filename))
if err != nil {
return span.Span{}, err
}
@ -209,7 +209,7 @@ func scannerErrorRange(ctx context.Context, fset *token.FileSet, pkg *pkg, posn
// spanToRange converts a span.Span to a protocol.Range,
// assuming that the span belongs to the package whose diagnostics are being computed.
func spanToRange(ctx context.Context, pkg *pkg, spn span.Span) (protocol.Range, error) {
ph, _, err := findFileInPackage(ctx, spn.URI(), pkg)
ph, _, err := findFileInPackage(pkg, spn.URI())
if err != nil {
return protocol.Range{}, err
}

View File

@ -126,26 +126,3 @@ func (s *snapshot) FindAnalysisError(ctx context.Context, id string, diag protoc
}
return nil, errors.Errorf("no matching diagnostic for %v", diag)
}
func findFileInPackage(ctx context.Context, uri span.URI, pkg source.Package) (source.ParseGoHandle, source.Package, error) {
queue := []source.Package{pkg}
seen := make(map[string]bool)
for len(queue) > 0 {
pkg := queue[0]
queue = queue[1:]
seen[pkg.ID()] = true
for _, ph := range pkg.Files() {
if ph.File().Identity().URI == uri {
return ph, pkg, nil
}
}
for _, dep := range pkg.Imports() {
if !seen[dep.ID()] {
queue = append(queue, dep)
}
}
}
return nil, nil, errors.Errorf("no file for %s in package %s", uri, pkg.ID())
}

View File

@ -141,7 +141,7 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI,
m, err := v.snapshot.load(ctx, source.DirectoryURI(folder))
var loadErr error
if err != nil && err != errNoPackagesFound {
loadErr = fmt.Errorf("Error loading packages: %v", err)
loadErr = fmt.Errorf("error loading packages: %v", err)
}
// Prepare CheckPackageHandles for every package that's been loaded.

View File

@ -301,16 +301,6 @@ func (v *view) Ignore(uri span.URI) bool {
return ok
}
func (v *view) findIgnoredFile(ctx context.Context, uri span.URI) (source.ParseGoHandle, source.Package, error) {
// Check the builtin package.
for _, h := range v.BuiltinPackage().Files() {
if h.File().Identity().URI == uri {
return h, nil, nil
}
}
return nil, nil, errors.Errorf("no ignored file for %s", uri)
}
func (v *view) BackgroundContext() context.Context {
v.mu.Lock()
defer v.mu.Unlock()
@ -455,12 +445,68 @@ func (v *view) openFiles(ctx context.Context, uris []span.URI) (results []source
return results
}
func (v *view) FindFileInPackage(ctx context.Context, uri span.URI, pkg source.Package) (source.ParseGoHandle, source.Package, error) {
// Special case for ignored files.
if v.Ignore(uri) {
return v.findIgnoredFile(ctx, uri)
func (v *view) FindPosInPackage(searchpkg source.Package, pos token.Pos) (*ast.File, *protocol.ColumnMapper, source.Package, error) {
tok := v.session.cache.fset.File(pos)
if tok == nil {
return nil, nil, nil, errors.Errorf("no file for pos in package %s", searchpkg.ID())
}
return findFileInPackage(ctx, uri, pkg)
uri := span.FileURI(tok.Name())
// Special case for ignored files.
var (
ph source.ParseGoHandle
pkg source.Package
err error
)
if v.Ignore(uri) {
ph, pkg, err = v.findIgnoredFile(uri)
} else {
ph, pkg, err = findFileInPackage(searchpkg, uri)
}
if err != nil {
return nil, nil, nil, err
}
file, m, _, err := ph.Cached()
if err != nil {
return nil, nil, nil, err
}
if !(file.Pos() <= pos && pos <= file.End()) {
return nil, nil, nil, err
}
return file, m, pkg, nil
}
func (v *view) findIgnoredFile(uri span.URI) (source.ParseGoHandle, source.Package, error) {
// Check the builtin package.
for _, h := range v.BuiltinPackage().Files() {
if h.File().Identity().URI == uri {
return h, nil, nil
}
}
return nil, nil, errors.Errorf("no ignored file for %s", uri)
}
func findFileInPackage(pkg source.Package, uri span.URI) (source.ParseGoHandle, source.Package, error) {
queue := []source.Package{pkg}
seen := make(map[string]bool)
for len(queue) > 0 {
pkg := queue[0]
queue = queue[1:]
seen[pkg.ID()] = true
for _, ph := range pkg.Files() {
if ph.File().Identity().URI == uri {
return ph, pkg, nil
}
}
for _, dep := range pkg.Imports() {
if !seen[dep.ID()] {
queue = append(queue, dep)
}
}
}
return nil, nil, errors.Errorf("no file for %s in package %s", uri, pkg.ID())
}
type debugView struct{ *view }

View File

@ -19,6 +19,9 @@ func (s *Server) implementation(ctx context.Context, params *protocol.Implementa
if err != nil {
return nil, err
}
return source.Implementation(ctx, view, f, params.Position)
ident, err := source.Identifier(ctx, view, f, params.Position)
if err != nil {
return nil, err
}
return ident.Implementation(ctx)
}

View File

@ -154,20 +154,11 @@ func (c *completer) item(cand candidate) (CompletionItem, error) {
if cand.imp != nil && cand.imp.pkg != nil {
searchPkg = cand.imp.pkg
}
ph, pkg, err := c.view.FindFileInPackage(c.ctx, uri, searchPkg)
file, _, pkg, err := c.view.FindPosInPackage(searchPkg, obj.Pos())
if err != nil {
log.Error(c.ctx, "error finding file in package", err, telemetry.URI.Of(uri), telemetry.Package.Of(searchPkg.ID()))
return item, nil
}
file, _, _, err := ph.Cached()
if err != nil {
log.Error(c.ctx, "no cached file", err, telemetry.URI.Of(uri))
return item, nil
}
if !(file.Pos() <= obj.Pos() && obj.Pos() <= file.End()) {
log.Error(c.ctx, "no file for object", errors.Errorf("no file for completion object %s", obj.Name()), telemetry.URI.Of(uri))
return item, nil
}
ident, err := findIdentifier(c.ctx, c.snapshot, pkg, file, obj.Pos())
if err != nil {
log.Error(c.ctx, "failed to findIdentifier", err, telemetry.URI.Of(uri))

View File

@ -242,18 +242,10 @@ func hasErrorType(obj types.Object) bool {
}
func objToNode(ctx context.Context, v View, pkg Package, obj types.Object) (ast.Decl, error) {
uri := span.FileURI(v.Session().Cache().FileSet().Position(obj.Pos()).Filename)
ph, _, err := v.FindFileInPackage(ctx, uri, pkg)
declAST, _, _, err := v.FindPosInPackage(pkg, obj.Pos())
if err != nil {
return nil, err
}
declAST, _, _, err := ph.Cached()
if declAST == nil {
return nil, err
}
if !(declAST.Pos() <= obj.Pos() && obj.Pos() <= declAST.End()) {
return nil, errors.Errorf("no file for %s", obj.Name())
}
path, _ := astutil.PathEnclosingInterval(declAST, obj.Pos(), obj.Pos())
if path == nil {
return nil, errors.Errorf("no path for object %v", obj.Name())

View File

@ -19,14 +19,8 @@ import (
"golang.org/x/tools/internal/lsp/protocol"
)
func Implementation(ctx context.Context, view View, f File, position protocol.Position) ([]protocol.Location, error) {
// Find all references to the identifier at the position.
ident, err := Identifier(ctx, view, f, position)
if err != nil {
return nil, err
}
res, err := ident.implementations(ctx)
func (i *IdentifierInfo) Implementation(ctx context.Context) ([]protocol.Location, error) {
res, err := i.implementations(ctx)
if err != nil {
return nil, err
}
@ -71,35 +65,14 @@ func Implementation(ctx context.Context, view View, f File, position protocol.Po
if pkgs[obj] == nil || len(pkg.Files()) == 0 {
continue
}
// Search for the identifier in each of the package's files.
var ident *IdentifierInfo
fset := view.Session().Cache().FileSet()
file := fset.File(obj.Pos())
var containingFile FileHandle
for _, f := range pkg.Files() {
if f.File().Identity().URI.Filename() == file.Name() {
containingFile = f.File()
}
}
if containingFile == nil {
return nil, fmt.Errorf("failed to find file %q in package %v", file.Name(), pkg.PkgPath())
}
uri := containingFile.Identity().URI
ph, _, err := view.FindFileInPackage(ctx, uri, pkgs[obj])
file, _, _, err := i.Snapshot.View().FindPosInPackage(pkgs[obj], obj.Pos())
if err != nil {
return nil, err
}
astFile, _, _, err := ph.Cached()
ident, err := findIdentifier(ctx, i.Snapshot, pkg, file, obj.Pos())
if err != nil {
return nil, err
}
ident, err = findIdentifier(ctx, view.Snapshot(), pkg, astFile, obj.Pos())
if err != nil {
return nil, err
}
decRange, err := ident.Declaration.Range()
if err != nil {
return nil, err

View File

@ -554,8 +554,12 @@ func (r *runner) Implementation(t *testing.T, spn span.Span, m tests.Implementat
if err != nil {
t.Fatalf("failed for %v: %v", m.Src, err)
}
ident, err := source.Identifier(ctx, r.view, f, loc.Range.Start)
if err != nil {
t.Fatalf("failed for %v: %v", m.Src, err)
}
var locs []protocol.Location
locs, err = source.Implementation(r.ctx, r.view, f, loc.Range.Start)
locs, err = ident.Implementation(r.ctx)
if err != nil {
t.Fatalf("failed for %v: %v", m.Src, err)
}

View File

@ -155,7 +155,7 @@ func nodeToMappedRange(ctx context.Context, view View, m *protocol.ColumnMapper,
}
func posToMappedRange(ctx context.Context, v View, pkg Package, pos, end token.Pos) (mappedRange, error) {
m, err := posToMapper(ctx, v, pkg, pos)
_, m, _, err := v.FindPosInPackage(pkg, pos)
if err != nil {
return mappedRange{}, err
}
@ -175,16 +175,6 @@ func posToRange(ctx context.Context, view View, m *protocol.ColumnMapper, pos, e
}, nil
}
func posToMapper(ctx context.Context, v View, pkg Package, pos token.Pos) (*protocol.ColumnMapper, error) {
posn := v.Session().Cache().FileSet().Position(pos)
ph, _, err := v.FindFileInPackage(ctx, span.FileURI(posn.Filename), pkg)
if err != nil {
return nil, err
}
_, m, _, err := ph.Cached()
return m, err
}
// Matches cgo generated comment as well as the proposed standard:
// https://golang.org/s/generatedcode
var generatedRx = regexp.MustCompile(`// .*DO NOT EDIT\.?`)

View File

@ -260,7 +260,7 @@ type View interface {
// FindFileInPackage returns the AST and type information for a file that may
// belong to or be part of a dependency of the given package.
FindFileInPackage(ctx context.Context, uri span.URI, pkg Package) (ParseGoHandle, Package, error)
FindPosInPackage(pkg Package, pos token.Pos) (*ast.File, *protocol.ColumnMapper, Package, error)
// Snapshot returns the current snapshot for the view.
Snapshot() Snapshot