internal/lsp: stop requiring file kind when fetching a file
This change consolidates the FileKind into only the FileHandle.
Previously, it had been set in multiple places, which required users to
pass in a FileKind when fetching a file. This resulted in confusion,
particularly in places when users did not have access to the file kind.
Change-Id: I9e07d7320c46a21d453ffe108d1431a615706a71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213459
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/builtin.go b/internal/lsp/cache/builtin.go
index 00d6832..f9d2fe5 100644
--- a/internal/lsp/cache/builtin.go
+++ b/internal/lsp/cache/builtin.go
@@ -44,7 +44,7 @@
pkg := pkgs[0]
files := make(map[string]*ast.File)
for _, filename := range pkg.GoFiles {
- fh := v.session.GetFile(span.FileURI(filename), source.Go)
+ fh := v.session.GetFile(span.FileURI(filename))
ph := v.session.cache.ParseGoHandle(fh, source.ParseFull)
v.builtin.files = append(v.builtin.files, ph)
file, _, _, err := ph.Parse(ctx)
diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go
index 41d3d80..e6a185a 100644
--- a/internal/lsp/cache/cache.go
+++ b/internal/lsp/cache/cache.go
@@ -56,8 +56,8 @@
err error
}
-func (c *cache) GetFile(uri span.URI, kind source.FileKind) source.FileHandle {
- underlying := c.fs.GetFile(uri, kind)
+func (c *cache) GetFile(uri span.URI) source.FileHandle {
+ underlying := c.fs.GetFile(uri)
key := fileKey{
identity: underlying.Identity(),
}
diff --git a/internal/lsp/cache/external.go b/internal/lsp/cache/external.go
index 2498bf6..3f98b13 100644
--- a/internal/lsp/cache/external.go
+++ b/internal/lsp/cache/external.go
@@ -27,11 +27,12 @@
identity source.FileIdentity
}
-func (fs *nativeFileSystem) GetFile(uri span.URI, kind source.FileKind) source.FileHandle {
+func (fs *nativeFileSystem) GetFile(uri span.URI) source.FileHandle {
identifier := "DOES NOT EXIST"
if fi, err := os.Stat(uri.Filename()); err == nil {
identifier = fi.ModTime().String()
}
+ kind := source.DetectLanguage("", uri.Filename())
return &nativeFileHandle{
fs: fs,
identity: source.FileIdentity{
diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go
deleted file mode 100644
index 534892b..0000000
--- a/internal/lsp/cache/file.go
+++ /dev/null
@@ -1,49 +0,0 @@
-// Copyright 2018 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package cache
-
-import (
- "go/token"
- "path/filepath"
- "strings"
-
- "golang.org/x/tools/internal/lsp/source"
- "golang.org/x/tools/internal/span"
-)
-
-// fileBase holds the common functionality for all files.
-// It is intended to be embedded in the file implementations
-type fileBase struct {
- uris []span.URI
- fname string
- kind source.FileKind
-
- view *view
-}
-
-func dir(filename string) string {
- return strings.ToLower(filepath.Dir(filename))
-}
-
-func basename(filename string) string {
- return strings.ToLower(filepath.Base(filename))
-}
-
-func (f *fileBase) URI() span.URI {
- return f.uris[0]
-}
-
-func (f *fileBase) filename() string {
- return f.fname
-}
-
-// View returns the view associated with the file.
-func (f *fileBase) View() source.View {
- return f.view
-}
-
-func (f *fileBase) FileSet() *token.FileSet {
- return f.view.Session().Cache().FileSet()
-}
diff --git a/internal/lsp/cache/overlay.go b/internal/lsp/cache/overlay.go
index 8e6b794..ad07a18 100644
--- a/internal/lsp/cache/overlay.go
+++ b/internal/lsp/cache/overlay.go
@@ -41,7 +41,7 @@
return o.text, o.hash, nil
}
-func (s *session) updateOverlay(ctx context.Context, c source.FileModification) error {
+func (s *session) updateOverlay(ctx context.Context, c source.FileModification) (source.FileKind, error) {
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
@@ -54,18 +54,18 @@
kind = source.DetectLanguage(c.LanguageID, c.URI.Filename())
default:
if !ok {
- return errors.Errorf("updateOverlay: modifying unopened overlay %v", c.URI)
+ return -1, errors.Errorf("updateOverlay: modifying unopened overlay %v", c.URI)
}
kind = o.kind
}
if kind == source.UnknownKind {
- return errors.Errorf("updateOverlay: unknown file kind for %s", c.URI)
+ return -1, errors.Errorf("updateOverlay: unknown file kind for %s", c.URI)
}
// Closing a file just deletes its overlay.
if c.Action == source.Close {
delete(s.overlays, c.URI)
- return nil
+ return kind, nil
}
// If the file is on disk, check if its content is the same as the overlay.
@@ -77,15 +77,15 @@
var sameContentOnDisk bool
switch c.Action {
case source.Open:
- _, h, err := s.cache.GetFile(c.URI, kind).Read(ctx)
+ _, h, err := s.cache.GetFile(c.URI).Read(ctx)
sameContentOnDisk = (err == nil && h == hash)
case source.Save:
// Make sure the version and content (if present) is the same.
if o.version != c.Version {
- return errors.Errorf("updateOverlay: saving %s at version %v, currently at %v", c.URI, c.Version, o.version)
+ return -1, errors.Errorf("updateOverlay: saving %s at version %v, currently at %v", c.URI, c.Version, o.version)
}
if c.Text != nil && o.hash != hash {
- return errors.Errorf("updateOverlay: overlay %s changed on save", c.URI)
+ return -1, errors.Errorf("updateOverlay: overlay %s changed on save", c.URI)
}
sameContentOnDisk = true
}
@@ -98,7 +98,7 @@
hash: hash,
sameContentOnDisk: sameContentOnDisk,
}
- return nil
+ return kind, nil
}
func (s *session) readOverlay(uri span.URI) *overlay {
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 2c23aa8..00f59f4 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -256,7 +256,8 @@
ctx = telemetry.URI.With(ctx, c.URI)
// Perform the session-specific updates.
- if err := s.updateOverlay(ctx, c); err != nil {
+ kind, err := s.updateOverlay(ctx, c)
+ if err != nil {
return nil, err
}
@@ -265,12 +266,11 @@
if view.Ignore(c.URI) {
return nil, errors.Errorf("ignored file %v", c.URI)
}
- // Set the content for the file, only for didChange and didClose events.
- f, err := view.getFileLocked(ctx, c.URI)
- if err != nil {
+ // Make sure to add the file to the view.
+ if _, err := view.getFileLocked(ctx, c.URI); err != nil {
return nil, err
}
- snapshots = append(snapshots, view.invalidateContent(ctx, c.URI, f.kind, c.Action))
+ snapshots = append(snapshots, view.invalidateContent(ctx, c.URI, kind, c.Action))
}
return snapshots, nil
}
@@ -283,12 +283,12 @@
return open
}
-func (s *session) GetFile(uri span.URI, kind source.FileKind) source.FileHandle {
+func (s *session) GetFile(uri span.URI) source.FileHandle {
if overlay := s.readOverlay(uri); overlay != nil {
return overlay
}
// Fall back to the cache-level file system.
- return s.cache.GetFile(uri, kind)
+ return s.cache.GetFile(uri)
}
func (s *session) DidChangeOutOfBand(ctx context.Context, uri span.URI, action source.FileAction) bool {
@@ -296,10 +296,12 @@
if err != nil {
return false
}
- f, err := view.getFileLocked(ctx, uri)
- if err != nil {
+ // Make sure that the file is part of the view.
+ if _, err := view.getFileLocked(ctx, uri); err != nil {
return false
}
- view.invalidateContent(ctx, f.URI(), f.kind, action)
+ // TODO(golang/go#31553): Remove this when this issue has been resolved.
+ kind := source.DetectLanguage("", uri.Filename())
+ view.invalidateContent(ctx, uri, kind, action)
return true
}
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index d9c4142..7e3d3b7 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -8,6 +8,8 @@
"context"
"io"
"os"
+ "path/filepath"
+ "strings"
"sync"
"golang.org/x/tools/go/analysis"
@@ -92,7 +94,7 @@
return nil, nil, err
}
// Go directly to disk to get the correct FileHandle, since we just copied the file without invalidating the snapshot.
- tempfh := s.view.Session().Cache().GetFile(span.FileURI(s.view.modfiles.temp), source.Mod)
+ tempfh := s.view.Session().Cache().GetFile(span.FileURI(s.view.modfiles.temp))
if tempfh == nil {
return nil, nil, errors.Errorf("temporary go.mod filehandle is nil")
}
@@ -533,9 +535,7 @@
s.view.mu.Lock()
defer s.view.mu.Unlock()
- // TODO(rstambler): Should there be a version that provides a kind explicitly?
- kind := source.DetectLanguage("", uri.Filename())
- f, err := s.view.getFile(ctx, uri, kind)
+ f, err := s.view.getFile(ctx, uri)
if err != nil {
return nil, err
}
@@ -547,7 +547,7 @@
defer s.mu.Unlock()
if _, ok := s.files[f.URI()]; !ok {
- s.files[f.URI()] = s.view.session.GetFile(f.URI(), f.kind)
+ s.files[f.URI()] = s.view.session.GetFile(f.URI())
}
return s.files[f.URI()]
}
@@ -633,7 +633,7 @@
result.files[k] = v
}
// Handle the invalidated file; it may have new contents or not exist.
- currentFH := s.view.session.GetFile(withoutURI, withoutFileKind)
+ currentFH := s.view.session.GetFile(withoutURI)
if _, _, err := currentFH.Read(ctx); os.IsNotExist(err) {
delete(result.files, withoutURI)
} else {
@@ -680,6 +680,10 @@
return result
}
+func dir(filename string) string {
+ return strings.ToLower(filepath.Dir(filename))
+}
+
func (s *snapshot) ID() uint64 {
return s.id
}
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index c546ff7..f335c9c 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -96,6 +96,28 @@
initializationError error
}
+// fileBase holds the common functionality for all files.
+// It is intended to be embedded in the file implementations
+type fileBase struct {
+ uris []span.URI
+ fname string
+
+ view *view
+}
+
+func (f *fileBase) URI() span.URI {
+ return f.uris[0]
+}
+
+func (f *fileBase) filename() string {
+ return f.fname
+}
+
+func (f *fileBase) addURI(uri span.URI) int {
+ f.uris = append(f.uris, uri)
+ return len(f.uris)
+}
+
// modfiles holds the real and temporary go.mod files that are attributed to a view.
type modfiles struct {
real, temp string
@@ -278,7 +300,7 @@
// and modules included by a replace directive. Return true if
// any of these file versions do not match.
for filename, version := range v.modFileVersions {
- if version != v.fileVersion(filename, source.Mod) {
+ if version != v.fileVersion(filename) {
return true
}
}
@@ -297,17 +319,101 @@
// and modules included by a replace directive in the resolver.
for _, mod := range r.ModsByModPath {
if (mod.Main || mod.Replace != nil) && mod.GoMod != "" {
- v.modFileVersions[mod.GoMod] = v.fileVersion(mod.GoMod, source.Mod)
+ v.modFileVersions[mod.GoMod] = v.fileVersion(mod.GoMod)
}
}
}
-func (v *view) fileVersion(filename string, kind source.FileKind) string {
+func (v *view) fileVersion(filename string) string {
uri := span.FileURI(filename)
- fh := v.session.GetFile(uri, kind)
+ fh := v.session.GetFile(uri)
return fh.Identity().String()
}
+func (v *view) mapFile(uri span.URI, f *fileBase) {
+ v.filesByURI[uri] = f
+ if f.addURI(uri) == 1 {
+ basename := basename(f.filename())
+ v.filesByBase[basename] = append(v.filesByBase[basename], f)
+ }
+}
+
+func basename(filename string) string {
+ return strings.ToLower(filepath.Base(filename))
+}
+
+// FindFile returns the file if the given URI is already a part of the view.
+func (v *view) findFileLocked(ctx context.Context, uri span.URI) *fileBase {
+ v.mu.Lock()
+ defer v.mu.Unlock()
+
+ f, err := v.findFile(uri)
+ if err != nil {
+ return nil
+ }
+ return f
+}
+
+// getFileLocked returns a File for the given URI. It will always succeed because it
+// adds the file to the managed set if needed.
+func (v *view) getFileLocked(ctx context.Context, uri span.URI) (*fileBase, error) {
+ v.mu.Lock()
+ defer v.mu.Unlock()
+
+ return v.getFile(ctx, uri)
+}
+
+// getFile is the unlocked internal implementation of GetFile.
+func (v *view) getFile(ctx context.Context, uri span.URI) (*fileBase, error) {
+ f, err := v.findFile(uri)
+ if err != nil {
+ return nil, err
+ } else if f != nil {
+ return f, nil
+ }
+ f = &fileBase{
+ view: v,
+ fname: uri.Filename(),
+ }
+ v.mapFile(uri, f)
+ return f, nil
+}
+
+// findFile checks the cache for any file matching the given uri.
+//
+// An error is only returned for an irreparable failure, for example, if the
+// filename in question does not exist.
+func (v *view) findFile(uri span.URI) (*fileBase, error) {
+ if f := v.filesByURI[uri]; f != nil {
+ // a perfect match
+ return f, nil
+ }
+ // no exact match stored, time to do some real work
+ // check for any files with the same basename
+ fname := uri.Filename()
+ basename := basename(fname)
+ if candidates := v.filesByBase[basename]; candidates != nil {
+ pathStat, err := os.Stat(fname)
+ if os.IsNotExist(err) {
+ return nil, err
+ }
+ if err != nil {
+ return nil, nil // the file may exist, return without an error
+ }
+ 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, nil
+ }
+ }
+ }
+ }
+ // no file with a matching name was found, it wasn't in our cache
+ return nil, nil
+}
+
func (v *view) Shutdown(ctx context.Context) {
v.session.removeView(ctx, v)
}
@@ -427,94 +533,6 @@
v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx)
}
-// FindFile returns the file if the given URI is already a part of the view.
-func (v *view) findFileLocked(ctx context.Context, uri span.URI) *fileBase {
- v.mu.Lock()
- defer v.mu.Unlock()
-
- f, err := v.findFile(uri)
- if err != nil {
- return nil
- }
- return f
-}
-
-// getFileLocked returns a File for the given URI. It will always succeed because it
-// adds the file to the managed set if needed.
-func (v *view) getFileLocked(ctx context.Context, uri span.URI) (*fileBase, error) {
- v.mu.Lock()
- defer v.mu.Unlock()
-
- // TODO(rstambler): Should there be a version that provides a kind explicitly?
- kind := source.DetectLanguage("", uri.Filename())
- return v.getFile(ctx, uri, kind)
-}
-
-// getFile is the unlocked internal implementation of GetFile.
-func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) (*fileBase, error) {
- f, err := v.findFile(uri)
- if err != nil {
- return nil, err
- } else if f != nil {
- return f, nil
- }
- f = &fileBase{
- view: v,
- fname: uri.Filename(),
- kind: kind,
- }
- v.mapFile(uri, f)
- return f, nil
-}
-
-// findFile checks the cache for any file matching the given uri.
-//
-// An error is only returned for an irreparable failure, for example, if the
-// filename in question does not exist.
-func (v *view) findFile(uri span.URI) (*fileBase, error) {
- if f := v.filesByURI[uri]; f != nil {
- // a perfect match
- return f, nil
- }
- // no exact match stored, time to do some real work
- // check for any files with the same basename
- fname := uri.Filename()
- basename := basename(fname)
- if candidates := v.filesByBase[basename]; candidates != nil {
- pathStat, err := os.Stat(fname)
- if os.IsNotExist(err) {
- return nil, err
- }
- if err != nil {
- return nil, nil // the file may exist, return without an error
- }
- 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, nil
- }
- }
- }
- }
- // no file with a matching name was found, it wasn't in our cache
- return nil, nil
-}
-
-func (f *fileBase) addURI(uri span.URI) int {
- f.uris = append(f.uris, uri)
- return len(f.uris)
-}
-
-func (v *view) mapFile(uri span.URI, f *fileBase) {
- v.filesByURI[uri] = f
- if f.addURI(uri) == 1 {
- basename := basename(f.filename())
- v.filesByBase[basename] = append(v.filesByBase[basename], f)
- }
-}
-
func (v *view) FindPosInPackage(searchpkg source.Package, pos token.Pos) (*ast.File, source.Package, error) {
tok := v.session.cache.fset.File(pos)
if tok == nil {
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 541d9e2..3dd8ec9 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -235,7 +235,7 @@
// FileSystem is the interface to something that provides file contents.
type FileSystem interface {
// GetFile returns a handle for the specified file.
- GetFile(uri span.URI, kind FileKind) FileHandle
+ GetFile(uri span.URI) FileHandle
}
// ParseGoHandle represents a handle to the AST for a file.
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index 3795e70..2f2ad81 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -144,7 +144,7 @@
}
func (s *Server) applyIncrementalChanges(ctx context.Context, uri span.URI, changes []protocol.TextDocumentContentChangeEvent) ([]byte, error) {
- content, _, err := s.session.GetFile(uri, source.UnknownKind).Read(ctx)
+ content, _, err := s.session.GetFile(uri).Read(ctx)
if err != nil {
return nil, jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "file not found (%v)", err)
}
