diff --git a/gopls/internal/regtest/diagnostics_test.go b/gopls/internal/regtest/diagnostics_test.go index 74687b81fc..c68aa8fc02 100644 --- a/gopls/internal/regtest/diagnostics_test.go +++ b/gopls/internal/regtest/diagnostics_test.go @@ -1594,3 +1594,36 @@ import ( }) }) } + +func TestMultipleModules_GO111MODULE_on(t *testing.T) { + const modules = ` +-- a/go.mod -- +module a.com + +go 1.12 +-- a/a.go -- +package a +-- b/go.mod -- +module b.com + +go 1.12 +-- b/b.go -- +package b +` + withOptions( + WithModes(WithoutExperiments), + EditorConfig{ + Env: map[string]string{ + "GO111MODULE": "on", + }, + }, + ).run(t, modules, func(t *testing.T, env *Env) { + env.OpenFile("a/a.go") + env.OpenFile("b/go.mod") + env.Await( + env.DiagnosticAtRegexp("a/a.go", "package a"), + env.DiagnosticAtRegexp("b/go.mod", "module b.com"), + OutstandingWork("Error loading workspace", "gopls requires a module at the root of your workspace."), + ) + }) +} diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 1abb844b8f..6a579fa04b 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -19,6 +19,7 @@ import ( "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/lsp/debug/tag" + "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/memoize" "golang.org/x/tools/internal/packagesinternal" @@ -123,9 +124,10 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error { } if len(pkgs) == 0 { if err != nil { - // Try to extract the error into a diagnostic. - if srcErrs := s.parseLoadError(ctx, err); srcErrs != nil { - return srcErrs + // Try to extract the load error into a structured error with + // diagnostics. + if criticalErr := s.parseLoadError(ctx, err); criticalErr != nil { + return criticalErr } } else { err = fmt.Errorf("no packages returned") @@ -167,8 +169,16 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error { return nil } -func (s *snapshot) parseLoadError(ctx context.Context, loadErr error) source.ErrorList { - var srcErrs source.ErrorList +func (s *snapshot) parseLoadError(ctx context.Context, loadErr error) *source.CriticalError { + // The error may be a result of the user's workspace layout. Check for + // a valid workspace configuration first. + if criticalErr := s.workspaceLayoutErrors(ctx, loadErr); criticalErr != nil { + return criticalErr + } + criticalErr := &source.CriticalError{ + MainError: loadErr, + } + // Attempt to place diagnostics in the relevant go.mod files, if any. for _, uri := range s.ModFiles() { fh, err := s.GetFile(ctx, uri) if err != nil { @@ -178,12 +188,82 @@ func (s *snapshot) parseLoadError(ctx context.Context, loadErr error) source.Err if srcErr == nil { continue } - if srcErrs == nil { - srcErrs = source.ErrorList{} - } - srcErrs = append(srcErrs, srcErr) + criticalErr.ErrorList = append(criticalErr.ErrorList, srcErr) } - return srcErrs + return criticalErr +} + +// workspaceLayoutErrors returns a diagnostic for every open file, as well as +// an error message if there are no open files. +func (s *snapshot) workspaceLayoutErrors(ctx context.Context, err error) *source.CriticalError { + // Assume the workspace is misconfigured only if we've detected an invalid + // build configuration. Currently, a valid build configuration is either a + // module at the root of the view or a GOPATH workspace. + if s.ValidBuildConfiguration() { + return nil + } + if len(s.workspace.getKnownModFiles()) == 0 { + return nil + } + // TODO(rstambler): Handle GO111MODULE=auto. + if s.view.go111module != "on" { + return nil + } + if s.workspace.moduleSource != legacyWorkspace { + return nil + } + // The user's workspace contains go.mod files and they have + // GO111MODULE=on, so we should guide them to create a + // workspace folder for each module. + + // Add a diagnostic to every open file, or return a general error if + // there aren't any. + var open []source.VersionedFileHandle + s.mu.Lock() + for _, fh := range s.files { + if s.isOpenLocked(fh.URI()) { + open = append(open, fh) + } + } + s.mu.Unlock() + + msg := `gopls requires a module at the root of your workspace. +You can work with multiple modules by opening each one as a workspace folder. +Improvements to this workflow will be coming soon (https://github.com/golang/go/issues/32394), +and you can learn more here: https://github.com/golang/go/issues/36899.` + + criticalError := &source.CriticalError{ + MainError: errors.New(msg), + } + if len(open) == 0 { + return criticalError + } + for _, fh := range open { + // Place the diagnostics on the package or module declarations. + var rng protocol.Range + switch fh.Kind() { + case source.Go: + if pgf, err := s.ParseGo(ctx, fh, source.ParseHeader); err == nil { + pkgDecl := span.NewRange(s.FileSet(), pgf.File.Package, pgf.File.Name.End()) + if spn, err := pkgDecl.Span(); err == nil { + rng, _ = pgf.Mapper.Range(spn) + } + } + case source.Mod: + if pmf, err := s.ParseMod(ctx, fh); err == nil { + if pmf.File.Module != nil && pmf.File.Module.Syntax != nil { + rng, _ = rangeFromPositions(pmf.Mapper, pmf.File.Module.Syntax.Start, pmf.File.Module.Syntax.End) + } + } + } + criticalError.ErrorList = append(criticalError.ErrorList, &source.Error{ + URI: fh.URI(), + Range: rng, + Kind: source.ListError, + Message: msg, + }) + } + return criticalError } type workspaceDirKey string diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 0697669810..d6b598bc62 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -923,7 +923,11 @@ func (s *snapshot) getFileLocked(ctx context.Context, f *fileBase) (source.Versi func (s *snapshot) IsOpen(uri span.URI) bool { s.mu.Lock() defer s.mu.Unlock() + return s.isOpenLocked(uri) +} + +func (s *snapshot) isOpenLocked(uri span.URI) bool { _, open := s.files[uri].(*overlay) return open } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 11d685be33..a603cb3ea1 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -517,6 +517,9 @@ func (s *snapshot) initialize(ctx context.Context, firstAttempt bool) { 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", @@ -552,7 +555,10 @@ func (s *snapshot) initialize(ctx context.Context, firstAttempt bool) { if err != nil { event.Error(ctx, "initial workspace load failed", err) if modErrors != nil { - s.initializedErr = errors.Errorf("errors loading modules: %v: %w", err, modErrors) + s.initializedErr = &source.CriticalError{ + MainError: errors.Errorf("errors loading modules: %v: %w", err, modErrors), + ErrorList: modErrors, + } } else { s.initializedErr = err } diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 16af4b63f5..d3d8474470 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -176,17 +176,14 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA if s.shouldIgnoreError(ctx, snapshot, err) { return false } + // 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 // will still be shown as a ShowMessage. If there is no error, any running // error progress reports will be closed. - s.showCriticalErrorStatus(ctx, err) + s.showCriticalErrorStatus(ctx, snapshot, err) if err != nil { - // Some error messages can also be displayed as diagnostics. - if errList := (source.ErrorList)(nil); errors.As(err, &errList) { - s.storeErrorDiagnostics(ctx, snapshot, typeCheckSource, errList) - } event.Error(ctx, "errors diagnosing workspace", err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder())) return false } @@ -361,7 +358,7 @@ func hasUndeclaredErrors(pkg source.Package) bool { // showCriticalErrorStatus shows the error as a progress report. // If the error is nil, it clears any existing error progress report. -func (s *Server) showCriticalErrorStatus(ctx context.Context, err error) { +func (s *Server) showCriticalErrorStatus(ctx context.Context, snapshot source.Snapshot, err error) { s.criticalErrorStatusMu.Lock() defer s.criticalErrorStatusMu.Unlock() @@ -369,6 +366,15 @@ func (s *Server) showCriticalErrorStatus(ctx context.Context, err error) { // status bar. var errMsg string if err != nil { + // Some error messages can also be displayed as diagnostics. But don't + // show source.ErrorLists as critical errors--only CriticalErrors + // should be shown. + if criticalErr := (*source.CriticalError)(nil); errors.As(err, &criticalErr) { + s.storeErrorDiagnostics(ctx, snapshot, typeCheckSource, criticalErr.ErrorList) + } else if srcErrList := (source.ErrorList)(nil); errors.As(err, &srcErrList) { + s.storeErrorDiagnostics(ctx, snapshot, typeCheckSource, srcErrList) + return + } errMsg = strings.Replace(err.Error(), "\n", " ", -1) } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index ddf8f6b477..04021d1c1f 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -536,15 +536,26 @@ type Package interface { Version() *module.Version } +type CriticalError struct { + MainError error + ErrorList +} + +func (err *CriticalError) Error() string { + if err.MainError == nil { + return "" + } + return err.MainError.Error() +} + type ErrorList []*Error func (err ErrorList) Error() string { - var b strings.Builder - b.WriteString("source error list:") + var list []string for _, e := range err { - b.WriteString(fmt.Sprintf("\n\t%s", e)) + list = append(list, e.Error()) } - return b.String() + return strings.Join(list, "\n\t") } // An Error corresponds to an LSP Diagnostic. @@ -577,6 +588,9 @@ const ( ) func (e *Error) Error() string { + if e.URI == "" { + return e.Message + } return fmt.Sprintf("%s:%s: %s", e.URI, e.Range, e.Message) }