gopls/internal/cache: detect and reinit on workspace vendor changes

No matter where the go.work file is, the addition or deletion of
workspace-vendored modules mandates reinitialization.

Add the necessary watch pattern, and detect that reinitialization is
required in snapshot.clone. The latter required threading through the
file modification, so that we can detect creations and deletions
explicitly.

This is a great example of where having a realistic fake file watcher
paid off.

Fixes golang/go#63375

Change-Id: Id3658e85bd2d915639d24fed31aba2d54ebc75e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/560717
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>

gopls/internal/cache/session.go

@@ -719,7 +719,7 @@ func (s *Session) ResetView(ctx context.Context, uri protocol.DocumentURI) (*Vie
 // TODO(rfindley): what happens if this function fails? It must leave us in a
 // broken state, which we should surface to the user, probably as a request to
 // restart gopls.
-func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modification) (map[*View][]protocol.DocumentURI, error) {
+func (s *Session) DidModifyFiles(ctx context.Context, modifications []file.Modification) (map[*View][]protocol.DocumentURI, error) {
 	s.viewMu.Lock()
 	defer s.viewMu.Unlock()
 
@@ -728,7 +728,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio
 	// This is done while holding viewMu because the set of open files affects
 	// the set of views, and to prevent views from seeing updated file content
 	// before they have processed invalidations.
-	replaced, err := s.updateOverlays(ctx, changes)
+	replaced, err := s.updateOverlays(ctx, modifications)
 	if err != nil {
 		return nil, err
 	}
@@ -739,7 +739,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio
 	checkViews := false
 
 	changed := make(map[protocol.DocumentURI]file.Handle)
-	for _, c := range changes {
+	for _, c := range modifications {
 		fh := mustReadFile(ctx, s, c.URI)
 		changed[c.URI] = fh
 
@@ -857,7 +857,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio
 	// We only want to run fast-path diagnostics (i.e. diagnoseChangedFiles) once
 	// for each changed file, in its best view.
 	viewsToDiagnose := map[*View][]protocol.DocumentURI{}
-	for _, mod := range changes {
+	for _, mod := range modifications {
 		v, err := s.viewOfLocked(ctx, mod.URI)
 		if err != nil {
 			// bestViewForURI only returns an error in the event of context
@@ -874,7 +874,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio
 	// ...but changes may be relevant to other views, for example if they are
 	// changes to a shared package.
 	for _, v := range s.views {
-		_, release, needsDiagnosis := s.invalidateViewLocked(ctx, v, StateChange{Files: changed})
+		_, release, needsDiagnosis := s.invalidateViewLocked(ctx, v, StateChange{Modifications: modifications, Files: changed})
 		release()
 
 		if needsDiagnosis || checkViews {

gopls/internal/cache/snapshot.go

@@ -952,6 +952,12 @@ func (s *Snapshot) fileWatchingGlobPatterns() map[protocol.RelativePattern]unit
 
 	var dirs []string
 	if s.view.moduleMode() {
+		if s.view.typ == GoWorkView {
+			workVendorDir := filepath.Join(s.view.gowork.Dir().Path(), "vendor")
+			workVendorURI := protocol.URIFromPath(workVendorDir)
+			patterns[protocol.RelativePattern{BaseURI: workVendorURI, Pattern: watchGoFiles}] = unit{}
+		}
+
 		// In module mode, watch directories containing active modules, and collect
 		// these dirs for later filtering the set of known directories.
 		//
@@ -1728,10 +1734,14 @@ func (s *Snapshot) clone(ctx, bgCtx context.Context, changed StateChange, done f
 	// one or more modules may have moved into or out of the
 	// vendor tree after 'go mod vendor' or 'rm -fr vendor/'.
 	//
+	// In this case, we consider the actual modification to see if was a creation
+	// or deletion.
+	//
 	// TODO(rfindley): revisit the location of this check.
-	for uri := range changedFiles {
-		if inVendor(uri) && s.initialErr != nil ||
-			strings.HasSuffix(string(uri), "/vendor/modules.txt") {
+	for _, mod := range changed.Modifications {
+		if inVendor(mod.URI) && (mod.Action == file.Create || mod.Action == file.Delete) ||
+			strings.HasSuffix(string(mod.URI), "/vendor/modules.txt") {
+
 			reinit = true
 			break
 		}
@@ -1741,6 +1751,11 @@ func (s *Snapshot) clone(ctx, bgCtx context.Context, changed StateChange, done f
 	// they exist. Importantly, we don't call ReadFile here: consider the case
 	// where a file is added on disk; we don't want to read the newly added file
 	// into the old snapshot, as that will break our change detection below.
+	//
+	// TODO(rfindley): it may be more accurate to rely on the modification type
+	// here, similarly to what we do for vendored files above. If we happened not
+	// to have read a file in the previous snapshot, that's not the same as it
+	// actually being created.
 	oldFiles := make(map[protocol.DocumentURI]file.Handle)
 	for uri := range changedFiles {
 		if fh, ok := s.files.get(uri); ok {

gopls/internal/cache/view.go

@@ -763,6 +763,7 @@ func (s *Snapshot) initialize(ctx context.Context, firstAttempt bool) {
 // By far the most common of these is a change to file state, but a query of
 // module upgrade information or vulnerabilities also affects gopls' behavior.
 type StateChange struct {
+	Modifications  []file.Modification // if set, the raw modifications originating this change
 	Files          map[protocol.DocumentURI]file.Handle
 	ModuleUpgrades map[protocol.DocumentURI]map[string]string
 	Vulns          map[protocol.DocumentURI]*vulncheck.Result

gopls/internal/test/integration/workspace/workspace_test.go

@@ -259,13 +259,6 @@ func TestWorkspaceVendoring(t *testing.T) {
 		env.AfterChange()
 		env.OpenFile("moda/a/a.go")
 		env.RunGoCommand("work", "vendor")
-
-		// Make an on-disk go.mod change to force a workspace reinitialization.
-		// This will be fixed in a follow-up CL.
-		env.OpenFile("moda/a/go.mod")
-		env.EditBuffer("moda/a/go.mod", protocol.TextEdit{NewText: "// arbitrary\n"})
-		env.SaveBuffer("moda/a/go.mod")
-
 		env.AfterChange()
 		loc := env.GoToDefinition(env.RegexpSearch("moda/a/a.go", "b.(Hello)"))
 		const want = "vendor/b.com/b/b.go"