internal/lsp: fix support for watching changed files

This is the beginning of the CLs to refactor the file watching code with
the normal text synchronization code. This hasn't yet been tested other
than with some minimal local testing, so follow-up CLs will be needed.

Updates golang/go#31553

Change-Id: Id081ecc59dd2903057164171bd95f0dc07baa5f1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214277
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2020-01-09 22:45:06 -05:00
parent 4a54ec1d38
commit 86d412b4c6
8 changed files with 121 additions and 172 deletions

View File

@ -20,9 +20,9 @@ type overlay struct {
version float64
kind source.FileKind
// sameContentOnDisk is true if a file has been saved on disk,
// 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.
sameContentOnDisk bool
saved bool
}
func (o *overlay) FileSystem() source.FileSystem {
@ -42,6 +42,11 @@ func (o *overlay) Read(ctx context.Context) ([]byte, string, error) {
}
func (s *session) updateOverlay(ctx context.Context, c source.FileModification) (source.FileKind, error) {
// Make sure that the file was not changed on disk.
if c.OnDisk {
return source.UnknownKind, errors.Errorf("updateOverlay called for an on-disk change: %s", c.URI)
}
s.overlayMu.Lock()
defer s.overlayMu.Unlock()
@ -90,13 +95,13 @@ func (s *session) updateOverlay(ctx context.Context, c source.FileModification)
sameContentOnDisk = true
}
s.overlays[c.URI] = &overlay{
session: s,
uri: c.URI,
version: c.Version,
text: text,
kind: kind,
hash: hash,
sameContentOnDisk: sameContentOnDisk,
session: s,
uri: c.URI,
version: c.Version,
text: text,
kind: kind,
hash: hash,
saved: sameContentOnDisk,
}
return kind, nil
}
@ -119,7 +124,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 {
if overlay.saved {
continue
}
overlays[uri.Filename()] = overlay.text

View File

@ -258,24 +258,47 @@ 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) ([]source.Snapshot, error) {
func (s *session) DidModifyFile(ctx context.Context, c source.FileModification) (snapshots []source.Snapshot, err error) {
ctx = telemetry.URI.With(ctx, c.URI)
// Perform the session-specific updates.
kind, err := s.updateOverlay(ctx, c)
if err != nil {
return nil, err
// Update overlays only if the file was changed in the editor.
var kind source.FileKind
if !c.OnDisk {
kind, err = s.updateOverlay(ctx, c)
if err != nil {
return nil, err
}
}
var snapshots []source.Snapshot
for _, view := range s.viewsOf(c.URI) {
if view.Ignore(c.URI) {
return nil, errors.Errorf("ignored file %v", c.URI)
}
// Make sure to add the file to the view.
if _, err := view.getFileLocked(c.URI); err != nil {
// If the file was changed or deleted on disk,
// do nothing if the view isn't already aware of the file.
if c.OnDisk {
switch c.Action {
case source.Change, source.Delete:
if !view.knownFile(c.URI) {
continue
}
}
}
// Make sure that the file is added to the view.
f, err := view.getFileLocked(c.URI)
if err != nil {
return nil, err
}
// If the file change was on disk, the file kind is not known.
if c.OnDisk {
// If the file was already known in the snapshot,
// then use the already known file kind. Otherwise,
// detect the file kind. This should only be needed for file creates.
if fh := view.getSnapshot().findFileHandle(ctx, f); fh != nil {
kind = fh.Identity().Kind
} else {
kind = source.DetectLanguage("", c.URI.Filename())
}
}
snapshots = append(snapshots, view.invalidateContent(ctx, c.URI, kind, c.Action))
}
return snapshots, nil
@ -296,18 +319,3 @@ 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) DidChangeOutOfBand(ctx context.Context, uri span.URI, action source.FileAction) bool {
view, err := s.viewOf(uri)
if err != nil {
return false
}
// Make sure that the file is part of the view.
if _, err := view.getFileLocked(uri); err != nil {
return false
}
// 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
}

View File

@ -522,15 +522,6 @@ func (s *snapshot) getFileURIs() []span.URI {
return uris
}
// FindFile returns the file if the given URI is already a part of the view.
func (s *snapshot) FindFile(uri span.URI) source.FileHandle {
f, err := s.view.findFileLocked(uri)
if f == nil || err != nil {
return nil
}
return s.getFileHandle(f)
}
// GetFile returns a File for the given URI. It will always succeed because it
// adds the file to the managed set if needed.
func (s *snapshot) GetFile(uri span.URI) (source.FileHandle, error) {
@ -551,7 +542,14 @@ func (s *snapshot) getFileHandle(f *fileBase) source.FileHandle {
return s.files[f.URI()]
}
func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutFileKind source.FileKind) *snapshot {
func (s *snapshot) findFileHandle(ctx context.Context, f *fileBase) source.FileHandle {
s.mu.Lock()
defer s.mu.Unlock()
return s.files[f.URI()]
}
func (s *snapshot) clone(ctx context.Context, withoutURI span.URI) *snapshot {
s.mu.Lock()
defer s.mu.Unlock()

View File

@ -342,12 +342,13 @@ 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(uri span.URI) (*fileBase, error) {
// knownFile returns true if the given URI is already a part of the view.
func (v *view) knownFile(uri span.URI) bool {
v.mu.Lock()
defer v.mu.Unlock()
return v.findFile(uri)
f, err := v.findFile(uri)
return f != nil && err == nil
}
// getFileLocked returns a File for the given URI. It will always succeed because it
@ -510,8 +511,13 @@ func (v *view) invalidateContent(ctx context.Context, uri span.URI, kind source.
// Cancel all still-running previous requests, since they would be
// operating on stale data.
//
// TODO(rstambler): All actions should lead to cancellation,
// but this will only be possible when all text synchronization events
// trigger diagnostics.
switch action {
case source.Change, source.Close:
case source.Save:
default:
v.cancelBackground()
}
@ -522,7 +528,7 @@ func (v *view) invalidateContent(ctx context.Context, uri span.URI, kind source.
v.snapshotMu.Lock()
defer v.snapshotMu.Unlock()
v.snapshot = v.snapshot.clone(ctx, uri, kind)
v.snapshot = v.snapshot.clone(ctx, uri)
return v.snapshot
}

View File

@ -29,7 +29,7 @@ func (s *Server) diagnoseSnapshot(ctx context.Context, snapshot source.Snapshot)
go func(id string) {
ph, err := snapshot.PackageHandle(ctx, id)
if err != nil {
log.Error(ctx, "diagnoseSnapshot: no PackageHandle for workspace package", err, telemetry.Package.Of(id))
log.Error(ctx, "diagnoseSnapshot: no PackageHandle for package", err, telemetry.Package.Of(id))
return
}
reports, _, err := source.PackageDiagnostics(ctx, snapshot, ph, false, snapshot.View().Options().DisabledAnalyses)
@ -37,7 +37,6 @@ func (s *Server) diagnoseSnapshot(ctx context.Context, snapshot source.Snapshot)
log.Error(ctx, "diagnoseSnapshot: no diagnostics", err, telemetry.Package.Of(ph.ID()))
return
}
// Don't publish empty diagnostics.
s.publishReports(ctx, reports, false)
}(id)
}
@ -122,7 +121,7 @@ func (s *Server) publishReports(ctx context.Context, reports map[source.FileIden
// If the file is open, and we've already delivered diagnostics for
// a later version, do nothing. This only works for open files,
// since their contents in the editor are the source of truth.
if s.session.IsOpen(fileID.URI) && fileID.Version < delivered.version {
if s.session.IsOpen(fileID.URI); fileID.Version < delivered.version {
continue
}
geqVersion := fileID.Version >= delivered.version && delivered.version > 0

View File

@ -28,10 +28,6 @@ type Snapshot interface {
// if it is not already part of the view.
GetFile(uri span.URI) (FileHandle, error)
// FindFile returns the file object for a given URI if it is
// already part of the view.
FindFile(uri span.URI) FileHandle
// Analyze runs the analyses for the given package at this snapshot.
Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) ([]*Error, error)
@ -157,16 +153,13 @@ type Session interface {
// content from the underlying cache if no overlay is present.
FileSystem
// IsOpen returns whether the editor currently has a file open.
// 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.
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.
DidChangeOutOfBand(ctx context.Context, uri span.URI, action FileAction) bool
// Options returns a copy of the SessionOptions for this session.
Options() Options
@ -179,8 +172,12 @@ type FileModification struct {
URI span.URI
Action FileAction
// OnDisk is true if a watched file is changed on disk.
// If true, Version will be -1 and Text will be nil.
OnDisk bool
// Version will be -1 and Text will be nil when they are not supplied,
// specifically on textDocument/didClose.
// specifically on textDocument/didClose and for on-disk changes.
Version float64
Text []byte

View File

@ -12,6 +12,7 @@ import (
"golang.org/x/tools/internal/jsonrpc2"
"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"
errors "golang.org/x/xerrors"
)
@ -87,6 +88,39 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo
return nil
}
func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) error {
// Keep track of each change's view and final snapshot.
views := make(map[source.View]source.Snapshot)
for _, change := range params.Changes {
uri := span.NewURI(change.URI)
ctx := telemetry.File.With(ctx, uri)
// Do nothing if the file is open in the editor.
// The editor is the source of truth.
if s.session.IsOpen(uri) {
continue
}
snapshots, err := s.session.DidModifyFile(ctx, source.FileModification{
URI: uri,
Action: changeTypeToFileAction(change.Type),
OnDisk: true,
})
if err != nil {
return err
}
snapshot, _, err := snapshotOf(s.session, uri, snapshots)
if err != nil {
return err
}
views[snapshot.View()] = snapshot
}
// Diagnose all resulting snapshots.
for view, snapshot := range views {
go s.diagnoseSnapshot(view.BackgroundContext(), snapshot)
}
return nil
}
func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocumentParams) error {
c := source.FileModification{
URI: span.NewURI(params.TextDocument.URI),
@ -189,3 +223,15 @@ func (s *Server) applyIncrementalChanges(ctx context.Context, uri span.URI, chan
}
return content, nil
}
func changeTypeToFileAction(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
}

View File

@ -1,110 +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 lsp
import (
"context"
"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"
"golang.org/x/tools/internal/telemetry/log"
)
func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) error {
for _, change := range params.Changes {
uri := span.NewURI(change.URI)
ctx := telemetry.File.With(ctx, uri)
for _, view := range s.session.Views() {
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, action) {
// If we had been tracking the given file,
// recompute diagnostics to reflect updated file contents.
snapshot := view.Snapshot()
fh, err := snapshot.GetFile(uri)
if err != nil {
return err
}
switch fh.Identity().Kind {
case source.Go:
go s.diagnoseFile(snapshot, fh)
case source.Mod:
go s.diagnoseModfile(snapshot)
}
return nil
}
case source.Delete:
snapshot := view.Snapshot()
fh := snapshot.FindFile(uri)
// If we have never seen this file before, there is nothing to do.
if fh == nil {
continue
}
phs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
log.Error(ctx, "didChangeWatchedFiles: CheckPackageHandles", err, telemetry.File)
continue
}
ph, err := source.WidestCheckPackageHandle(phs)
if err != nil {
log.Error(ctx, "didChangeWatchedFiles: WidestCheckPackageHandle", err, telemetry.File)
continue
}
// Find a different file in the same package we can use to trigger diagnostics.
// TODO(rstambler): Allow diagnostics to be called per-package to avoid this.
var otherFile source.FileHandle
for _, pgh := range ph.CompiledGoFiles() {
if pgh.File().Identity().URI == fh.Identity().URI {
continue
}
if f := snapshot.FindFile(pgh.File().Identity().URI); f != nil && s.session.IsOpen(fh.Identity().URI) {
otherFile = f
break
}
}
// Notify the view of the deletion of the file.
s.session.DidChangeOutOfBand(ctx, uri, action)
// If this was the only file in the package, clear its diagnostics.
if otherFile == nil {
if err := s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
URI: protocol.NewURI(uri),
Version: fh.Identity().Version,
}); err != nil {
log.Error(ctx, "failed to clear diagnostics", err, telemetry.URI.Of(uri))
}
return nil
}
// Refresh diagnostics for the package the file belonged to.
go s.diagnoseFile(view.Snapshot(), otherFile)
}
}
}
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
}