diff --git a/internal/lsp/cache/overlay.go b/internal/lsp/cache/overlay.go deleted file mode 100644 index 6d3dbd146c..0000000000 --- a/internal/lsp/cache/overlay.go +++ /dev/null @@ -1,118 +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 ( - "context" - - "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/span" - errors "golang.org/x/xerrors" -) - -type overlay struct { - session *session - uri span.URI - text []byte - hash string - version float64 - kind source.FileKind - - // saved is true if a file has been saved on disk, - // and therefore does not need to be part of the overlay sent to go/packages. - saved bool -} - -func (o *overlay) FileSystem() source.FileSystem { - return o.session -} - -func (o *overlay) Identity() source.FileIdentity { - return source.FileIdentity{ - URI: o.uri, - Identifier: o.hash, - Version: o.version, - Kind: o.kind, - } -} -func (o *overlay) Read(ctx context.Context) ([]byte, string, error) { - return o.text, o.hash, nil -} - -func (s *session) updateOverlay(ctx context.Context, c source.FileModification) error { - // Make sure that the file was not changed on disk. - if c.OnDisk { - return errors.Errorf("updateOverlay called for an on-disk change: %s", c.URI) - } - - s.overlayMu.Lock() - defer s.overlayMu.Unlock() - - o, ok := s.overlays[c.URI] - - // Determine the file kind on open, otherwise, assume it has been cached. - var kind source.FileKind - switch c.Action { - case source.Open: - kind = source.DetectLanguage(c.LanguageID, c.URI.Filename()) - default: - if !ok { - return 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) - } - - // Closing a file just deletes its overlay. - if c.Action == source.Close { - delete(s.overlays, c.URI) - return nil - } - - // If the file is on disk, check if its content is the same as the overlay. - text := c.Text - if text == nil { - text = o.text - } - hash := hashContents(text) - var sameContentOnDisk bool - switch c.Action { - case source.Open: - _, 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) - } - if c.Text != nil && o.hash != hash { - return errors.Errorf("updateOverlay: overlay %s changed on save", c.URI) - } - sameContentOnDisk = true - } - s.overlays[c.URI] = &overlay{ - session: s, - uri: c.URI, - version: c.Version, - text: text, - kind: kind, - hash: hash, - saved: sameContentOnDisk, - } - return nil -} - -func (s *session) readOverlay(uri span.URI) *overlay { - s.overlayMu.Lock() - defer s.overlayMu.Unlock() - - // We might have the content saved in an overlay. - if overlay, ok := s.overlays[uri]; ok { - return overlay - } - return nil -} diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 2c009dd963..6111fa5b0c 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -33,6 +33,35 @@ type session struct { overlays map[span.URI]*overlay } +type overlay struct { + session *session + uri span.URI + text []byte + hash string + version float64 + kind source.FileKind + + // saved is true if a file has been saved on disk, + // and therefore does not need to be part of the overlay sent to go/packages. + saved bool +} + +func (o *overlay) FileSystem() source.FileSystem { + return o.session +} + +func (o *overlay) Identity() source.FileIdentity { + return source.FileIdentity{ + URI: o.uri, + Identifier: o.hash, + Version: o.version, + Kind: o.kind, + } +} +func (o *overlay) Read(ctx context.Context) ([]byte, string, error) { + return o.text, o.hash, nil +} + func (s *session) Options() source.Options { return s.options } @@ -249,14 +278,17 @@ func (s *session) dropView(ctx context.Context, v *view) (int, error) { } func (s *session) DidModifyFiles(ctx context.Context, changes []source.FileModification) ([]source.Snapshot, error) { - views := make(map[*view][]span.URI) + views := make(map[*view]map[span.URI]source.FileHandle) + overlays, err := s.updateOverlays(ctx, changes) + if err != nil { + return nil, err + } for _, c := range changes { - // Only update overlays for in-editor changes. - if !c.OnDisk { - if err := s.updateOverlay(ctx, c); err != nil { - return nil, err - } + // Do nothing if the file is open in the editor and we receive + // an on-disk action. The editor is the source of truth. + if s.isOpen(c.URI) && c.OnDisk { + continue } for _, view := range s.viewsOf(c.URI) { if view.Ignore(c.URI) { @@ -276,7 +308,14 @@ func (s *session) DidModifyFiles(ctx context.Context, changes []source.FileModif if _, err := view.getFile(c.URI); err != nil { return nil, err } - views[view] = append(views[view], c.URI) + if _, ok := views[view]; !ok { + views[view] = make(map[span.URI]source.FileHandle) + } + if o, ok := overlays[c.URI]; ok { + views[view][c.URI] = o + } else { + views[view][c.URI] = s.cache.GetFile(c.URI) + } } } var snapshots []source.Snapshot @@ -286,7 +325,7 @@ func (s *session) DidModifyFiles(ctx context.Context, changes []source.FileModif return snapshots, nil } -func (s *session) IsOpen(uri span.URI) bool { +func (s *session) isOpen(uri span.URI) bool { s.overlayMu.Lock() defer s.overlayMu.Unlock() @@ -294,6 +333,80 @@ func (s *session) IsOpen(uri span.URI) bool { return open } +func (s *session) updateOverlays(ctx context.Context, changes []source.FileModification) (map[span.URI]*overlay, error) { + s.overlayMu.Lock() + defer s.overlayMu.Unlock() + + for _, c := range changes { + // Don't update overlays for on-disk changes. + if c.OnDisk { + continue + } + + o, ok := s.overlays[c.URI] + + // Determine the file kind on open, otherwise, assume it has been cached. + var kind source.FileKind + switch c.Action { + case source.Open: + kind = source.DetectLanguage(c.LanguageID, c.URI.Filename()) + default: + if !ok { + return nil, errors.Errorf("updateOverlays: modifying unopened overlay %v", c.URI) + } + kind = o.kind + } + if kind == source.UnknownKind { + return nil, errors.Errorf("updateOverlays: unknown file kind for %s", c.URI) + } + + // Closing a file just deletes its overlay. + if c.Action == source.Close { + delete(s.overlays, c.URI) + continue + } + + // If the file is on disk, check if its content is the same as the overlay. + text := c.Text + if text == nil { + text = o.text + } + hash := hashContents(text) + var sameContentOnDisk bool + switch c.Action { + case source.Open: + _, 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 nil, errors.Errorf("updateOverlays: saving %s at version %v, currently at %v", c.URI, c.Version, o.version) + } + if c.Text != nil && o.hash != hash { + return nil, errors.Errorf("updateOverlays: overlay %s changed on save", c.URI) + } + sameContentOnDisk = true + } + o = &overlay{ + session: s, + uri: c.URI, + version: c.Version, + text: text, + kind: kind, + hash: hash, + saved: sameContentOnDisk, + } + s.overlays[c.URI] = o + } + + // Get the overlays for each change while the session's overlay map is locked. + overlays := make(map[span.URI]*overlay) + for _, c := range changes { + overlays[c.URI] = s.overlays[c.URI] + } + return overlays, nil +} + func (s *session) GetFile(uri span.URI) source.FileHandle { if overlay := s.readOverlay(uri); overlay != nil { return overlay @@ -301,3 +414,13 @@ func (s *session) GetFile(uri span.URI) source.FileHandle { // Fall back to the cache-level file system. return s.cache.GetFile(uri) } + +func (s *session) readOverlay(uri span.URI) *overlay { + s.overlayMu.Lock() + defer s.overlayMu.Unlock() + + if overlay, ok := s.overlays[uri]; ok { + return overlay + } + return nil +} diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 7488a98bb1..41a69502df 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -108,6 +108,9 @@ func (s *snapshot) Config(ctx context.Context) *packages.Config { } func (s *snapshot) buildOverlay() map[string][]byte { + s.mu.Lock() + defer s.mu.Unlock() + overlays := make(map[string][]byte) for uri, fh := range s.files { overlay, ok := fh.(*overlay) @@ -569,15 +572,24 @@ func (s *snapshot) GetFile(uri span.URI) (source.FileHandle, error) { if err != nil { return nil, err } + s.mu.Lock() defer s.mu.Unlock() if _, ok := s.files[f.URI()]; !ok { - s.files[f.URI()] = s.view.session.GetFile(f.URI()) + s.files[f.URI()] = s.view.session.cache.GetFile(uri) } return s.files[f.URI()], nil } +func (s *snapshot) IsOpen(uri span.URI) bool { + s.mu.Lock() + defer s.mu.Unlock() + + _, open := s.files[uri].(*overlay) + return open +} + func (s *snapshot) awaitLoaded(ctx context.Context) error { // Do not return results until the snapshot's view has been initialized. s.view.awaitInitialized(ctx) @@ -685,7 +697,7 @@ func contains(views []*view, view *view) bool { return false } -func (s *snapshot) clone(ctx context.Context, withoutURIs []span.URI) *snapshot { +func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.FileHandle) *snapshot { s.mu.Lock() defer s.mu.Unlock() @@ -716,7 +728,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs []span.URI) *snapshot // If an ID's value is true, invalidate its metadata too. transitiveIDs := make(map[packageID]bool) - for _, withoutURI := range withoutURIs { + for withoutURI, currentFH := range withoutURIs { directIDs := map[packageID]struct{}{} // Collect all of the package IDs that correspond to the given file. @@ -724,8 +736,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs []span.URI) *snapshot for _, id := range s.ids[withoutURI] { directIDs[id] = struct{}{} } - // Get the current and original FileHandles for this URI. - currentFH := s.view.session.GetFile(withoutURI) + // The original FileHandle for this URI is cached on the snapshot. originalFH := s.files[withoutURI] // Check if the file's package name or imports have changed, diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index fc9066bb4c..4525853edc 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -239,8 +239,9 @@ func (v *view) buildBuiltinPackage(ctx context.Context, goFiles []string) error uri := span.FileURI(goFiles[0]) v.addIgnoredFile(uri) // to avoid showing diagnostics for builtin.go - // Get the FileHandle through the session to avoid adding it to the snapshot. - pgh := v.session.cache.ParseGoHandle(v.session.GetFile(uri), source.ParseFull) + // Get the FileHandle through the cache to avoid adding it to the snapshot + // and to get the file content from disk. + pgh := v.session.cache.ParseGoHandle(v.session.cache.GetFile(uri), source.ParseFull) fset := v.session.cache.fset h := v.session.cache.store.Bind(pgh.File().Identity(), func(ctx context.Context) interface{} { data := &builtinPackageData{} @@ -540,7 +541,7 @@ func (v *view) awaitInitialized(ctx context.Context) { // 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, uris []span.URI) source.Snapshot { +func (v *view) invalidateContent(ctx context.Context, uris map[span.URI]source.FileHandle) source.Snapshot { // Detach the context so that content invalidation cannot be canceled. ctx = xcontext.Detach(ctx) diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index fcc777c1c7..411344f3da 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -88,7 +88,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA // Only run analyses for packages with open files. withAnalyses := alwaysAnalyze for _, fh := range ph.CompiledGoFiles() { - if s.session.IsOpen(fh.File().Identity().URI) { + if snapshot.IsOpen(fh.File().Identity().URI) { withAnalyses = true } } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index fefbb9aa8a..2c8f3e9396 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -105,7 +105,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missi // If no file is associated with the error, pick an open file from the package. if e.URI.Filename() == "" { for _, ph := range pkg.CompiledGoFiles() { - if snapshot.View().Session().IsOpen(ph.File().Identity().URI) { + if snapshot.IsOpen(ph.File().Identity().URI) { e.URI = ph.File().Identity().URI } } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 3f0e30d02e..21c7a1d416 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -33,6 +33,10 @@ type Snapshot interface { // if it is not already part of the view. GetFile(uri span.URI) (FileHandle, error) + // IsOpen returns whether the editor currently has a file open, + // and if its contents are saved on disk or not. + IsOpen(uri span.URI) bool + // Analyze runs the analyses for the given package at this snapshot. Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) ([]*Error, error) @@ -164,10 +168,6 @@ type Session interface { // content from the underlying cache if no overlay is present. FileSystem - // IsOpen returns whether the editor currently has a file open, - // and if its contents are saved on disk or not. - IsOpen(uri span.URI) bool - // DidModifyFile reports a file modification to the session. // It returns the resulting snapshots, a guaranteed one per view. DidModifyFiles(ctx context.Context, changes []FileModification) ([]Snapshot, error) diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 3d95a85291..652132d2c1 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -65,15 +65,8 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) error { var modifications []source.FileModification for _, change := range params.Changes { - uri := span.NewURI(change.URI) - - // Do nothing if the file is open in the editor. - // The editor is the source of truth. - if s.session.IsOpen(uri) { - continue - } modifications = append(modifications, source.FileModification{ - URI: uri, + URI: span.NewURI(change.URI), Action: changeTypeToFileAction(change.Type), OnDisk: true, })