internal/lsp: handle initial workspace load failure per module
This CL adds diagnostics to the go.mod file if one of the modules in
the workspace is invalid and causes the initial workspace load to fail.
When the module is fixed, the initial workspace load will be retried.
This CL also introduces the *source.ErrorList error type, which will be
useful in the future as a way of producing diagnostics out of errors.
Updates golang/go#32394
Change-Id: Ib8860a4f16c1983b8512f75e26354512d5a9a86d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/254753
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/regtest/workspace_test.go b/gopls/internal/regtest/workspace_test.go
index 2c1a1d9..8629917 100644
--- a/gopls/internal/regtest/workspace_test.go
+++ b/gopls/internal/regtest/workspace_test.go
@@ -311,3 +311,50 @@
}
})
}
+
+// This test confirms that a gopls workspace can recover from initialization
+// with one invalid module.
+func TestOneBrokenModule(t *testing.T) {
+ const multiModule = `
+-- moda/a/go.mod --
+module a.com
+
+require b.com v1.2.3
+
+-- moda/a/a.go --
+package a
+
+import (
+ "b.com/b"
+)
+
+func main() {
+ var x int
+ _ = b.Hello()
+}
+-- modb/go.mod --
+modul b.com // typo here
+
+-- modb/b/b.go --
+package b
+
+func Hello() int {
+ var x int
+}
+`
+ run(t, multiModule, func(t *testing.T, env *Env) {
+ env.Await(InitialWorkspaceLoad)
+ env.OpenFile("modb/go.mod")
+ env.Await(
+ OnceMet(
+ CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 1),
+ DiagnosticAt("modb/go.mod", 0, 0),
+ ),
+ )
+ env.RegexpReplace("modb/go.mod", "modul", "module")
+ env.Editor.SaveBufferWithoutActions(env.Ctx, "modb/go.mod")
+ env.Await(
+ env.DiagnosticAtRegexp("modb/b/b.go", "x"),
+ )
+ })
+}
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index e625030e..d8296c1 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -1402,7 +1402,7 @@
if err != nil {
return nil, err
}
- if parsed.File.Module == nil {
+ if parsed.File == nil || parsed.File.Module == nil {
return nil, fmt.Errorf("no module declaration for %s", mod.modURI)
}
path := parsed.File.Module.Mod.Path
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index e7fe544..157168c 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -711,24 +711,35 @@
// If we have multiple modules, we need to load them by paths.
var scopes []interface{}
+ var modErrors *source.ErrorList
+ addError := func(uri span.URI, err error) {
+ if modErrors == nil {
+ modErrors = &source.ErrorList{}
+ }
+ *modErrors = append(*modErrors, &source.Error{
+ URI: uri,
+ Category: "compiler",
+ Kind: source.ListError,
+ Message: err.Error(),
+ })
+ }
if len(s.modules) > 0 {
- // TODO(rstambler): Retry the initial workspace load for whichever
- // modules we failed to load.
for _, mod := range s.modules {
fh, err := s.GetFile(ctx, mod.modURI)
if err != nil {
- s.view.initializedErr = err
+ addError(mod.modURI, err)
continue
}
parsed, err := s.ParseMod(ctx, fh)
if err != nil {
- s.view.initializedErr = err
+ addError(mod.modURI, err)
continue
}
path := parsed.File.Module.Mod.Path
scopes = append(scopes, moduleLoadScope(path))
}
- } else {
+ }
+ if len(scopes) == 0 {
scopes = append(scopes, viewLoadScope("LOAD_VIEW"))
}
err := s.load(ctx, append(scopes, packagePath("builtin"))...)
@@ -737,8 +748,12 @@
}
if err != nil {
event.Error(ctx, "initial workspace load failed", err)
+ if modErrors != nil {
+ s.view.initializedErr = errors.Errorf("errors loading modules: %v: %w", err, modErrors)
+ } else {
+ s.view.initializedErr = err
+ }
}
- s.view.initializedErr = err
})
}
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 71662f3..d455df9 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -97,6 +97,15 @@
if errors.Is(err, context.Canceled) {
return nil, nil
}
+ // Some error messages can be displayed as diagnostics.
+ if errList := (*source.ErrorList)(nil); errors.As(err, &errList) {
+ if r, err := errorsToDiagnostic(ctx, snapshot, *errList); err == nil {
+ for k, v := range r {
+ reports[k] = v
+ }
+ return reports, nil
+ }
+ }
// Try constructing a more helpful error message out of this error.
if s.handleFatalErrors(ctx, snapshot, modErr, err) {
return nil, nil
@@ -212,6 +221,32 @@
return fmt.Sprintf("%x", sha256.Sum256([]byte(key)))
}
+func errorsToDiagnostic(ctx context.Context, snapshot source.Snapshot, errors []*source.Error) (map[idWithAnalysis]map[string]*source.Diagnostic, error) {
+ reports := make(map[idWithAnalysis]map[string]*source.Diagnostic)
+ for _, e := range errors {
+ diagnostic := &source.Diagnostic{
+ Range: e.Range,
+ Message: e.Message,
+ Related: e.Related,
+ Severity: protocol.SeverityError,
+ Source: e.Category,
+ }
+ fh, err := snapshot.GetFile(ctx, e.URI)
+ if err != nil {
+ return nil, err
+ }
+ id := idWithAnalysis{
+ id: fh.VersionedFileIdentity(),
+ withAnalysis: false,
+ }
+ if _, ok := reports[id]; !ok {
+ reports[id] = make(map[string]*source.Diagnostic)
+ }
+ reports[id][diagnosticKey(diagnostic)] = diagnostic
+ }
+ return reports, nil
+}
+
func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, reports map[idWithAnalysis]map[string]*source.Diagnostic) {
// Check for context cancellation before publishing diagnostics.
if ctx.Err() != nil {
@@ -322,7 +357,8 @@
func (s *Server) handleFatalErrors(ctx context.Context, snapshot source.Snapshot, modErr, loadErr error) bool {
modURI := snapshot.View().ModFile()
- // If the folder has no Go code in it, we shouldn't spam the user with a warning.
+ // If the folder has no Go code in it, we shouldn't spam the user with a
+ // warning.
var hasGo bool
_ = filepath.Walk(snapshot.View().Folder().Filename(), func(path string, info os.FileInfo, err error) error {
if !strings.HasSuffix(info.Name(), ".go") {
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 9e5abcf..c19dc89 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -12,6 +12,7 @@
"go/token"
"go/types"
"io"
+ "strings"
"golang.org/x/mod/modfile"
"golang.org/x/mod/module"
@@ -484,6 +485,17 @@
Version() *module.Version
}
+type ErrorList []*Error
+
+func (err *ErrorList) Error() string {
+ var b strings.Builder
+ b.WriteString("source error list:")
+ for _, e := range *err {
+ b.WriteString(fmt.Sprintf("\n\t%s", e))
+ }
+ return b.String()
+}
+
type Error struct {
URI span.URI
Range protocol.Range
