From c6b416e8a41dc8e1c3df588301c4740ce59da876 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Mon, 22 Apr 2019 17:05:43 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/source/analysis.go | 38 +++++++++++++++++------------- internal/lsp/source/diagnostics.go | 22 ++++++++++------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/internal/lsp/source/analysis.go b/internal/lsp/source/analysis.go index 6cf8df43..b72095e0 100644 --- a/internal/lsp/source/analysis.go +++ b/internal/lsp/source/analysis.go @@ -18,17 +18,22 @@ 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) @@ -36,9 +41,9 @@ func analyze(ctx context.Context, v View, pkgs []Package, analyzers []*analysis. } // 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 diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index fb600f7d..b052be58 100644 --- a/internal/lsp/source/diagnostics.go +++ b/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 }