diff --git a/go/analysis/cmd/vet-lite/main.go b/go/analysis/cmd/vet-lite/main.go index 38f23efd..b4b0e631 100644 --- a/go/analysis/cmd/vet-lite/main.go +++ b/go/analysis/cmd/vet-lite/main.go @@ -8,6 +8,7 @@ package main import ( "flag" + "fmt" "log" "os" "strings" @@ -41,7 +42,6 @@ import ( ) var analyzers = []*analysis.Analyzer{ - // For now, just the traditional vet suite: asmdecl.Analyzer, assign.Analyzer, atomic.Analyzer, @@ -56,7 +56,6 @@ var analyzers = []*analysis.Analyzer{ nilfunc.Analyzer, pkgfact.Analyzer, printf.Analyzer, - // shadow.Analyzer, // experimental; not enabled by default shift.Analyzer, stdmethods.Analyzer, structtag.Analyzer, @@ -77,47 +76,49 @@ func main() { // Flags for legacy vet compatibility. // - // These flags, plus the shims in analysisflags, enable all + // These flags, plus the shims in analysisflags, enable // 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. + // Legacy vet had the concept of "experimental" checkers. There + // was exactly one, shadow, and it had to be explicitly enabled + // by the -shadow flag, which would of course disable all the + // other tristate flags, requiring the -all flag to reenable them. + // (By itself, -all did not enable all checkers.) + // The -all flag is no longer needed, so it is a no-op. // - // 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. + // The shadow analyzer has been removed from the suite, + // but can be run using these additional commands: + // $ go install golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow + // $ go vet -vettool=$(which shadow) + // Alternatively, one could build a multichecker containing all + // the desired checks (vet's suite + shadow) and run it in a + // single "go vet" command. for _, name := range []string{"source", "v", "all"} { - flag.Var(warnBoolFlag(name), name, "no effect (deprecated)") + _ = flag.Bool(name, false, "no effect (deprecated)") } flag.Usage = func() { - analysisflags.Help("vet", analyzers, nil) + fmt.Fprintln(os.Stderr, `Usage of vet: + vet unit.cfg # execute analysis specified by config file + vet help # general help + vet help name # help on specific analyzer and its flags`) + flag.PrintDefaults() os.Exit(1) } analyzers = analysisflags.Parse(analyzers, true) args := flag.Args() + if len(args) == 0 { + flag.Usage() + } + if args[0] == "help" { + analysisflags.Help("vet", analyzers, args[1:]) + os.Exit(0) + } 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) } - -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/cmd/vet/vet.go b/go/analysis/cmd/vet/vet.go index 3830f013..db8180ba 100644 --- a/go/analysis/cmd/vet/vet.go +++ b/go/analysis/cmd/vet/vet.go @@ -7,10 +7,10 @@ // using the golang.org/x/tools/go/packages API to load packages in any // build system. // -// Each analysis flag name is preceded by the analysis name: --analysis.flag. -// In addition, the --analysis.enabled flag controls whether the -// diagnostics of that analysis are displayed. (A disabled analysis may yet -// be run if it is required by some other analysis that is enabled.) +// Each analyzer flag name is preceded by the analyzer name: -NAME.flag. +// In addition, the -NAME flag itself controls whether the +// diagnostics of that analyzer are displayed. (A disabled analyzer may yet +// be run if it is required by some other analyzer that is enabled.) package main import ( @@ -58,9 +58,7 @@ func main() { loopclosure.Analyzer, lostcancel.Analyzer, nilfunc.Analyzer, - pkgfact.Analyzer, printf.Analyzer, - // shadow.Analyzer, // experimental; not enabled by default shift.Analyzer, stdmethods.Analyzer, structtag.Analyzer, @@ -72,12 +70,9 @@ func main() { // for debugging: findcall.Analyzer, + pkgfact.Analyzer, - // use SSA: + // uses SSA: nilness.Analyzer, - - // Work in progress: - // httpheader.Analyzer, - // deadcode.Analyzer, ) } diff --git a/go/analysis/internal/analysisflags/flags.go b/go/analysis/internal/analysisflags/flags.go index 3ab46f39..37e1bf6c 100644 --- a/go/analysis/internal/analysisflags/flags.go +++ b/go/analysis/internal/analysisflags/flags.go @@ -20,7 +20,7 @@ import ( ) // Parse creates a flag for each of the analyzer's flags, -// including (in multi mode) an --analysis.enable flag, +// including (in multi mode) a flag named after the analyzer, // parses the flags, then filters and returns the list of // analyzers enabled by flags. func Parse(analyzers []*analysis.Analyzer, multi bool) []*analysis.Analyzer { @@ -36,16 +36,15 @@ func Parse(analyzers []*analysis.Analyzer, multi bool) []*analysis.Analyzer { for _, a := range analyzers { var prefix string - // Add -analysis.enable flag. + // Add -NAME flag to enable it. if multi { prefix = a.Name + "." enable := new(triState) - enableName := prefix + "enable" enableUsage := "enable " + a.Name + " analysis" - flag.Var(enable, enableName, enableUsage) + flag.Var(enable, a.Name, enableUsage) enabled[a] = enable - analysisFlags = append(analysisFlags, analysisFlag{enableName, true, enableUsage}) + analysisFlags = append(analysisFlags, analysisFlag{a.Name, true, enableUsage}) } a.Flags.VisitAll(func(f *flag.Flag) { @@ -73,8 +72,7 @@ func Parse(analyzers []*analysis.Analyzer, multi bool) []*analysis.Analyzer { 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.Var(newFlag.Value, old, "deprecated alias for -"+new) } } @@ -90,8 +88,8 @@ func Parse(analyzers []*analysis.Analyzer, multi bool) []*analysis.Analyzer { 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 any -NAME flag is true, run only those analyzers. Otherwise, + // if any -NAME flag is false, run all but those analyzers. if multi { var hasTrue, hasFalse bool for _, ts := range enabled { @@ -204,7 +202,7 @@ 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" + // "invalid boolean value %q for -NAME: %s" return fmt.Errorf("want true or false") } if b { @@ -234,59 +232,11 @@ func (ts triState) IsBoolFlag() bool { // 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. +// new names. The old names will continue to work. 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", + "compositewhitelist": "composites.whitelist", + "printfuncs": "printf.funcs", + "shadowstrict": "shadow.strict", "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/flags_test.go b/go/analysis/internal/analysisflags/flags_test.go index d2310acf..1f055dde 100644 --- a/go/analysis/internal/analysisflags/flags_test.go +++ b/go/analysis/internal/analysisflags/flags_test.go @@ -45,11 +45,11 @@ func TestExec(t *testing.T) { 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]"}, + {"-a1=0", "[a2 a3]"}, + {"-a1=1", "[a1]"}, + {"-a1", "[a1]"}, + {"-a1=1 -a3=1", "[a1 a3]"}, + {"-a1=1 -a3=0", "[a1]"}, } { cmd := exec.Command(progname, "-test.run=TestExec") cmd.Env = append(os.Environ(), "ANALYSISFLAGS_CHILD=1", "FLAGS="+test.flags) diff --git a/go/analysis/internal/analysisflags/help.go b/go/analysis/internal/analysisflags/help.go index 843c3d6f..dc7ba066 100644 --- a/go/analysis/internal/analysisflags/help.go +++ b/go/analysis/internal/analysisflags/help.go @@ -46,8 +46,8 @@ func Help(progname string, analyzers []*analysis.Analyzer, args []string) { 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.") + fmt.Println("To select specific analyzers, use the -NAME flag for each one,") + fmt.Println(" or -NAME=false to run all analyzers not explicitly disabled.") // Show only the core command-line flags. fmt.Println("\nCore flags:")