internal/lsp/cache: attach ModTidyHandle to snapshot instance

This change attaches a modTidyHandle to the snapshot instance and tries to reuse it as often as possible. It also modifies the modTidyKey to include the files that have not yet been saved. This was necessary because `go mod tidy` only runs using contents on disk.

So, if a user decides to add an import that is not in their modcache, we run diagnostics as normal and the ModTidyKey's imports field gets updated with the new import, we run `go mod tidy` but since the file was not saved yet, nothing changes. Then when the user eventually saves their file, we do not rerun `go mod tidy` because the imports hash has not changed from the time the file was in overlay to the time the file was saved on disk. To be able to account for this, we need the invalidate the ModTidyKey when imports change between saves.

Change-Id: I9e210a15cf009d222cecd7824c2a1a927957483b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219477
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/cache/mod.go

@@ -38,10 +38,11 @@ type modKey struct {
 }
 
 type modTidyKey struct {
-	cfg     string
+	cfg             string
-	gomod   string
+	gomod           string
-	imports string
+	imports         string
-	view    string
+	unsavedOverlays string
+	view            string
 }
 
 type modHandle struct {
@@ -274,6 +275,10 @@ func (mh *modHandle) Tidy(ctx context.Context) (*modfile.File, *protocol.ColumnM
 }
 
 func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle) (source.ModTidyHandle, error) {
+	if handle := s.getModTidyHandle(); handle != nil {
+		return handle, nil
+	}
+
 	realURI, tempURI := s.view.ModFiles()
 	cfg := s.Config(ctx)
 	options := s.View().Options()
@@ -291,10 +296,11 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle)
 		return nil, err
 	}
 	key := modTidyKey{
-		view:    folder,
+		view:            folder,
-		imports: imports,
+		imports:         imports,
-		gomod:   realfh.Identity().Identifier,
+		unsavedOverlays: hashUnsavedOverlays(s.files),
-		cfg:     hashConfig(cfg),
+		gomod:           realfh.Identity().Identifier,
+		cfg:             hashConfig(cfg),
 	}
 	h := s.view.session.cache.store.Bind(key, func(ctx context.Context) interface{} {
 		data := &modData{}
@@ -391,11 +397,14 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle)
 		}
 		return data
 	})
-	return &modHandle{
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.modTidyHandle = &modHandle{
 		handle: h,
 		file:   realfh,
 		cfg:    cfg,
-	}, nil
+	}
+	return s.modTidyHandle, nil
 }
 
 // extractModParseErrors processes the raw errors returned by modfile.Parse,

internal/lsp/cache/snapshot.go

@@ -11,6 +11,8 @@ import (
 	"go/token"
 	"os"
 	"path/filepath"
+	"sort"
+	"strings"
 	"sync"
 
 	"golang.org/x/tools/go/analysis"
@@ -60,6 +62,10 @@ type snapshot struct {
 
 	// modHandles keeps track of any ParseModHandles for this snapshot.
 	modHandles map[span.URI]*modHandle
+
+	// modTidyHandle is the saved modTidyHandle for this snapshot, it is attached to the
+	// snapshot so we can reuse it without having to call "go mod tidy" everytime.
+	modTidyHandle *modHandle
 }
 
 type packageKey struct {
@@ -129,6 +135,17 @@ func (s *snapshot) buildOverlay() map[string][]byte {
 	return overlays
 }
 
+func hashUnsavedOverlays(files map[span.URI]source.FileHandle) string {
+	var unsaved []string
+	for uri, fh := range files {
+		if overlay, ok := fh.(*overlay); ok && !overlay.saved {
+			unsaved = append(unsaved, uri.Filename())
+		}
+	}
+	sort.Strings(unsaved)
+	return hashContents([]byte(strings.Join(unsaved, "")))
+}
+
 func (s *snapshot) PackageHandles(ctx context.Context, fh source.FileHandle) ([]source.PackageHandle, error) {
 	if fh.Identity().Kind != source.Go {
 		panic("called PackageHandles on a non-Go FileHandle")
@@ -230,6 +247,12 @@ func (s *snapshot) getModHandle(uri span.URI) *modHandle {
 	return s.modHandles[uri]
 }
 
+func (s *snapshot) getModTidyHandle() *modHandle {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.modTidyHandle
+}
+
 func (s *snapshot) getImportedBy(id packageID) []packageID {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -625,6 +648,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Fi
 		workspacePackages: make(map[packageID]packagePath),
 		unloadableFiles:   make(map[span.URI]struct{}),
 		modHandles:        make(map[span.URI]*modHandle),
+		modTidyHandle:     s.modTidyHandle,
 	}
 
 	// Copy all of the FileHandles.
@@ -660,11 +684,19 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Fi
 		// and if so, invalidate this file's packages' metadata.
 		invalidateMetadata := s.shouldInvalidateMetadata(ctx, originalFH, currentFH)
 
-		// If a go.mod file's contents have changed, invalidate the metadata
+		// Invalidate the previous modTidyHandle if any of the files have been
-		// for all of the packages in the workspace.
+		// saved or if any of the metadata has been invalidated.
-		if invalidateMetadata && currentFH.Identity().Kind == source.Mod {
+		if invalidateMetadata || fileWasSaved(originalFH, currentFH) {
-			for id := range s.workspacePackages {
+			result.modTidyHandle = nil
-				directIDs[id] = struct{}{}
+		}
+
+		if currentFH.Identity().Kind == source.Mod {
+			// If the view's go.mod file's contents have changed, invalidate the metadata
+			// for all of the packages in the workspace.
+			if invalidateMetadata {
+				for id := range s.workspacePackages {
+					directIDs[id] = struct{}{}
+				}
 			}
 			delete(result.modHandles, withoutURI)
 		}
@@ -713,7 +745,6 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Fi
 		// Make sure to remove the changed file from the unloadable set.
 		delete(result.unloadableFiles, withoutURI)
 	}
-
 	// Copy the set of initally loaded packages.
 	for k, v := range s.workspacePackages {
 		result.workspacePackages[k] = v
@@ -753,10 +784,25 @@ outer:
 	}
 	// Don't bother copying the importedBy graph,
 	// as it changes each time we update metadata.
-
 	return result
 }
 
+// fileWasSaved returns true if the FileHandle passed in has been saved.
+// It accomplishes this by checking to see if the original and current FileHandles
+// are both overlays, and if the current FileHandles is saved while the original FileHandle
+// was not saved.
+func fileWasSaved(originalFH, currentFH source.FileHandle) bool {
+	c, ok := currentFH.(*overlay)
+	if ok {
+		return true
+	}
+	if originalFH == nil {
+		return c.saved
+	}
+	o, ok := originalFH.(*overlay)
+	return ok && !o.saved && c.saved
+}
+
 // shouldInvalidateMetadata reparses a file's package and import declarations to
 // determine if the file requires a metadata reload.
 func (s *snapshot) shouldInvalidateMetadata(ctx context.Context, originalFH, currentFH source.FileHandle) bool {