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 <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

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
 }
 

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 act.exec(ctx, fset)
-			return nil
 		})
 	}
 	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)
+		act.result, act.err = pass.Analyzer.Run(pass)
-		if err == nil {
+		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

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 {