go/analysis: change AllObjectFacts and AllPackageFacts to filter facts

Facts are intended to be private to an analysis. Even though it's hard
to guarantee with the information we have that facts won't leak to the
wrong analysis, we can filter some analyses that couldn't possibly have
come from the same analysis. AllObjectFacts and AllPackageFacts will
now filter facts if their type isn't specified in an analysis's FactTypes.

Change-Id: I2794437a5810e08fe6a9652b3569c5e3c17e159f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/189037
Run-TryBot: Michael Matloob <[email protected]>
Reviewed-by: Dominik Honnef <[email protected]>

Author: matloob

go/analysis/analysis.go

@@ -128,11 +128,13 @@ type Pass struct {
 	// See comments for ExportObjectFact.
 	ExportPackageFact func(fact Fact)
 
-	// AllPackageFacts returns a new slice containing all package facts in unspecified order.
+	// AllPackageFacts returns a new slice containing all package facts of the analysis's FactTypes
+	// in unspecified order.
 	// WARNING: This is an experimental API and may change in the future.
 	AllPackageFacts func() []PackageFact
 
-	// AllObjectFacts returns a new slice containing all object facts in unspecified order.
+	// AllObjectFacts returns a new slice containing all object facts of the analysis's FactTypes
+	// in unspecified order.
 	// WARNING: This is an experimental API and may change in the future.
 	AllObjectFacts func() []ObjectFact
 

go/analysis/internal/facts/facts.go

@@ -99,10 +99,10 @@ func (s *Set) ExportObjectFact(obj types.Object, fact analysis.Fact) {
 	s.mu.Unlock()
 }
 
-func (s *Set) AllObjectFacts() []analysis.ObjectFact {
+func (s *Set) AllObjectFacts(filter map[reflect.Type]bool) []analysis.ObjectFact {
 	var facts []analysis.ObjectFact
 	for k, v := range s.m {
-		if k.obj != nil {
+		if k.obj != nil && filter[k.t] {
 			facts = append(facts, analysis.ObjectFact{k.obj, v})
 		}
 	}
@@ -132,10 +132,10 @@ func (s *Set) ExportPackageFact(fact analysis.Fact) {
 	s.mu.Unlock()
 }
 
-func (s *Set) AllPackageFacts() []analysis.PackageFact {
+func (s *Set) AllPackageFacts(filter map[reflect.Type]bool) []analysis.PackageFact {
 	var facts []analysis.PackageFact
 	for k, v := range s.m {
-		if k.obj == nil {
+		if k.obj == nil && filter[k.t] {
 			facts = append(facts, analysis.PackageFact{k.pkg, v})
 		}
 	}

go/analysis/internal/facts/facts_test.go

@@ -10,6 +10,7 @@ import (
 	"go/token"
 	"go/types"
 	"os"
+	"reflect"
 	"testing"
 
 	"golang.org/x/tools/go/analysis/analysistest"
@@ -172,3 +173,52 @@ func load(dir string, path string) (*types.Package, error) {
 	}
 	return pkgs[0].Types, nil
 }
+
+type otherFact struct {
+	S string
+}
+
+func (f *otherFact) String() string { return fmt.Sprintf("otherFact(%s)", f.S) }
+func (f *otherFact) AFact()         {}
+
+func TestFactFilter(t *testing.T) {
+	files := map[string]string{
+		"a/a.go": `package a; type A int`,
+	}
+	dir, cleanup, err := analysistest.WriteFiles(files)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanup()
+
+	pkg, err := load(dir, "a")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	obj := pkg.Scope().Lookup("A")
+	s, err := facts.Decode(pkg, func(string) ([]byte, error) { return nil, nil })
+	if err != nil {
+		t.Fatal(err)
+	}
+	s.ExportObjectFact(obj, &myFact{"good object fact"})
+	s.ExportPackageFact(&myFact{"good package fact"})
+	s.ExportObjectFact(obj, &otherFact{"bad object fact"})
+	s.ExportPackageFact(&otherFact{"bad package fact"})
+
+	filter := map[reflect.Type]bool{
+		reflect.TypeOf(&myFact{}): true,
+	}
+
+	pkgFacts := s.AllPackageFacts(filter)
+	wantPkgFacts := `[{package a ("a") myFact(good package fact)}]`
+	if got := fmt.Sprintf("%v", pkgFacts); got != wantPkgFacts {
+		t.Errorf("AllPackageFacts: got %v, want %v", got, wantPkgFacts)
+	}
+
+	objFacts := s.AllObjectFacts(filter)
+	wantObjFacts := "[{type a.A int myFact(good object fact)}]"
+	if got := fmt.Sprintf("%v", objFacts); got != wantObjFacts {
+		t.Errorf("AllObjectFacts: got %v, want %v", got, wantObjFacts)
+	}
+}

go/analysis/unitchecker/unitchecker.go

@@ -42,6 +42,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strings"
 	"sync"
@@ -322,6 +323,11 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re
 				return
 			}
 
+			factFilter := make(map[reflect.Type]bool)
+			for _, f := range a.FactTypes {
+				factFilter[reflect.TypeOf(f)] = true
+			}
+
 			pass := &analysis.Pass{
 				Analyzer:          a,
 				Fset:              fset,
@@ -334,10 +340,10 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re
 				Report:            func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) },
 				ImportObjectFact:  facts.ImportObjectFact,
 				ExportObjectFact:  facts.ExportObjectFact,
-				AllObjectFacts:    facts.AllObjectFacts,
+				AllObjectFacts:    func() []analysis.ObjectFact { return facts.AllObjectFacts(factFilter) },
 				ImportPackageFact: facts.ImportPackageFact,
 				ExportPackageFact: facts.ExportPackageFact,
-				AllPackageFacts:   facts.AllPackageFacts,
+				AllPackageFacts:   func() []analysis.PackageFact { return facts.AllPackageFacts(factFilter) },
 			}
 
 			t0 := time.Now()