diff --git a/go/analysis/cmd/analyze/analyze.go b/go/analysis/cmd/analyze/analyze.go index f4df80185e..b77bbbd7af 100644 --- a/go/analysis/cmd/analyze/analyze.go +++ b/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, diff --git a/go/analysis/internal/analysisflags/flags.go b/go/analysis/internal/analysisflags/flags.go new file mode 100644 index 0000000000..d6c13f2685 --- /dev/null +++ b/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 +} diff --git a/go/analysis/internal/analysisflags/flags_test.go b/go/analysis/internal/analysisflags/flags_test.go new file mode 100644 index 0000000000..d2310acfbe --- /dev/null +++ b/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) + } + } +} diff --git a/go/analysis/multichecker/multichecker.go b/go/analysis/multichecker/multichecker.go index 0621cf66e8..a849031ef4 100644 --- a/go/analysis/multichecker/multichecker.go +++ b/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. - 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 - } + analyzers = analysisflags.Parse(analyzers, true) args := flag.Args() if len(args) == 0 { - fmt.Fprintln(os.Stderr, usage) - fmt.Fprintln(os.Stderr, `Run 'analyze help' for more detail, - or 'analyze help name' for details and flags of a specific analyzer.`) + fmt.Fprintln(os.Stderr, strings.ReplaceAll(usage, "PROGNAME", progname)) + fmt.Fprintf(os.Stderr, `Run '%[1]s help' for more detail, + 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 } diff --git a/go/analysis/singlechecker/singlechecker.go b/go/analysis/singlechecker/singlechecker.go index c66fb65602..da54022081 100644 --- a/go/analysis/singlechecker/singlechecker.go +++ b/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 flag.Lookup(f.Name) != nil { - log.Printf("%s flag -%s would conflict with driver; skipping", a.Name, f.Name) - return - } - flag.Var(f.Value, f.Name, f.Usage) - }) + if err := analysis.Validate(analyzers); err != nil { + log.Fatal(err) + } + + checker.RegisterFlags() 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) } }