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 <[email protected]>

Author: ianthehat

internal/lsp/cache/check.go

@@ -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

internal/lsp/cache/file.go

@@ -9,14 +9,19 @@ import (
 	"go/ast"
 	"go/token"
 	"io/ioutil"
+	"path/filepath"
+	"strings"
 
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/span"
 )
 
 // 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
 	}

internal/lsp/cache/view.go

@@ -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,
-			view: v,
+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.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
+				}
+			}
 		}
-		v.files[uri] = f
 	}
-	return f
+	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)
+	}
 }

internal/lsp/source/diagnostics.go

@@ -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)