internal/lsp: show critical error pop-ups as progress reports

We've been looking for a way to show unintrusive error status reports to
users--try using progress reports as a way of populating a status bar.
This avoids the problem of annoying the user with constant pop-ups.

Whenever an error is returns from (*snapshot).WorkspacePackages, we
start a progress report with the error message. If the error goes away
on the next call to diagnostics, or the error message changes, we will
either remove or update the progress report.

Screencast: https://drive.google.com/file/d/1tG9pc_tPsLoqkQHiJqdY8b06TzTkPVae/view?usp=sharing&resourcekey=0-CEG_LhGHYiFp9S37ut_kgw

Updates golang/go#42250

Change-Id: I8a529a911883092bc08af32246719d883dc5f5a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/268677
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>

Author: stamblerre

gopls/internal/regtest/diagnostics_test.go

@@ -1452,3 +1452,35 @@ package foo_`
 		)
 	})
 }
+
+// TestProgressBarErrors confirms that critical workspace load errors are shown
+// and updated via progress reports.
+func TestProgressBarErrors(t *testing.T) {
+	testenv.NeedsGo1Point(t, 14)
+
+	const pkg = `
+-- go.mod --
+modul mod.com
+
+go 1.12
+-- main.go --
+package main
+`
+	run(t, pkg, func(t *testing.T, env *Env) {
+		env.OpenFile("go.mod")
+		env.Await(
+			OutstandingWork("Error loading workspace", "unknown directive"),
+		)
+		env.EditBuffer("go.mod", fake.NewEdit(0, 0, 3, 0, `module mod.com
+
+go 1.hello
+`))
+		env.Await(
+			OutstandingWork("Error loading workspace", "invalid go version"),
+		)
+		env.RegexpReplace("go.mod", "go 1.hello", "go 1.12")
+		env.Await(
+			NoOutstandingWork(),
+		)
+	})
+}

gopls/internal/regtest/env.go

@@ -58,8 +58,8 @@ type State struct {
 }
 
 type workProgress struct {
-	title   string
-	percent float64
+	title, msg string
+	percent    float64
 }
 
 func (s State) String() string {
@@ -200,10 +200,16 @@ func (e *Env) onProgress(_ context.Context, m *protocol.ProgressParams) error {
 	switch kind := v["kind"]; kind {
 	case "begin":
 		work.title = v["title"].(string)
+		if msg, ok := v["message"]; ok {
+			work.msg = msg.(string)
+		}
 	case "report":
 		if pct, ok := v["percentage"]; ok {
 			work.percent = pct.(float64)
 		}
+		if msg, ok := v["message"]; ok {
+			work.msg = msg.(string)
+		}
 	case "end":
 		title := e.state.outstandingWork[m.Token].title
 		e.state.completedWork[title] = e.state.completedWork[title] + 1

gopls/internal/regtest/expectation.go

@@ -202,6 +202,24 @@ func CompletedWork(title string, atLeast int) SimpleExpectation {
 	}
 }
 
+// OutstandingWork expects a work item to be outstanding. The given title must
+// be an exact match, whereas the given msg must only be contained in the work
+// item's message.
+func OutstandingWork(title, msg string) SimpleExpectation {
+	check := func(s State) Verdict {
+		for _, work := range s.outstandingWork {
+			if work.title == title && strings.Contains(work.msg, msg) {
+				return Met
+			}
+		}
+		return Unmet
+	}
+	return SimpleExpectation{
+		check:       check,
+		description: fmt.Sprintf("outstanding work: %s", title),
+	}
+}
+
 // LogExpectation is an expectation on the log messages received by the editor
 // from gopls.
 type LogExpectation struct {

internal/lsp/diagnostics.go

@@ -182,34 +182,26 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
 
 	// Diagnose all of the packages in the workspace.
 	wsPkgs, err := snapshot.WorkspacePackages(ctx)
+	if s.shouldIgnoreError(ctx, snapshot, err) {
+		return nil, 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.showCriticalErrorStatus(ctx, err)
+
 	if err != nil {
-		if errors.Is(err, context.Canceled) {
-			return nil, nil
-		}
-		// Some error messages can be displayed as diagnostics.
+		// Some error messages can also be displayed as diagnostics.
 		if errList := (*source.ErrorList)(nil); errors.As(err, &errList) {
 			if err := errorsToDiagnostic(ctx, snapshot, *errList, reports); err == nil {
 				return reports, nil
 			}
 		}
-		// Try constructing a more helpful error message out of this error.
-		if s.handleFatalErrors(ctx, snapshot, modErr, err) {
-			return nil, nil
-		}
 		event.Error(ctx, "errors diagnosing workspace", err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder()))
-		// Present any `go list` errors directly to the user.
-		if errors.Is(err, source.PackagesLoadError) {
-			if err := s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
-				Type: protocol.Error,
-				Message: fmt.Sprintf(`The code in the workspace failed to compile (see the error message below).
-If you believe this is a mistake, please file an issue: https://github.com/golang/go/issues/new.
-%v`, err),
-			}); err != nil {
-				event.Error(ctx, "ShowMessage failed", err, tag.Directory.Of(snapshot.View().Folder().Filename()))
-			}
-		}
 		return nil, nil
 	}
+
 	var (
 		showMsgMu sync.Mutex
 		showMsg   *protocol.ShowMessageParams
@@ -299,6 +291,36 @@ If you believe this is a mistake, please file an issue: https://github.com/golan
 	return reports, showMsg
 }
 
+// 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) {
+	s.criticalErrorStatusMu.Lock()
+	defer s.criticalErrorStatusMu.Unlock()
+
+	// Remove all newlines so that the error message can be formatted in a
+	// status bar.
+	var errMsg string
+	if err != nil {
+		errMsg = strings.Replace(err.Error(), "\n", " ", -1)
+	}
+
+	if s.criticalErrorStatus == nil {
+		if errMsg != "" {
+			s.criticalErrorStatus = s.progress.start(ctx, "Error loading workspace", errMsg, nil, nil)
+		}
+		return
+	}
+
+	// If an error is already shown to the user, update it or mark it as
+	// resolved.
+	if errMsg == "" {
+		s.criticalErrorStatus.end("Done.")
+		s.criticalErrorStatus = nil
+	} else {
+		s.criticalErrorStatus.report(errMsg, 0)
+	}
+}
+
 // checkForOrphanedFile checks that the given URIs can be mapped to packages.
 // If they cannot and the workspace is not otherwise unloaded, it also surfaces
 // a warning, suggesting that the user check the file for build tags.
@@ -470,7 +492,13 @@ func toProtocolDiagnostics(diagnostics []*source.Diagnostic) []protocol.Diagnost
 	return reports
 }
 
-func (s *Server) handleFatalErrors(ctx context.Context, snapshot source.Snapshot, modErr, loadErr error) bool {
+func (s *Server) shouldIgnoreError(ctx context.Context, snapshot source.Snapshot, err error) bool {
+	if err == nil { // if there is no error at all
+		return false
+	}
+	if errors.Is(err, context.Canceled) {
+		return true
+	}
 	// 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 {

internal/lsp/server.go

@@ -96,13 +96,19 @@ type Server struct {
 	gcOptimizationDetailsMu sync.Mutex
 	gcOptimizationDetails   map[span.URI]struct{}
 
-	// diagnosticsSema limits the concurrency of diagnostics runs, which can be expensive.
+	// diagnosticsSema limits the concurrency of diagnostics runs, which can be
+	// expensive.
 	diagnosticsSema chan struct{}
 
 	progress *progressTracker
 
 	// debouncer is used for debouncing diagnostics.
 	debouncer *debouncer
+
+	// When the workspace fails to load, we show its status through a progress
+	// report with an error message.
+	criticalErrorStatusMu sync.Mutex
+	criticalErrorStatus   *workDone
 }
 
 // sentDiagnostics is used to cache diagnostics that have been sent for a given file.