internal/lsp: prepare for deletion of view.modURI

Splitting this CL out of CL 257417 to minimize the number of changes.
A few of the view's methods are moved to the snapshot, as they will
soon rely on the snapshot's modules field. Some dead code is also
deleted.

We now populate the snapshot's modules field even when
ExperimentalWorkspaceModule is not true, but we stop looking for modules
after searching the view's root.

Change-Id: Id0068ec10fafcfa6f7694dfcb8aaee8cb025078f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257961
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>

Author: stamblerre

internal/lsp/cache/load.go

@@ -58,16 +58,6 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
 			query = append(query, string(scope))
 		case fileURI:
 			query = append(query, fmt.Sprintf("file=%s", span.URI(scope).Filename()))
-		case directoryURI:
-			filename := span.URI(scope).Filename()
-			q := fmt.Sprintf("%s/...", filename)
-			// Simplify the query if it will be run in the requested directory.
-			// This ensures compatibility with Go 1.12 that doesn't allow
-			// <directory>/... in GOPATH mode.
-			if s.view.rootURI.Filename() == filename {
-				q = "./..."
-			}
-			query = append(query, q)
 		case moduleLoadScope:
 			query = append(query, fmt.Sprintf("%s/...", scope))
 		case viewLoadScope:
@@ -82,7 +72,7 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
 			panic(fmt.Sprintf("unknown scope type %T", scope))
 		}
 		switch scope.(type) {
-		case directoryURI, viewLoadScope:
+		case viewLoadScope:
 			containsDir = true
 		}
 	}

internal/lsp/cache/pkg.go

