diff --git a/go/analysis/cmd/vet-lite/main.go b/go/analysis/cmd/vet-lite/main.go index 70d22fbb26e..38f23efd8c1 100644 --- a/go/analysis/cmd/vet-lite/main.go +++ b/go/analysis/cmd/vet-lite/main.go @@ -1,7 +1,7 @@ // 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 +// $ go vet -vettool=$(which vet-lite) // // For a checker also capable of running standalone, use multichecker. package main @@ -9,6 +9,7 @@ package main import ( "flag" "log" + "os" "strings" "golang.org/x/tools/go/analysis" @@ -74,6 +75,34 @@ func main() { log.Fatal(err) } + // Flags for legacy vet compatibility. + // + // These flags, plus the shims in analysisflags, enable all + // existing scripts that run vet to continue to work. + // + // We still need to deal with legacy vet's "experimental" + // checkers. In vet there is exactly one such checker, shadow, + // and it must be enabled explicitly with the -shadow flag, but + // of course setting it disables all the other tristate flags, + // requiring the -all flag to reenable them. + // + // I don't believe this feature carries its weight. I propose we + // simply skip shadow for now; the few users that want it can + // run "go vet -vettool=..." using a vet tool that includes + // shadow, either as an additional step, with a shadow + // "singlechecker", or in place of the regular vet step on a + // multichecker with a hand-picked suite of checkers. + // Or, we could improve the shadow checker to the point where it + // need not be experimental. + for _, name := range []string{"source", "v", "all"} { + flag.Var(warnBoolFlag(name), name, "no effect (deprecated)") + } + + flag.Usage = func() { + analysisflags.Help("vet", analyzers, nil) + os.Exit(1) + } + analyzers = analysisflags.Parse(analyzers, true) args := flag.Args() @@ -83,3 +112,12 @@ func main() { unitchecker.Main(args[0], analyzers) } + +type warnBoolFlag string + +func (f warnBoolFlag) Set(s string) error { + log.Printf("warning: deprecated flag -%s has no effect", string(f)) + return nil +} +func (f warnBoolFlag) IsBoolFlag() bool { return true } +func (f warnBoolFlag) String() string { return "false" } diff --git a/go/analysis/internal/analysisflags/flags.go b/go/analysis/internal/analysisflags/flags.go index d6c13f2685c..3ab46f39386 100644 --- a/go/analysis/internal/analysisflags/flags.go +++ b/go/analysis/internal/analysisflags/flags.go @@ -69,6 +69,15 @@ func Parse(analyzers []*analysis.Analyzer, multi bool) []*analysis.Analyzer { printflags := flag.Bool("flags", false, "print analyzer flags in JSON") addVersionFlag() + // Add shims for legacy vet flags. + for old, new := range vetLegacyFlags { + newFlag := flag.Lookup(new) + if newFlag != nil && flag.Lookup(old) == nil { + oldFlag := &warnFlag{old, new, newFlag.Value} + flag.Var(oldFlag, old, "deprecated alias for -"+new) + } + } + flag.Parse() // (ExitOnError) // -flags: print flags so that go vet knows which ones are legitimate. @@ -221,3 +230,63 @@ func (ts *triState) String() string { func (ts triState) IsBoolFlag() bool { return true } + +// Legacy flag support + +// vetLegacyFlags maps flags used by legacy vet to their corresponding +// new names. Uses of the old names will continue to work, but with a +// log message. TODO(adonovan): delete this mechanism after Nov 2019. +var vetLegacyFlags = map[string]string{ + "asmdecl": "asmdecl.enable", + "assign": "assign.enable", + "atomic": "atomic.enable", + "bool": "bools.enable", + "buildtags": "buildtag.enable", + "cgocall": "cgocall.enable", + "composites": "composites.enable", + "compositewhitelist": "composites.whitelist", + "copylocks": "copylocks.enable", + "flags": "flags.enable", + "httpresponse": "httpresponse.enable", + "lostcancel": "lostcancel.enable", + "methods": "stdmethods.enable", + "nilfunc": "nilfunc.enable", + "printf": "printf.enable", + "printfuncs": "printf.funcs", + "rangeloops": "loopclosure.enable", + "shadow": "shadow.enable", + "shadowstrict": "shadow.strict", + "shift": "shift.enable", + "source": "source.enable", + "structtags": "structtag.enable", + "tests": "tests.enable", + // "unmarshal": "unmarshal.enable", // TODO(adonovan) a new checker + "unreachable": "unreachable.enable", + "unsafeptr": "unsafeptr.enable", + "unusedresult": "unusedresult.enable", + "unusedfuncs": "unusedresult.funcs", + "unusedstringmethods": "unusedresult.stringmethods", +} + +type warnFlag struct { + old, new string + flag.Value +} + +func (f *warnFlag) Set(s string) error { + log.Printf("warning: deprecated flag -%s; use -%s instead", f.old, f.new) + return f.Value.Set(s) +} + +func (f *warnFlag) IsBoolFlag() bool { + b, ok := f.Value.(interface{ IsBoolFlag() bool }) + return ok && b.IsBoolFlag() +} + +func (f *warnFlag) String() string { + // The flag package may call new(warnFlag).String. + if f.Value == nil { + return "" + } + return f.Value.String() +} diff --git a/go/analysis/internal/analysisflags/help.go b/go/analysis/internal/analysisflags/help.go new file mode 100644 index 00000000000..843c3d6fe02 --- /dev/null +++ b/go/analysis/internal/analysisflags/help.go @@ -0,0 +1,100 @@ +package analysisflags + +import ( + "flag" + "fmt" + "io" + "log" + "os" + "path/filepath" + "sort" + "strings" + + "golang.org/x/tools/go/analysis" +) + +const usage = `PROGNAME is a tool for static analysis of Go programs. + +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: PROGNAME [-flag] [package] +` + +// PrintUsage prints the usage message to stderr. +func PrintUsage(out io.Writer) { + progname := filepath.Base(os.Args[0]) + fmt.Fprintln(out, strings.Replace(usage, "PROGNAME", progname, -1)) +} + +// Help implements the help subcommand for a multichecker or vet-lite +// style command. The optional args specify the analyzers to describe. +// Help calls log.Fatal if no such analyzer exists. +func Help(progname string, analyzers []*analysis.Analyzer, args []string) { + // No args: show summary of all analyzers. + if len(args) == 0 { + PrintUsage(os.Stdout) + fmt.Println("Registered analyzers:") + fmt.Println() + sort.Slice(analyzers, func(i, j int) bool { + return analyzers[i].Name < analyzers[j].Name + }) + for _, a := range analyzers { + title := strings.Split(a.Doc, "\n\n")[0] + fmt.Printf(" %-12s %s\n", a.Name, title) + } + fmt.Println("\nBy default all analyzers are run.") + fmt.Println("To select specific analyzers, use the -NAME.enable flag for each one,") + fmt.Println(" or -NAME.enable=false to run all analyzers not explicitly disabled.") + + // Show only the core command-line flags. + fmt.Println("\nCore flags:") + fmt.Println() + fs := flag.NewFlagSet("", flag.ExitOnError) + flag.VisitAll(func(f *flag.Flag) { + if !strings.Contains(f.Name, ".") { + fs.Var(f.Value, f.Name, f.Usage) + } + }) + fs.PrintDefaults() + + fmt.Printf("\nTo see details and flags of a specific analyzer, run '%s help name'.\n", progname) + + return + } + + // Show help on specific analyzer(s). +outer: + for _, arg := range args { + for _, a := range analyzers { + if a.Name == arg { + paras := strings.Split(a.Doc, "\n\n") + title := paras[0] + fmt.Printf("%s: %s\n", a.Name, title) + + // Show only the flags relating to this analysis, + // properly prefixed. + first := true + fs := flag.NewFlagSet(a.Name, flag.ExitOnError) + a.Flags.VisitAll(func(f *flag.Flag) { + if first { + first = false + fmt.Println("\nAnalyzer flags:") + fmt.Println() + } + fs.Var(f.Value, a.Name+"."+f.Name, f.Usage) + }) + fs.PrintDefaults() + + if len(paras) > 1 { + fmt.Printf("\n%s\n", strings.Join(paras[1:], "\n\n")) + } + + continue outer + } + } + log.Fatalf("Analyzer %q not registered", arg) + } +} diff --git a/go/analysis/internal/unitchecker/unitchecker.go b/go/analysis/internal/unitchecker/unitchecker.go index b67c943b4e2..32b50c1be00 100644 --- a/go/analysis/internal/unitchecker/unitchecker.go +++ b/go/analysis/internal/unitchecker/unitchecker.go @@ -6,7 +6,7 @@ // 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 +// $ go vet -vettool=$(which vet) // // It supports the following command-line protocol: // diff --git a/go/analysis/internal/unitchecker/unitchecker_test.go b/go/analysis/internal/unitchecker/unitchecker_test.go index 7466cd8d0df..c8362bfcebd 100644 --- a/go/analysis/internal/unitchecker/unitchecker_test.go +++ b/go/analysis/internal/unitchecker/unitchecker_test.go @@ -6,8 +6,9 @@ package unitchecker_test -// This test depends on go1.12 features such as go vet's support for -// GOVETTOOL, and the (*os/exec.ExitError).ExitCode method. +// This test depends on features such as +// go vet's support for vetx files (1.11) and +// the (*os.ProcessState).ExitCode method (1.12). import ( "flag" @@ -71,9 +72,8 @@ func TestIntegration(t *testing.T) { testdata := analysistest.TestData() - cmd := exec.Command("go", "vet", "b") + cmd := exec.Command("go", "vet", "-vettool="+os.Args[0], "b") cmd.Env = append(os.Environ(), - "GOVETTOOL="+os.Args[0], "UNITCHECKER_CHILD=1", "GOPATH="+testdata, ) diff --git a/go/analysis/multichecker/multichecker.go b/go/analysis/multichecker/multichecker.go index 9038eac1dd9..3e9f699d388 100644 --- a/go/analysis/multichecker/multichecker.go +++ b/go/analysis/multichecker/multichecker.go @@ -13,8 +13,6 @@ import ( "log" "os" "path/filepath" - "sort" - "strings" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/internal/analysisflags" @@ -28,20 +26,10 @@ import ( // so it should be used as guidance only, not as a firm indicator of // program correctness." -const usage = `PROGNAME is a tool for static analysis of Go programs. - -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: PROGNAME [-flag] [package] -` - func Main(analyzers ...*analysis.Analyzer) { progname := filepath.Base(os.Args[0]) log.SetFlags(0) - log.SetPrefix(filepath.Base(os.Args[0]) + ": ") // e.g. "vet: " + log.SetPrefix(progname + ": ") // e.g. "vet: " if err := analysis.Validate(analyzers); err != nil { log.Fatal(err) @@ -53,7 +41,7 @@ func Main(analyzers ...*analysis.Analyzer) { args := flag.Args() if len(args) == 0 { - fmt.Fprintln(os.Stderr, strings.Replace(usage, "PROGNAME", progname, -1)) + analysisflags.PrintUsage(os.Stderr) fmt.Fprintf(os.Stderr, "Run '%[1]s help' for more detail,\n"+ " or '%[1]s help name' for details and flags of a specific analyzer.\n", progname) @@ -61,7 +49,7 @@ func Main(analyzers ...*analysis.Analyzer) { } if args[0] == "help" { - help(progname, analyzers, args[1:]) + analysisflags.Help(progname, analyzers, args[1:]) os.Exit(0) } @@ -69,69 +57,3 @@ func Main(analyzers ...*analysis.Analyzer) { log.Fatal(err) } } - -func help(progname string, analyzers []*analysis.Analyzer, args []string) { - // No args: show summary of all analyzers. - if len(args) == 0 { - fmt.Println(strings.Replace(usage, "PROGNAME", progname, -1)) - fmt.Println("Registered analyzers:") - fmt.Println() - sort.Slice(analyzers, func(i, j int) bool { - return analyzers[i].Name < analyzers[j].Name - }) - for _, a := range analyzers { - title := strings.Split(a.Doc, "\n\n")[0] - fmt.Printf(" %-12s %s\n", a.Name, title) - } - fmt.Println("\nBy default all analyzers are run.") - fmt.Println("To select specific analyzers, use the -NAME.enable flag for each one.") - - // Show only the core command-line flags. - fmt.Println("\nCore flags:") - fmt.Println() - fs := flag.NewFlagSet("", flag.ExitOnError) - flag.VisitAll(func(f *flag.Flag) { - if !strings.Contains(f.Name, ".") { - fs.Var(f.Value, f.Name, f.Usage) - } - }) - fs.PrintDefaults() - - fmt.Printf("\nTo see details and flags of a specific analyzer, run '%s help name'.\n", progname) - - return - } - - // Show help on specific analyzer(s). -outer: - for _, arg := range args { - for _, a := range analyzers { - if a.Name == arg { - paras := strings.Split(a.Doc, "\n\n") - title := paras[0] - fmt.Printf("%s: %s\n", a.Name, title) - - // Show only the flags relating to this analysis, - // properly prefixed. - first := true - fs := flag.NewFlagSet(a.Name, flag.ExitOnError) - a.Flags.VisitAll(func(f *flag.Flag) { - if first { - first = false - fmt.Println("\nAnalyzer flags:") - fmt.Println() - } - fs.Var(f.Value, a.Name+"."+f.Name, f.Usage) - }) - fs.PrintDefaults() - - if len(paras) > 1 { - fmt.Printf("\n%s\n", strings.Join(paras[1:], "\n\n")) - } - - continue outer - } - } - log.Fatalf("Analyzer %q not registered", arg) - } -}