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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>

Author: stamblerre

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
 }

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)
 		}

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)
 		}

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

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
 	}