1
0
mirror of https://github.com/golang/go synced 2024-11-18 16:04:44 -07:00

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>
This commit is contained in:
Rebecca Stambler 2019-11-13 13:14:21 -05:00
parent bd32a3e739
commit faa69481e7
7 changed files with 55 additions and 28 deletions

View File

@ -33,25 +33,30 @@ func (s *snapshot) CheckPackageHandles(ctx context.Context, f source.File) ([]so
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

View File

@ -15,7 +15,6 @@ import (
"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 @@ func (s *session) SetOverlay(uri span.URI, kind source.FileKind, version float64
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 @@ func (s *session) openOverlay(ctx context.Context, uri span.URI, kind source.Fil
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 @@ func (s *session) buildOverlay() map[string][]byte {
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 {

View File

@ -7,7 +7,6 @@ import (
"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 @@ func (s *snapshot) clone(ctx context.Context, withoutURI *span.URI, withoutTypes
// 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 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, changeType
// 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 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, changeType
}
}
}
// If a file has been explicitly created, make sure that its original file handle is nil.
originalFH = nil
}
@ -402,7 +400,7 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, changeType
}
// 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())
}

View File

@ -376,9 +376,9 @@ func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind)
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

View File

@ -7,12 +7,12 @@ package cache
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 @@ type WatchMap struct {
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) Watch(key interface{}, callback func(protocol.FileChangeType)
}
}
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 @@ func (w *WatchMap) Notify(key interface{}, changeType protocol.FileChangeType) b
var result bool
for _, entry := range entriesCopy {
result = entry.callback(changeType) || result
result = entry.callback(action) || result
}
return result
}

View File

@ -194,7 +194,7 @@ type Session interface {
// 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 @@ type Session interface {
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.

View File

@ -23,19 +23,20 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
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 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
}
// 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 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
}
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
}