internal/lsp: add more error handling to analysis

Updates golang/go#30786

Change-Id: Icf054b9bcd62b36e3aa55288946a9f0d1c845300
Reviewed-on: https://go-review.googlesource.com/c/tools/+/172972
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Cottrell <[email protected]>

Author: stamblerre

internal/lsp/source/analysis.go

@@ -18,27 +18,32 @@ import (
 	"sync"
 	"time"
 
+	"golang.org/x/sync/errgroup"
 	"golang.org/x/tools/go/analysis"
 )
 
-func analyze(ctx context.Context, v View, pkgs []Package, analyzers []*analysis.Analyzer) []*Action {
+func analyze(ctx context.Context, v View, pkgs []Package, analyzers []*analysis.Analyzer) ([]*Action, error) {
+	if ctx.Err() != nil {
+		return nil, ctx.Err()
+	}
+
 	// Build nodes for initial packages.
 	var roots []*Action
 	for _, a := range analyzers {
 		for _, pkg := range pkgs {
 			root, err := pkg.GetActionGraph(ctx, a)
 			if err != nil {
-				continue
+				return nil, err
 			}
 			root.isroot = true
 			roots = append(roots, root)
 		}
 	}
 
 	// Execute the graph in parallel.
-	execAll(v.FileSet(), roots)
+	execAll(ctx, v.FileSet(), roots)
 
-	return roots
+	return roots, nil
 }
 
 // An action represents one unit of analysis work: the application of
@@ -75,28 +80,27 @@ func (act *Action) String() string {
 	return fmt.Sprintf("%s@%s", act.Analyzer, act.Pkg)
 }
 
-func execAll(fset *token.FileSet, actions []*Action) {
-	var wg sync.WaitGroup
+func execAll(ctx context.Context, fset *token.FileSet, actions []*Action) error {
+	g, ctx := errgroup.WithContext(ctx)
 	for _, act := range actions {
-		wg.Add(1)
-		work := func(act *Action) {
-			act.exec(fset)
-			wg.Done()
-		}
-		go work(act)
+		act := act
+		g.Go(func() error {
+			act.exec(ctx, fset)
+			return nil
+		})
 	}
-	wg.Wait()
+	return g.Wait()
 }
 
-func (act *Action) exec(fset *token.FileSet) {
+func (act *Action) exec(ctx context.Context, fset *token.FileSet) {
 	act.once.Do(func() {
-		act.execOnce(fset)
+		act.execOnce(ctx, fset)
 	})
 }
 
-func (act *Action) execOnce(fset *token.FileSet) {
+func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) {
 	// Analyze dependencies.
-	execAll(fset, act.Deps)
+	execAll(ctx, fset, act.Deps)
 
 	// Report an error if any dependency failed.
 	var failed []string

internal/lsp/source/diagnostics.go

@@ -101,13 +101,12 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag
 		return reports, nil
 	}
 	// Type checking and parsing succeeded. Run analyses.
-	runAnalyses(ctx, v, pkg, func(a *analysis.Analyzer, diag analysis.Diagnostic) {
+	runAnalyses(ctx, v, pkg, func(a *analysis.Analyzer, diag analysis.Diagnostic) error {
 		r := span.NewRange(v.FileSet(), diag.Pos, 0)
 		s, err := r.Span()
 		if err != nil {
-			//TODO: we could not process the diag.Pos, and thus have no valid span
-			//we don't have anywhere to put this error though
-			v.Logger().Errorf(ctx, "%v", err)
+			// The diagnostic has an invalid position, so we don't have a valid span.
+			return err
 		}
 		category := a.Name
 		if diag.Category != "" {
@@ -119,6 +118,7 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag
 			Message:  diag.Message,
 			Severity: SeverityWarning,
 		})
+		return nil
 	})
 
 	return reports, nil
@@ -168,8 +168,8 @@ func singleDiagnostic(uri span.URI, format string, a ...interface{}) map[span.UR
 	}
 }
 
-func runAnalyses(ctx context.Context, v View, pkg Package, report func(a *analysis.Analyzer, diag analysis.Diagnostic)) error {
-	// the traditional vet suite:
+func runAnalyses(ctx context.Context, v View, pkg Package, report func(a *analysis.Analyzer, diag analysis.Diagnostic) error) error {
+	// The traditional vet suite:
 	analyzers := []*analysis.Analyzer{
 		asmdecl.Analyzer,
 		assign.Analyzer,
@@ -195,7 +195,10 @@ func runAnalyses(ctx context.Context, v View, pkg Package, report func(a *analys
 		unusedresult.Analyzer,
 	}
 
-	roots := analyze(ctx, v, []Package{pkg}, analyzers)
+	roots, err := analyze(ctx, v, []Package{pkg}, analyzers)
+	if err != nil {
+		return err
+	}
 
 	// Report diagnostics and errors from root analyzers.
 	for _, r := range roots {
@@ -205,9 +208,10 @@ func runAnalyses(ctx context.Context, v View, pkg Package, report func(a *analys
 				// which isn't super useful...
 				return r.err
 			}
-			report(r.Analyzer, diag)
+			if err := report(r.Analyzer, diag); err != nil {
+				return err
+			}
 		}
 	}
-
 	return nil
 }