close_issue_1
/
tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

internal/lsp: match files by identity 

Instead of using a simple path map we now attempt to match files with
os.SameFile with fast paths for exact path matches. This should fix issues both
with symlinked directories (the mac tmp folder) and with case sensitivity
(windows)

Change-Id: I014dd01f89d08a348e7de7697cbc3a2512a6e5b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/169699
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Ian Cottrell 2019-03-27 09:25:30 -04:00
parent dbeab5af4b
commit c70d86f8b7
4 changed files with 96 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
internal/lsp/cache/check.go vendored
View File

@ -19,7 +19,7 @@ import (
"golang.org/x/tools/internal/span"
)
func (v *View) parse(ctx context.Context, uri span.URI) ([]packages.Error, error) {
func (v *View) parse(ctx context.Context, f *File) ([]packages.Error, error) {
v.mcache.mu.Lock()
defer v.mcache.mu.Unlock()
@ -28,12 +28,6 @@ func (v *View) parse(ctx context.Context, uri span.URI) ([]packages.Error, error
return nil, err
}
f := v.files[uri]
// This should never happen.
if f == nil {
return nil, fmt.Errorf("no file for %v", uri)
}
// If the package for the file has not been invalidated by the application
// of the pending changes, there is no need to continue.
if f.isPopulated() {
@ -45,7 +39,7 @@ func (v *View) parse(ctx context.Context, uri span.URI) ([]packages.Error, error
return errs, err
}
if f.meta == nil {
return nil, fmt.Errorf("no metadata found for %v", uri)
return nil, fmt.Errorf("no metadata found for %v", f.filename)
}
imp := &importer{
view: v,
@ -65,7 +59,7 @@ func (v *View) parse(ctx context.Context, uri span.URI) ([]packages.Error, error
// If we still have not found the package for the file, something is wrong.
if f.pkg == nil {
return nil, fmt.Errorf("no package found for %v", uri)
return nil, fmt.Errorf("parse: no package found for %v", f.filename)
}
return nil, nil
}
@ -83,7 +77,11 @@ func (v *View) cachePackage(pkg *Package) {
continue
}
fURI := span.FileURI(tok.Name())
f := v.getFile(fURI)
f, err := v.getFile(fURI)
if err != nil {
log.Printf("no file: %v", err)
continue
}
f.token = tok
f.ast = file
f.imports = f.ast.Imports
@ -92,17 +90,13 @@ func (v *View) cachePackage(pkg *Package) {
}
func (v *View) checkMetadata(ctx context.Context, f *File) ([]packages.Error, error) {
filename, err := f.uri.Filename()
if err != nil {
return nil, err
}
if v.reparseImports(ctx, f, filename) {
if v.reparseImports(ctx, f, f.filename) {
cfg := v.Config
cfg.Mode = packages.LoadImports
pkgs, err := packages.Load(&cfg, fmt.Sprintf("file=%s", filename))
pkgs, err := packages.Load(&cfg, fmt.Sprintf("file=%s", f.filename))
if len(pkgs) == 0 {
if err == nil {
err = fmt.Errorf("no packages found for %s", filename)
err = fmt.Errorf("no packages found for %s", f.filename)
}
return nil, err
}
@ -156,7 +150,7 @@ func (v *View) link(pkgPath string, pkg *packages.Package, parent *metadata) *me
m.name = pkg.Name
m.files = pkg.CompiledGoFiles
for _, filename := range m.files {
if f, ok := v.files[span.FileURI(filename)]; ok {
if f := v.findFile(span.FileURI(filename)); f != nil {
f.meta = m
}
}
@ -347,7 +341,7 @@ func (v *View) parseFiles(filenames []string) ([]*ast.File, []error) {
}
// First, check if we have already cached an AST for this file.
f := v.files[span.FileURI(filename)]
f := v.findFile(span.FileURI(filename))
var fAST *ast.File
if f != nil {
fAST = f.ast

26
internal/lsp/cache/file.go vendored
View File

@ -9,6 +9,8 @@ import (
"go/ast"
"go/token"
"io/ioutil"
"path/filepath"
"strings"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
@ -16,7 +18,10 @@ import (
// File holds all the information we know about a file.
type File struct {
uri span.URI
uris []span.URI
filename string
basename string
view *View
active bool
content []byte
@ -27,8 +32,12 @@ type File struct {
imports []*ast.ImportSpec
}
func basename(filename string) string {
return strings.ToLower(filepath.Base(filename))
}
func (f *File) URI() span.URI {
return f.uri
return f.uris[0]
}
// GetContent returns the contents of the file, reading it from file system if needed.
@ -52,7 +61,7 @@ func (f *File) GetToken(ctx context.Context) *token.File {
defer f.view.mu.Unlock()
if f.token == nil || len(f.view.contentChanges) > 0 {
if _, err := f.view.parse(ctx, f.uri); err != nil {
if _, err := f.view.parse(ctx, f); err != nil {
return nil
}
}
@ -64,7 +73,7 @@ func (f *File) GetAST(ctx context.Context) *ast.File {
defer f.view.mu.Unlock()
if f.ast == nil || len(f.view.contentChanges) > 0 {
if _, err := f.view.parse(ctx, f.uri); err != nil {
if _, err := f.view.parse(ctx, f); err != nil {
return nil
}
}
@ -74,9 +83,8 @@ func (f *File) GetAST(ctx context.Context) *ast.File {
func (f *File) GetPackage(ctx context.Context) source.Package {
f.view.mu.Lock()
defer f.view.mu.Unlock()
if f.pkg == nil || len(f.view.contentChanges) > 0 {
errs, err := f.view.parse(ctx, f.uri)
errs, err := f.view.parse(ctx, f)
if err != nil {
// Create diagnostics for errors if we are able to.
if len(errs) > 0 {
@ -105,11 +113,7 @@ func (f *File) read(ctx context.Context) {
}
}
// We don't know the content yet, so read it.
filename, err := f.uri.Filename()
if err != nil {
return
}
content, err := ioutil.ReadFile(filename)
content, err := ioutil.ReadFile(f.filename)
if err != nil {
return
}

82
internal/lsp/cache/view.go vendored
View File

@ -7,6 +7,7 @@ package cache
import (
"context"
"go/token"
"os"
"sync"
"golang.org/x/tools/go/packages"
@ -30,8 +31,10 @@ type View struct {
// go/packages API. It is shared across all views.
Config packages.Config
// files caches information for opened files in a view.
files map[span.URI]*File
// keep track of files by uri and by basename, a single file may be mapped
// to multiple uris, and the same basename may map to multiple files
filesByURI map[span.URI]*File
filesByBase map[string][]*File
// contentChanges saves the content changes for a given state of the view.
// When type information is requested by the view, all of the dirty changes
@ -76,7 +79,8 @@ func NewView(config *packages.Config) *View {
backgroundCtx: ctx,
cancel: cancel,
Config: *config,
files: make(map[span.URI]*File),
filesByURI: make(map[span.URI]*File),
filesByBase: make(map[string][]*File),
contentChanges: make(map[span.URI]func()),
mcache: &metadataCache{
packages: make(map[string]*metadata),
@ -137,7 +141,10 @@ func (v *View) applyContentChanges(ctx context.Context) error {
// setContent applies a content update for a given file. It assumes that the
// caller is holding the view's mutex.
func (v *View) applyContentChange(uri span.URI, content []byte) {
f := v.getFile(uri)
f, err := v.getFile(uri)
if err != nil {
return
}
f.content = content
// TODO(rstambler): Should we recompute these here?
@ -153,16 +160,12 @@ func (v *View) applyContentChange(uri span.URI, content []byte) {
case f.active && content == nil:
// The file was active, so we need to forget its content.
f.active = false
if filename, err := f.uri.Filename(); err == nil {
delete(f.view.Config.Overlay, filename)
}
delete(f.view.Config.Overlay, f.filename)
f.content = nil
case content != nil:
// This is an active overlay, so we update the map.
f.active = true
if filename, err := f.uri.Filename(); err == nil {
f.view.Config.Overlay[filename] = f.content
}
f.view.Config.Overlay[f.filename] = f.content
}
}
@ -180,7 +183,7 @@ func (v *View) remove(pkgPath string) {
// All of the files in the package may also be holding a pointer to the
// invalidated package.
for _, filename := range m.files {
if f, ok := v.files[span.FileURI(filename)]; ok {
if f := v.findFile(span.FileURI(filename)); f != nil {
f.pkg = nil
}
}
@ -197,18 +200,61 @@ func (v *View) GetFile(ctx context.Context, uri span.URI) (source.File, error) {
return nil, ctx.Err()
}
return v.getFile(uri), nil
return v.getFile(uri)
}
// getFile is the unlocked internal implementation of GetFile.
func (v *View) getFile(uri span.URI) *File {
f, found := v.files[uri]
if !found {
f = &File{
uri: uri,
func (v *View) getFile(uri span.URI) (*File, error) {
if f := v.findFile(uri); f != nil {
return f, nil
}
filename, err := uri.Filename()
if err != nil {
return nil, err
}
f := &File{
view: v,
filename: filename,
}
v.files[uri] = f
v.mapFile(uri, f)
return f, nil
}
func (v *View) findFile(uri span.URI) *File {
if f := v.filesByURI[uri]; f != nil {
// a perfect match
return f
}
// no exact match stored, time to do some real work
// check for any files with the same basename
fname, err := uri.Filename()
if err != nil {
return nil
}
basename := basename(fname)
if candidates := v.filesByBase[basename]; candidates != nil {
pathStat, err := os.Stat(fname)
if err == nil {
return nil
}
for _, c := range candidates {
if cStat, err := os.Stat(c.filename); err == nil {
if os.SameFile(pathStat, cStat) {
// same file, map it
v.mapFile(uri, c)
return c
}
}
}
}
return nil
}
func (v *View) mapFile(uri span.URI, f *File) {
v.filesByURI[uri] = f
f.uris = append(f.uris, uri)
if f.basename == "" {
f.basename = basename(f.filename)
v.filesByBase[f.basename] = append(v.filesByBase[f.basename], f)
}
}

2
internal/lsp/source/diagnostics.go
View File

@ -58,7 +58,7 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag
}
pkg := f.GetPackage(ctx)
if pkg == nil {
return nil, fmt.Errorf("no package found for %v", f.URI())
return nil, fmt.Errorf("diagnostics: no package found for %v", f.URI())
}
// Prepare the reports we will send for this package.
reports := make(map[span.URI][]Diagnostic)