internal/lsp: start handling watched file deletes

Now when a file is deleted we force the file's packages to refresh
go/packages metadata, and kick off diagnostics.

I made a couple other changes to watched file handling:
- Kick off diagnostics in a goroutine to match how DidChange works.
  This will allow us to work through big sets of file changes faster,
  and will save duplicated work once type checking can be canceled.
- Don't assume a watched file is only part of one view.

Two interesting cases we don't handle yet:
- If the deleted file was the only file in the package, we don't
  currently update diagnostics for dependent packages. This requires
  rejiggering how diagnostics are invoked a bit.
- If the deleted file is still open in the editor and then later
  closed, we don't trigger metadata/diagnostics refresh on DidClose.

Updates golang/go#31553

Change-Id: I65768614c24d9800ffea149ccdbdbd3cb7b2f3d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193121
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/cache/load.go

@@ -321,20 +321,3 @@ func (v *view) link(ctx context.Context, g *importGraph) error {
 	}
 	return nil
 }
-
-func identicalFileHandles(old, new []source.ParseGoHandle) bool {
-	if len(old) != len(new) {
-		return false
-	}
-	oldByIdentity := make(map[string]struct{}, len(old))
-	for _, ph := range old {
-		oldByIdentity[hashParseKey(ph)] = struct{}{}
-	}
-	for _, ph := range new {
-		if _, found := oldByIdentity[hashParseKey(ph)]; !found {
-			return false
-		}
-		delete(oldByIdentity, hashParseKey(ph))
-	}
-	return len(oldByIdentity) == 0
-}

internal/lsp/cache/session.go

