x/tools/go/analysis/checker: an importable analysis driver based on go/packages

[adonovan]

We propose to create a new package, golang.org/x/tools/go/analysis/checker, to provide a function, Analyze, that applies a set of analyzers to a set of packages and returns a data structure providing access to the complete results of the operation.

Background: the go/analysis framework defines an interface for static checkers that inspect a parsed, type-checked Go package and report mistakes or suggest fixes. The interface allows checkers (e.g. printf, nilness) to be written independent of the driver, and drivers (e.g. go vet) to be written independent of the checkers. The x/tools repo provides two drivers. The first, unitchecker, used by go vet, performs "separate analysis", of one package at a time, using serialized types and facts for dependencies. The second, internal/checker, uses go/packages to load packages and all their dependencies from source. Currently it has two public interfaces, singlechecker and multichecker, that differ only trivially: a singlechecker has only one analyzer, whereas a multichecker contains many and has a slightly more complicated command-line interface. However, both of these public interfaces consist only of a Main function that does the entire job and then calls os.Exit.

Users reported that this was not a very useful building block, and that they wanted to incorporate analysis into larger applications, or add some extra steps after package loading but before analysis, or some extra postprocessing of the analysis results. See:

• proposal: x/tools/go/analysis: change singlechecker/multichecker to allow customization of package loading #53215
• x/tools/go/analysis: provide driver for running analysis programmatically #31897
• x/tools/go/analysis: feature request: need a way of accessing analysis results from another go package #50265
• proposal: x/tools/go/analysis: export internal/analysisflags and internal/checker #53336
• x/tools/go/analysis/singlechecker: don't use flag.CommandLine #30231
• x/tools/go/analysis/singlechecker: don't use os.Exit in Main #30219
• cmd/cover: provide a mechanism to flush out coverage report #31007

We propose to create a new package with the following public API.

package checker // golang.org/x/tools/go/analysis/checker

// Analyze runs the specified analyzers on the initial packages.
//
// For each analyzer that uses facts, Analyze also runs it on all the
// dependencies of the initial packages. (In this case the program
// must have been loaded using the packages.LoadAllSyntax flag.)
//
// On success, it returns a Result whose Roots holds one item per (a,
// p) in the cross-product of analyzers and pkgs.
func Analyze(analyzers []*analysis.Analyzer, pkgs []*packages.Package, opts *Options) (*Graph, error)

type Options struct {
	// Verbose     bool // -v: log each step  // NOTE: removed from earlier draft.
	Sequential  bool // -p: disable parallelism
	SanityCheck bool // -s: perform additional sanity checks
	ShowFacts   bool // -f: log each exported fact

	/* more fields in future */
}

// Graph holds the results of a batch of analysis, including
// information about the requested actions (analyzers applied to
// packages) plus any dependent actions that it was necessary to
// compute.
type Graph struct { // NOTE: was Result in earlier draft
	// Roots contains the roots of the action graph.
	// Each node (a, p) in the action graph represents the
	// application of one analyzer a to one package p. (A node
	// thus corresponds to one analysis.Pass instance.)
	// The root actions are the product Input.Packages ×
	// Input.Analyzers.
	//
	// Each element of Action.Deps represents an edge in the
	// action graph: a dependency from one action to another.
	// An edge of the form (a, p) -> (a, p2) indicates that the
	// analysis of package p requires information ("facts") from
	// the same analyzer applied to one of p's dependencies, p2.
	// An edge of the form (a, p) -> (a2, p) indicates that the
	// analysis of package p requires information ("results")
	// from a different analyzer applied to the same package.
	// Such edges are sometimes termed "vertical" and "horizontal",
	// respectively.
	Roots []*Action

	/* more fields in future */
}

// An Action represents one unit of analysis work by the driver: the
// application of one analysis to one package. It provides the inputs
// to and records the outputs of a single analysis.Pass.
//
// Actions form a DAG, both within a package (as different analyzers
// are applied, either in sequence or parallel), and across packages
// (as dependencies are analyzed).
type Action struct {
	Analyzer    *analysis.Analyzer
	Package         *packages.Package // NOTE: was Pkg in earlier draft.
	IsRoot      bool // whether this is a root node of the graph
	Deps        []*Action
	Result      interface{} // computed result of Analyzer.run, if any (and if IsRoot)
	Err         error       // error result of Analyzer.run
	Diagnostics []analysis.Diagnostic
	Duration    time.Duration // execution time of this step

	/* more fields in future */
}
func (act *Action) AllObjectFacts() []analysis.ObjectFact
func (act *Action) AllPackageFacts() []analysis.PackageFact
func (act *Action) ObjectFact(obj types.Object, ptr analysis.Fact) bool
func (act *Action) PackageFact(pkg *types.Package, ptr analysis.Fact) bool

