From 782573673af31588817cb7e79a1baeca1570609c Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Thu, 28 Dec 2023 15:13:05 -0500 Subject: [PATCH] gopls/internal/lsp/cache: simplify critical errors The critical error logic was hard to follow, and ill defined because it was constructed in two places: once during initialization, and another time in Snapshot.CriticalError. CriticalError is now rebranded as an InitializationError, and constructed only during snapshot initialization. It covers a load error, and an unparsable go.work or go.mod file. Critical errors are applied to orphaned files. For golang/go#57979 Change-Id: Ib3cdf602954202be0c87594c26dbbd0ff7e6458a Reviewed-on: https://go-review.googlesource.com/c/tools/+/553097 LUCI-TryBot-Result: Go LUCI Reviewed-by: Alan Donovan --- gopls/internal/lsp/cache/check.go | 2 +- gopls/internal/lsp/cache/diagnostics.go | 14 +- gopls/internal/lsp/cache/load.go | 76 ------- gopls/internal/lsp/cache/mod_tidy.go | 9 - gopls/internal/lsp/cache/snapshot.go | 200 ++++-------------- gopls/internal/lsp/cache/view.go | 81 +++---- gopls/internal/server/diagnostics.go | 23 +- .../diagnostics/diagnostics_test.go | 10 +- .../internal/test/integration/expectation.go | 2 +- .../integration/workspace/zero_config_test.go | 24 +++ 10 files changed, 142 insertions(+), 299 deletions(-) diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go index 079b37161ee..502ebb2149c 100644 --- a/gopls/internal/lsp/cache/check.go +++ b/gopls/internal/lsp/cache/check.go @@ -203,7 +203,7 @@ func (s *Snapshot) resolveImportGraph() (*importGraph, error) { s.mu.Unlock() openPackages := make(map[PackageID]bool) - for _, fh := range s.overlays() { + for _, fh := range s.Overlays() { mps, err := s.MetadataForFile(ctx, fh.URI()) if err != nil { return nil, err diff --git a/gopls/internal/lsp/cache/diagnostics.go b/gopls/internal/lsp/cache/diagnostics.go index 11d83ac3c2c..76c82630cc6 100644 --- a/gopls/internal/lsp/cache/diagnostics.go +++ b/gopls/internal/lsp/cache/diagnostics.go @@ -12,14 +12,18 @@ import ( "golang.org/x/tools/gopls/internal/util/bug" ) -// A CriticalError is a workspace-wide error that generally prevents gopls from -// functioning correctly. In the presence of critical errors, other diagnostics -// in the workspace may not make sense. -type CriticalError struct { +// A InitializationError is an error that causes snapshot initialization to fail. +// It is either the error returned from go/packages.Load, or an error parsing a +// workspace go.work or go.mod file. +// +// Such an error generally indicates that the View is malformed, and will never +// be usable. +type InitializationError struct { // MainError is the primary error. Must be non-nil. MainError error - // Diagnostics contains any supplemental (structured) diagnostics. + // Diagnostics contains any supplemental (structured) diagnostics extracted + // from the load error. Diagnostics map[protocol.DocumentURI][]*Diagnostic } diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go index a932567b72e..9831a4d2512 100644 --- a/gopls/internal/lsp/cache/load.go +++ b/gopls/internal/lsp/cache/load.go @@ -297,82 +297,6 @@ func (m *moduleErrorMap) Error() string { return buf.String() } -// workspaceLayoutError returns an error describing 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, []*Diagnostic) { - // TODO(rfindley): both of the checks below should be delegated to the workspace. - - if s.view.adjustedGO111MODULE() == "off" { - return nil, nil - } - - // If the user is using a go.work file, assume that they know what they are - // doing. - // - // TODO(golang/go#53880): improve orphaned file diagnostics when using go.work. - if s.view.typ == GoWorkView { - return nil, nil - } - - // Apply diagnostics about the workspace configuration to relevant open - // files. - openFiles := s.overlays() - - // If the snapshot does not have a valid build configuration, it may be - // that the user has opened a directory that contains multiple modules. - // Check for that an warn about it. - if s.view.typ == AdHocView { - msg := `gopls was not able to find modules in your workspace. -When outside of GOPATH, gopls needs to know which modules you are working on. -You can fix this by opening your workspace to a folder inside a Go module, or -by using a go.work file to specify multiple modules. -See the documentation for more information on setting up your workspace: -https://github.com/golang/tools/blob/master/gopls/doc/workspace.md.` - return fmt.Errorf(msg), s.applyCriticalErrorToFiles(ctx, msg, openFiles) - } - - return nil, nil -} - -func (s *Snapshot) applyCriticalErrorToFiles(ctx context.Context, msg string, files []*Overlay) []*Diagnostic { - var srcDiags []*Diagnostic - for _, fh := range files { - // Place the diagnostics on the package or module declarations. - var rng protocol.Range - switch s.FileKind(fh) { - case file.Go: - if pgf, err := s.ParseGo(ctx, fh, ParseHeader); err == nil { - // Check that we have a valid `package foo` range to use for positioning the error. - if pgf.File.Package.IsValid() && pgf.File.Name != nil && pgf.File.Name.End().IsValid() { - rng, _ = pgf.PosRange(pgf.File.Package, pgf.File.Name.End()) - } - } - case file.Mod: - if pmf, err := s.ParseMod(ctx, fh); err == nil { - if mod := pmf.File.Module; mod != nil && mod.Syntax != nil { - rng, _ = pmf.Mapper.OffsetRange(mod.Syntax.Start.Byte, mod.Syntax.End.Byte) - } - } - } - srcDiags = append(srcDiags, &Diagnostic{ - URI: fh.URI(), - Range: rng, - Severity: protocol.SeverityError, - Source: ListError, - Message: msg, - }) - } - return srcDiags -} - // buildMetadata populates the updates map with metadata updates to // apply, based on the given pkg. It recurs through pkg.Imports to ensure that // metadata exists for all dependencies. diff --git a/gopls/internal/lsp/cache/mod_tidy.go b/gopls/internal/lsp/cache/mod_tidy.go index 222b5a4c9dc..67c6d64549a 100644 --- a/gopls/internal/lsp/cache/mod_tidy.go +++ b/gopls/internal/lsp/cache/mod_tidy.go @@ -72,15 +72,6 @@ func (s *Snapshot) ModTidy(ctx context.Context, pm *ParsedModule) (*TidiedModule } } - if criticalErr := s.CriticalError(ctx); criticalErr != nil { - return &TidiedModule{ - Diagnostics: criticalErr.Diagnostics[fh.URI()], - }, 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 760991bf3ec..1b1ed129a79 100644 --- a/gopls/internal/lsp/cache/snapshot.go +++ b/gopls/internal/lsp/cache/snapshot.go @@ -39,7 +39,6 @@ import ( "golang.org/x/tools/gopls/internal/util/bug" "golang.org/x/tools/gopls/internal/util/constraints" "golang.org/x/tools/gopls/internal/util/immutable" - "golang.org/x/tools/gopls/internal/util/maps" "golang.org/x/tools/gopls/internal/util/pathutil" "golang.org/x/tools/gopls/internal/util/persistent" "golang.org/x/tools/gopls/internal/util/slices" @@ -93,18 +92,23 @@ type Snapshot struct { // TODO(rfindley): use atomic.Int32 on Go 1.19+. refcount int + // mu guards all of the maps in the snapshot, as well as the builtin URI and + // initialized. + mu sync.Mutex + // initialized reports whether the snapshot has been initialized. Concurrent // initialization is guarded by the view.initializationSema. Each snapshot is // initialized at most once: concurrent initialization is guarded by // view.initializationSema. initialized bool - // initializedErr holds the last error resulting from initialization. If + + // initialErr holds the last error resulting from initialization. If // initialization fails, we only retry when the workspace modules change, // to avoid too many go/packages calls. - initializedErr *CriticalError - - // mu guards all of the maps in the snapshot, as well as the builtin URI. - mu sync.Mutex + // If initialized is false, initialErr stil holds the error resulting from + // the previous initialization. + // TODO(rfindley): can we unify the lifecycle of initialized and initialErr. + initialErr *InitializationError // builtin is the location of builtin.go in GOROOT. // @@ -617,7 +621,7 @@ func (s *Snapshot) goCommandInvocation(ctx context.Context, flags InvocationFlag func (s *Snapshot) buildOverlay() map[string][]byte { overlays := make(map[string][]byte) - for _, overlay := range s.overlays() { + for _, overlay := range s.Overlays() { if overlay.saved { continue } @@ -629,7 +633,11 @@ func (s *Snapshot) buildOverlay() map[string][]byte { return overlays } -func (s *Snapshot) overlays() []*Overlay { +// Overlays returns the set of overlays at this snapshot. +// +// Note that this may differ from the set of overlays on the server, if the +// snapshot observed a historical state. +func (s *Snapshot) Overlays() []*Overlay { s.mu.Lock() defer s.mu.Unlock() @@ -1328,149 +1336,20 @@ func (s *Snapshot) MetadataGraph() *metadata.Graph { return s.meta } -func (s *Snapshot) awaitLoaded(ctx context.Context) error { - loadErr := s.awaitLoadedAllErrors(ctx) - - // TODO(rfindley): eliminate this function as part of simplifying - // CriticalErrors. - if loadErr != nil { - return loadErr.MainError - } - return nil -} - -// CriticalError returns any critical errors in the workspace. -// -// A nil result may mean success, or context cancellation. -func (s *Snapshot) CriticalError(ctx context.Context) *CriticalError { - // If we couldn't compute workspace mod files, then the load below is - // invalid. - // - // TODO(rfindley): is this a clear error to present to the user? - if s.view.workspaceModFilesErr != nil { - return &CriticalError{MainError: s.view.workspaceModFilesErr} - } - - loadErr := s.awaitLoadedAllErrors(ctx) - if loadErr != nil && errors.Is(loadErr.MainError, context.Canceled) { - return nil - } - - // Even if packages didn't fail to load, we still may want to show - // additional warnings. - if loadErr == nil { - active, _ := s.WorkspaceMetadata(ctx) - if msg := shouldShowAdHocPackagesWarning(s, active); msg != "" { - return &CriticalError{ - MainError: errors.New(msg), - } - } - // Even if workspace packages were returned, there still may be an error - // with the user's workspace layout. Workspace packages that only have the - // ID "command-line-arguments" are usually a symptom of a bad workspace - // configuration. - // - // This heuristic is path-dependent: we only get command-line-arguments - // packages when we've loaded using file scopes, which only occurs - // on-demand or via orphaned file reloading. - // - // TODO(rfindley): re-evaluate this heuristic. - if containsCommandLineArguments(active) { - err, diags := s.workspaceLayoutError(ctx) - if err != nil { - if ctx.Err() != nil { - return nil // see the API documentation for Snapshot - } - return &CriticalError{ - MainError: err, - Diagnostics: maps.Group(diags, byURI), - } - } - } - return nil - } - - if errMsg := loadErr.MainError.Error(); strings.Contains(errMsg, "cannot find main module") || strings.Contains(errMsg, "go.mod file not found") { - err, diags := s.workspaceLayoutError(ctx) - if err != nil { - if ctx.Err() != nil { - return nil // see the API documentation for Snapshot - } - return &CriticalError{ - MainError: err, - Diagnostics: maps.Group(diags, byURI), - } - } - } - return loadErr -} - -// A portion of this text is expected by TestBrokenWorkspace_OutsideModule. -const adHocPackagesWarning = `You are outside of a module and outside of $GOPATH/src. -If you are using modules, please open your editor to a directory in your module. -If you believe this warning is incorrect, please file an issue: https://github.com/golang/go/issues/new.` - -func shouldShowAdHocPackagesWarning(snapshot *Snapshot, active []*metadata.Package) string { - if snapshot.view.typ == AdHocView { - for _, mp := range active { - // A blank entry in DepsByImpPath - // indicates a missing dependency. - for _, importID := range mp.DepsByImpPath { - if importID == "" { - return adHocPackagesWarning - } - } - } - } - return "" -} - -func containsCommandLineArguments(metas []*metadata.Package) bool { - for _, mp := range metas { - if metadata.IsCommandLineArguments(mp.ID) { - return true - } - } - return false +// InitializationError returns the last error from initialization. +func (s *Snapshot) InitializationError() *InitializationError { + s.mu.Lock() + defer s.mu.Unlock() + return s.initialErr } -func (s *Snapshot) awaitLoadedAllErrors(ctx context.Context) *CriticalError { +// awaitLoaded awaits initialization and package reloading, and returns +// ctx.Err(). +func (s *Snapshot) awaitLoaded(ctx context.Context) error { // Do not return results until the snapshot's view has been initialized. s.AwaitInitialized(ctx) - - // TODO(rfindley): Should we be more careful about returning the - // initialization error? Is it possible for the initialization error to be - // corrected without a successful reinitialization? - if err := s.getInitializationError(); err != nil { - return err - } - - // TODO(rfindley): revisit this handling. Calling reloadWorkspace with a - // cancelled context should have the same effect, so this preemptive handling - // should not be necessary. - // - // Also: GetCriticalError ignores context cancellation errors. Should we be - // returning nil here? - if ctx.Err() != nil { - return &CriticalError{MainError: ctx.Err()} - } - - if err := s.reloadWorkspace(ctx); err != nil { - diags := s.extractGoCommandErrors(ctx, err) - return &CriticalError{ - MainError: err, - Diagnostics: maps.Group(diags, byURI), - } - } - - return nil -} - -func (s *Snapshot) getInitializationError() *CriticalError { - s.mu.Lock() - defer s.mu.Unlock() - - return s.initializedErr + s.reloadWorkspace(ctx) + return ctx.Err() } // AwaitInitialized waits until the snapshot's view is initialized. @@ -1486,7 +1365,7 @@ func (s *Snapshot) AwaitInitialized(ctx context.Context) { } // reloadWorkspace reloads the metadata for all invalidated workspace packages. -func (s *Snapshot) reloadWorkspace(ctx context.Context) error { +func (s *Snapshot) reloadWorkspace(ctx context.Context) { var scopes []loadScope var seen map[PackagePath]bool s.mu.Lock() @@ -1505,7 +1384,7 @@ func (s *Snapshot) reloadWorkspace(ctx context.Context) error { s.mu.Unlock() if len(scopes) == 0 { - return nil + return } // For an ad-hoc view, we cannot reload by package path. Just reload the view. @@ -1519,9 +1398,10 @@ func (s *Snapshot) reloadWorkspace(ctx context.Context) error { // of the metadata we attempted to load. if !errors.Is(err, context.Canceled) { s.clearShouldLoad(scopes...) + if err != nil { + event.Error(ctx, "reloading workspace", err, s.Labels()...) + } } - - return err } func (s *Snapshot) orphanedFileDiagnostics(ctx context.Context, overlays []*Overlay) ([]*Diagnostic, error) { @@ -1582,6 +1462,8 @@ searchOverlays: } } + initialErr := s.InitializationError() + for _, fh := range orphaned { pgf, rng, ok := orphanedFileDiagnosticRange(ctx, s.view.parseCache, fh) if !ok { @@ -1592,11 +1474,13 @@ searchOverlays: msg string // if non-empty, report a diagnostic with this message suggestedFixes []SuggestedFix // associated fixes, if any ) - // If we have a relevant go.mod file, check whether the file is orphaned - // due to its go.mod file being inactive. We could also offer a - // prescriptive diagnostic in the case that there is no go.mod file, but it - // is harder to be precise in that case, and less important. - if goMod, err := nearestModFile(ctx, fh.URI(), s); err == nil && goMod != "" { + if initialErr != nil { + msg = fmt.Sprintf("initialization failed: %v", initialErr.MainError) + } else if goMod, err := nearestModFile(ctx, fh.URI(), s); err == nil && goMod != "" { + // If we have a relevant go.mod file, check whether the file is orphaned + // due to its go.mod file being inactive. We could also offer a + // prescriptive diagnostic in the case that there is no go.mod file, but it + // is harder to be precise in that case, and less important. if _, ok := loadedModFiles[goMod]; !ok { modDir := filepath.Dir(goMod.Path()) viewDir := s.view.folder.Dir.Path() @@ -1801,7 +1685,7 @@ func (s *Snapshot) clone(ctx, bgCtx context.Context, changed StateChange) (*Snap cancel: cancel, builtin: s.builtin, initialized: s.initialized, - initializedErr: s.initializedErr, + initialErr: s.initialErr, packages: s.packages.Clone(), activePackages: s.activePackages.Clone(), files: s.files.Clone(changedFiles), @@ -1849,7 +1733,7 @@ func (s *Snapshot) clone(ctx, bgCtx context.Context, changed StateChange) (*Snap // // TODO(rfindley): revisit the location of this check. for uri := range changedFiles { - if inVendor(uri) && s.initializedErr != nil || + if inVendor(uri) && s.initialErr != nil || strings.HasSuffix(string(uri), "/vendor/modules.txt") { reinit = true break diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go index 3111346aa1c..49fa6988767 100644 --- a/gopls/internal/lsp/cache/view.go +++ b/gopls/internal/lsp/cache/view.go @@ -608,7 +608,24 @@ func (v *View) Snapshot() (*Snapshot, func(), error) { return v.snapshot, v.snapshot.Acquire(), nil } +// initialize loads the metadata (and currently, file contents, due to +// golang/go#57558) for the main package query of the View, which depends on +// the view type (see ViewType). If s.initialized is already true, initialize +// is a no op. +// +// The first attempt--which populates the first snapshot for a new view--must +// be allowed to run to completion without being cancelled. +// +// Subsequent attempts are triggered by conditions where gopls can't enumerate +// specific packages that require reloading, such as a change to a go.mod file. +// These attempts may be cancelled, and then retried by a later call. +// +// Postcondition: if ctx was not cancelled, s.initialized is true, s.initialErr +// holds the error resulting from initialization, if any, and s.metadata holds +// the resulting metadata graph. func (s *Snapshot) initialize(ctx context.Context, firstAttempt bool) { + // Acquire initializationSema, which is + // (in effect) a mutex with a timeout. select { case <-ctx.Done(): return @@ -627,25 +644,7 @@ func (s *Snapshot) initialize(ctx context.Context, firstAttempt bool) { return } - s.loadWorkspace(ctx, firstAttempt) -} - -func (s *Snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) (loadErr error) { - // A failure is retryable if it may have been due to context cancellation, - // and this is not the initial workspace load (firstAttempt==true). - // - // The IWL runs on a detached context with a long (~10m) timeout, so - // if the context was canceled we consider loading to have failed - // permanently. - retryableFailure := func() bool { - return loadErr != nil && ctx.Err() != nil && !firstAttempt - } defer func() { - if !retryableFailure() { - s.mu.Lock() - s.initialized = true - s.mu.Unlock() - } if firstAttempt { close(s.view.initialWorkspaceLoad) } @@ -668,8 +667,6 @@ func (s *Snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) (loadEr }) } - // TODO(rfindley): this should be predicated on the s.view.moduleMode(). - // There is no point loading ./... if we have an empty go.work. if len(s.view.workspaceModFiles) > 0 { for modURI := range s.view.workspaceModFiles { // Verify that the modfile is valid before trying to load it. @@ -683,7 +680,7 @@ func (s *Snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) (loadEr fh, err := s.ReadFile(ctx, modURI) if err != nil { if ctx.Err() != nil { - return ctx.Err() + return } addError(modURI, err) continue @@ -691,7 +688,7 @@ func (s *Snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) (loadEr parsed, err := s.ParseMod(ctx, fh) if err != nil { if ctx.Err() != nil { - return ctx.Err() + return } addError(modURI, err) continue @@ -716,43 +713,47 @@ func (s *Snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) (loadEr if len(scopes) > 0 { scopes = append(scopes, packageLoadScope("builtin")) } - loadErr = s.load(ctx, true, scopes...) + loadErr := s.load(ctx, true, scopes...) - if retryableFailure() { - return loadErr + // A failure is retryable if it may have been due to context cancellation, + // and this is not the initial workspace load (firstAttempt==true). + // + // The IWL runs on a detached context with a long (~10m) timeout, so + // if the context was canceled we consider loading to have failed + // permanently. + if loadErr != nil && ctx.Err() != nil && !firstAttempt { + return } - var criticalErr *CriticalError + var initialErr *InitializationError switch { case loadErr != nil && ctx.Err() != nil: event.Error(ctx, fmt.Sprintf("initial workspace load: %v", loadErr), loadErr) - criticalErr = &CriticalError{ + initialErr = &InitializationError{ MainError: loadErr, } case loadErr != nil: event.Error(ctx, "initial workspace load failed", loadErr) extractedDiags := s.extractGoCommandErrors(ctx, loadErr) - criticalErr = &CriticalError{ + initialErr = &InitializationError{ MainError: loadErr, - Diagnostics: maps.Group(append(modDiagnostics, extractedDiags...), byURI), + Diagnostics: maps.Group(extractedDiags, byURI), } - case len(modDiagnostics) == 1: - criticalErr = &CriticalError{ - MainError: fmt.Errorf(modDiagnostics[0].Message), - Diagnostics: maps.Group(modDiagnostics, byURI), + case s.view.workspaceModFilesErr != nil: + initialErr = &InitializationError{ + MainError: s.view.workspaceModFilesErr, } - case len(modDiagnostics) > 1: - criticalErr = &CriticalError{ - MainError: fmt.Errorf("error loading module names"), - Diagnostics: maps.Group(modDiagnostics, byURI), + case len(modDiagnostics) > 0: + initialErr = &InitializationError{ + MainError: fmt.Errorf(modDiagnostics[0].Message), } } - // Lock the snapshot when setting the initialized error. s.mu.Lock() defer s.mu.Unlock() - s.initializedErr = criticalErr - return loadErr + + s.initialized = true + s.initialErr = initialErr } // A StateChange describes external state changes that may affect a snapshot. diff --git a/gopls/internal/server/diagnostics.go b/gopls/internal/server/diagnostics.go index b34d1d649ca..460e119c77d 100644 --- a/gopls/internal/server/diagnostics.go +++ b/gopls/internal/server/diagnostics.go @@ -368,20 +368,27 @@ func (s *server) diagnose(ctx context.Context, snapshot *cache.Snapshot) (diagMa return diagnostics, ctx.Err() } - criticalErr := snapshot.CriticalError(ctx) - if ctx.Err() != nil { // must check ctx after GetCriticalError + initialErr := snapshot.InitializationError() + if ctx.Err() != nil { + // Don't update initialization status if the context is cancelled. return nil, ctx.Err() } - if criticalErr != nil { - store("critical error", criticalErr.Diagnostics, nil) + if initialErr != nil { + store("critical error", initialErr.Diagnostics, nil) } // 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.updateCriticalErrorStatus(ctx, snapshot, criticalErr) + statusErr := initialErr + if len(snapshot.Overlays()) == 0 { + // Don't report a hanging status message if there are no open files at this + // snapshot. + statusErr = nil + } + s.updateCriticalErrorStatus(ctx, snapshot, statusErr) // Diagnose template (.tmpl) files. tmplReports := template.Diagnostics(snapshot) @@ -597,8 +604,10 @@ const WorkspaceLoadFailure = "Error loading workspace" // updateCriticalErrorStatus updates the critical error progress notification // based on err. -// If err is nil, it clears any existing error progress report. -func (s *server) updateCriticalErrorStatus(ctx context.Context, snapshot *cache.Snapshot, err *cache.CriticalError) { +// +// If err is nil, or if there are no open files, it clears any existing error +// progress report. +func (s *server) updateCriticalErrorStatus(ctx context.Context, snapshot *cache.Snapshot, err *cache.InitializationError) { s.criticalErrorStatusMu.Lock() defer s.criticalErrorStatusMu.Unlock() diff --git a/gopls/internal/test/integration/diagnostics/diagnostics_test.go b/gopls/internal/test/integration/diagnostics/diagnostics_test.go index f6022602990..81720e743d7 100644 --- a/gopls/internal/test/integration/diagnostics/diagnostics_test.go +++ b/gopls/internal/test/integration/diagnostics/diagnostics_test.go @@ -554,8 +554,14 @@ func f() { Run(t, noModule, func(t *testing.T, env *Env) { env.OpenFile("a.go") env.AfterChange( - // Expect the adHocPackagesWarning. - OutstandingWork(server.WorkspaceLoadFailure, "outside of a module"), + // AdHoc views are not critical errors, but their missing import + // diagnostics should specifically mention GOROOT or GOPATH (and not + // modules). + NoOutstandingWork(nil), + Diagnostics( + env.AtRegexp("a.go", `"mod.com`), + WithMessage("GOROOT or GOPATH"), + ), ) // Deleting the import dismisses the warning. env.RegexpReplace("a.go", `import "mod.com/hello"`, "") diff --git a/gopls/internal/test/integration/expectation.go b/gopls/internal/test/integration/expectation.go index 97b77db1e42..eee7473dc22 100644 --- a/gopls/internal/test/integration/expectation.go +++ b/gopls/internal/test/integration/expectation.go @@ -500,7 +500,7 @@ func NoOutstandingWork(ignore func(title, msg string) bool) Expectation { // the "begin" notification, work should not be in progress. continue } - if ignore(w.title, w.msg) { + if ignore != nil && ignore(w.title, w.msg) { continue } return Unmet diff --git a/gopls/internal/test/integration/workspace/zero_config_test.go b/gopls/internal/test/integration/workspace/zero_config_test.go index a1991b5930e..dd75c591ddb 100644 --- a/gopls/internal/test/integration/workspace/zero_config_test.go +++ b/gopls/internal/test/integration/workspace/zero_config_test.go @@ -161,3 +161,27 @@ package a checkViews(summary()) }) } + +func TestCriticalErrorsInOrphanedFiles(t *testing.T) { + // This test checks that as we open and close files requiring a different + // port, the set of Views is adjusted accordingly. + const files = ` +-- go.mod -- +modul golang.org/lsptests/broken + +go 1.20 + +-- a.go -- +package broken + +const C = 0 +` + + Run(t, files, func(t *testing.T, env *Env) { + env.OpenFile("a.go") + env.AfterChange( + Diagnostics(env.AtRegexp("go.mod", "modul")), + Diagnostics(env.AtRegexp("a.go", "broken"), WithMessage("initialization failed")), + ) + }) +}