@@ -14,6 +14,7 @@ import (
 	"sync/atomic"
 
 	"golang.org/x/tools/internal/lsp/debug"
+	"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"
@@ -336,8 +337,15 @@ func (s *session) buildOverlay() map[string][]byte {
 	return overlays
 }
 
-func (s *session) DidChangeOutOfBand(uri span.URI) {
+func (s *session) DidChangeOutOfBand(ctx context.Context, f source.GoFile, changeType protocol.FileChangeType) {
-	s.filesWatchMap.Notify(uri)
+	if changeType == protocol.Deleted {
+		// After a deletion we must invalidate the package's metadata to
+		// force a go/packages invocation to refresh the package's file
+		// list.
+		f.(*goFile).invalidateMeta(ctx)
+	}
+
+	s.filesWatchMap.Notify(f.URI())
 }
 
 func (o *overlay) FileSystem() source.FileSystem {

internal/lsp/cache/view.go

@@ -365,6 +365,31 @@ func (f *goFile) invalidateContent(ctx context.Context) {
 	f.handle = nil
 }
 
+// invalidateMeta invalides package metadata for all files in f's
+// package. This forces f's package's metadata to be reloaded next
+// time the package is checked.
+func (f *goFile) invalidateMeta(ctx context.Context) {
+	pkgs, err := f.GetPackages(ctx)
+	if err != nil {
+		log.Error(ctx, "invalidateMeta: GetPackages", err, telemetry.File.Of(f.URI()))
+		return
+	}
+
+	for _, pkg := range pkgs {
+		for _, pgh := range pkg.GetHandles() {
+			uri := pgh.File().Identity().URI
+			if gof, _ := f.view.FindFile(ctx, uri).(*goFile); gof != nil {
+				gof.mu.Lock()
+				gof.meta = nil
+				gof.mu.Unlock()
+			}
+		}
+		f.view.mcache.mu.Lock()
+		delete(f.view.mcache.packages, packageID(pkg.ID()))
+		f.view.mcache.mu.Unlock()
+	}
+}
+
 // remove invalidates a package and its reverse dependencies in the view's
 // package cache. It is assumed that the caller has locked both the mutexes
 // of both the mcache and the pcache.

internal/lsp/source/view.go

@@ -14,6 +14,7 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/internal/imports"
+	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/span"
 )
 
@@ -189,7 +190,7 @@ type Session interface {
 
 	// DidChangeOutOfBand is called when a file under the root folder
 	// changes. The file is not necessarily open in the editor.
-	DidChangeOutOfBand(uri span.URI)
+	DidChangeOutOfBand(ctx context.Context, f GoFile, change protocol.FileChangeType)
 
 	// Options returns a copy of the SessionOptions for this session.
 	Options() SessionOptions
@@ -270,10 +271,10 @@ type GoFile interface {
 	// GetPackages returns the CheckPackageHandles of the packages that this file belongs to.
 	GetCheckPackageHandles(ctx context.Context) ([]CheckPackageHandle, error)
 
-	// GetPackage returns the CheckPackageHandle for the package that this file belongs to.
+	// GetPackage returns the Package that this file belongs to.
 	GetPackage(ctx context.Context) (Package, error)
 
-	// GetPackages returns the CheckPackageHandles of the packages that this file belongs to.
+	// GetPackages returns the Packages that this file belongs to.
 	GetPackages(ctx context.Context) ([]Package, error)
 
 	// GetActiveReverseDeps returns the active files belonging to the reverse

internal/lsp/watched_files.go

@@ -8,9 +8,11 @@ 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"
-	"golang.org/x/tools/internal/telemetry/tag"
+	"golang.org/x/tools/internal/telemetry/trace"
 )
 
 func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) error {
@@ -22,31 +24,75 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
 	for _, change := range params.Changes {
 		uri := span.NewURI(change.URI)
 
-		switch change.Type {
+		ctx := telemetry.File.With(ctx, uri)
-		case protocol.Changed:
+
-			view := s.session.ViewOf(uri)
+		for _, view := range s.session.Views() {
+			gof, _ := view.FindFile(ctx, uri).(source.GoFile)
 
 			// If we have never seen this file before, there is nothing to do.
-			if view.FindFile(ctx, uri) == nil {
+			if gof == nil {
-				break
+				continue
 			}
 
-			log.Print(ctx, "watched file changed", tag.Of("uri", uri))
-
 			// 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
 			}
 
-			s.session.DidChangeOutOfBand(uri)
+			switch change.Type {
+			case protocol.Changed:
+				log.Print(ctx, "watched file changed", telemetry.File)
 
-			// Refresh diagnostics to reflect updated file contents.
+				s.session.DidChangeOutOfBand(ctx, gof, change.Type)
-			s.Diagnostics(ctx, view, uri)
+
-		case protocol.Created:
+				// Refresh diagnostics to reflect updated file contents.
-			log.Print(ctx, "watched file created", tag.Of("uri", uri))
+				go func(view source.View) {
-		case protocol.Deleted:
+					ctx := view.BackgroundContext()
-			log.Print(ctx, "watched file deleted", tag.Of("uri", uri))
+					ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
+					defer done()
+					s.Diagnostics(ctx, view, uri)
+				}(view)
+			case protocol.Created:
+				log.Print(ctx, "watched file created", telemetry.File)
+			case protocol.Deleted:
+				log.Print(ctx, "watched file deleted", telemetry.File)
+
+				pkg, err := gof.GetPackage(ctx)
+				if err != nil {
+					log.Error(ctx, "didChangeWatchedFiles: GetPackage", err, telemetry.File)
+					continue
+				}
+
+				// Find a different file in the same package we can use to
+				// trigger diagnostics.
+				var otherFile source.GoFile
+				for _, pgh := range pkg.GetHandles() {
+					ident := pgh.File().Identity()
+					if ident.URI == gof.URI() {
+						continue
+					}
+					otherFile, _ = view.FindFile(ctx, ident.URI).(source.GoFile)
+					if otherFile != nil {
+						break
+					}
+				}
+
+				s.session.DidChangeOutOfBand(ctx, gof, change.Type)
+
+				if otherFile != nil {
+					// Refresh diagnostics to reflect updated file contents.
+					go func(view source.View) {
+						ctx := view.BackgroundContext()
+						ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
+						defer done()
+						s.Diagnostics(ctx, view, otherFile.URI())
+					}(view)
+				} else {
+					// TODO: Handle case when there is no other file (i.e. deleted
+					//       file was the only file in the package).
+				}
+			}
 		}
 	}