gopls/internal/lsp/cache: improve ad-hoc warning for nested modules

Our warning for the specific (and somewhat arbitrary) case of opening a
nested module was stale. Update it for workspaces.

This is a very weak distillation of CL 448695, targeted at improving
this one error message.

Change-Id: Ic78e2f8ff8004a78ac2a0650b40767de9dfe153c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/454563
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

gopls/internal/lsp/cache/load.go

@@ -275,26 +275,34 @@ func (m *moduleErrorMap) Error() string {
 	return buf.String()
 }
 
-// workspaceLayoutErrors returns a diagnostic for every open file, as well as
-// an error message if there are no open files.
-func (s *snapshot) workspaceLayoutError(ctx context.Context) *source.CriticalError {
+// workspaceLayoutErrors returns an error decribing a misconfiguration of the
+// workspace, along with related diagnostic.
+//
+// The unusual argument ordering of results is intentional: if the resulting
+// error is nil, so must be the resulting diagnostics.
+//
+// If ctx is cancelled, it may return ctx.Err(), nil.
+//
+// TODO(rfindley): separate workspace diagnostics from critical workspace
+// errors.
+func (s *snapshot) workspaceLayoutError(ctx context.Context) (error, []*source.Diagnostic) {
 	// TODO(rfindley): do we really not want to show a critical error if the user
 	// has no go.mod files?
 	if len(s.workspace.getKnownModFiles()) == 0 {
-		return nil
+		return nil, nil
 	}
 
 	// TODO(rfindley): both of the checks below should be delegated to the workspace.
 	if s.view.effectiveGO111MODULE() == off {
-		return nil
+		return nil, nil
 	}
 	if s.workspace.moduleSource != legacyWorkspace {
-		return nil
+		return nil, nil
 	}
 
 	// If the user has one module per view, there is nothing to warn about.
 	if s.ValidBuildConfiguration() && len(s.workspace.getKnownModFiles()) == 1 {
-		return nil
+		return nil, nil
 	}
 
 	// Apply diagnostics about the workspace configuration to relevant open
@@ -320,51 +328,57 @@ go workspaces (go.work files).
 See the documentation for more information on setting up your workspace:
 https://github.com/golang/tools/blob/master/gopls/doc/workspace.md.`
 		}
-		return &source.CriticalError{
-			MainError:   fmt.Errorf(msg),
-			Diagnostics: s.applyCriticalErrorToFiles(ctx, msg, openFiles),
-		}
+		return fmt.Errorf(msg), s.applyCriticalErrorToFiles(ctx, msg, openFiles)
 	}
 
 	// If the user has one active go.mod file, they may still be editing files
 	// in nested modules. Check the module of each open file and add warnings
 	// that the nested module must be opened as a workspace folder.
 	if len(s.workspace.ActiveModFiles()) == 1 {
 		// Get the active root go.mod file to compare against.
-		var rootModURI span.URI
+		var rootMod string
 		for uri := range s.workspace.ActiveModFiles() {
-			rootModURI = uri
+			rootMod = uri.Filename()
 		}
-		nestedModules := map[string][]source.VersionedFileHandle{}
+		rootDir := filepath.Dir(rootMod)
+		nestedModules := make(map[string][]source.VersionedFileHandle)
 		for _, fh := range openFiles {
-			modURI := moduleForURI(s.workspace.knownModFiles, fh.URI())
-			if modURI != rootModURI {
-				modDir := filepath.Dir(modURI.Filename())
+			mod, err := findRootPattern(ctx, filepath.Dir(fh.URI().Filename()), "go.mod", s)
+			if err != nil {
+				if ctx.Err() != nil {
+					return ctx.Err(), nil
+				}
+				continue
+			}
+			if mod == "" {
+				continue
+			}
+			if mod != rootMod && source.InDir(rootDir, mod) {
+				modDir := filepath.Dir(mod)
 				nestedModules[modDir] = append(nestedModules[modDir], fh)
 			}
 		}
+		var multiModuleMsg string
+		if s.view.goversion >= 18 {
+			multiModuleMsg = `To work on multiple modules at once, please use a go.work file.
+See https://github.com/golang/tools/blob/master/gopls/doc/workspace.md for more information on using workspaces.`
+		} else {
+			multiModuleMsg = `To work on multiple modules at once, please upgrade to Go 1.18 and use a go.work file.
+See https://github.com/golang/tools/blob/master/gopls/doc/workspace.md for more information on using workspaces.`
+		}
 		// Add a diagnostic to each file in a nested module to mark it as
 		// "orphaned". Don't show a general diagnostic in the progress bar,
 		// because the user may still want to edit a file in a nested module.
 		var srcDiags []*source.Diagnostic
 		for modDir, uris := range nestedModules {
-			msg := fmt.Sprintf(`This file is in %s, which is a nested module in the %s module.
-gopls currently requires one module per workspace folder.
-Please open %s as a separate workspace folder.
-You can learn more here: https://github.com/golang/tools/blob/master/gopls/doc/workspace.md.
-`, modDir, filepath.Dir(rootModURI.Filename()), modDir)
+			msg := fmt.Sprintf("This file is in %s, which is a nested module in the %s module.\n%s", modDir, rootMod, multiModuleMsg)
 			srcDiags = append(srcDiags, s.applyCriticalErrorToFiles(ctx, msg, uris)...)
 		}
 		if len(srcDiags) != 0 {
-			return &source.CriticalError{
-				MainError: fmt.Errorf(`You are working in a nested module.
-Please open it as a separate workspace folder. Learn more:
-https://github.com/golang/tools/blob/master/gopls/doc/workspace.md.`),
-				Diagnostics: srcDiags,
-			}
+			return fmt.Errorf("You have opened a nested module.\n%s", multiModuleMsg), srcDiags
 		}
 	}
-	return nil
+	return nil, nil
 }
 
 func (s *snapshot) applyCriticalErrorToFiles(ctx context.Context, msg string, files []source.VersionedFileHandle) []*source.Diagnostic {

gopls/internal/lsp/cache/mod_tidy.go

@@ -63,6 +63,9 @@ func (s *snapshot) ModTidy(ctx context.Context, pm *source.ParsedModule) (*sourc
 				Diagnostics: criticalErr.Diagnostics,
 			}, nil
 		}
