From e2857128af6b14510c22a7ba5f85839bff1c4b1e Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 24 Sep 2018 17:32:33 -0400 Subject: [PATCH] go/analysis: several API renamings Analysis -> Analyzer Unit -> Pass Output -> Result Inputs -> ResultOf Lemma -> Fact Set{Object,Package}Lemma -> Export{Object,Package}Fact {Object,Package}Lemma -> Import{Object,Package}Fact LemmaTypes -> UsesFacts bool plugins/ -> passes/ Notes: - Unit.Output is no longer a field; it's the result of calling Analyzer.Run. - Because analyzers no longer declare their LemmaTypes, they, not the driver, are now responsible for registering Fact types with Gob. A follow-up change will additionally rename: Finding -> Report Pass.Syntax -> Pass.Files Pass.Info -> Pass.TypesInfo Change-Id: Iccbdadbea5a0aafe732e23a344dd57fd93681931 Reviewed-on: https://go-review.googlesource.com/137095 Reviewed-by: Ian Cottrell --- go/analysis/analysis.go | 208 +++++++++--------- .../{plugin => passes}/findcall/findcall.go | 14 +- go/analysis/passes/pkgfact/pkgfact.go | 110 +++++++++ go/analysis/plugin/README | 8 - go/analysis/plugin/pkglemma/pkglemma.go | 102 --------- go/analysis/validate.go | 45 +--- 6 files changed, 224 insertions(+), 263 deletions(-) rename go/analysis/{plugin => passes}/findcall/findcall.go (69%) create mode 100644 go/analysis/passes/pkgfact/pkgfact.go delete mode 100644 go/analysis/plugin/README delete mode 100644 go/analysis/plugin/pkglemma/pkglemma.go diff --git a/go/analysis/analysis.go b/go/analysis/analysis.go index e103fdeb..b1befed7 100644 --- a/go/analysis/analysis.go +++ b/go/analysis/analysis.go @@ -1,4 +1,4 @@ -// The analysis package defines a uniform interface for static checkers +// The analysis package defines a uniform interface for static checkers ("Analyzers") // of Go source code. By implementing a common interface, checkers from // a variety of sources can be easily selected, incorporated, and reused // in a wide range of programs including command-line tools, text @@ -6,7 +6,7 @@ // and batch pipelines for large code bases. For the design, see // https://docs.google.com/document/d/1-azPLXaLgTCKeKDNg0HVMq2ovMlD-e7n1ZHzZVzOlJk // -// Each analysis is invoked once per Go package, and is provided the +// Each analyzer is invoked once per Go package, and is provided the // abstract syntax trees (ASTs) and type information for that package. // // The principal data types of this package are structs, not interfaces, @@ -22,65 +22,74 @@ import ( "reflect" ) -// An Analysis describes an analysis function and its options. -type Analysis struct { - // The Name of the analysis must be a valid Go identifier +// An Analyzer describes an analysis function and its options. +type Analyzer struct { + // The Name of the analyzer must be a valid Go identifier // as it may appear in command-line flags, URLs, and so on. Name string - // Doc is the documentation for the analysis. + // Doc is the documentation for the analyzer. Doc string - // Flags defines any flags accepted by the analysis. + // Flags defines any flags accepted by the analyzer. // The manner in which these flags are exposed to the user - // depends on the driver which runs the analysis. + // depends on the driver which runs the analyzer. Flags flag.FlagSet - // Run applies the analysis to a package. - // It returns an error if the analysis failed. - Run func(*Unit) error + // Run applies the analyzer to a package. + // It returns an error if the analyzer failed. + // + // On success, the Run function may return a result + // computed by the Analyzer; its type must match ResultType. + // The driver makes this result available as an input to + // another Analyzer that depends directly on this one (see + // Requires) when it analyzes the same package. + // + // To pass analysis results between packages (and thus + // potentially between address spaces), use Facts, which are + // serializable. + Run func(*Pass) (interface{}, error) // RunDespiteErrors allows the driver to invoke - // the Run method of this analysis even on a + // the Run method of this analyzer even on a // package that contains parse or type errors. RunDespiteErrors bool - // Requires is a set of analyses that must run successfully - // before this one on a given package. This analysis may inspect - // the outputs produced by each analysis in Requires. - // The graph over analyses implied by Requires edges must be acyclic. + // Requires is a set of analyzers that must run successfully + // before this one on a given package. This analyzer may inspect + // the outputs produced by each analyzer in Requires. + // The graph over analyzers implied by Requires edges must be acyclic. // // Requires establishes a "horizontal" dependency between - // analysis units (different analyses, same package). - Requires []*Analysis + // analysis passes (different analyzers, same package). + Requires []*Analyzer - // OutputType is the type of the optional Output value - // computed by this analysis and stored in Unit.Output. - // (The Output is provided as an Input to - // each analysis that Requires this one.) - OutputType reflect.Type + // ResultType is the type of the optional result of the Run function. + ResultType reflect.Type - // LemmaTypes is the set of types of lemmas produced and - // consumed by this analysis. An analysis that uses lemmas - // may assume that its import dependencies have been - // similarly analyzed before it runs. Lemmas are pointers. + // UsesFacts indicates that this analyzer produces and consumes Facts. + // An analyzer that uses facts may assume that its import + // dependencies have been similarly analyzed before it runs. + // Facts are pointers. // - // LemmaTypes establishes a "vertical" dependency between - // analysis units (same analysis, different packages). - LemmaTypes []reflect.Type + // UsesFacts establishes a "vertical" dependency between + // analysis passes (same analyzer, different packages). + UsesFacts bool } -func (a *Analysis) String() string { return a.Name } +func (a *Analyzer) String() string { return a.Name } -// A Unit provides information to the Run function that -// applies a specific analysis to a single Go package. +// A Pass provides information to the Run function that +// applies a specific analyzer to a single Go package. // // It forms the interface between the analysis logic and the driver // program, and has both input and an output components. -type Unit struct { +// +// As in a compiler, one pass may depend on the result computed by another. +type Pass struct { // -- inputs -- - Analysis *Analysis // the identity of the current analysis + Analyzer *Analyzer // the identity of the current analyzer // syntax and type information Fset *token.FileSet // file position information @@ -88,28 +97,25 @@ type Unit struct { Pkg *types.Package // type information about the package Info *types.Info // type information about the syntax trees - // Inputs provides the inputs to this analysis unit, which are - // the corresponding outputs of its prerequisite analysis. + // ResultOf provides the inputs to this analysis pass, which are + // the corresponding results of its prerequisite analyzers. // The map keys are the elements of Analysis.Required, // and the type of each corresponding value is the required - // analysis's OutputType. - Inputs map[*Analysis]interface{} + // analysis's ResultType. + ResultOf map[*Analyzer]interface{} - // ObjectLemma retrieves a lemma associated with obj. - // Given a value ptr of type *T, where *T satisfies Lemma, - // ObjectLemma copies the value to *ptr. + // ImportObjectFact retrieves a fact associated with obj. + // Given a value ptr of type *T, where *T satisfies Fact, + // ImportObjectFact copies the value to *ptr. // - // ObjectLemma may panic if applied to a lemma type that - // the analysis did not declare among its LemmaTypes, - // or if called after analysis of the unit is complete. - // - // ObjectLemma is not concurrency-safe. - ObjectLemma func(obj types.Object, lemma Lemma) bool + // ImportObjectFact panics if called after the pass is complete. + // ImportObjectFact is not concurrency-safe. + ImportObjectFact func(obj types.Object, fact Fact) bool - // PackageLemma retrives a lemma associated with package pkg, - // which must be this package or one if its dependencies. - // See comments for ObjectLemma. - PackageLemma func(pkg *types.Package, lemma Lemma) bool + // ImportPackageFact retrieves a fact associated with package pkg, + // which must be this package or one of its dependencies. + // See comments for ImportObjectFact. + ImportPackageFact func(pkg *types.Package, fact Fact) bool // -- outputs -- @@ -118,32 +124,17 @@ type Unit struct { // It is populated by the Run function. Findings []*Finding - // SetObjectLemma associates a lemma of type *T with the obj, - // replacing any previous lemma of that type. + // ExportObjectFact associates a fact of type *T with the obj, + // replacing any previous fact of that type. // - // SetObjectLemma panics if the lemma's type is not among - // Analysis.LemmaTypes, or if obj does not belong to the package - // being analyzed, or if it is called after analysis of the unit - // is complete. - // - // SetObjectLemma is not concurrency-safe. - SetObjectLemma func(obj types.Object, lemma Lemma) + // ExportObjectFact panics if it is called after the pass is + // complete, or if obj does not belong to the package being analyzed. + // ExportObjectFact is not concurrency-safe. + ExportObjectFact func(obj types.Object, fact Fact) - // SetPackageLemma associates a lemma with the current package. - // See comments for SetObjectLemma. - SetPackageLemma func(lemma Lemma) - - // Output is an immutable result computed by this analysis unit - // and set by the Run function. - // It will be made available as an input to any analysis that - // depends directly on this one; see Analysis.Requires. - // Its type must match Analysis.OutputType. - // - // Outputs are available as Inputs to later analyses of the - // same package. To pass analysis results between packages (and - // thus potentially between address spaces), use Lemmas, which - // are serializable. - Output interface{} + // ExportPackageFact associates a fact with the current package. + // See comments for ExportObjectFact. + ExportPackageFact func(fact Fact) /* Further fields may be added in future. */ // For example, suggested or applied refactorings. @@ -151,58 +142,59 @@ type Unit struct { // Findingf is a helper function that creates a new Finding using the // specified position and formatted error message, appends it to -// unit.Findings, and returns it. -func (unit *Unit) Findingf(pos token.Pos, format string, args ...interface{}) *Finding { +// pass.Findings, and returns it. +func (pass *Pass) Findingf(pos token.Pos, format string, args ...interface{}) *Finding { msg := fmt.Sprintf(format, args...) f := &Finding{Pos: pos, Message: msg} - unit.Findings = append(unit.Findings, f) + pass.Findings = append(pass.Findings, f) return f } -func (unit *Unit) String() string { - return fmt.Sprintf("%s@%s", unit.Analysis.Name, unit.Pkg.Path()) +func (pass *Pass) String() string { + return fmt.Sprintf("%s@%s", pass.Analyzer.Name, pass.Pkg.Path()) } -// A Lemma is an intermediate fact produced during analysis. +// A Fact is an intermediate fact produced during analysis. // -// Each lemma is associated with a named declaration (a types.Object). -// A single object may have multiple associated lemmas, but only one of -// any particular lemma type. +// Each fact is associated with a named declaration (a types.Object). +// A single object may have multiple associated facts, but only one of +// any particular fact type. // -// A Lemma represents a predicate such as "never returns", but does not +// A Fact represents a predicate such as "never returns", but does not // represent the subject of the predicate such as "function F". // -// Lemmas may be produced in one analysis unit and consumed by another -// analysis unit even if these are in different address spaces. -// If package P imports Q, all lemmas about objects of Q produced during +// Facts may be produced in one analysis pass and consumed by another +// analysis pass even if these are in different address spaces. +// If package P imports Q, all facts about objects of Q produced during // analysis of that package will be available during later analysis of P. -// Lemmas are analogous to type export data in a build system: -// just as export data enables separate compilation of several units, -// lemmas enable "separate analysis". +// Facts are analogous to type export data in a build system: +// just as export data enables separate compilation of several passes, +// facts enable "separate analysis". // -// Each unit of analysis starts with the set of lemmas produced by the -// same analysis applied to the packages directly imported by the -// current one. The analysis may add additional lemmas to the set, and -// they may be exported in turn. An analysis's Run function may retrieve -// lemmas by calling Unit.Lemma and set them using Unit.SetLemma. +// Each pass (a, p) starts with the set of facts produced by the +// same analyzer a applied to the packages directly imported by p. +// The analysis may add facts to the set, and they may be exported in turn. +// An analysis's Run function may retrieve facts by calling +// Pass.Import{Object,Package}Fact and update them using +// Pass.Export{Object,Package}Fact. // -// Each type of Lemma may be produced by at most one Analysis. -// Lemmas are logically private to their Analysis; to pass values -// between different analysis, use the Input/Output mechanism. +// A fact is logically private to its Analysis. To pass values +// between different analyzers, use the results mechanism; +// see Analyzer.Requires, Analyzer.ResultType, and Pass.ResultOf. // -// A Lemma type must be a pointer. (Unit.GetLemma relies on it.) -// Lemmas are encoded and decoded using encoding/gob. -// A Lemma may implement the GobEncoder/GobDecoder interfaces -// to customize its encoding; Lemma encoding should not fail. +// A Fact type must be a pointer. +// Facts are encoded and decoded using encoding/gob. +// A Fact may implement the GobEncoder/GobDecoder interfaces +// to customize its encoding. Fact encoding should not fail. // -// A Lemma should not be modified once passed to SetLemma. -type Lemma interface { - IsLemma() // dummy method to avoid type errors +// A Fact should not be modified once exported. +type Fact interface { + AFact() // dummy method to avoid type errors } // A Finding is a message associated with a source location. // -// An Analysis may return a variety of findings; the optional Category, +// An Analyzer may return a variety of findings; the optional Category, // which should be a constant, may be used to classify them. // It is primarily intended to make it easy to look up documentation. type Finding struct { diff --git a/go/analysis/plugin/findcall/findcall.go b/go/analysis/passes/findcall/findcall.go similarity index 69% rename from go/analysis/plugin/findcall/findcall.go rename to go/analysis/passes/findcall/findcall.go index c7942551..5955498e 100644 --- a/go/analysis/plugin/findcall/findcall.go +++ b/go/analysis/passes/findcall/findcall.go @@ -9,7 +9,7 @@ import ( "golang.org/x/tools/go/analysis" ) -var Analysis = &analysis.Analysis{ +var Analyzer = &analysis.Analyzer{ Name: "findcall", Doc: "find calls to a particular function", Run: findcall, @@ -19,11 +19,11 @@ var Analysis = &analysis.Analysis{ var name = "println" // --name flag func init() { - Analysis.Flags.StringVar(&name, "name", name, "name of the function to find") + Analyzer.Flags.StringVar(&name, "name", name, "name of the function to find") } -func findcall(unit *analysis.Unit) error { - for _, f := range unit.Syntax { +func findcall(pass *analysis.Pass) (interface{}, error) { + for _, f := range pass.Syntax { ast.Inspect(f, func(n ast.Node) bool { if call, ok := n.(*ast.CallExpr); ok { var id *ast.Ident @@ -33,13 +33,13 @@ func findcall(unit *analysis.Unit) error { case *ast.SelectorExpr: id = fun.Sel } - if id != nil && !unit.Info.Types[id].IsType() && id.Name == name { - unit.Findingf(call.Lparen, "call of %s(...)", id.Name) + if id != nil && !pass.Info.Types[id].IsType() && id.Name == name { + pass.Findingf(call.Lparen, "call of %s(...)", id.Name) } } return true }) } - return nil + return nil, nil } diff --git a/go/analysis/passes/pkgfact/pkgfact.go b/go/analysis/passes/pkgfact/pkgfact.go new file mode 100644 index 00000000..092b4cc6 --- /dev/null +++ b/go/analysis/passes/pkgfact/pkgfact.go @@ -0,0 +1,110 @@ +// The pkgfact package is a demonstration and test of the package fact +// mechanism. +// +// The output of the pkgfact analysis is a set of key/values pairs +// gathered from the analyzed package and its imported dependencies. +// Each key/value pair comes from a top-level constant declaration +// whose name starts with "_". For example: +// +// package p +// +// const _greeting = "hello" +// const _audience = "world" +// +// the pkgfact analysis output for package p would be: +// +// {"greeting": "hello", "audience": "world"}. +// +// In addition, the analysis reports a finding at each import +// showing which key/value pairs it contributes. +package pkgfact + +import ( + "fmt" + "go/ast" + "go/token" + "go/types" + "reflect" + "sort" + "strings" + + "golang.org/x/tools/go/analysis" +) + +var Analyzer = &analysis.Analyzer{ + Name: "pkgfact", + Doc: "gather name/value pairs from constant declarations", + Run: run, + UsesFacts: true, // *pairsFact + ResultType: reflect.TypeOf(map[string]string{}), +} + +// A pairsFact is a package-level fact that records +// an set of key=value strings accumulated from constant +// declarations in this package and its dependencies. +// Elements are ordered by keys, which are unique. +type pairsFact []string + +func (*pairsFact) AFact() {} + +func run(pass *analysis.Pass) (interface{}, error) { + result := make(map[string]string) + + // At each import, print the fact from the imported + // package and accumulate its information into the result. + // (Warning: accumulation leads to quadratic growth of work.) + doImport := func(spec *ast.ImportSpec) { + pkg := pass.Info.Defs[spec.Name].(*types.PkgName).Imported() + var fact pairsFact + if pass.ImportPackageFact(pkg, &fact) { + for _, pair := range fact { + eq := strings.IndexByte(pair, '=') + result[pair[:eq]] = pair[1+eq:] + } + pass.Findingf(spec.Pos(), "%s", strings.Join(fact, " ")) + } + } + + // At each "const _name = value", add a fact into env. + doConst := func(spec *ast.ValueSpec) { + if len(spec.Names) == len(spec.Values) { + for i := range spec.Names { + name := spec.Names[i].Name + if strings.HasPrefix(name, "_") { + key := name[1:] + value := pass.Info.Types[spec.Values[i]].Value.String() + result[key] = value + } + } + } + } + + for _, f := range pass.Syntax { + for _, decl := range f.Decls { + if decl, ok := decl.(*ast.GenDecl); ok { + for _, spec := range decl.Specs { + switch decl.Tok { + case token.IMPORT: + doImport(spec.(*ast.ImportSpec)) + case token.CONST: + doConst(spec.(*ast.ValueSpec)) + } + } + } + } + } + + // Sort/deduplicate the result and save it as a package fact. + keys := make([]string, 0, len(result)) + for key := range result { + keys = append(keys, key) + } + sort.Strings(keys) + var fact pairsFact + for _, key := range keys { + fact = append(fact, fmt.Sprintf("%s=%s", key, result[key])) + } + pass.ExportPackageFact(&fact) + + return result, nil +} diff --git a/go/analysis/plugin/README b/go/analysis/plugin/README deleted file mode 100644 index add86bdb..00000000 --- a/go/analysis/plugin/README +++ /dev/null @@ -1,8 +0,0 @@ - -This directory does not contain a Go package, -but acts as a container for various analyses -that implement the golang.org/x/tools/go/analysis -API and may be imported into an analysis tool. - -By convention, each package foo provides the analysis, -and each command foo/cmd/foo provides a standalone driver. diff --git a/go/analysis/plugin/pkglemma/pkglemma.go b/go/analysis/plugin/pkglemma/pkglemma.go deleted file mode 100644 index 94b4e17a..00000000 --- a/go/analysis/plugin/pkglemma/pkglemma.go +++ /dev/null @@ -1,102 +0,0 @@ -// The pkglemma package is a demonstration and test of the package lemma -// mechanism. -// -// The output of the pkglemma analysis is a set of key/values pairs -// gathered from the analyzed package and its imported dependencies. -// Each key/value pair comes from a top-level constant declaration -// whose name starts with "_". For example: -// -// package p -// -// const _greeting = "hello" -// const _audience = "world" -// -// the pkglemma analysis output for package p would be: -// -// {"greeting": "hello", "audience": "world"}. -// -// In addition, the analysis reports a finding at each import -// showing which key/value pairs it contributes. -package pkglemma - -import ( - "fmt" - "go/ast" - "go/token" - "go/types" - "reflect" - "sort" - "strings" - - "golang.org/x/tools/go/analysis" -) - -var Analysis = &analysis.Analysis{ - Name: "pkglemma", - Doc: "gather name/value pairs from constant declarations", - Run: run, - LemmaTypes: []reflect.Type{reflect.TypeOf(new(note))}, - OutputType: reflect.TypeOf(map[string]string{}), -} - -// A note is a package-level lemma that records -// key/value pairs accumulated from constant -// declarations in this package and its dependencies. -type note struct { - M map[string]string -} - -func (*note) IsLemma() {} - -func run(unit *analysis.Unit) error { - m := make(map[string]string) - - // At each import, print the lemma from the imported - // package and accumulate its information into m. - doImport := func(spec *ast.ImportSpec) { - pkg := unit.Info.Defs[spec.Name].(*types.PkgName).Imported() - var lemma note - if unit.PackageLemma(pkg, &lemma) { - var lines []string - for k, v := range lemma.M { - m[k] = v - lines = append(lines, fmt.Sprintf("%s=%s", k, v)) - } - sort.Strings(lines) - unit.Findingf(spec.Pos(), "%s", strings.Join(lines, " ")) - } - } - - // At each "const _name = value", add a fact into m. - doConst := func(spec *ast.ValueSpec) { - if len(spec.Names) == len(spec.Values) { - for i := range spec.Names { - name := spec.Names[i].Name - if strings.HasPrefix(name, "_") { - m[name[1:]] = unit.Info.Types[spec.Values[i]].Value.String() - } - } - } - } - - for _, f := range unit.Syntax { - for _, decl := range f.Decls { - if decl, ok := decl.(*ast.GenDecl); ok { - for _, spec := range decl.Specs { - switch decl.Tok { - case token.IMPORT: - doImport(spec.(*ast.ImportSpec)) - case token.CONST: - doConst(spec.(*ast.ValueSpec)) - } - } - } - } - } - - unit.Output = m - - unit.SetPackageLemma(¬e{m}) - - return nil -} diff --git a/go/analysis/validate.go b/go/analysis/validate.go index 7fbb3b54..e91729ca 100644 --- a/go/analysis/validate.go +++ b/go/analysis/validate.go @@ -2,28 +2,21 @@ package analysis import ( "fmt" - "reflect" "unicode" ) -// Validate reports an error if any of the analyses are misconfigured. +// Validate reports an error if any of the analyzers are misconfigured. // Checks include: // - that the name is a valid identifier; // - that analysis names are unique; -// - that the Requires graph is acylic; -// - that analyses' lemma and output types are unique. -// - that each lemma type is a pointer. -func Validate(analyses []*Analysis) error { +// - that the Requires graph is acylic. +func Validate(analyzers []*Analyzer) error { names := make(map[string]bool) - // Map each lemma/output type to its sole generating analysis. - lemmaTypes := make(map[reflect.Type]*Analysis) - outputTypes := make(map[reflect.Type]*Analysis) - // Traverse the Requires graph, depth first. - color := make(map[*Analysis]uint8) // 0=white 1=grey 2=black - var visit func(a *Analysis) error - visit = func(a *Analysis) error { + color := make(map[*Analyzer]uint8) // 0=white 1=grey 2=black + var visit func(a *Analyzer) error + visit = func(a *Analyzer) error { if a == nil { return fmt.Errorf("nil *Analysis") } @@ -43,30 +36,6 @@ func Validate(analyses []*Analysis) error { return fmt.Errorf("analysis %q is undocumented", a) } - // lemma types - for _, t := range a.LemmaTypes { - if t == nil { - return fmt.Errorf("analysis %s has nil LemmaType", a) - } - if prev := lemmaTypes[t]; prev != nil { - return fmt.Errorf("lemma type %s registered by two analyses: %v, %v", - t, a, prev) - } - if t.Kind() != reflect.Ptr { - return fmt.Errorf("%s: lemma type %s is not a pointer", a, t) - } - lemmaTypes[t] = a - } - - // output types - if a.OutputType != nil { - if prev := outputTypes[a.OutputType]; prev != nil { - return fmt.Errorf("output type %s registered by two analyses: %v, %v", - a.OutputType, a, prev) - } - outputTypes[a.OutputType] = a - } - // recursion for i, req := range a.Requires { if err := visit(req); err != nil { @@ -78,7 +47,7 @@ func Validate(analyses []*Analysis) error { return nil } - for _, a := range analyses { + for _, a := range analyzers { if err := visit(a); err != nil { return err }