go/analysis/internal/unitchecker: a 'go vet'-compatible driver
Also:
- add cmd/vet-lite, a version of cmd/vet that doesn't depend on
go/packages and must be run under "go vet". This will be vendored
into $GOROOT/src/cmd/vet.
- add an integration test for a unitchecker-based command under "go vet".
Change-Id: Id613dac2812816c6d6372fa6d1536c8d4e4c2676
Reviewed-on: https://go-review.googlesource.com/c/143418
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/go/analysis/cmd/vet-lite/main.go b/go/analysis/cmd/vet-lite/main.go
new file mode 100644
index 0000000..ae66a7d
--- /dev/null
+++ b/go/analysis/cmd/vet-lite/main.go
@@ -0,0 +1,83 @@
+// The vet-lite command is a driver for static checkers conforming to
+// the golang.org/x/tools/go/analysis API. It must be run by go vet:
+//
+// $ GOVETTOOL=$(which vet-lite) go vet
+//
+// For a checker also capable of running standalone, use multichecker.
+package main
+
+import (
+ "flag"
+ "log"
+ "strings"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/internal/analysisflags"
+ "golang.org/x/tools/go/analysis/internal/unitchecker"
+
+ "golang.org/x/tools/go/analysis/passes/asmdecl"
+ "golang.org/x/tools/go/analysis/passes/assign"
+ "golang.org/x/tools/go/analysis/passes/atomic"
+ "golang.org/x/tools/go/analysis/passes/bools"
+ "golang.org/x/tools/go/analysis/passes/buildtag"
+ "golang.org/x/tools/go/analysis/passes/cgocall"
+ "golang.org/x/tools/go/analysis/passes/composite"
+ "golang.org/x/tools/go/analysis/passes/copylock"
+ "golang.org/x/tools/go/analysis/passes/httpresponse"
+ "golang.org/x/tools/go/analysis/passes/loopclosure"
+ "golang.org/x/tools/go/analysis/passes/lostcancel"
+ "golang.org/x/tools/go/analysis/passes/nilfunc"
+ "golang.org/x/tools/go/analysis/passes/pkgfact"
+ "golang.org/x/tools/go/analysis/passes/printf"
+ "golang.org/x/tools/go/analysis/passes/shift"
+ "golang.org/x/tools/go/analysis/passes/stdmethods"
+ "golang.org/x/tools/go/analysis/passes/structtag"
+ "golang.org/x/tools/go/analysis/passes/tests"
+ "golang.org/x/tools/go/analysis/passes/unreachable"
+ "golang.org/x/tools/go/analysis/passes/unsafeptr"
+ "golang.org/x/tools/go/analysis/passes/unusedresult"
+)
+
+var analyzers = []*analysis.Analyzer{
+ // For now, just the traditional vet suite:
+ asmdecl.Analyzer,
+ assign.Analyzer,
+ atomic.Analyzer,
+ bools.Analyzer,
+ buildtag.Analyzer,
+ cgocall.Analyzer,
+ composite.Analyzer,
+ copylock.Analyzer,
+ httpresponse.Analyzer,
+ loopclosure.Analyzer,
+ lostcancel.Analyzer,
+ nilfunc.Analyzer,
+ pkgfact.Analyzer,
+ printf.Analyzer,
+ // shadow.Analyzer, // experimental; not enabled by default
+ shift.Analyzer,
+ stdmethods.Analyzer,
+ structtag.Analyzer,
+ tests.Analyzer,
+ unreachable.Analyzer,
+ unsafeptr.Analyzer,
+ unusedresult.Analyzer,
+}
+
+func main() {
+ log.SetFlags(0)
+ log.SetPrefix("vet: ")
+
+ if err := analysis.Validate(analyzers); err != nil {
+ log.Fatal(err)
+ }
+
+ analyzers = analysisflags.Parse(analyzers, true)
+
+ args := flag.Args()
+ if len(args) != 1 || !strings.HasSuffix(args[0], ".cfg") {
+ log.Fatalf("invalid command: want .cfg file (this reduced version of vet is intended to be run only by the 'go vet' command)")
+ }
+
+ unitchecker.Main(args[0], analyzers)
+}
diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go
index a7c7826..e0c1029 100644
--- a/go/analysis/internal/checker/checker.go
+++ b/go/analysis/internal/checker/checker.go
@@ -25,6 +25,7 @@
"time"
"golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/internal/unitchecker"
"golang.org/x/tools/go/packages"
)
@@ -106,6 +107,18 @@
}()
}
+ // The undocumented protocol used by 'go vet'
+ // is that a vet-like tool must support:
+ //
+ // -flags describe flags in JSON
+ // -V=full describe executable for build caching
+ // foo.cfg perform separate modular analyze on the single
+ // unit described by a JSON config file foo.cfg.
+ if len(args) == 1 && strings.HasSuffix(args[0], ".cfg") {
+ unitchecker.Main(args[0], analyzers)
+ panic("unreachable")
+ }
+
// Load the packages.
if dbg('v') {
log.SetPrefix("")
diff --git a/go/analysis/internal/facts/facts.go b/go/analysis/internal/facts/facts.go
index c1edc90..468f148 100644
--- a/go/analysis/internal/facts/facts.go
+++ b/go/analysis/internal/facts/facts.go
@@ -217,7 +217,7 @@
s.mu.Lock()
for k, fact := range s.m {
if debug {
- log.Printf("%#v => %s\n", k, fact)
+ log.Printf("%v => %s\n", k, fact)
}
var object objectpath.Path
if k.obj != nil {
@@ -259,15 +259,6 @@
if len(gobFacts) > 0 {
if err := gob.NewEncoder(&buf).Encode(gobFacts); err != nil {
// Fact encoding should never fail. Identify the culprit.
- //
- // TODO(adonovan): what's the right thing to do here?
- // The error is clearly a bug, so log.Fatal leads to early
- // detection, but it could potentially bring down a big
- // job because of an obscure dynamic bug in a fact.
- // But perhaps that's fine: other bugs in Analyzers
- // have the same potential to cause failures.
- // Alternatively we could discard the bad facts with a
- // log message, but who reads logs?
for _, gf := range gobFacts {
if err := gob.NewEncoder(ioutil.Discard).Encode(gf); err != nil {
fact := gf.Fact
diff --git a/go/analysis/internal/unitchecker/testdata/src/a/a.go b/go/analysis/internal/unitchecker/testdata/src/a/a.go
new file mode 100644
index 0000000..9765e19
--- /dev/null
+++ b/go/analysis/internal/unitchecker/testdata/src/a/a.go
@@ -0,0 +1,7 @@
+package a
+
+func _() {
+ MyFunc123()
+}
+
+func MyFunc123() {}
diff --git a/go/analysis/internal/unitchecker/testdata/src/b/b.go b/go/analysis/internal/unitchecker/testdata/src/b/b.go
new file mode 100644
index 0000000..a7b6125
--- /dev/null
+++ b/go/analysis/internal/unitchecker/testdata/src/b/b.go
@@ -0,0 +1,10 @@
+package b
+
+import "a"
+
+func _() {
+ a.MyFunc123()
+ MyFunc123()
+}
+
+func MyFunc123() {}
diff --git a/go/analysis/internal/unitchecker/unitchecker.go b/go/analysis/internal/unitchecker/unitchecker.go
new file mode 100644
index 0000000..b67c943
--- /dev/null
+++ b/go/analysis/internal/unitchecker/unitchecker.go
@@ -0,0 +1,306 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// The unitchecker package defines the main function for an analysis
+// driver that analyzes a single compilation unit during a build.
+// It is invoked by a build system such as "go vet":
+//
+// $ GOVETTOOL=$(which vet) go vet
+//
+// It supports the following command-line protocol:
+//
+// -V=full describe executable (to the build tool)
+// -flags describe flags (to the build tool)
+// foo.cfg description of compilation unit (from the build tool)
+//
+// This package does not depend on go/packages.
+// If you need a standalone tool, use multichecker,
+// which supports this mode but can also load packages
+// from source using go/packages.
+package unitchecker
+
+// TODO(adonovan):
+// - with gccgo, go build does not build standard library,
+// so we will not get to analyze it. Yet we must in order
+// to create base facts for, say, the fmt package for the
+// printf checker.
+// - support JSON output, factored with multichecker.
+
+import (
+ "encoding/gob"
+ "encoding/json"
+ "fmt"
+ "go/ast"
+ "go/build"
+ "go/importer"
+ "go/parser"
+ "go/token"
+ "go/types"
+ "io"
+ "io/ioutil"
+ "log"
+ "os"
+ "sort"
+ "strings"
+ "sync"
+ "time"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/internal/facts"
+)
+
+// A Config describes a compilation unit to be analyzed.
+// It is provided to the tool in a JSON-encoded file
+// whose name ends with ".cfg".
+type Config struct {
+ Compiler string
+ Dir string
+ ImportPath string
+ GoFiles []string
+ OtherFiles []string // TODO(adonovan): make go vet populate this (github.com/golang/go/issues/27665)
+ ImportMap map[string]string
+ PackageFile map[string]string
+ Standard map[string]bool
+ PackageVetx map[string]string
+ VetxOnly bool
+ VetxOutput string
+ SucceedOnTypecheckFailure bool
+}
+
+// Main reads the *.cfg file, runs the analysis,
+// and calls os.Exit with an appropriate error code.
+func Main(configFile string, analyzers []*analysis.Analyzer) {
+ cfg, err := readConfig(configFile)
+ if err != nil {
+ log.Fatal(err)
+ }
+
+ fset := token.NewFileSet()
+ diags, err := run(fset, cfg, analyzers)
+ if err != nil {
+ log.Fatal(err)
+ }
+
+ if len(diags) > 0 {
+ for _, diag := range diags {
+ fmt.Fprintf(os.Stderr, "%s: %s\n", fset.Position(diag.Pos), diag.Message)
+ }
+ os.Exit(1)
+ }
+
+ os.Exit(0)
+}
+
+func readConfig(filename string) (*Config, error) {
+ data, err := ioutil.ReadFile(filename)
+ if err != nil {
+ return nil, err
+ }
+ cfg := new(Config)
+ if err := json.Unmarshal(data, cfg); err != nil {
+ return nil, fmt.Errorf("cannot decode JSON config file %s: %v", filename, err)
+ }
+ if len(cfg.GoFiles) == 0 {
+ // The go command disallows packages with no files.
+ // The only exception is unsafe, but the go command
+ // doesn't call vet on it.
+ return nil, fmt.Errorf("package has no files: %s", cfg.ImportPath)
+ }
+ return cfg, nil
+}
+
+func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]analysis.Diagnostic, error) {
+ // Load, parse, typecheck.
+ var files []*ast.File
+ for _, name := range cfg.GoFiles {
+ f, err := parser.ParseFile(fset, name, nil, parser.ParseComments)
+ if err != nil {
+ if cfg.SucceedOnTypecheckFailure {
+ // Silently succeed; let the compiler
+ // report parse errors.
+ err = nil
+ }
+ return nil, err
+ }
+ files = append(files, f)
+ }
+ compilerImporter := importer.For(cfg.Compiler, func(path string) (io.ReadCloser, error) {
+ // path is a resolved package path, not an import path.
+ file, ok := cfg.PackageFile[path]
+ if !ok {
+ if cfg.Compiler == "gccgo" && cfg.Standard[path] {
+ return nil, nil // fall back to default gccgo lookup
+ }
+ return nil, fmt.Errorf("no package file for %q", path)
+ }
+ return os.Open(file)
+ })
+ importer := importerFunc(func(importPath string) (*types.Package, error) {
+ path, ok := cfg.ImportMap[importPath] // resolve vendoring, etc
+ if !ok {
+ return nil, fmt.Errorf("can't resolve import %q", path)
+ }
+ return compilerImporter.Import(path)
+ })
+ tc := &types.Config{
+ Importer: importer,
+ Sizes: types.SizesFor("gc", build.Default.GOARCH), // assume gccgo ≡ gc?
+ }
+ info := &types.Info{
+ Types: make(map[ast.Expr]types.TypeAndValue),
+ Defs: make(map[*ast.Ident]types.Object),
+ Uses: make(map[*ast.Ident]types.Object),
+ Implicits: make(map[ast.Node]types.Object),
+ Scopes: make(map[ast.Node]*types.Scope),
+ Selections: make(map[*ast.SelectorExpr]*types.Selection),
+ }
+ pkg, err := tc.Check(cfg.ImportPath, fset, files, info)
+ if err != nil {
+ if cfg.SucceedOnTypecheckFailure {
+ // Silently succeed; let the compiler
+ // report type errors.
+ err = nil
+ }
+ return nil, err
+ }
+
+ // Register fact types with gob.
+ // In VetxOnly mode, analyzers are only for their facts,
+ // so we can skip any analysis that neither produces facts
+ // nor depends on any analysis that produces facts.
+ // Also build a map to hold working state and result.
+ type action struct {
+ once sync.Once
+ result interface{}
+ err error
+ usesFacts bool // (transitively uses)
+ diagnostics []analysis.Diagnostic
+ }
+ actions := make(map[*analysis.Analyzer]*action)
+ var registerFacts func(a *analysis.Analyzer) bool
+ registerFacts = func(a *analysis.Analyzer) bool {
+ act, ok := actions[a]
+ if !ok {
+ act = new(action)
+ var usesFacts bool
+ for _, f := range a.FactTypes {
+ usesFacts = true
+ gob.Register(f)
+ }
+ for _, req := range a.Requires {
+ if registerFacts(req) {
+ usesFacts = true
+ }
+ }
+ act.usesFacts = usesFacts
+ actions[a] = act
+ }
+ return act.usesFacts
+ }
+ var filtered []*analysis.Analyzer
+ for _, a := range analyzers {
+ if registerFacts(a) || !cfg.VetxOnly {
+ filtered = append(filtered, a)
+ }
+ }
+ analyzers = filtered
+
+ // Read facts from imported packages.
+ read := func(path string) ([]byte, error) {
+ if vetx, ok := cfg.PackageVetx[path]; ok {
+ return ioutil.ReadFile(vetx)
+ }
+ return nil, nil // no .vetx file, no facts
+ }
+ facts, err := facts.Decode(pkg, read)
+ if err != nil {
+ return nil, err
+ }
+
+ // In parallel, execute the DAG of analyzers.
+ var exec func(a *analysis.Analyzer) *action
+ var execAll func(analyzers []*analysis.Analyzer)
+ exec = func(a *analysis.Analyzer) *action {
+ act := actions[a]
+ act.once.Do(func() {
+ execAll(a.Requires) // prefetch dependencies in parallel
+
+ // The inputs to this analysis are the
+ // results of its prerequisites.
+ inputs := make(map[*analysis.Analyzer]interface{})
+ var failed []string
+ for _, req := range a.Requires {
+ reqact := exec(req)
+ if reqact.err != nil {
+ failed = append(failed, req.String())
+ continue
+ }
+ inputs[req] = reqact.result
+ }
+
+ // Report an error if any dependency failed.
+ if failed != nil {
+ sort.Strings(failed)
+ act.err = fmt.Errorf("failed prerequisites: %s", strings.Join(failed, ", "))
+ return
+ }
+
+ pass := &analysis.Pass{
+ Analyzer: a,
+ Fset: fset,
+ Files: files,
+ OtherFiles: cfg.OtherFiles,
+ Pkg: pkg,
+ TypesInfo: info,
+ ResultOf: inputs,
+ Report: func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) },
+ ImportObjectFact: facts.ImportObjectFact,
+ ExportObjectFact: facts.ExportObjectFact,
+ ImportPackageFact: facts.ImportPackageFact,
+ ExportPackageFact: facts.ExportPackageFact,
+ }
+
+ t0 := time.Now()
+ act.result, act.err = a.Run(pass)
+ if false {
+ log.Printf("analysis %s = %s", pass, time.Since(t0))
+ }
+ })
+ return act
+ }
+ execAll = func(analyzers []*analysis.Analyzer) {
+ var wg sync.WaitGroup
+ for _, a := range analyzers {
+ wg.Add(1)
+ go func(a *analysis.Analyzer) {
+ _ = exec(a)
+ wg.Done()
+ }(a)
+ }
+ wg.Wait()
+ }
+
+ execAll(analyzers)
+
+ // Return diagnostics from root analyzers.
+ var diags []analysis.Diagnostic
+ for _, a := range analyzers {
+ act := actions[a]
+ if act.err != nil {
+ return nil, act.err // some analysis failed
+ }
+ diags = append(diags, act.diagnostics...)
+ }
+
+ data := facts.Encode()
+ if err := ioutil.WriteFile(cfg.VetxOutput, data, 0666); err != nil {
+ return nil, fmt.Errorf("failed to write analysis facts: %v", err)
+ }
+
+ return diags, nil
+}
+
+type importerFunc func(path string) (*types.Package, error)
+
+func (f importerFunc) Import(path string) (*types.Package, error) { return f(path) }
diff --git a/go/analysis/internal/unitchecker/unitchecker_test.go b/go/analysis/internal/unitchecker/unitchecker_test.go
new file mode 100644
index 0000000..872275f
--- /dev/null
+++ b/go/analysis/internal/unitchecker/unitchecker_test.go
@@ -0,0 +1,97 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package unitchecker_test
+
+import (
+ "flag"
+ "log"
+ "os"
+ "os/exec"
+ "runtime"
+ "strings"
+ "testing"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/internal/analysisflags"
+ "golang.org/x/tools/go/analysis/internal/unitchecker"
+ "golang.org/x/tools/go/analysis/passes/findcall"
+ "golang.org/x/tools/go/analysis/passes/printf"
+)
+
+func TestMain(m *testing.M) {
+ if os.Getenv("UNITCHECKER_CHILD") == "1" {
+ // child process
+ main()
+ panic("unreachable")
+ }
+
+ // test
+ flag.Parse()
+ os.Exit(m.Run())
+}
+
+func main() {
+ findcall.Analyzer.Flags.Set("name", "MyFunc123")
+
+ var analyzers = []*analysis.Analyzer{
+ findcall.Analyzer,
+ printf.Analyzer,
+ }
+
+ if err := analysis.Validate(analyzers); err != nil {
+ log.Fatal(err)
+ }
+ analyzers = analysisflags.Parse(analyzers, true)
+
+ args := flag.Args()
+ if len(args) != 1 || !strings.HasSuffix(args[0], ".cfg") {
+ log.Fatalf("invalid command: want .cfg file")
+ }
+
+ unitchecker.Main(args[0], analyzers)
+}
+
+// This is a very basic integration test of modular
+// analysis with facts using unitchecker under "go vet".
+// It fork/execs the main function above.
+func TestIntegration(t *testing.T) {
+ if runtime.GOOS != "linux" {
+ t.Skipf("skipping fork/exec test on this platform")
+ }
+
+ testdata := analysistest.TestData()
+
+ cmd := exec.Command("go", "vet", "b")
+ cmd.Env = append(os.Environ(),
+ "GOVETTOOL="+os.Args[0],
+ "UNITCHECKER_CHILD=1",
+ "GOPATH="+testdata,
+ )
+
+ out, err := cmd.CombinedOutput()
+ exitcode := -1
+ if exitErr, ok := err.(*exec.ExitError); ok {
+ exitcode = exitErr.ExitCode()
+ }
+ if exitcode != 2 {
+ t.Errorf("got exit code %d, want 2", exitcode)
+ }
+
+ want := `
+# a
+testdata/src/a/a.go:4:11: call of MyFunc123(...)
+# b
+testdata/src/b/b.go:6:13: call of MyFunc123(...)
+testdata/src/b/b.go:7:11: call of MyFunc123(...)
+`[1:]
+ if got := string(out); got != want {
+ t.Errorf("got <<%s>>, want <<%s>>", got, want)
+ }
+
+ if t.Failed() {
+ t.Logf("err=%v stderr=<<%s>", err, cmd.Stderr)
+ }
+}
diff --git a/go/analysis/multichecker/multichecker.go b/go/analysis/multichecker/multichecker.go
index 27406fa..9038eac 100644
--- a/go/analysis/multichecker/multichecker.go
+++ b/go/analysis/multichecker/multichecker.go
@@ -41,7 +41,7 @@
func Main(analyzers ...*analysis.Analyzer) {
progname := filepath.Base(os.Args[0])
log.SetFlags(0)
- log.SetPrefix(progname + ": ")
+ log.SetPrefix(filepath.Base(os.Args[0]) + ": ") // e.g. "vet: "
if err := analysis.Validate(analyzers); err != nil {
log.Fatal(err)
diff --git a/go/analysis/passes/pkgfact/pkgfact.go b/go/analysis/passes/pkgfact/pkgfact.go
index eca0f25..e053086 100644
--- a/go/analysis/passes/pkgfact/pkgfact.go
+++ b/go/analysis/passes/pkgfact/pkgfact.go
@@ -70,14 +70,14 @@
}
}
- // At each "const _name = value", add a fact into env.
+ // 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, "_") && strings.HasSuffix(name, "_") {
- if key := strings.Trim(name[1:], "_"); key != "" {
+ if key := strings.Trim(name, "_"); key != "" {
value := pass.TypesInfo.Types[spec.Values[i]].Value.String()
result[key] = value
}