+		if ctx.Err() != nil { // must check ctx after GetCriticalError
+			return nil, ctx.Err()
+		}
 
 		if err := s.awaitLoaded(ctx); err != nil {
 			return nil, err

gopls/internal/lsp/cache/snapshot.go

@@ -1212,6 +1212,8 @@ func (s *snapshot) CachedImportPaths(ctx context.Context) (map[PackagePath]sourc
 	return results, nil
 }
 
+// TODO(rfindley): clarify that this is only active modules. Or update to just
+// use findRootPattern.
 func (s *snapshot) GoModForFile(uri span.URI) span.URI {
 	return moduleForURI(s.workspace.activeModFiles, uri)
 }
@@ -1396,13 +1398,31 @@ func (s *snapshot) GetCriticalError(ctx context.Context) *source.CriticalError {
 		//
 		// TODO(rfindley): re-evaluate this heuristic.
 		if containsCommandLineArguments(wsPkgs) {
-			return s.workspaceLayoutError(ctx)
+			err, diags := s.workspaceLayoutError(ctx)
+			if err != nil {
+				if ctx.Err() != nil {
+					return nil // see the API documentation for source.Snapshot
+				}
+				return &source.CriticalError{
+					MainError:   err,
+					Diagnostics: diags,
+				}
+			}
 		}
 		return nil
 	}
 
 	if errMsg := loadErr.MainError.Error(); strings.Contains(errMsg, "cannot find main module") || strings.Contains(errMsg, "go.mod file not found") {
-		return s.workspaceLayoutError(ctx)
+		err, diags := s.workspaceLayoutError(ctx)
+		if err != nil {
+			if ctx.Err() != nil {
+				return nil // see the API documentation for source.Snapshot
+			}
+			return &source.CriticalError{
+				MainError:   err,
+				Diagnostics: diags,
+			}
+		}
 	}
 	return loadErr
 }

gopls/internal/lsp/cache/view.go