// -- utilities (all pure functions) --

func (*Graph) Visit(f func(*Action) error) error) 
// NOTE: was ForEach in earlier draft:
// func ForEach(roots []*Action, f func(*Action) error) error 

func (*Graph) JSONDiagnostics(w.io.Writer) 
func (*Graph) TextDiagnostics(w.io.Writer, contextLines int) 
// NOTE: in earlier draft, these were:
// func PrintDiagnostics(out io.Writer, roots []*Action, context int) (exitcode int)
// func PrintJSON(out io.Writer, roots []*Action)

// NOTE: removed from earlier draft
// func PrintTiming(out io.Writer, roots []*Action)

// NOTE: Removed from earlier draft; will be dealt with later:
// type FixFilterFunc = func(*Action, analysis.Diagnostic, analysis.SuggestedFix) bool
// func ApplyFixes(actions []*Action, filter FixFilterFunc) error

The primary entry point is Analyze; it accepts a set of packages and analyzers and runs a unit of analysis (an "Action") for each element of their cross product, plus all necessary horizontal and vertical dependencies. The result is exposed as a graph, only the roots of which are provided in the Result struct; this approach is similar to the one used in go/packages.Load.

To reduce churn and skew, the above API code is just the declarations but not all of the comments. You can see the complete API and implementation in https://go.dev/cl/411907.

Questions:

• should the package name be something other than checker, so as not to suggest commonality with unitchecker? "srcchecker", perhaps? (If we could start over, I would use "driver" for {single,multi,unit}checker and this package, and use "checker" for analysis.Analyzer.)
• see also the [rough, evolving] analysistest proposal proposal: x/tools/go/analysis/analysistest: improved, module-aware API #61336. Does this proposal have implications for analysistest?

[gopherbot]

Change https://go.dev/cl/411907 mentions this issue: go/analysis/checker: a go/packages-based driver library

[lfolger]

I like the proposal and have two questions about this.

1. Do you have an opinion how error handling works in with this package, especially in the light of panics?
   Imagine you are running multiple analyzers on multiple packages . If one of these analyzers panics during the analysis does the Analyze recover the panic?
   If not, it would prevent you from getting any result for any of the analyzers on any package.

2. Is is possible to make the facts part of the output/input?
   This would allow users to shard the execution of many packages into batches and distribute them between workers (potentially in different processes on different machines).

[adonovan]

1. Do you have an opinion how error handling works in with this package, especially in the light of panics?
   Imagine you are running multiple analyzers on multiple packages . If one of these analyzers panics during the analysis does the Analyze recover the panic?

The go/analysis/checker driver uses defer + recover around Analyzer.Run, so if the analyzer is single-threaded (as ~all are), it will catch the panic and treat it as a failure of that Action. Like compilation, all downstream actions will not be executed, but independent actions will continue as usual. All this information can be gleaned from the action graph returned by checker.Analyze.

2. Is is possible to make the facts part of the output/input?

The Fact data structures are part of the output: see the All{Object,Package}Facts and {Object,Package}Fact methods. (Their encoding isn't, but the point of internal/checker is that it loads, parses, type-checks and analyzes everything from source, so the encoding of types and facts don't come into play.)

This would allow users to shard the execution of many packages into batches and distribute them between workers (potentially in different processes on different machines).

If you want to distribute units of analysis, you should be using unitchecker, or something like it (it's not a lot of code), as it assumes that intermediate results between units are serialized. Perhaps I should add an Example in go/analysis (but not checker) that shows how to construct a system analogous to go vet + unitchecker: in other words, the parent task constructs the graph of units of work and the child task(s) do the actual work, using serialized intermediaries for types and facts.

[lfolger]

I see. I think I missed this part:

For each analyzer that uses facts, Analyze also runs it on all the dependencies of the initial packages.

Thanks for clarifying (I should learn to read comment 😄 )

[jba]

I think checker is a fine name.

Since this is a non-main package, I don't think it should log anything. Consider instead providing that functionality as callbacks. For instance, replace Verbose with OnStep func(s string) (or maybe something richer than a string).