internal/lsp/cache: add finer-grained control of file changes
This change is the first step in centralizing control of modifications
to different files, either within the workspace or outside of it. We add
a source.FileAction type to pass into the internal/lsp/cache package and
handle the difference between opening and creating a file.
Now that we load all packages in a workspace by default, we no longer
need to re-load a file on open. This CL should enable CL 206883 to work
correctly.
Change-Id: I2ddb21ca2dd33720d668066e73283f5629d02867
Reviewed-on: https://go-review.googlesource.com/c/tools/+/206888
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go
index f55a818..39c5a7c 100644
--- a/internal/lsp/cache/gofile.go
+++ b/internal/lsp/cache/gofile.go
@@ -33,25 +33,30 @@
fh := s.Handle(ctx, f)
// Determine if we need to type-check the package.
- m, cphs, load, check := s.shouldCheck(fh)
+ metadata, cphs, load, check := s.shouldCheck(fh)
// We may need to re-load package metadata.
// We only need to this if it has been invalidated, and is therefore unvailable.
if load {
var err error
- m, err = s.load(ctx, source.FileURI(f.URI()))
+ m, err := s.load(ctx, source.FileURI(f.URI()))
if err != nil {
return nil, err
}
// If load has explicitly returned nil metadata and no error,
// it means that we should not re-type-check the packages.
- if m == nil {
+ // Return the cached CheckPackageHandles, if we had them.
+ if m == nil && len(cphs) > 0 {
return cphs, nil
}
+ // If metadata was returned, from the load call, use it.
+ if m != nil {
+ metadata = m
+ }
}
if check {
var results []source.CheckPackageHandle
- for _, m := range m {
+ for _, m := range metadata {
cph, err := s.checkPackageHandle(ctx, m.id, source.ParseFull)
if err != nil {
return nil, err
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 1b8cd35..eb1c85c 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -15,7 +15,6 @@
"sync/atomic"
"golang.org/x/tools/internal/lsp/debug"
- "golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/lsp/telemetry"
"golang.org/x/tools/internal/span"
@@ -338,7 +337,7 @@
s.overlayMu.Lock()
defer func() {
s.overlayMu.Unlock()
- s.filesWatchMap.Notify(uri, protocol.Changed)
+ s.filesWatchMap.Notify(uri, source.Change)
}()
if data == nil {
@@ -374,7 +373,7 @@
s.overlayMu.Lock()
defer func() {
s.overlayMu.Unlock()
- s.filesWatchMap.Notify(uri, protocol.Created)
+ s.filesWatchMap.Notify(uri, source.Open)
}()
s.overlays[uri] = &overlay{
session: s,
@@ -418,8 +417,8 @@
return overlays
}
-func (s *session) DidChangeOutOfBand(ctx context.Context, uri span.URI, changeType protocol.FileChangeType) bool {
- return s.filesWatchMap.Notify(uri, changeType)
+func (s *session) DidChangeOutOfBand(ctx context.Context, uri span.URI, action source.FileAction) bool {
+ return s.filesWatchMap.Notify(uri, action)
}
func (o *overlay) FileSystem() source.FileSystem {
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index db332dc..dcd95e4 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -7,7 +7,6 @@
"sync"
"golang.org/x/tools/go/analysis"
- "golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/telemetry/log"
@@ -352,7 +351,7 @@
// invalidateContent invalidates the content of a Go file,
// including any position and type information that depends on it.
-func (v *view) invalidateContent(ctx context.Context, f source.File, changeType protocol.FileChangeType) bool {
+func (v *view) invalidateContent(ctx context.Context, f source.File, kind source.FileKind, action source.FileAction) bool {
var (
withoutTypes = make(map[span.URI]struct{})
withoutMetadata = make(map[span.URI]struct{})
@@ -370,8 +369,8 @@
// Get the original FileHandle for the URI, if it exists.
originalFH := v.snapshot.getFile(f.URI())
- switch changeType {
- case protocol.Created:
+ switch action {
+ case source.Create:
// If this is a file we don't yet know about,
// then we do not yet know what packages it should belong to.
// Make a rough estimate of what metadata to invalidate by finding the package IDs
@@ -388,7 +387,6 @@
}
}
}
- // If a file has been explicitly created, make sure that its original file handle is nil.
originalFH = nil
}
@@ -402,7 +400,7 @@
}
// Make sure to clear out the content if there has been a deletion.
- if changeType == protocol.Deleted {
+ if action == source.Delete {
v.session.clearOverlay(f.URI())
}
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 155a374..e2a4236 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -376,9 +376,9 @@
fname: uri.Filename(),
kind: kind,
}
- v.session.filesWatchMap.Watch(uri, func(changeType protocol.FileChangeType) bool {
+ v.session.filesWatchMap.Watch(uri, func(action source.FileAction) bool {
ctx := xcontext.Detach(ctx)
- return v.invalidateContent(ctx, f, changeType)
+ return v.invalidateContent(ctx, f, kind, action)
})
v.mapFile(uri, f)
return f, nil
diff --git a/internal/lsp/cache/watcher.go b/internal/lsp/cache/watcher.go
index 53fe4dc..b955e1a 100644
--- a/internal/lsp/cache/watcher.go
+++ b/internal/lsp/cache/watcher.go
@@ -7,12 +7,12 @@
import (
"sync"
- "golang.org/x/tools/internal/lsp/protocol"
+ "golang.org/x/tools/internal/lsp/source"
)
type watcher struct {
id uint64
- callback func(changeType protocol.FileChangeType) bool
+ callback func(action source.FileAction) bool
}
type WatchMap struct {
@@ -24,7 +24,8 @@
func NewWatchMap() *WatchMap {
return &WatchMap{watchers: make(map[interface{}][]watcher)}
}
-func (w *WatchMap) Watch(key interface{}, callback func(protocol.FileChangeType) bool) func() {
+
+func (w *WatchMap) Watch(key interface{}, callback func(source.FileAction) bool) func() {
w.mu.Lock()
defer w.mu.Unlock()
id := w.nextID
@@ -49,7 +50,7 @@
}
}
-func (w *WatchMap) Notify(key interface{}, changeType protocol.FileChangeType) bool {
+func (w *WatchMap) Notify(key interface{}, action source.FileAction) bool {
// Make a copy of the watcher callbacks so we don't need to hold
// the mutex during the callbacks (to avoid deadlocks).
w.mu.Lock()
@@ -60,7 +61,7 @@
var result bool
for _, entry := range entriesCopy {
- result = entry.callback(changeType) || result
+ result = entry.callback(action) || result
}
return result
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index aa1e2a8..16907b5 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -194,7 +194,7 @@
// DidChangeOutOfBand is called when a file under the root folder changes.
// If the file was open in the editor, it returns true.
- DidChangeOutOfBand(ctx context.Context, uri span.URI, change protocol.FileChangeType) bool
+ DidChangeOutOfBand(ctx context.Context, uri span.URI, action FileAction) bool
// Options returns a copy of the SessionOptions for this session.
Options() Options
@@ -203,6 +203,17 @@
SetOptions(Options)
}
+type FileAction int
+
+const (
+ Open = FileAction(iota)
+ Close
+ Change
+ Create
+ Delete
+ UnknownFileAction
+)
+
// View represents a single workspace.
// This is the level at which we maintain configuration like working directory
// and build tags.
diff --git a/internal/lsp/watched_files.go b/internal/lsp/watched_files.go
index b6238e0..10e1a9a 100644
--- a/internal/lsp/watched_files.go
+++ b/internal/lsp/watched_files.go
@@ -23,19 +23,20 @@
if !view.Options().WatchFileChanges {
continue
}
- switch change.Type {
- case protocol.Changed, protocol.Created:
+ action := toFileAction(change.Type)
+ switch action {
+ case source.Change, source.Create:
// If client has this file open, don't do anything.
// The client's contents must remain the source of truth.
if s.session.IsOpen(uri) {
break
}
- if s.session.DidChangeOutOfBand(ctx, uri, change.Type) {
+ if s.session.DidChangeOutOfBand(ctx, uri, action) {
// If we had been tracking the given file,
// recompute diagnostics to reflect updated file contents.
go s.diagnostics(view, uri)
}
- case protocol.Deleted:
+ case source.Delete:
f := view.FindFile(ctx, uri)
// If we have never seen this file before, there is nothing to do.
if f == nil {
@@ -65,7 +66,7 @@
}
// Notify the view of the deletion of the file.
- s.session.DidChangeOutOfBand(ctx, uri, change.Type)
+ s.session.DidChangeOutOfBand(ctx, uri, action)
// If this was the only file in the package, clear its diagnostics.
if otherFile == nil {
@@ -82,3 +83,15 @@
}
return nil
}
+
+func toFileAction(ct protocol.FileChangeType) source.FileAction {
+ switch ct {
+ case protocol.Changed:
+ return source.Change
+ case protocol.Created:
+ return source.Create
+ case protocol.Deleted:
+ return source.Delete
+ }
+ return source.UnknownFileAction
+}