@@ -859,33 +859,38 @@ func (s *Session) getWorkspaceInformation(ctx context.Context, folder span.URI,
 // Otherwise, it returns folder.
 // TODO (rFindley): move this to workspace.go
 // TODO (rFindley): simplify this once workspace modules are enabled by default.
-func findWorkspaceRoot(ctx context.Context, folder span.URI, fs source.FileSource, excludePath func(string) bool, experimental bool) (span.URI, error) {
+func findWorkspaceRoot(ctx context.Context, folderURI span.URI, fs source.FileSource, excludePath func(string) bool, experimental bool) (span.URI, error) {
 	patterns := []string{"go.work", "go.mod"}
 	if experimental {
 		patterns = []string{"go.work", "gopls.mod", "go.mod"}
 	}
+	folder := folderURI.Filename()
 	for _, basename := range patterns {
-		dir, err := findRootPattern(ctx, folder, basename, fs)
+		match, err := findRootPattern(ctx, folder, basename, fs)
 		if err != nil {
-			return "", fmt.Errorf("finding %s: %w", basename, err)
+			if ctxErr := ctx.Err(); ctxErr != nil {
+				return "", ctxErr
+			}
+			return "", err
 		}
-		if dir != "" {
+		if match != "" {
+			dir := span.URIFromPath(filepath.Dir(match))
 			return dir, nil
 		}
 	}
 
 	// The experimental workspace can handle nested modules at this point...
 	if experimental {
-		return folder, nil
+		return folderURI, nil
 	}
 
 	// ...else we should check if there's exactly one nested module.
-	all, err := findModules(folder, excludePath, 2)
+	all, err := findModules(folderURI, excludePath, 2)
 	if err == errExhausted {
 		// Fall-back behavior: if we don't find any modules after searching 10000
 		// files, assume there are none.
 		event.Log(ctx, fmt.Sprintf("stopped searching for modules after %d files", fileLimit))
-		return folder, nil
+		return folderURI, nil
 	}
 	if err != nil {
 		return "", err
@@ -896,19 +901,24 @@ func findWorkspaceRoot(ctx context.Context, folder span.URI, fs source.FileSourc
 			return dirURI(uri), nil
 		}
 	}
-	return folder, nil
+	return folderURI, nil
 }
 
-func findRootPattern(ctx context.Context, folder span.URI, basename string, fs source.FileSource) (span.URI, error) {
-	dir := folder.Filename()
+// findRootPattern looks for files with the given basename in dir or any parent
+// directory of dir, using the provided FileSource. It returns the first match,
+// starting from dir and search parents.
+//
+// The resulting string is either the file path of a matching file with the
+// given basename, or "" if none was found.
+func findRootPattern(ctx context.Context, dir, basename string, fs source.FileSource) (string, error) {
 	for dir != "" {
 		target := filepath.Join(dir, basename)
 		exists, err := fileExists(ctx, span.URIFromPath(target), fs)
 		if err != nil {
-			return "", err
+			return "", err // not readable or context cancelled
 		}
 		if exists {
-			return span.URIFromPath(dir), nil
+			return target, nil
 		}
 		// Trailing separators must be trimmed, otherwise filepath.Split is a noop.
 		next, _ := filepath.Split(strings.TrimRight(dir, string(filepath.Separator)))

gopls/internal/lsp/diagnostics.go

@@ -295,6 +295,9 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn
 		return
 	}
 	criticalErr := snapshot.GetCriticalError(ctx)
+	if ctx.Err() != nil { // must check ctx after GetCriticalError
+		return
+	}
 
 	// Show the error as a progress error report so that it appears in the
 	// status bar. If a client doesn't support progress reports, the error

gopls/internal/lsp/source/view.go

@@ -214,6 +214,8 @@ type Snapshot interface {
 	MetadataForFile(ctx context.Context, uri span.URI) ([]*Metadata, error)
 
 	// GetCriticalError returns any critical errors in the workspace.
+	//
+	// A nil result may mean success, or context cancellation.
 	GetCriticalError(ctx context.Context) *CriticalError
 
 	// BuildGoplsMod generates a go.mod file for all modules in the workspace.