internal/lsp: stop using structured errors
Using structured errors in gopls has proven to be difficult to manage:
it's hard to know whether a given error return is expected to be
structured without any type information. We have mostly eliminated
them; finish the job.
I don't intend any semantic changes here.
I considered eliminating CriticalError altogether, but it does seem
useful to have a convenient bundle for return values. So I left it
alone for now.
Change-Id: I4b5f85a8de9712babffbc1383088151b596bd815
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287792
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 7bea58e..6a3995a 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -60,7 +60,7 @@
// initializedErr holds the last error resulting from initialization. If
// initialization fails, we only retry when the the workspace modules change,
// to avoid too many go/packages calls.
- initializedErr error
+ initializedErr *source.CriticalError
// mu guards all of the maps in the snapshot.
mu sync.Mutex
@@ -1012,7 +1012,7 @@
wsPkgs, _ := s.WorkspacePackages(ctx)
if msg := shouldShowAdHocPackagesWarning(s, wsPkgs); msg != "" {
return &source.CriticalError{
- MainError: fmt.Errorf(msg),
+ MainError: errors.New(msg),
}
}
// Even if workspace packages were returned, there still may be an error
@@ -1084,7 +1084,10 @@
// TODO(rstambler): Should we be more careful about returning the
// initialization error? Is it possible for the initialization error to be
// corrected without a successful reinitialization?
- return s.initializedErr
+ if s.initializedErr == nil {
+ return nil
+ }
+ return s.initializedErr.MainError
}
func (s *snapshot) AwaitInitialized(ctx context.Context) {
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 5b767d1..55c52eb 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -9,7 +9,6 @@
"context"
"encoding/json"
"fmt"
- exec "golang.org/x/sys/execabs"
"io"
"io/ioutil"
"os"
@@ -23,6 +22,7 @@
"golang.org/x/mod/modfile"
"golang.org/x/mod/semver"
+ exec "golang.org/x/sys/execabs"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/gocommand"
@@ -581,7 +581,9 @@
ErrorList: modErrors,
}
} else {
- s.initializedErr = err
+ s.initializedErr = &source.CriticalError{
+ MainError: err,
+ }
}
} else {
// Clear out the initialization error, in case it had been set
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index adf19ad..ceee5f0 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -350,13 +350,9 @@
// status bar.
var errMsg string
if err != nil {
- event.Error(ctx, "errors loading workspace", err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder()))
-
- // Some error messages can also be displayed as diagnostics.
- if criticalErr := (*source.CriticalError)(nil); errors.As(err, &criticalErr) {
- s.storeErrorDiagnostics(ctx, snapshot, modSource, criticalErr.ErrorList)
- }
- errMsg = strings.Replace(err.Error(), "\n", " ", -1)
+ event.Error(ctx, "errors loading workspace", err.MainError, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder()))
+ s.storeErrorDiagnostics(ctx, snapshot, modSource, err.ErrorList)
+ errMsg = strings.Replace(err.MainError.Error(), "\n", " ", -1)
}
if s.criticalErrorStatus == nil {
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 7a576a6d..2e0b851 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -556,17 +556,12 @@
}
type CriticalError struct {
+ // MainError is the primary error. Must be non-nil.
MainError error
+ // ErrorList contains any supplemental (structured) errors.
ErrorList []*Error
}
-func (err *CriticalError) Error() string {
- if err.MainError == nil {
- return ""
- }
- return err.MainError.Error()
-}
-
// An Error corresponds to an LSP Diagnostic.
// https://microsoft.github.io/language-server-protocol/specification#diagnostic
type Error struct {
@@ -601,13 +596,6 @@
UpgradeNotification
)
-func (e *Error) Error() string {
- if e.URI == "" {
- return e.Message
- }
- return fmt.Sprintf("%s:%s: %s", e.URI, e.Range, e.Message)
-}
-
var (
PackagesLoadError = errors.New("packages.Load error")
)
