internal/lsp: add support for analyzers with dependencies on other analyzers

Steal alan's parallel analysis-graph-running code from multichecker.
Facts are still not supported.

Change-Id: I22f83375d7a314b49d4f458d6dd40c33febc795b
Reviewed-on: https://go-review.googlesource.com/c/161659
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/lsp_test.go

@@ -36,7 +36,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	// We hardcode the expected number of test cases to ensure that all tests
 	// are being executed. If a test is added, this number must be changed.
 	const expectedCompletionsCount = 63
-	const expectedDiagnosticsCount = 15
+	const expectedDiagnosticsCount = 16
 	const expectedFormatCount = 3
 	const expectedDefinitionsCount = 16
 	const expectedTypeDefinitionsCount = 2
@@ -206,7 +206,7 @@ func diffDiagnostics(filename string, want, got []protocol.Diagnostic) string {
 			if g.Range.Start != g.Range.End || w.Range.Start != g.Range.End {
 				goto Failed
 			}
-		} else {
+		} else if g.Range.End != g.Range.Start { // Accept any 'want' range if the diagnostic returns a zero-length range.
 			if w.Range.End != g.Range.End {
 				goto Failed
 			}

internal/lsp/source/diagnostics.go

@@ -9,8 +9,30 @@ import (
 	"context"
 	"fmt"
 	"go/token"
+	"sort"
 	"strconv"
 	"strings"
+	"sync"
+
+	"golang.org/x/tools/go/analysis/passes/asmdecl"
+	"golang.org/x/tools/go/analysis/passes/assign"
+	"golang.org/x/tools/go/analysis/passes/atomic"
+	"golang.org/x/tools/go/analysis/passes/atomicalign"
+	"golang.org/x/tools/go/analysis/passes/bools"
+	"golang.org/x/tools/go/analysis/passes/buildtag"
+	"golang.org/x/tools/go/analysis/passes/cgocall"
+	"golang.org/x/tools/go/analysis/passes/composite"
+	"golang.org/x/tools/go/analysis/passes/copylock"
+	"golang.org/x/tools/go/analysis/passes/httpresponse"
+	"golang.org/x/tools/go/analysis/passes/loopclosure"
+	"golang.org/x/tools/go/analysis/passes/nilfunc"
+	"golang.org/x/tools/go/analysis/passes/shift"
+	"golang.org/x/tools/go/analysis/passes/stdmethods"
+	"golang.org/x/tools/go/analysis/passes/structtag"
+	"golang.org/x/tools/go/analysis/passes/unmarshal"
+	"golang.org/x/tools/go/analysis/passes/unreachable"
+	"golang.org/x/tools/go/analysis/passes/unsafeptr"
+	"golang.org/x/tools/go/analysis/passes/unusedresult"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/tests"
@@ -166,34 +188,128 @@ func identifierEnd(content []byte, l, c int) (int, error) {
 }
 
 func runAnalyses(pkg *packages.Package, report func(a *analysis.Analyzer, diag analysis.Diagnostic)) error {
+	// These are the analyses in the vetsuite, except for lostcancel and printf.
+	// Those rely on facts from other packages.
+	// TODO(matloob): Add fact support.
 	analyzers := []*analysis.Analyzer{
-		tests.Analyzer, // an analyzer that doesn't have facts or requires
+		asmdecl.Analyzer,
+		assign.Analyzer,
+		atomic.Analyzer,
+		atomicalign.Analyzer,
+		bools.Analyzer,
+		buildtag.Analyzer,
+		cgocall.Analyzer,
+		composite.Analyzer,
+		copylock.Analyzer,
+		httpresponse.Analyzer,
+		loopclosure.Analyzer,
+		nilfunc.Analyzer,
+		shift.Analyzer,
+		stdmethods.Analyzer,
+		structtag.Analyzer,
+		tests.Analyzer,
+		unmarshal.Analyzer,
+		unreachable.Analyzer,
+		unsafeptr.Analyzer,
+		unusedresult.Analyzer,
+	}
+
+	// Execute actions in parallel. Based on unitchecker's analysis execution logic.
+	type action struct {
+		once        sync.Once
+		result      interface{}
+		err         error
+		diagnostics []analysis.Diagnostic
+	}
+	actions := make(map[*analysis.Analyzer]*action)
+
+	var registerActions func(a *analysis.Analyzer)
+	registerActions = func(a *analysis.Analyzer) {
+		act, ok := actions[a]
+		if !ok {
+			act = new(action)
+			for _, req := range a.Requires {
+				registerActions(req)
+			}
+			actions[a] = act
+		}
 	}
 	for _, a := range analyzers {
-		if len(a.FactTypes) > 0 {
-			panic("for analyzer " + a.Name + " modular analyses needing facts are not yet supported")
-		}
-		if len(a.Requires) > 0 {
-			panic("for analyzer " + a.Name + " analyses requiring results are not yet supported")
-		}
-		pass := &analysis.Pass{
-			Analyzer: a,
+		registerActions(a)
+	}
 
-			Fset:       pkg.Fset,
-			Files:      pkg.Syntax,
-			OtherFiles: pkg.OtherFiles,
-			Pkg:        pkg.Types,
-			TypesInfo:  pkg.TypesInfo,
-			TypesSizes: pkg.TypesSizes,
+	// In parallel, execute the DAG of analyzers.
+	var exec func(a *analysis.Analyzer) *action
+	var execAll func(analyzers []*analysis.Analyzer)
+	exec = func(a *analysis.Analyzer) *action {
+		act := actions[a]
+		act.once.Do(func() {
+			if len(a.FactTypes) > 0 {
+				panic("for analyzer " + a.Name + " modular analyses needing facts are not yet supported")
+			}
 
-			Report: func(diagnostic analysis.Diagnostic) { report(a, diagnostic) },
+			execAll(a.Requires) // prefetch dependencies in parallel
 
-			// TODO(matloob): Fill in the fields ResultOf, ImportObjectFact, ImportPackageFact,
-			// ExportObjectFact, ExportPackageFact once modular facts and results are supported.
+			// The inputs to this analysis are the
+			// results of its prerequisites.
+			inputs := make(map[*analysis.Analyzer]interface{})
+			var failed []string
+			for _, req := range a.Requires {
+				reqact := exec(req)
+				if reqact.err != nil {
+					failed = append(failed, req.String())
+					continue
+				}
+				inputs[req] = reqact.result
+			}
+
+			// Report an error if any dependency failed.
+			if failed != nil {
+				sort.Strings(failed)
+				act.err = fmt.Errorf("failed prerequisites: %s", strings.Join(failed, ", "))
+				return
+			}
+
+			pass := &analysis.Pass{
+				Analyzer:   a,
+				Fset:       pkg.Fset,
+				Files:      pkg.Syntax,
+				OtherFiles: pkg.OtherFiles,
+				Pkg:        pkg.Types,
+				TypesInfo:  pkg.TypesInfo,
+				TypesSizes: pkg.TypesSizes,
+				ResultOf:   inputs,
+				Report:     func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) },
+			}
+
+			act.result, act.err = a.Run(pass)
+		})
+		return act
+	}
+	execAll = func(analyzers []*analysis.Analyzer) {
+		var wg sync.WaitGroup
+		for _, a := range analyzers {
+			wg.Add(1)
+			go func(a *analysis.Analyzer) {
+				_ = exec(a)
+				wg.Done()
+			}(a)
 		}
-		_, err := a.Run(pass)
-		if err != nil {
-			return err
+		wg.Wait()
+	}
+
+	execAll(analyzers)
+
+	// Report diagnostics and errors from root analyzers.
+	for _, a := range analyzers {
+		act := actions[a]
+		if act.err != nil {
+			// TODO(matloob): This isn't quite right: we might return a failed prerequisites error,
+			// which isn't super useful...
+			return act.err
+		}
+		for _, diag := range act.diagnostics {
+			report(a, diag)
 		}
 	}
 

internal/lsp/testdata/analyzer/bad_test.go

@@ -1,7 +1,11 @@
 package analyzer
 
-import "testing"
+import (
+	"sync"
+	"testing"
+)
 
 func Testbad(t *testing.T) { //@diag("", "tests", "Testbad has malformed name: first letter after 'Test' must not be lowercase")
-
+	var x sync.Mutex
+	_ = x //@diag("x", "copylocks", "assignment copies lock value to _: sync.Mutex")
 }