@@ -42,7 +42,6 @@ type (
 // Declare explicit types for files and directories to distinguish between the two.
 type (
 	fileURI         span.URI
-	directoryURI    span.URI
 	moduleLoadScope string
 	viewLoadScope   span.URI
 )

internal/lsp/cache/session.go

@@ -169,11 +169,9 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 	// If workspace module mode is enabled, find all of the modules in the
 	// workspace.
 	var modules map[span.URI]*moduleRoot
-	if options.ExperimentalWorkspaceModule {
-		modules, err = findWorkspaceModules(ctx, ws.rootURI, options)
-		if err != nil {
-			return nil, nil, func() {}, err
-		}
+	modules, err = findWorkspaceModules(ctx, ws.rootURI, options)
+	if err != nil {
+		return nil, nil, func() {}, err
 	}
 
 	// Now that we have set all required fields,
@@ -276,17 +274,14 @@ func findWorkspaceModules(ctx context.Context, root span.URI, options *source.Op
 			}
 		}
 		// We're only interested in go.mod files.
-		if filepath.Base(path) != "go.mod" {
-			return nil
+		if filepath.Base(path) == "go.mod" {
+			m := newModule(ctx, span.URIFromPath(path))
+			modules[m.rootURI] = m
 		}
-		// At this point, we definitely have a go.mod file in the workspace,
-		// so add it to the view.
-		modURI := span.URIFromPath(path)
-		rootURI := span.URIFromPath(filepath.Dir(path))
-		modules[rootURI] = &moduleRoot{
-			rootURI: rootURI,
-			modURI:  modURI,
-			sumURI:  span.URIFromPath(sumFilename(modURI)),
+		// If we are not using experimental workspace modules, don't continue
+		// the search past the view's root.
+		if !options.ExperimentalWorkspaceModule {
+			return filepath.SkipDir
 		}
 		return nil
 	})

internal/lsp/cache/snapshot.go

@@ -597,28 +597,25 @@ func (s *snapshot) CachedImportPaths(ctx context.Context) (map[string]source.Pac
 	return results, nil
 }
 
-func (s *snapshot) GoModForFile(ctx context.Context, fh source.FileHandle) (source.VersionedFileHandle, error) {
-	if fh.Kind() != source.Go {
-		return nil, fmt.Errorf("expected Go file, got %s", fh.Kind())
-	}
+func (s *snapshot) GoModForFile(ctx context.Context, uri span.URI) (span.URI, error) {
 	if len(s.modules) == 0 {
 		if s.view.modURI == "" {
-			return nil, errors.New("no modules in this view")
+			return "", errors.New("no modules in this view")
 		}
-		return s.GetFile(ctx, s.view.modURI)
+		return s.view.modURI, nil
 	}
 	var match span.URI
 	for _, m := range s.modules {
 		// Add an os.PathSeparator to the end of each directory to make sure
 		// that foo/apple/banana does not match foo/a.
-		if !strings.HasPrefix(fh.URI().Filename()+string(os.PathSeparator), m.rootURI.Filename()+string(os.PathSeparator)) {
+		if !strings.HasPrefix(uri.Filename()+string(os.PathSeparator), m.rootURI.Filename()+string(os.PathSeparator)) {
 			continue
 		}
 		if len(m.modURI) > len(match) {
 			match = m.modURI
 		}
 	}
-	return s.GetFile(ctx, match)
+	return match, nil
 }
 
 func (s *snapshot) getPackage(id packageID, mode source.ParseMode) *packageHandle {
@@ -1065,7 +1062,8 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Ve
 			rootURI := span.URIFromPath(filepath.Dir(withoutURI.Filename()))
 			if currentMod {
 				if _, ok := result.modules[rootURI]; !ok {
-					result.addModule(ctx, currentFH.URI())
+					m := newModule(ctx, currentFH.URI())
+					result.modules[m.rootURI] = m
 					result.view.definitelyReinitialize()
 				}
 			} else if originalMod {
@@ -1542,13 +1540,13 @@ func (s *snapshot) BuildWorkspaceModFile(ctx context.Context) (*modfile.File, er
 	return file, nil
 }
 
-func (s *snapshot) addModule(ctx context.Context, modURI span.URI) {
+func newModule(ctx context.Context, modURI span.URI) *moduleRoot {
 	rootURI := span.URIFromPath(filepath.Dir(modURI.Filename()))
 	sumURI := span.URIFromPath(sumFilename(modURI))
 	if info, _ := os.Stat(sumURI.Filename()); info == nil {
 		sumURI = ""
 	}
-	s.modules[rootURI] = &moduleRoot{
+	return &moduleRoot{
 		rootURI: rootURI,
 		modURI:  modURI,
 		sumURI:  sumURI,

internal/lsp/cache/view.go

@@ -209,15 +209,15 @@ func (f *fileBase) addURI(uri span.URI) int {
 
 func (v *View) ID() string { return v.id }
 
-func (v *View) ValidBuildConfiguration() bool {
-	return v.hasValidBuildConfiguration
+func (s *snapshot) ValidBuildConfiguration() bool {
+	return s.view.hasValidBuildConfiguration
 }
 
-func (v *View) ModFiles() []span.URI {
-	if v.modURI == "" {
+func (s *snapshot) ModFiles() []span.URI {
+	if s.view.modURI == "" {
 		return nil
 	}
-	return []span.URI{v.modURI}
+	return []span.URI{s.view.modURI}
 }
 
 // tempModFile creates a temporary go.mod file based on the contents of the
@@ -340,13 +340,13 @@ func (v *View) Rebuild(ctx context.Context) (source.Snapshot, func(), error) {
 	return snapshot, release, nil
 }
 
-func (v *View) WriteEnv(ctx context.Context, w io.Writer) error {
-	v.optionsMu.Lock()
-	env, buildFlags := v.envLocked()
-	v.optionsMu.Unlock()
+func (s *snapshot) WriteEnv(ctx context.Context, w io.Writer) error {
+	s.view.optionsMu.Lock()
+	env, buildFlags := s.view.envLocked()
+	s.view.optionsMu.Unlock()
 
 	fullEnv := make(map[string]string)
-	for k, v := range v.goEnv {
+	for k, v := range s.view.goEnv {
 		fullEnv[k] = v
 	}
 	for _, v := range env {
@@ -360,7 +360,7 @@ func (v *View) WriteEnv(ctx context.Context, w io.Writer) error {
 
 	}
 	fmt.Fprintf(w, "go env for %v\n(root %s)\n(valid build configuration = %v)\n(build flags: %v)\n",
-		v.folder.Filename(), v.rootURI.Filename(), v.hasValidBuildConfiguration, buildFlags)
+		s.view.folder.Filename(), s.view.rootURI.Filename(), s.view.hasValidBuildConfiguration, buildFlags)
 	for k, v := range fullEnv {
 		fmt.Fprintf(w, "%s=%s\n", k, v)
 	}
@@ -646,18 +646,18 @@ func (v *View) BackgroundContext() context.Context {
 	return v.backgroundCtx
 }
 
-func (v *View) IgnoredFile(uri span.URI) bool {
+func (s *snapshot) IgnoredFile(uri span.URI) bool {
 	filename := uri.Filename()
 	var prefixes []string
-	if v.modURI == "" {
-		for _, entry := range filepath.SplitList(v.gopath) {
+	if len(s.modules) == 0 {
+		for _, entry := range filepath.SplitList(s.view.gopath) {
 			prefixes = append(prefixes, filepath.Join(entry, "src"))
 		}
 	} else {
-		mainMod := filepath.Dir(v.modURI.Filename())
-		prefixes = []string{mainMod, v.gomodcache}
+		for _, m := range s.modules {
+			prefixes = []string{m.rootURI.Filename(), s.view.gomodcache}
+		}
 	}
-
 	for _, prefix := range prefixes {
 		if strings.HasPrefix(filename, prefix) {
 			return checkIgnored(filename[len(prefix):])

internal/lsp/code_action.go

@@ -459,11 +459,11 @@ func moduleQuickFixes(ctx context.Context, snapshot source.Snapshot, fh source.V
 	case source.Mod:
 		modFH = fh
 	case source.Go:
-		var err error
-		modFH, err = snapshot.GoModForFile(ctx, fh)
+		modURI, err := snapshot.GoModForFile(ctx, fh.URI())
 		if err != nil {
 			return nil, err
 		}
+		modFH, err = snapshot.GetFile(ctx, modURI)
 	}
 	tidied, err := snapshot.ModTidy(ctx, modFH)
 	if err == source.ErrTmpModfileUnsupported {

internal/lsp/command.go

@@ -339,7 +339,7 @@ func (s *Server) runTests(ctx context.Context, snapshot source.Snapshot, uri pro
 	})
 }
 
-func (s *Server) runGoGenerate(ctx context.Context, snapshot source.Snapshot, uri span.URI, recursive bool, work *workDone) error {
+func (s *Server) runGoGenerate(ctx context.Context, snapshot source.Snapshot, dir span.URI, recursive bool, work *workDone) error {
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
@@ -353,7 +353,7 @@ func (s *Server) runGoGenerate(ctx context.Context, snapshot source.Snapshot, ur
 
 	stderr := io.MultiWriter(er, workDoneWriter{work})
 
-	if err := snapshot.RunGoCommandPiped(ctx, uri.Filename(), "generate", args, er, stderr); err != nil {
+	if err := snapshot.RunGoCommandPiped(ctx, dir.Filename(), "generate", args, er, stderr); err != nil {
 		return err
 	}
 	return nil

internal/lsp/diagnostics.go

@@ -130,9 +130,11 @@ If you believe this is a mistake, please file an issue: https://github.com/golan
 		return nil, nil
 	}
 	var (
-		showMsg *protocol.ShowMessageParams
-		wg      sync.WaitGroup
+		showMsg     *protocol.ShowMessageParams
+		wg          sync.WaitGroup
+		seenFilesMu sync.Mutex
 	)
+	seenFiles := make(map[span.URI]struct{})
 	for _, pkg := range wsPkgs {
 		wg.Add(1)
 		go func(pkg source.Package) {
@@ -141,6 +143,10 @@ If you believe this is a mistake, please file an issue: https://github.com/golan
 			withAnalysis := alwaysAnalyze // only run analyses for packages with open files
 			var gcDetailsDir span.URI     // find the package's optimization details, if available
 			for _, pgf := range pkg.CompiledGoFiles() {
+				seenFilesMu.Lock()
+				seenFiles[pgf.URI] = struct{}{}
+				seenFilesMu.Unlock()
+
 				if snapshot.IsOpen(pgf.URI) {
 					withAnalysis = true
 				}
@@ -159,7 +165,7 @@ If you believe this is a mistake, please file an issue: https://github.com/golan
 
 			// Check if might want to warn the user about their build configuration.
 			// Our caller decides whether to send the message.
-			if warn && !snapshot.View().ValidBuildConfiguration() {
+			if warn && !snapshot.ValidBuildConfiguration() {
 				showMsg = &protocol.ShowMessageParams{
 					Type:    protocol.Warning,
 					Message: `You are neither in a module nor in your GOPATH. If you are using modules, please open your editor to a directory in your module. If you believe this warning is incorrect, please file an issue: https://github.com/golang/go/issues/new.`,
@@ -187,10 +193,18 @@ If you believe this is a mistake, please file an issue: https://github.com/golan
 			}
 		}(pkg)
 	}
+	wg.Wait()
 	// Confirm that every opened file belongs to a package (if any exist in
 	// the workspace). Otherwise, add a diagnostic to the file.
 	if len(wsPkgs) > 0 {
 		for _, o := range s.session.Overlays() {
+			seenFilesMu.Lock()
+			_, ok := seenFiles[o.URI()]
+			seenFilesMu.Unlock()
+			if ok {
+				continue
+			}
+
 			diagnostic := s.checkForOrphanedFile(ctx, snapshot, o.URI())
 			if diagnostic == nil {
 				continue
@@ -200,7 +214,6 @@ If you believe this is a mistake, please file an issue: https://github.com/golan
 			addReport(o.VersionedFileIdentity(), true, []*source.Diagnostic{diagnostic})
 		}
 	}
-	wg.Wait()
 	return reports, showMsg
 }
 
@@ -405,7 +418,7 @@ func (s *Server) handleFatalErrors(ctx context.Context, snapshot source.Snapshot
 	}
 
 	// All other workarounds are for errors associated with modules.
-	if len(snapshot.View().ModFiles()) == 0 {
+	if len(snapshot.ModFiles()) == 0 {
 		return false
 	}
 
@@ -431,7 +444,7 @@ See https://github.com/golang/go/issues/39164 for more detail on this issue.`,
 		// to a specific module, so this will re-run `go mod vendor` in every
 		// known module with a vendor directory.
 		// TODO(rstambler): Only re-run `go mod vendor` in the relevant module.
-		for _, uri := range snapshot.View().ModFiles() {
+		for _, uri := range snapshot.ModFiles() {
 			// Check that there is a vendor directory in the module before
 			// running `go mod vendor`.
 			if info, _ := os.Stat(filepath.Join(filepath.Dir(uri.Filename()), "vendor")); info == nil {
@@ -459,7 +472,7 @@ See https://github.com/golang/go/issues/39164 for more detail on this issue.`,
 	if errors.Is(loadErr, source.PackagesLoadError) {
 		// TODO(rstambler): Construct the diagnostics in internal/lsp/cache
 		// so that we can avoid this here.
-		for _, uri := range snapshot.View().ModFiles() {
+		for _, uri := range snapshot.ModFiles() {
 			fh, err := snapshot.GetFile(ctx, uri)
 			if err != nil {
 				return false

internal/lsp/general.go

@@ -219,7 +219,7 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol
 
 		// Print each view's environment.
 		buf := &bytes.Buffer{}
-		if err := view.WriteEnv(ctx, buf); err != nil {
+		if err := snapshot.WriteEnv(ctx, buf); err != nil {
 			event.Error(ctx, "failed to write environment", err, tag.Directory.Of(view.Folder().Filename()))
 			continue
 		}

internal/lsp/lsp_test.go

@@ -49,8 +49,7 @@ func testLSP(t *testing.T, datum *tests.Data) {
 	tests.DefaultOptions(options)
 	session.SetOptions(options)
 	options.Env = datum.Config.Env
-	view, _, release, err := session.NewView(ctx, datum.Config.Dir, span.URIFromPath(datum.Config.Dir), options)
-	release()
+	view, snapshot, release, err := session.NewView(ctx, datum.Config.Dir, span.URIFromPath(datum.Config.Dir), options)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -63,7 +62,8 @@ func testLSP(t *testing.T, datum *tests.Data) {
 	view.SetOptions(ctx, options)
 
 	// Only run the -modfile specific tests in module mode with Go 1.14 or above.
-	datum.ModfileFlagAvailable = len(view.ModFiles()) > 0 && testenv.Go1Point() >= 14
+	datum.ModfileFlagAvailable = len(snapshot.ModFiles()) > 0 && testenv.Go1Point() >= 14
+	release()
 
 	var modifications []source.FileModification
 	for filename, content := range datum.Config.Overlay {

internal/lsp/mod/diagnostics.go

@@ -27,7 +27,7 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.Vers
 	defer done()
 
 	reports := map[source.VersionedFileIdentity][]*source.Diagnostic{}
-	for _, uri := range snapshot.View().ModFiles() {
+	for _, uri := range snapshot.ModFiles() {
 		fh, err := snapshot.GetFile(ctx, uri)
 		if err != nil {
 			return nil, err

internal/lsp/mod/hover.go

@@ -17,7 +17,7 @@ import (
 
 func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, position protocol.Position) (*protocol.Hover, error) {
 	var found bool
-	for _, uri := range snapshot.View().ModFiles() {
+	for _, uri := range snapshot.ModFiles() {
 		if fh.URI() == uri {
 			found = true
 			break

internal/lsp/source/diagnostics.go

@@ -41,7 +41,7 @@ type RelatedInformation struct {
 func Diagnostics(ctx context.Context, snapshot Snapshot, pkg Package, withAnalysis bool) (map[VersionedFileIdentity][]*Diagnostic, bool, error) {
 	onlyIgnoredFiles := true
 	for _, pgf := range pkg.CompiledGoFiles() {
-		onlyIgnoredFiles = onlyIgnoredFiles && snapshot.View().IgnoredFile(pgf.URI)
+		onlyIgnoredFiles = onlyIgnoredFiles && snapshot.IgnoredFile(pgf.URI)
 	}
 	if onlyIgnoredFiles {
 		return nil, false, nil