go/analysis: revert UsesFacts to FactTypes

Forcing clients to register their Fact types with gob made for an
unfriendly API. Now the driver is again responsible for doing it.

The FactTypes field is now a slice of Fact values (not reflect.Types)
used only for their dynamic type, which is slightly more convenient.

Change-Id: I01219edb24bd2371ba642bb56508aa80c19a9b61
Reviewed-on: https://go-review.googlesource.com/137836
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This commit is contained in:
Alan Donovan 2018-09-26 16:34:31 -04:00
parent ef04bbebd8
commit 308f0c7c09
4 changed files with 39 additions and 13 deletions

View File

@ -11,6 +11,9 @@
// //
// The principal data types of this package are structs, not interfaces, // The principal data types of this package are structs, not interfaces,
// to permit later addition of optional fields as the API evolves. // 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 package analysis
import ( import (
@ -67,14 +70,15 @@ type Analyzer struct {
// ResultType is the type of the optional result of the Run function. // ResultType is the type of the optional result of the Run function.
ResultType reflect.Type 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 // An analyzer that uses facts may assume that its import
// dependencies have been similarly analyzed before it runs. // 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). // analysis passes (same analyzer, different packages).
UsesFacts bool FactTypes []Fact
} }
func (a *Analyzer) String() string { return a.Name } func (a *Analyzer) String() string { return a.Name }

View File

@ -188,7 +188,7 @@ func analyze(pkgs []*packages.Package, analyzers []*analysis.Analyzer) []*action
// An analysis that consumes/produces facts // An analysis that consumes/produces facts
// must run on the package's dependencies too. // must run on the package's dependencies too.
if a.UsesFacts { if len(a.FactTypes) > 0 {
paths := make([]string, 0, len(pkg.Imports)) paths := make([]string, 0, len(pkg.Imports))
for path := range pkg.Imports { for path := range pkg.Imports {
paths = append(paths, path) paths = append(paths, path)
@ -374,7 +374,7 @@ func needFacts(analyzers []*analysis.Analyzer) bool {
q = q[1:] q = q[1:]
if !seen[a] { if !seen[a] {
seen[a] = true seen[a] = true
if a.UsesFacts { if len(a.FactTypes) > 0 {
return true return true
} }
q = append(q, a.Requires...) q = append(q, a.Requires...)

View File

@ -35,7 +35,7 @@ var Analyzer = &analysis.Analyzer{
Name: "pkgfact", Name: "pkgfact",
Doc: "gather name/value pairs from constant declarations", Doc: "gather name/value pairs from constant declarations",
Run: run, Run: run,
UsesFacts: true, // *pairsFact FactTypes: []analysis.Fact{new(pairsFact)},
ResultType: reflect.TypeOf(map[string]string{}), ResultType: reflect.TypeOf(map[string]string{}),
} }

View File

@ -2,38 +2,60 @@ package analysis
import ( import (
"fmt" "fmt"
"reflect"
"unicode" "unicode"
) )
// Validate reports an error if any of the analyzers are misconfigured. // Validate reports an error if any of the analyzers are misconfigured.
// Checks include: // Checks include:
// - that the name is a valid identifier; // - that the name is a valid identifier;
// - that analysis names are unique; // - that analyzer names are unique;
// - that the Requires graph is acylic. // - that the Requires graph is acylic;
// - that analyzer fact types are unique;
// - that each fact type is a pointer.
func Validate(analyzers []*Analyzer) error { func Validate(analyzers []*Analyzer) error {
names := make(map[string]bool) 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. // Traverse the Requires graph, depth first.
color := make(map[*Analyzer]uint8) // 0=white 1=grey 2=black color := make(map[*Analyzer]uint8) // 0=white 1=grey 2=black
var visit func(a *Analyzer) error var visit func(a *Analyzer) error
visit = func(a *Analyzer) error { visit = func(a *Analyzer) error {
if a == nil { if a == nil {
return fmt.Errorf("nil *Analysis") return fmt.Errorf("nil *Analyzer")
} }
if color[a] == 0 { // white if color[a] == 0 { // white
color[a] = 1 // grey color[a] = 1 // grey
// names // names
if !validIdent(a.Name) { if !validIdent(a.Name) {
return fmt.Errorf("invalid analysis name %q", a) return fmt.Errorf("invalid analyzer name %q", a)
} }
if names[a.Name] { if names[a.Name] {
return fmt.Errorf("duplicate analysis name %q", a) return fmt.Errorf("duplicate analyzer name %q", a)
} }
names[a.Name] = true names[a.Name] = true
if a.Doc == "" { 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 // recursion