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>
diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go
index 19b21c6..f79109a 100644
--- a/gopls/internal/lsp/cache/load.go
+++ b/gopls/internal/lsp/cache/load.go
@@ -275,26 +275,34 @@
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,10 +328,7 @@
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
@@ -331,40 +336,49 @@
// 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 {
diff --git a/gopls/internal/lsp/cache/mod_tidy.go b/gopls/internal/lsp/cache/mod_tidy.go
index f433211..0a4b88e 100644
--- a/gopls/internal/lsp/cache/mod_tidy.go
+++ b/gopls/internal/lsp/cache/mod_tidy.go
@@ -63,6 +63,9 @@
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
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 0b56120..9650655 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -1212,6 +1212,8 @@
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 @@
//
// 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
}
diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go
index bc623ff..4fad654 100644
--- a/gopls/internal/lsp/cache/view.go
+++ b/gopls/internal/lsp/cache/view.go
@@ -859,33 +859,38 @@
// 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 @@
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)))
diff --git a/gopls/internal/lsp/diagnostics.go b/gopls/internal/lsp/diagnostics.go
index 19bc3ce..6b83cf8 100644
--- a/gopls/internal/lsp/diagnostics.go
+++ b/gopls/internal/lsp/diagnostics.go
@@ -295,6 +295,9 @@
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
diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go
index fa55cde..e82a89a 100644
--- a/gopls/internal/lsp/source/view.go
+++ b/gopls/internal/lsp/source/view.go
@@ -214,6 +214,8 @@
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.
