Add nogo configuration.

This splits the nogo rules into a separate configuration yaml file, and
allows for multiple files to be provided.

Because attrs cannot be passed down to aspects, this required that all
findings are propagated up the aspect Provider. This doesn't mean that
any extra work must be done, just that this information must be carried
through the graph, and some additional starlark complexity is required.

PiperOrigin-RevId: 339076357

1 files changed, 31 insertions, 144 deletions

diff --git a/tools/nogo/nogo.go b/tools/nogo/nogo.go
index e19e3c237..779d4d6d8 100644
--- a/tools/nogo/nogo.go
+++ b/tools/nogo/nogo.go
@@ -21,7 +21,6 @@ package nogo
 import (
 	"encoding/json"
 	"errors"
-	"flag"
 	"fmt"
 	"go/ast"
 	"go/build"
@@ -45,20 +44,20 @@ import (
 	"gvisor.dev/gvisor/tools/checkescape"
 )
 
-// stdlibConfig is serialized as the configuration.
+// StdlibConfig is serialized as the configuration.
 //
 // This contains everything required for stdlib analysis.
-type stdlibConfig struct {
+type StdlibConfig struct {
 	Srcs   []string
 	GOOS   string
 	GOARCH string
 	Tags   []string
 }
 
-// packageConfig is serialized as the configuration.
+// PackageConfig is serialized as the configuration.
 //
 // This contains everything required for single package analysis.
-type packageConfig struct {
+type PackageConfig struct {
 	ImportPath  string
 	GoFiles     []string
 	NonGoFiles  []string
@@ -84,7 +83,7 @@ type saver func([]byte) error
 //
 // This is done because all stdlib data is stored together, and we don't want
 // to load this data many times over.
-func (c *packageConfig) factLoader() (loader, error) {
+func (c *PackageConfig) factLoader() (loader, error) {
 	allFacts := make(map[string][]byte)
 	if c.StdlibFacts != "" {
 		data, err := ioutil.ReadFile(c.StdlibFacts)
@@ -114,7 +113,7 @@ func (c *packageConfig) factLoader() (loader, error) {
 // shouldInclude indicates whether the file should be included.
 //
 // NOTE: This does only basic parsing of tags.
-func (c *packageConfig) shouldInclude(path string) (bool, error) {
+func (c *PackageConfig) shouldInclude(path string) (bool, error) {
 	ctx := build.Default
 	ctx.GOOS = c.GOOS
 	ctx.GOARCH = c.GOARCH
@@ -128,7 +127,7 @@ func (c *packageConfig) shouldInclude(path string) (bool, error) {
 // files, and the facts. Note that this importer implementation will always
 // pass when a given package is not available.
 type importer struct {
-	*packageConfig
+	*PackageConfig
 	fset     *token.FileSet
 	cache    map[string]*types.Package
 	lastErr  error
@@ -185,14 +184,14 @@ func (i *importer) Import(path string) (*types.Package, error) {
 // ErrSkip indicates the package should be skipped.
 var ErrSkip = errors.New("skipped")
 
-// checkStdlib checks the standard library.
+// CheckStdlib checks the standard library.
 //
 // This constructs a synthetic package configuration for each library in the
-// standard library sources, and call checkPackage repeatedly.
+// standard library sources, and call CheckPackage repeatedly.
 //
 // Note that not all parts of the source are expected to build. We skip obvious
 // test files, and cmd files, which should not be dependencies.
-func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]string, []byte, error) {
+func CheckStdlib(config *StdlibConfig, analyzers []*analysis.Analyzer) (allFindings []Finding, facts []byte, err error) {
 	if len(config.Srcs) == 0 {
 		return nil, nil, nil
 	}
@@ -225,7 +224,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str
 	}
 
 	// Aggregate all files by directory.
-	packages := make(map[string]*packageConfig)
+	packages := make(map[string]*PackageConfig)
 	for _, file := range config.Srcs {
 		if !strings.HasPrefix(file, rootSrcPrefix) {
 			// Superflouous file.
@@ -243,7 +242,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str
 		}
 		c, ok := packages[pkg]
 		if !ok {
-			c = &packageConfig{
+			c = &PackageConfig{
 				ImportPath: pkg,
 				GOOS:       config.GOOS,
 				GOARCH:     config.GOARCH,
@@ -262,7 +261,6 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str
 	}
 
 	// Closure to check a single package.
-	allFindings := make([]string, 0)
 	stdlibFacts := make(map[string][]byte)
 	stdlibErrs := make(map[string]error)
 	var checkOne func(pkg string) error // Recursive.
@@ -301,7 +299,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str
 		}()
 
 		// Run the analysis.
-		findings, factData, err := checkPackage(config, ac, checkOne)
+		findings, factData, err := CheckPackage(config, analyzers, checkOne)
 		if err != nil {
 			// If we can't analyze a package from the standard library,
 			// then we skip it. It will simply not have any findings.
@@ -344,7 +342,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str
 	return allFindings, factData, nil
 }
 
-// checkPackage runs all analyzers.
+// CheckPackage runs all given analyzers.
 //
 // The implementation was adapted from [1], which was in turn adpated from [2].
 // This returns a list of matching analysis issues, or an error if the analysis
@@ -352,9 +350,9 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str
 //
 // [1] bazelbuid/rules_go/tools/builders/nogo_main.go
 // [2] golang.org/x/tools/go/checker/internal/checker
-func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, importCallback func(string) error) ([]string, []byte, error) {
+func CheckPackage(config *PackageConfig, analyzers []*analysis.Analyzer, importCallback func(string) error) (findings []Finding, factData []byte, err error) {
 	imp := &importer{
-		packageConfig: config,
+		PackageConfig: config,
 		fset:          token.NewFileSet(),
 		cache:         make(map[string]*types.Package),
 		callback:      importCallback,
@@ -406,7 +404,6 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo
 	// Register fact types and establish dependencies between analyzers.
 	// The visit closure will execute recursively, and populate results
 	// will all required analysis results.
-	diagnostics := make(map[*analysis.Analyzer][]analysis.Diagnostic)
 	results := make(map[*analysis.Analyzer]interface{})
 	var visit func(*analysis.Analyzer) error // For recursion.
 	visit = func(a *analysis.Analyzer) error {
@@ -421,27 +418,25 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo
 			}
 		}
 
-		// Prepare the matcher.
-		m := ac[a]
-		report := func(d analysis.Diagnostic) {
-			if m.ShouldReport(d, imp.fset) {
-				diagnostics[a] = append(diagnostics[a], d)
-			}
-		}
-
 		// Run the analysis.
 		factFilter := make(map[reflect.Type]bool)
 		for _, f := range a.FactTypes {
 			factFilter[reflect.TypeOf(f)] = true
 		}
 		p := &analysis.Pass{
-			Analyzer:          a,
-			Fset:              imp.fset,
-			Files:             syntax,
-			Pkg:               types,
-			TypesInfo:         typesInfo,
-			ResultOf:          results, // All results.
-			Report:            report,
+			Analyzer:  a,
+			Fset:      imp.fset,
+			Files:     syntax,
+			Pkg:       types,
+			TypesInfo: typesInfo,
+			ResultOf:  results, // All results.
+			Report: func(d analysis.Diagnostic) {
+				findings = append(findings, Finding{
+					Category: AnalyzerName(a.Name),
+					Position: imp.fset.Position(d.Pos),
+					Message:  d.Message,
+				})
+			},
 			ImportPackageFact: facts.ImportPackageFact,
 			ExportPackageFact: facts.ExportPackageFact,
 			ImportObjectFact:  facts.ImportObjectFact,
@@ -464,7 +459,7 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo
 	}
 
 	// Visit all analyzers recursively.
-	for a, _ := range ac {
+	for _, a := range analyzers {
 		if imp.lastErr == ErrSkip {
 			continue // No local analysis.
 		}
@@ -473,114 +468,6 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo
 		}
 	}
 
-	// Convert all diagnostics to strings.
-	findings := make([]string, 0, len(diagnostics))
-	for a, ds := range diagnostics {
-		for _, d := range ds {
-			// Include the anlyzer name for debugability and configuration.
-			findings = append(findings, fmt.Sprintf("%s: %s: %s", a.Name, imp.fset.Position(d.Pos), d.Message))
-		}
-	}
-
 	// Return all findings.
-	factData := facts.Encode()
-	return findings, factData, nil
-}
-
-var (
-	packageFile    = flag.String("package", "", "package configuration file (in JSON format)")
-	stdlibFile     = flag.String("stdlib", "", "stdlib configuration file (in JSON format)")
-	findingsOutput = flag.String("findings", "", "output file (or stdout, if not specified)")
-	factsOutput    = flag.String("facts", "", "output file for facts (optional)")
-	escapesOutput  = flag.String("escapes", "", "output file for escapes (optional)")
-)
-
-func loadConfig(file string, config interface{}) interface{} {
-	// Load the configuration.
-	f, err := os.Open(file)
-	if err != nil {
-		log.Fatalf("unable to open configuration %q: %v", file, err)
-	}
-	defer f.Close()
-	dec := json.NewDecoder(f)
-	dec.DisallowUnknownFields()
-	if err := dec.Decode(config); err != nil {
-		log.Fatalf("unable to decode configuration: %v", err)
-	}
-	return config
-}
-
-// Main is the entrypoint; it should be called directly from main.
-//
-// N.B. This package registers it's own flags.
-func Main() {
-	// Parse all flags.
-	flag.Parse()
-
-	var (
-		findings []string
-		factData []byte
-		err      error
-	)
-
-	// Check the configuration.
-	if *packageFile != "" && *stdlibFile != "" {
-		log.Fatalf("unable to perform stdlib and package analysis; provide only one!")
-	} else if *stdlibFile != "" {
-		// Perform basic analysis.
-		c := loadConfig(*stdlibFile, new(stdlibConfig)).(*stdlibConfig)
-		findings, factData, err = checkStdlib(c, analyzerConfig)
-	} else if *packageFile != "" {
-		// Perform basic analysis.
-		c := loadConfig(*packageFile, new(packageConfig)).(*packageConfig)
-		findings, factData, err = checkPackage(c, analyzerConfig, nil)
-		// Do we need to do escape analysis?
-		if *escapesOutput != "" {
-			f, err := os.OpenFile(*escapesOutput, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
-			if err != nil {
-				log.Fatalf("unable to open output %q: %v", *escapesOutput, err)
-			}
-			defer f.Close()
-			escapes, _, err := checkPackage(c, escapesConfig, nil)
-			if err != nil {
-				log.Fatalf("error performing escape analysis: %v", err)
-			}
-			for _, escape := range escapes {
-				fmt.Fprintf(f, "%s\n", escape)
-			}
-		}
-	} else {
-		log.Fatalf("please provide at least one of package or stdlib!")
-	}
-
-	// Save facts.
-	if *factsOutput != "" {
-		if err := ioutil.WriteFile(*factsOutput, factData, 0644); err != nil {
-			log.Fatalf("error saving findings to %q: %v", *factsOutput, err)
-		}
-	}
-
-	// Open the output file.
-	var w io.Writer = os.Stdout
-	if *findingsOutput != "" {
-		f, err := os.OpenFile(*findingsOutput, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
-		if err != nil {
-			log.Fatalf("unable to open output %q: %v", *findingsOutput, err)
-		}
-		defer f.Close()
-		w = f
-	}
-
-	// Handle findings & errors.
-	if err != nil {
-		log.Fatalf("error checking package: %v", err)
-	}
-	if len(findings) == 0 {
-		return
-	}
-
-	// Print findings.
-	for _, finding := range findings {
-		fmt.Fprintf(w, "%s\n", finding)
-	}
+	return findings, facts.Encode(), nil
 }