diff --git a/go/analysis/analysis.go b/go/analysis/analysis.go index 127f1d3f..8fa329c1 100644 --- a/go/analysis/analysis.go +++ b/go/analysis/analysis.go @@ -11,6 +11,9 @@ // // The principal data types of this package are structs, not interfaces, // to permit later addition of optional fields as the API evolves. +// +// THIS INTERFACE IS EXPERIMENTAL AND SUBJECT TO CHANGE. +// We aim to finalize it by November 2018. package analysis import ( @@ -67,14 +70,15 @@ type Analyzer struct { // ResultType is the type of the optional result of the Run function. ResultType reflect.Type - // UsesFacts indicates that this analyzer produces and consumes Facts. + // FactTypes indicates that this analyzer imports and exports + // Facts of the specified concrete types. // An analyzer that uses facts may assume that its import // dependencies have been similarly analyzed before it runs. - // Facts are pointers. + // Facts must be pointers. // - // UsesFacts establishes a "vertical" dependency between + // FactTypes establishes a "vertical" dependency between // analysis passes (same analyzer, different packages). - UsesFacts bool + FactTypes []Fact } func (a *Analyzer) String() string { return a.Name } diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index d4388c53..d9647c0e 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -188,7 +188,7 @@ func analyze(pkgs []*packages.Package, analyzers []*analysis.Analyzer) []*action // An analysis that consumes/produces facts // must run on the package's dependencies too. - if a.UsesFacts { + if len(a.FactTypes) > 0 { paths := make([]string, 0, len(pkg.Imports)) for path := range pkg.Imports { paths = append(paths, path) @@ -374,7 +374,7 @@ func needFacts(analyzers []*analysis.Analyzer) bool { q = q[1:] if !seen[a] { seen[a] = true - if a.UsesFacts { + if len(a.FactTypes) > 0 { return true } q = append(q, a.Requires...) diff --git a/go/analysis/passes/pkgfact/pkgfact.go b/go/analysis/passes/pkgfact/pkgfact.go index 4c98cb7a..ddb18341 100644 --- a/go/analysis/passes/pkgfact/pkgfact.go +++ b/go/analysis/passes/pkgfact/pkgfact.go @@ -35,7 +35,7 @@ var Analyzer = &analysis.Analyzer{ Name: "pkgfact", Doc: "gather name/value pairs from constant declarations", Run: run, - UsesFacts: true, // *pairsFact + FactTypes: []analysis.Fact{new(pairsFact)}, ResultType: reflect.TypeOf(map[string]string{}), } diff --git a/go/analysis/validate.go b/go/analysis/validate.go index e91729ca..ff176e48 100644 --- a/go/analysis/validate.go +++ b/go/analysis/validate.go @@ -2,38 +2,60 @@ package analysis import ( "fmt" + "reflect" "unicode" ) // 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 analyzer names are unique; +// - that the Requires graph is acylic; +// - that analyzer fact types are unique; +// - that each fact type is a pointer. func Validate(analyzers []*Analyzer) error { names := make(map[string]bool) + // Map each fact type to its sole generating analyzer. + factTypes := make(map[reflect.Type]*Analyzer) + // Traverse the Requires graph, depth first. 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") + return fmt.Errorf("nil *Analyzer") } if color[a] == 0 { // white color[a] = 1 // grey // names if !validIdent(a.Name) { - return fmt.Errorf("invalid analysis name %q", a) + return fmt.Errorf("invalid analyzer name %q", a) } if names[a.Name] { - return fmt.Errorf("duplicate analysis name %q", a) + return fmt.Errorf("duplicate analyzer name %q", a) } names[a.Name] = true if a.Doc == "" { - return fmt.Errorf("analysis %q is undocumented", a) + return fmt.Errorf("analyzer %q is undocumented", a) + } + + // fact types + for _, f := range a.FactTypes { + if f == nil { + return fmt.Errorf("analyzer %s has nil FactType", a) + } + t := reflect.TypeOf(f) + if prev := factTypes[t]; prev != nil { + return fmt.Errorf("fact type %s registered by two analyzers: %v, %v", + t, a, prev) + } + if t.Kind() != reflect.Ptr { + return fmt.Errorf("%s: fact type %s is not a pointer", a, t) + } + factTypes[t] = a } // recursion