go/analysis/internal/analysisflags: common flag handling

The analysisflags package provides a function to help
ensure that all drivers support consistent command-line
interfaces.  In particular, -analyzer.enable flags use
tristate logic as in vet, and the -flags flag dumps
a list of flags in JSON for use by 'go vet' and other
build systems.

This code is in a separate package from internal/checker
(the common parts of multichecker, singlechecker)
because we don't want the forthcoming vet-lite (formerly
known as doctor) driver to have an unnecessary dependency
on go/packages.  (When go/packages is promoted to the
standard library we can consolidate them.)

+ Test of tristate analyzer selection logic.

Change-Id: I5ea4e556e0f56505df06eb8fa9dd9eed884a1b47
Reviewed-on: https://go-review.googlesource.com/c/143197
Reviewed-by: Michael Matloob <matloob@golang.org>

go/analysis/cmd/analyze/analyze.go

@@ -1,3 +1,7 @@
+// 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 analyze command is a static checker for Go programs, similar to
 // vet, but with pluggable analyzers defined using the analysis
 // interface, and using the go/packages API to load packages in any
@@ -10,8 +14,6 @@
 package main
 
 import (
-	"log"
-
 	"golang.org/x/tools/go/analysis/multichecker"
 
 	// analysis plug-ins
@@ -41,9 +43,6 @@ import (
 )
 
 func main() {
-	log.SetFlags(0)
-	log.SetPrefix("analyze: ")
-
 	multichecker.Main(
 		// the traditional vet suite:
 		asmdecl.Analyzer,

go/analysis/internal/analysisflags/flags.go

@@ -0,0 +1,223 @@
+// 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 analysisflags defines helpers for processing flags of
+// analysis driver tools.
+package analysisflags
+
+import (
+	"crypto/sha256"
+	"encoding/json"
+	"flag"
+	"fmt"
+	"io"
+	"log"
+	"os"
+	"strconv"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// Parse creates a flag for each of the analyzer's flags,
+// including (in multi mode) an --analysis.enable flag,
+// parses the flags, then filters and returns the list of
+// analyzers enabled by flags.
+func Parse(analyzers []*analysis.Analyzer, multi bool) []*analysis.Analyzer {
+	// Connect each analysis flag to the command line as -analysis.flag.
+	type analysisFlag struct {
+		Name  string
+		Bool  bool
+		Usage string
+	}
+	var analysisFlags []analysisFlag
+
+	enabled := make(map[*analysis.Analyzer]*triState)
+	for _, a := range analyzers {
+		var prefix string
+
+		// Add -analysis.enable flag.
+		if multi {
+			prefix = a.Name + "."
+
+			enable := new(triState)
+			enableName := prefix + "enable"
+			enableUsage := "enable " + a.Name + " analysis"
+			flag.Var(enable, enableName, enableUsage)
+			enabled[a] = enable
+			analysisFlags = append(analysisFlags, analysisFlag{enableName, true, enableUsage})
+		}
+
+		a.Flags.VisitAll(func(f *flag.Flag) {
+			if !multi && flag.Lookup(f.Name) != nil {
+				log.Printf("%s flag -%s would conflict with driver; skipping", a.Name, f.Name)
+				return
+			}
+
+			name := prefix + f.Name
+			flag.Var(f.Value, name, f.Usage)
+
+			var isBool bool
+			if b, ok := f.Value.(interface{ IsBoolFlag() bool }); ok {
+				isBool = b.IsBoolFlag()
+			}
+			analysisFlags = append(analysisFlags, analysisFlag{name, isBool, f.Usage})
+		})
+	}
+
+	// standard flags: -flags, -V.
+	printflags := flag.Bool("flags", false, "print analyzer flags in JSON")
+	addVersionFlag()
+
+	flag.Parse() // (ExitOnError)
+
+	// -flags: print flags so that go vet knows which ones are legitimate.
+	if *printflags {
+		data, err := json.MarshalIndent(analysisFlags, "", "\t")
+		if err != nil {
+			log.Fatal(err)
+		}
+		os.Stdout.Write(data)
+		os.Exit(0)
+	}
+
+	// If any --foo.enable flag is true,  run only those analyzers. Otherwise,
+	// if any --foo.enable flag is false, run all but those analyzers.
+	if multi {
+		var hasTrue, hasFalse bool
+		for _, ts := range enabled {
+			switch *ts {
+			case setTrue:
+				hasTrue = true
+			case setFalse:
+				hasFalse = true
+			}
+		}
+
+		var keep []*analysis.Analyzer
+		if hasTrue {
+			for _, a := range analyzers {
+				if *enabled[a] == setTrue {
+					keep = append(keep, a)
+				}
+			}
+			analyzers = keep
+		} else if hasFalse {
+			for _, a := range analyzers {
+				if *enabled[a] != setFalse {
+					keep = append(keep, a)
+				}
+			}
+			analyzers = keep
+		}
+	}
+
+	return analyzers
+}
+
+// addVersionFlag registers a -V flag that, if set,
+// prints the executable version and exits 0.
+//
+// It is a variable not a function to permit easy
+// overriding in the copy vendored in $GOROOT/src/cmd/vet:
+//
+// func init() { addVersionFlag = objabi.AddVersionFlag }
+var addVersionFlag = func() {
+	flag.Var(versionFlag{}, "V", "print version and exit")
+}
+
+// versionFlag minimally complies with the -V protocol required by "go vet".
+type versionFlag struct{}
+
+func (versionFlag) IsBoolFlag() bool { return true }
+func (versionFlag) Get() interface{} { return nil }
+func (versionFlag) String() string   { return "" }
+func (versionFlag) Set(s string) error {
+	if s != "full" {
+		log.Fatalf("unsupported flag value: -V=%s", s)
+	}
+
+	// This replicates the miminal subset of
+	// cmd/internal/objabi.AddVersionFlag, which is private to the
+	// go tool yet forms part of our command-line interface.
+	// TODO(adonovan): clarify the contract.
+
+	// Print the tool version so the build system can track changes.
+	// Formats:
+	//   $progname version devel ... buildID=...
+	//   $progname version go1.9.1
+	progname := os.Args[0]
+	f, err := os.Open(progname)
+	if err != nil {
+		log.Fatal(err)
+	}
+	h := sha256.New()
+	if _, err := io.Copy(h, f); err != nil {
+		log.Fatal(err)
+	}
+	f.Close()
+	fmt.Printf("%s version devel comments-go-here buildID=%02x\n",
+		progname, string(h.Sum(nil)))
+	os.Exit(0)
+	return nil
+}
+
+// A triState is a boolean that knows whether
+// it has been set to either true or false.
+// It is used to identify whether a flag appears;
+// the standard boolean flag cannot
+// distinguish missing from unset.
+// It also satisfies flag.Value.
+type triState int
+
+const (
+	unset triState = iota
+	setTrue
+	setFalse
+)
+
+func triStateFlag(name string, value triState, usage string) *triState {
+	flag.Var(&value, name, usage)
+	return &value
+}
+
+// triState implements flag.Value, flag.Getter, and flag.boolFlag.
+// They work like boolean flags: we can say vet -printf as well as vet -printf=true
+func (ts *triState) Get() interface{} {
+	return *ts == setTrue
+}
+
+func (ts triState) isTrue() bool {
+	return ts == setTrue
+}
+
+func (ts *triState) Set(value string) error {
+	b, err := strconv.ParseBool(value)
+	if err != nil {
+		// This error message looks poor but package "flag" adds
+		// "invalid boolean value %q for -foo.enable: %s"
+		return fmt.Errorf("want true or false")
+	}
+	if b {
+		*ts = setTrue
+	} else {
+		*ts = setFalse
+	}
+	return nil
+}
+
+func (ts *triState) String() string {
+	switch *ts {
+	case unset:
+		return "true"
+	case setTrue:
+		return "true"
+	case setFalse:
+		return "false"
+	}
+	panic("not reached")
+}
+
+func (ts triState) IsBoolFlag() bool {
+	return true
+}

go/analysis/internal/analysisflags/flags_test.go

@@ -0,0 +1,67 @@
+// 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 analysisflags_test
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"runtime"
+	"strings"
+	"testing"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/internal/analysisflags"
+)
+
+func main() {
+	fmt.Println(analysisflags.Parse([]*analysis.Analyzer{
+		{Name: "a1", Doc: "a1"},
+		{Name: "a2", Doc: "a2"},
+		{Name: "a3", Doc: "a3"},
+	}, true))
+	os.Exit(0)
+}
+
+// This test fork/execs the main function above.
+func TestExec(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skipf("skipping fork/exec test on this platform")
+	}
+
+	progname := os.Args[0]
+
+	if os.Getenv("ANALYSISFLAGS_CHILD") == "1" {
+		// child process
+		os.Args = strings.Fields(progname + " " + os.Getenv("FLAGS"))
+		main()
+		panic("unreachable")
+	}
+
+	for _, test := range []struct {
+		flags string
+		want  string
+	}{
+		{"", "[a1 a2 a3]"},
+		{"-a1.enable=0", "[a2 a3]"},
+		{"-a1.enable=1", "[a1]"},
+		{"-a1.enable", "[a1]"},
+		{"-a1.enable=1 -a3.enable=1", "[a1 a3]"},
+		{"-a1.enable=1 -a3.enable=0", "[a1]"},
+	} {
+		cmd := exec.Command(progname, "-test.run=TestExec")
+		cmd.Env = append(os.Environ(), "ANALYSISFLAGS_CHILD=1", "FLAGS="+test.flags)
+
+		output, err := cmd.CombinedOutput()
+		if err != nil {
+			t.Fatalf("exec failed: %v; output=<<%s>>", err, output)
+		}
+
+		got := strings.TrimSpace(string(output))
+		if got != test.want {
+			t.Errorf("got %s, want %s", got, test.want)
+		}
+	}
+}

go/analysis/multichecker/multichecker.go

@@ -1,3 +1,7 @@
+// 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 multichecker defines the main function for an analysis driver
 // with several analyzers. This package makes it easy for anyone to build
 // an analysis tool containing just the analyzers they need.
@@ -8,16 +12,15 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"path/filepath"
 	"sort"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/internal/analysisflags"
 	"golang.org/x/tools/go/analysis/internal/checker"
 )
 
-// TODO(adonovan): support tri-state enable flags so -printf.enable=true means
-// "run only printf" and -printf.enable=false means "run all but printf"
-
 // TODO(adonovan): document (and verify) the exit codes:
 // "Vet's exit code is 2 for erroneous invocation of the tool, 1 if a
 // problem was reported, and 0 otherwise. Note that the tool does not
@@ -25,62 +28,39 @@ import (
 // so it should be used as guidance only, not as a firm indicator of
 // program correctness."
 
-const usage = `Analyze is a tool for static analysis of Go programs.
+const usage = `PROGNAME is a tool for static analysis of Go programs.
 
-Analyze examines Go source code and reports suspicious constructs, such as Printf
+PROGNAME examines Go source code and reports suspicious constructs, such as Printf
 calls whose arguments do not align with the format string. It uses heuristics
 that do not guarantee all reports are genuine problems, but it can find errors
 not caught by the compilers.
 
-Usage: analyze [-flag] [package]
+Usage: PROGNAME [-flag] [package]
 `
 
 func Main(analyzers ...*analysis.Analyzer) {
+	progname := filepath.Base(os.Args[0])
+	log.SetFlags(0)
+	log.SetPrefix(progname + ": ")
+
 	if err := analysis.Validate(analyzers); err != nil {
 		log.Fatal(err)
 	}
 
 	checker.RegisterFlags()
 
-	// Connect each analysis flag to the command line as --analysis.flag.
+	analyzers = analysisflags.Parse(analyzers, true)
-	enabled := make(map[*analysis.Analyzer]*bool)
-	for _, a := range analyzers {
-		prefix := a.Name + "."
-
-		// Add --foo.enable flag.
-		enable := new(bool)
-		flag.BoolVar(enable, prefix+"enable", false, "enable only "+a.Name+" analysis")
-		enabled[a] = enable
-
-		a.Flags.VisitAll(func(f *flag.Flag) {
-			flag.Var(f.Value, prefix+f.Name, f.Usage)
-		})
-	}
-
-	flag.Parse() // (ExitOnError)
-
-	// If any --foo.enable flag is set,
-	// run only those analyzers.
-	var keep []*analysis.Analyzer
-	for _, a := range analyzers {
-		if *enabled[a] {
-			keep = append(keep, a)
-		}
-	}
-	if keep != nil {
-		analyzers = keep
-	}
 
 	args := flag.Args()
 	if len(args) == 0 {
-		fmt.Fprintln(os.Stderr, usage)
+		fmt.Fprintln(os.Stderr, strings.ReplaceAll(usage, "PROGNAME", progname))
-		fmt.Fprintln(os.Stderr, `Run 'analyze help' for more detail,
+		fmt.Fprintf(os.Stderr, `Run '%[1]s help' for more detail,
- or 'analyze help name' for details and flags of a specific analyzer.`)
+ or '%[1]s help name' for details and flags of a specific analyzer.\n`, progname)
 		os.Exit(1)
 	}
 
 	if args[0] == "help" {
-		help(analyzers, args[1:])
+		help(progname, analyzers, args[1:])
 		os.Exit(0)
 	}
 
@@ -89,10 +69,10 @@ func Main(analyzers ...*analysis.Analyzer) {
 	}
 }
 
-func help(analyzers []*analysis.Analyzer, args []string) {
+func help(progname string, analyzers []*analysis.Analyzer, args []string) {
 	// No args: show summary of all analyzers.
 	if len(args) == 0 {
-		fmt.Println(usage)
+		fmt.Println(strings.ReplaceAll(usage, "PROGNAME", progname))
 		fmt.Println("Registered analyzers:")
 		fmt.Println()
 		sort.Slice(analyzers, func(i, j int) bool {
@@ -116,7 +96,7 @@ func help(analyzers []*analysis.Analyzer, args []string) {
 		})
 		fs.PrintDefaults()
 
-		fmt.Println("\nTo see details and flags of a specific analyzer, run 'analyze help name'.")
+		fmt.Printf("\nTo see details and flags of a specific analyzer, run '%s help name'.\n", progname)
 
 		return
 	}

go/analysis/singlechecker/singlechecker.go

@@ -1,3 +1,7 @@
+// 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 singlechecker defines the main function for an analysis
 // driver with only a single analysis.
 // This package makes it easy for a provider of an analysis package to
@@ -27,6 +31,7 @@ import (
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/internal/analysisflags"
 	"golang.org/x/tools/go/analysis/internal/checker"
 )
 
@@ -35,15 +40,13 @@ func Main(a *analysis.Analyzer) {
 	log.SetFlags(0)
 	log.SetPrefix(a.Name + ": ")
 
-	checker.RegisterFlags()
+	analyzers := []*analysis.Analyzer{a}
 
-	a.Flags.VisitAll(func(f *flag.Flag) {
+	if err := analysis.Validate(analyzers); err != nil {
-		if flag.Lookup(f.Name) != nil {
+		log.Fatal(err)
-			log.Printf("%s flag -%s would conflict with driver; skipping", a.Name, f.Name)
+	}
-			return
+
-		}
+	checker.RegisterFlags()
-		flag.Var(f.Value, f.Name, f.Usage)
-	})
 
 	flag.Usage = func() {
 		paras := strings.Split(a.Doc, "\n\n")
@@ -55,16 +58,16 @@ func Main(a *analysis.Analyzer) {
 		fmt.Println("\nFlags:")
 		flag.PrintDefaults()
 	}
-	flag.Parse()
+
+	analyzers = analysisflags.Parse(analyzers, false)
 
 	args := flag.Args()
-
 	if len(args) == 0 {
 		flag.Usage()
 		os.Exit(1)
 	}
 
-	if err := checker.Run(args, []*analysis.Analyzer{a}); err != nil {
+	if err := checker.Run(args, analyzers); err != nil {
 		log.Fatal(err)
 	}
 }