From e54d0edf47019ad6bf745b362d59786ef596b455 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Wed, 22 Jan 2020 21:34:21 -0500 Subject: [PATCH] internal/lsp: support batched on-disk changes in source.DidModifyFiles We don't yet propagate these batched changes in text_synchronization.go, but this is the next step in moving towards a batched approach. Updates golang/go#31553 Change-Id: Id6496af9d5422cc50ccb995f81c71ec1886f965a Reviewed-on: https://go-review.googlesource.com/c/tools/+/215907 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/cache/session.go | 59 +++++++++++++++++----------- internal/lsp/lsp_test.go | 14 ++++--- internal/lsp/source/source_test.go | 14 ++++--- internal/lsp/source/view.go | 2 +- internal/lsp/text_synchronization.go | 12 +++--- 5 files changed, 60 insertions(+), 41 deletions(-) diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 58fe475ecba..ff423a1baa8 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -13,7 +13,6 @@ import ( "golang.org/x/tools/internal/lsp/debug" "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/trace" "golang.org/x/tools/internal/xcontext" @@ -253,34 +252,48 @@ 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) (snapshots []source.Snapshot, err error) { - ctx = telemetry.URI.With(ctx, c.URI) +func (s *session) DidModifyFiles(ctx context.Context, changes []source.FileModification) ([]source.Snapshot, error) { + views := make(map[*view][]span.URI) + saves := make(map[*view]bool) - // Update overlays only if the file was changed in the editor. - if !c.OnDisk { - if err := s.updateOverlay(ctx, c); err != nil { - return nil, err - } - } - for _, view := range s.viewsOf(c.URI) { - if view.Ignore(c.URI) { - return nil, errors.Errorf("ignored file %v", c.URI) + 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 + } } - // 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 + for _, view := range s.viewsOf(c.URI) { + if view.Ignore(c.URI) { + return nil, errors.Errorf("ignored file %v", c.URI) + } + // If the file change is on-disk and not a create, + // make sure the file is known to the view already. + if c.OnDisk { + switch c.Action { + case source.Change, source.Delete: + if !view.knownFile(c.URI) { + continue + } + case source.Save: + panic("save considered an on-disk change") } } + // Make sure that the file is added to the view. + if _, err := view.getFile(c.URI); err != nil { + return nil, err + } + views[view] = append(views[view], c.URI) + saves[view] = len(changes) == 1 && !changes[0].OnDisk && changes[0].Action == source.Save } - // Make sure that the file is added to the view. - if _, err := view.getFile(c.URI); err != nil { - return nil, err + } + var snapshots []source.Snapshot + for view, uris := range views { + containsFileSave, ok := saves[view] + if !ok { + panic("unknown view") } - snapshots = append(snapshots, view.invalidateContent(ctx, []span.URI{c.URI}, c.Action == source.Save)) + snapshots = append(snapshots, view.invalidateContent(ctx, uris, containsFileSave)) } return snapshots, nil } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 6384e0c9d5d..be6cef54aee 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -64,12 +64,14 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { if kind != source.Go { continue } - if _, err := session.DidModifyFile(ctx, source.FileModification{ - URI: span.FileURI(filename), - Action: source.Open, - Version: -1, - Text: content, - LanguageID: "go", + if _, err := session.DidModifyFiles(ctx, []source.FileModification{ + { + URI: span.FileURI(filename), + Action: source.Open, + Version: -1, + Text: content, + LanguageID: "go", + }, }); err != nil { t.Fatal(err) } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 892d46dbf59..0db99477f1f 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -65,12 +65,14 @@ func testSource(t *testing.T, exporter packagestest.Exporter) { if kind != source.Go { continue } - if _, err := session.DidModifyFile(ctx, source.FileModification{ - URI: span.FileURI(filename), - Action: source.Open, - Version: -1, - Text: content, - LanguageID: "go", + if _, err := session.DidModifyFiles(ctx, []source.FileModification{ + { + URI: span.FileURI(filename), + Action: source.Open, + Version: -1, + Text: content, + LanguageID: "go", + }, }); err != nil { t.Fatal(err) } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index ed4ffef0bca..ade81d42f12 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -164,7 +164,7 @@ type Session interface { IsOpen(uri span.URI) bool // DidModifyFile reports a file modification to the session. - DidModifyFile(ctx context.Context, c FileModification) ([]Snapshot, error) + DidModifyFiles(ctx context.Context, changes []FileModification) ([]Snapshot, error) // Options returns a copy of the SessionOptions for this session. Options() Options diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 96fe881cb23..8e38f478a16 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -69,10 +69,12 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did if s.session.IsOpen(uri) { continue } - snapshots, err := s.session.DidModifyFile(ctx, source.FileModification{ - URI: uri, - Action: changeTypeToFileAction(change.Type), - OnDisk: true, + snapshots, err := s.session.DidModifyFiles(ctx, []source.FileModification{ + { + URI: uri, + Action: changeTypeToFileAction(change.Type), + OnDisk: true, + }, }) if err != nil { return err @@ -114,7 +116,7 @@ func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocu } func (s *Server) didModifyFile(ctx context.Context, c source.FileModification) (source.Snapshot, error) { - snapshots, err := s.session.DidModifyFile(ctx, c) + snapshots, err := s.session.DidModifyFiles(ctx, []source.FileModification{c}) if err != nil { return nil, err }