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>

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)
+}

go/analysis/internal/checker/checker.go

@@ -25,6 +25,7 @@ import (
 	"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 @@ func Run(args []string, analyzers []*analysis.Analyzer) error {
 		}()
 	}
 
+	// 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("")

go/analysis/internal/facts/facts.go

@@ -217,7 +217,7 @@ func (s *Set) Encode() []byte {
 	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 @@ func (s *Set) Encode() []byte {
 	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

go/analysis/internal/unitchecker/testdata/src/a/a.go

@@ -0,0 +1,7 @@
+package a
+
+func _() {
+	MyFunc123()
+}
+
+func MyFunc123() {}

go/analysis/internal/unitchecker/testdata/src/b/b.go

@@ -0,0 +1,10 @@
+package b
+
+import "a"
+
+func _() {
+	a.MyFunc123()
+	MyFunc123()
+}
+
+func MyFunc123() {}

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) }

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)
+	}
+}

go/analysis/multichecker/multichecker.go

@@ -41,7 +41,7 @@ Usage: PROGNAME [-flag] [package]
 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)

go/analysis/passes/pkgfact/pkgfact.go

@@ -70,14 +70,14 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		}
 	}
 
-	// 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
 					}