diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go index e3a1396954..41d3d807c2 100644 --- a/internal/lsp/cache/cache.go +++ b/internal/lsp/cache/cache.go @@ -76,11 +76,10 @@ func (c *cache) GetFile(uri span.URI, kind source.FileKind) source.FileHandle { func (c *cache) NewSession(ctx context.Context) source.Session { index := atomic.AddInt64(&sessionIndex, 1) s := &session{ - cache: c, - id: strconv.FormatInt(index, 10), - options: source.DefaultOptions, - overlays: make(map[span.URI]*overlay), - filesWatchMap: NewWatchMap(), + cache: c, + id: strconv.FormatInt(index, 10), + options: source.DefaultOptions, + overlays: make(map[span.URI]*overlay), } debug.AddSession(debugSession{s}) return s diff --git a/internal/lsp/cache/overlay.go b/internal/lsp/cache/overlay.go index e793f15d94..a0e7a75819 100644 --- a/internal/lsp/cache/overlay.go +++ b/internal/lsp/cache/overlay.go @@ -5,10 +5,12 @@ package cache import ( + "bytes" "context" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" + errors "golang.org/x/xerrors" ) type overlay struct { @@ -40,43 +42,50 @@ func (o *overlay) Read(ctx context.Context) ([]byte, string, error) { return o.data, o.hash, nil } -func (s *session) setOverlay(f source.File, version float64, data []byte) { +func (s *session) setOverlay(uri span.URI, version float64, data []byte) error { s.overlayMu.Lock() - defer func() { - s.overlayMu.Unlock() - s.filesWatchMap.Notify(f.URI(), source.Change) - }() + defer s.overlayMu.Unlock() - if data == nil { - delete(s.overlays, f.URI()) - return + o, ok := s.overlays[uri] + if !ok { + return errors.Errorf("setting overlay for unopened file %s", uri) } - - s.overlays[f.URI()] = &overlay{ + s.overlays[uri] = &overlay{ session: s, - uri: f.URI(), - kind: f.Kind(), + uri: uri, + kind: o.kind, data: data, hash: hashContents(data), version: version, } + return nil } -func (s *session) clearOverlay(uri span.URI) { +func (s *session) closeOverlay(uri span.URI) error { + s.openFiles.Delete(uri) + s.overlayMu.Lock() defer s.overlayMu.Unlock() + _, ok := s.overlays[uri] + if !ok { + return errors.Errorf("closing unopened overlay %s", uri) + } delete(s.overlays, uri) + return nil } -// openOverlay adds the file content to the overlay. -// It also checks if the provided content is equivalent to the file's content on disk. -func (s *session) openOverlay(ctx context.Context, uri span.URI, kind source.FileKind, version float64, data []byte) { +func (s *session) openOverlay(ctx context.Context, uri span.URI, languageID string, version float64, data []byte) error { + kind := source.DetectLanguage(languageID, uri.Filename()) + if kind == source.UnknownKind { + return errors.Errorf("openOverlay: unknown file kind for %s", uri) + } + + s.openFiles.Store(uri, true) + s.overlayMu.Lock() - defer func() { - s.overlayMu.Unlock() - s.filesWatchMap.Notify(uri, source.Open) - }() + defer s.overlayMu.Unlock() + s.overlays[uri] = &overlay{ session: s, uri: uri, @@ -91,6 +100,30 @@ func (s *session) openOverlay(ctx context.Context, uri span.URI, kind source.Fil s.overlays[uri].sameContentOnDisk = true } } + return nil +} + +func (s *session) saveOverlay(uri span.URI, version float64, data []byte) error { + s.overlayMu.Lock() + defer s.overlayMu.Unlock() + + o, ok := s.overlays[uri] + if !ok { + return errors.Errorf("saveOverlay: unopened overlay %s", uri) + } + if o.version != version { + return errors.Errorf("saveOverlay: saving %s at version %v, currently at %v", uri, version, o.version) + } + if data != nil { + if !bytes.Equal(o.data, data) { + return errors.Errorf("saveOverlay: overlay %s changed on save", uri) + } + o.data = data + } + o.sameContentOnDisk = true + o.version = version + + return nil } func (s *session) readOverlay(uri span.URI) *overlay { @@ -110,6 +143,7 @@ func (s *session) buildOverlay() map[string][]byte { overlays := make(map[string][]byte) for uri, overlay := range s.overlays { + // TODO(rstambler): Make sure not to send overlays outside of the current view. if overlay.sameContentOnDisk { continue } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 32d44226e1..c8389bae67 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -6,7 +6,6 @@ package cache import ( "context" - "path/filepath" "strconv" "strings" "sync" @@ -35,8 +34,7 @@ type session struct { overlayMu sync.Mutex overlays map[span.URI]*overlay - openFiles sync.Map - filesWatchMap *WatchMap + openFiles sync.Map } func (s *session) Options() source.Options { @@ -275,79 +273,41 @@ func (s *session) dropView(ctx context.Context, v *view) (int, error) { return -1, errors.Errorf("view %s for %v not found", v.Name(), v.Folder()) } -func (s *session) DidModifyFile(ctx context.Context, c source.FileModification) error { +func (s *session) DidModifyFile(ctx context.Context, c source.FileModification) ([]source.Snapshot, error) { ctx = telemetry.URI.With(ctx, c.URI) + + // Perform session-specific actions. + switch c.Action { + case source.Open: + if err := s.openOverlay(ctx, c.URI, c.LanguageID, c.Version, c.Text); err != nil { + return nil, err + } + case source.Change: + if err := s.setOverlay(c.URI, c.Version, c.Text); err != nil { + return nil, err + } + case source.Save: + if err := s.saveOverlay(c.URI, c.Version, c.Text); err != nil { + return nil, err + } + case source.Close: + if err := s.closeOverlay(c.URI); err != nil { + return nil, err + } + } + var snapshots []source.Snapshot for _, view := range s.viewsOf(c.URI) { - switch c.Action { - case source.Open: - kind := source.DetectLanguage(c.LanguageID, c.URI.Filename()) - if kind == source.UnknownKind { - return errors.Errorf("DidModifyFile: unknown file kind for %s", c.URI) - } - return s.didOpen(ctx, c.URI, kind, c.Version, c.Text) - case source.Save: - s.didSave(c.URI, c.Version) - case source.Close: - s.didClose(c.URI) + 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. - switch c.Action { - case source.Change, source.Close: - f, err := view.GetFile(ctx, c.URI) - if err != nil { - return err - } - view.setContent(ctx, f, c.Version, c.Text) + f, err := view.GetFile(ctx, c.URI) + if err != nil { + return nil, err } + snapshots = append(snapshots, view.invalidateContent(ctx, f, c.Action)) } - return nil -} - -// TODO: Propagate the language ID through to the view. -func (s *session) didOpen(ctx context.Context, uri span.URI, kind source.FileKind, version float64, text []byte) error { - ctx = telemetry.File.With(ctx, uri) - - // Files with _ prefixes are ignored. - if strings.HasPrefix(filepath.Base(uri.Filename()), "_") { - for _, view := range s.views { - view.ignoredURIsMu.Lock() - view.ignoredURIs[uri] = struct{}{} - view.ignoredURIsMu.Unlock() - } - return nil - } - - // Make sure that the file gets added to the session's file watch map. - view, err := s.bestView(uri) - if err != nil { - return err - } - if _, err := view.GetFile(ctx, uri); err != nil { - return err - } - - // Mark the file as open. - s.openFiles.Store(uri, true) - - // Read the file on disk and compare it to the text provided. - // If it is the same as on disk, we can avoid sending it as an overlay to go/packages. - s.openOverlay(ctx, uri, kind, version, text) - return nil -} - -func (s *session) didSave(uri span.URI, version float64) { - s.overlayMu.Lock() - defer s.overlayMu.Unlock() - - if overlay, ok := s.overlays[uri]; ok { - overlay.sameContentOnDisk = true - overlay.version = version - } -} - -func (s *session) didClose(uri span.URI) { - s.openFiles.Delete(uri) + return snapshots, nil } func (s *session) IsOpen(uri span.URI) bool { @@ -364,5 +324,14 @@ func (s *session) GetFile(uri span.URI, kind source.FileKind) source.FileHandle } func (s *session) DidChangeOutOfBand(ctx context.Context, uri span.URI, action source.FileAction) bool { - return s.filesWatchMap.Notify(uri, action) + view, err := s.viewOf(uri) + if err != nil { + return false + } + f, err := view.GetFile(ctx, uri) + if err != nil { + return false + } + view.invalidateContent(ctx, f, action) + return true } diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 7aae7d2f1b..0062904b54 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -590,22 +590,6 @@ func (s *snapshot) ID() uint64 { return s.id } -// invalidateContent invalidates the content of a Go file, -// including any position and type information that depends on it. -// It returns true if we were already tracking the given file, false otherwise. -func (v *view) invalidateContent(ctx context.Context, f source.File, action source.FileAction) { - // This should be the only time we hold the view's snapshot lock for any period of time. - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() - - // If we are deleting a file, make sure to clear out the overlay. - if action == source.Delete { - v.session.clearOverlay(f.URI()) - } - - v.snapshot = v.snapshot.clone(ctx, f) -} - func (s *snapshot) clearAndRebuildImportGraph() { s.mu.Lock() defer s.mu.Unlock() diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 76eda8c05c..c6cea0b59e 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -12,6 +12,7 @@ import ( "go/token" "os" "os/exec" + "path/filepath" "reflect" "strings" "sync" @@ -25,7 +26,6 @@ import ( "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" "golang.org/x/tools/internal/telemetry/tag" - "golang.org/x/tools/internal/xcontext" errors "golang.org/x/xerrors" ) @@ -313,6 +313,13 @@ func (v *view) Ignore(uri span.URI) bool { defer v.ignoredURIsMu.Unlock() _, ok := v.ignoredURIs[uri] + + // Files with _ prefixes are always ignored. + if !ok && strings.HasPrefix(filepath.Base(uri.Filename()), "_") { + v.ignoredURIs[uri] = struct{}{} + return true + } + return ok } @@ -338,21 +345,31 @@ func (v *view) getSnapshot() *snapshot { return v.snapshot } -// setContent sets the overlay contents for a file. -func (v *view) setContent(ctx context.Context, f source.File, version float64, content []byte) { +// invalidateContent invalidates the content of a Go file, +// including any position and type information that depends on it. +// It returns true if we were already tracking the given file, false otherwise. +func (v *view) invalidateContent(ctx context.Context, f source.File, action source.FileAction) source.Snapshot { + // Cancel all still-running previous requests, since they would be + // operating on stale data. + switch action { + case source.Change, source.Close: + v.cancelBackground() + } + + // This should be the only time we hold the view's snapshot lock for any period of time. + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + v.snapshot = v.snapshot.clone(ctx, f) + return v.snapshot +} + +func (v *view) cancelBackground() { v.mu.Lock() defer v.mu.Unlock() - if v.Ignore(f.URI()) { - return - } - - // Cancel all still-running previous requests, since they would be - // operating on stale data. v.cancel() v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx) - - v.session.setOverlay(f, version, content) } // FindFile returns the file if the given URI is already a part of the view. @@ -390,11 +407,6 @@ func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) fname: uri.Filename(), kind: kind, } - v.session.filesWatchMap.Watch(uri, func(action source.FileAction) bool { - ctx := xcontext.Detach(ctx) - v.invalidateContent(ctx, f, action) - return true // we're the only watcher - }) v.mapFile(uri, f) return f, nil } diff --git a/internal/lsp/cache/watcher.go b/internal/lsp/cache/watcher.go deleted file mode 100644 index b955e1a4b2..0000000000 --- a/internal/lsp/cache/watcher.go +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2019 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 ( - "sync" - - "golang.org/x/tools/internal/lsp/source" -) - -type watcher struct { - id uint64 - callback func(action source.FileAction) bool -} - -type WatchMap struct { - mu sync.Mutex - nextID uint64 - watchers map[interface{}][]watcher -} - -func NewWatchMap() *WatchMap { - return &WatchMap{watchers: make(map[interface{}][]watcher)} -} - -func (w *WatchMap) Watch(key interface{}, callback func(source.FileAction) bool) func() { - w.mu.Lock() - defer w.mu.Unlock() - id := w.nextID - w.nextID++ - w.watchers[key] = append(w.watchers[key], watcher{ - id: id, - callback: callback, - }) - return func() { - // unwatch if invoked - w.mu.Lock() - defer w.mu.Unlock() - // find and delete the watcher entry - entries := w.watchers[key] - for i, entry := range entries { - if entry.id == id { - // found it - entries[i] = entries[len(entries)-1] - entries = entries[:len(entries)-1] - } - } - } -} - -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() - entries := w.watchers[key] - entriesCopy := make([]watcher, len(entries)) - copy(entriesCopy, entries) - w.mu.Unlock() - - var result bool - for _, entry := range entriesCopy { - result = entry.callback(action) || result - } - return result -} diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 664279e6c9..ce636ead54 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -61,7 +61,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { if kind != source.Go { continue } - if err := session.DidModifyFile(ctx, source.FileModification{ + if _, err := session.DidModifyFile(ctx, source.FileModification{ URI: span.FileURI(filename), Action: source.Open, Version: -1, diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 41bf6bcdeb..1697756995 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -65,7 +65,7 @@ func testSource(t *testing.T, exporter packagestest.Exporter) { if kind != source.Go { continue } - if err := session.DidModifyFile(ctx, source.FileModification{ + if _, err := session.DidModifyFile(ctx, source.FileModification{ URI: span.FileURI(filename), Action: source.Open, Version: -1, diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index bcd1ec7d1b..c64cae3f06 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -169,7 +169,7 @@ type Session interface { IsOpen(uri span.URI) bool // DidModifyFile reports a file modification to the session. - DidModifyFile(ctx context.Context, c FileModification) error + DidModifyFile(ctx context.Context, c FileModification) ([]Snapshot, error) // DidChangeOutOfBand is called when a file under the root folder changes. // If the file was open in the editor, it returns true. diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 91125354d4..09a0f9ca46 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -19,16 +19,17 @@ import ( func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) error { // Confirm that the file's language ID is related to Go. uri := span.NewURI(params.TextDocument.URI) - if err := s.session.DidModifyFile(ctx, source.FileModification{ + snapshots, err := s.session.DidModifyFile(ctx, source.FileModification{ URI: uri, Action: source.Open, Version: params.TextDocument.Version, Text: []byte(params.TextDocument.Text), LanguageID: params.TextDocument.LanguageID, - }); err != nil { + }) + if err != nil { return err } - view, err := s.session.ViewOf(uri) + snapshot, view, err := snapshotOf(s.session, uri, snapshots) if err != nil { return err } @@ -37,28 +38,7 @@ func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocume return err } // Always run diagnostics when a file is opened. - return s.diagnose(view.Snapshot(), f) -} - -func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocumentParams) error { - c := source.FileModification{ - URI: span.NewURI(params.TextDocument.URI), - Action: source.Save, - Version: params.TextDocument.Version, - } - if params.Text != nil { - c.Text = []byte(*params.Text) - } - return s.session.DidModifyFile(ctx, c) -} - -func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { - return s.session.DidModifyFile(ctx, source.FileModification{ - URI: span.NewURI(params.TextDocument.URI), - Action: source.Close, - Version: -1, - Text: nil, - }) + return s.diagnose(snapshot, f) } func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDocumentParams) error { @@ -67,15 +47,16 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo if err != nil { return err } - if err := s.session.DidModifyFile(ctx, source.FileModification{ + snapshots, err := s.session.DidModifyFile(ctx, source.FileModification{ URI: uri, Action: source.Change, Version: params.TextDocument.Version, Text: text, - }); err != nil { + }) + if err != nil { return err } - view, err := s.session.ViewOf(uri) + snapshot, view, err := snapshotOf(s.session, uri, snapshots) if err != nil { return err } @@ -92,7 +73,44 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo return err } // Always update diagnostics after a file change. - return s.diagnose(view.Snapshot(), f) + return s.diagnose(snapshot, f) +} + +func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocumentParams) error { + c := source.FileModification{ + URI: span.NewURI(params.TextDocument.URI), + Action: source.Save, + Version: params.TextDocument.Version, + } + if params.Text != nil { + c.Text = []byte(*params.Text) + } + _, err := s.session.DidModifyFile(ctx, c) + return err +} + +func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { + _, err := s.session.DidModifyFile(ctx, source.FileModification{ + URI: span.NewURI(params.TextDocument.URI), + Action: source.Close, + Version: -1, + Text: nil, + }) + return err +} + +// snapshotOf returns the snapshot corresponding to the view for the given file URI. +func snapshotOf(session source.Session, uri span.URI, snapshots []source.Snapshot) (source.Snapshot, source.View, error) { + view, err := session.ViewOf(uri) + if err != nil { + return nil, nil, err + } + for _, s := range snapshots { + if s.View() == view { + return s, view, nil + } + } + return nil, nil, errors.Errorf("bestSnapshot: no snapshot for %s", uri) } func (s *Server) wasFirstChange(uri span.URI) bool {