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>
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 2082b47..707730a 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -19,7 +19,7 @@
"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 @@
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 @@
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 @@
// 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 @@
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) 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 @@
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 @@
}
// 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
diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go
index 3b1269e..db16db6 100644
--- a/internal/lsp/cache/file.go
+++ b/internal/lsp/cache/file.go
@@ -9,6 +9,8 @@
"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 @@
// 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 @@
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 @@
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 @@
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) 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 @@
}
}
// 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
}
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index ba7b221..d8f609c 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -7,6 +7,7 @@
import (
"context"
"go/token"
+ "os"
"sync"
"golang.org/x/tools/go/packages"
@@ -30,8 +31,10 @@
// 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 @@
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 @@
// 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 @@
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 @@
// 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 @@
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,
- view: v,
- }
- v.files[uri] = f
+func (v *View) getFile(uri span.URI) (*File, error) {
+ if f := v.findFile(uri); f != nil {
+ return f, nil
}
- return f
+ filename, err := uri.Filename()
+ if err != nil {
+ return nil, err
+ }
+ f := &File{
+ view: v,
+ filename: filename,
+ }
+ 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)
+ }
}
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 2fede2b..c110c24 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -58,7 +58,7 @@
}
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)
