From e58f34171df65cd22cff6eb9040ad9798f622cf7 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 23 Apr 2019 14:36:30 -0400 Subject: [PATCH] internal/lsp: add more error propagation and logging for analyses This change propagates more errors from analyses, instead of just saving them in act.err, we actually return immediately. Ultimately, we'd want to return to the previous behavior, but this will help us figure out what's going wrong. Updates golang/go#30786 Change-Id: I9d3288fd113c43775140e5c008e3e300b6d28c2a Reviewed-on: https://go-review.googlesource.com/c/tools/+/173497 Run-TryBot: Rebecca Stambler Reviewed-by: Ian Cottrell --- internal/lsp/diagnostics.go | 3 +-- internal/lsp/source/analysis.go | 36 +++++++++++++++++------------- internal/lsp/source/diagnostics.go | 8 +++++++ 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index f587cbb5..244dc40a 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -67,8 +67,7 @@ func (s *Server) cacheAndDiagnose(ctx context.Context, uri span.URI, content str } }() - s.log.Debugf(ctx, "cacheAndDiagnose: done computing diagnostics for %s", uri) - + s.log.Debugf(ctx, "cacheAndDiagnose: returned from diagnostics for %s", uri) return nil } diff --git a/internal/lsp/source/analysis.go b/internal/lsp/source/analysis.go index b72095e0..a1bb2e3b 100644 --- a/internal/lsp/source/analysis.go +++ b/internal/lsp/source/analysis.go @@ -36,13 +36,15 @@ func analyze(ctx context.Context, v View, pkgs []Package, analyzers []*analysis. return nil, err } root.isroot = true + root.view = v roots = append(roots, root) } } // Execute the graph in parallel. - execAll(ctx, v.FileSet(), roots) - + if err := execAll(ctx, v.FileSet(), roots); err != nil { + return nil, err + } return roots, nil } @@ -64,6 +66,7 @@ type Action struct { diagnostics []analysis.Diagnostic err error duration time.Duration + view View } type objectFactKey struct { @@ -85,22 +88,25 @@ func execAll(ctx context.Context, fset *token.FileSet, actions []*Action) error for _, act := range actions { act := act g.Go(func() error { - act.exec(ctx, fset) - return nil + return act.exec(ctx, fset) }) } return g.Wait() } -func (act *Action) exec(ctx context.Context, fset *token.FileSet) { +func (act *Action) exec(ctx context.Context, fset *token.FileSet) error { + var err error act.once.Do(func() { - act.execOnce(ctx, fset) + err = act.execOnce(ctx, fset) }) + return err } -func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) { +func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) error { // Analyze dependencies. - execAll(ctx, fset, act.Deps) + if err := execAll(ctx, fset, act.Deps); err != nil { + return err + } // Report an error if any dependency failed. var failed []string @@ -112,7 +118,7 @@ func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) { if failed != nil { sort.Strings(failed) act.err = fmt.Errorf("failed prerequisites: %s", strings.Join(failed, ", ")) - return + return act.err } // Plumb the output values of the dependencies @@ -152,24 +158,24 @@ func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) { } act.pass = pass - var err error if len(act.Pkg.GetErrors()) > 0 && !pass.Analyzer.RunDespiteErrors { - err = fmt.Errorf("analysis skipped due to errors in package") + act.err = fmt.Errorf("analysis skipped due to errors in package") } else { - act.result, err = pass.Analyzer.Run(pass) - if err == nil { + act.result, act.err = pass.Analyzer.Run(pass) + if act.err == nil { if got, want := reflect.TypeOf(act.result), pass.Analyzer.ResultType; got != want { - err = fmt.Errorf( + act.err = fmt.Errorf( "internal error: on package %s, analyzer %s returned a result of type %v, but declared ResultType %v", pass.Pkg.Path(), pass.Analyzer, got, want) } } } - act.err = err // disallow calls after Run pass.ExportObjectFact = nil pass.ExportPackageFact = nil + + return act.err } // inheritFacts populates act.facts with diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index b052be58..67e28c10 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -98,8 +98,12 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag } } if len(diags) > 0 { + v.Logger().Debugf(ctx, "found parse or type-checking errors for %s, returning", uri) return reports, nil } + + v.Logger().Debugf(ctx, "running `go vet` analyses for %s", uri) + // Type checking and parsing succeeded. Run analyses. runAnalyses(ctx, v, pkg, func(a *analysis.Analyzer, diag analysis.Diagnostic) error { r := span.NewRange(v.FileSet(), diag.Pos, 0) @@ -121,6 +125,8 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag return nil }) + v.Logger().Debugf(ctx, "completed reporting `go vet` analyses for %s", uri) + return reports, nil } @@ -200,6 +206,8 @@ func runAnalyses(ctx context.Context, v View, pkg Package, report func(a *analys return err } + v.Logger().Debugf(ctx, "analyses have completed for %s", pkg.GetTypes().Path()) + // Report diagnostics and errors from root analyzers. for _, r := range roots { for _, diag := range r.diagnostics {