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 <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/source/analysis.go b/internal/lsp/source/analysis.go
index 6cf8df4..b72095e 100644
--- a/internal/lsp/source/analysis.go
+++ b/internal/lsp/source/analysis.go
@@ -18,17 +18,22 @@
"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)
@@ -36,9 +41,9 @@
}
// 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 @@
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
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index fb600f7..b052be5 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -101,13 +101,12 @@
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 @@
Message: diag.Message,
Severity: SeverityWarning,
})
+ return nil
})
return reports, nil
@@ -168,8 +168,8 @@
}
}
-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 @@
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 @@
// which isn't super useful...
return r.err
}
- report(r.Analyzer, diag)
+ if err := report(r.Analyzer, diag); err != nil {
+ return err
+ }
}
}
-
return nil
}
