go/analysis/cmd/vet-lite: remove deprecation warnings

Per discussion with Russ,
the -all/-source/-v flags now silently do nothing, and
the -printffuncs (et al) shims now silently delegate to -printf.funcs, and
the -NAME.enable (et al) flags are now called just -NAME.

Various minor tweaks to command-line help messages.

Change-Id: If6587937f58446e605eca4d3a5be0aaf6287065d
Reviewed-on: https://go-review.googlesource.com/c/148879
Reviewed-by: Russ Cox <rsc@golang.org>

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"
+	// Legacy vet had the concept of "experimental" checkers. There
-	// checkers. In vet there is exactly one such checker, shadow,
+	// was exactly one, shadow, and it had to be explicitly enabled
-	// and it must be enabled explicitly with the -shadow flag, but
+	// by the -shadow flag, which would of course disable all the
-	// of course setting it disables all the other tristate flags,
+	// other tristate flags, requiring the -all flag to reenable them.
-	// 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
+	// The shadow analyzer has been removed from the suite,
-	// simply skip shadow for now; the few users that want it can
+	// but can be run using these additional commands:
-	// run "go vet -vettool=..." using a vet tool that includes
+	//   $ go install golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
-	// shadow, either as an additional step, with a shadow
+	//   $ go vet -vettool=$(which shadow)
-	// "singlechecker", or in place of the regular vet step on a
+	// Alternatively, one could build a multichecker containing all
-	// multichecker with a hand-picked suite of checkers.
+	// the desired checks (vet's suite + shadow) and run it in a
-	// Or, we could improve the shadow checker to the point where it
+	// single "go vet" command.
-	// need not be experimental.
 	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" }

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.
+// Each analyzer flag name is preceded by the analyzer name: -NAME.flag.
-// In addition, the --analysis.enabled flag controls whether the
+// In addition, the -NAME flag itself controls whether the
-// diagnostics of that analysis are displayed. (A disabled analysis may yet
+// diagnostics of that analyzer are displayed. (A disabled analyzer may yet
-// be run if it is required by some other analysis that is enabled.)
+// 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,
 	)
 }

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(newFlag.Value, old, "deprecated alias for -"+new)
-			flag.Var(oldFlag, 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 -NAME 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 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
+// new names. The old names will continue to work.
-// 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()
-}

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=0", "[a2 a3]"},
-		{"-a1.enable=1", "[a1]"},
+		{"-a1=1", "[a1]"},
-		{"-a1.enable", "[a1]"},
+		{"-a1", "[a1]"},
-		{"-a1.enable=1 -a3.enable=1", "[a1 a3]"},
+		{"-a1=1 -a3=1", "[a1 a3]"},
-		{"-a1.enable=1 -a3.enable=0", "[a1]"},
+		{"-a1=1 -a3=0", "[a1]"},
 	} {
 		cmd := exec.Command(progname, "-test.run=TestExec")
 		cmd.Env = append(os.Environ(), "ANALYSISFLAGS_CHILD=1", "FLAGS="+test.flags)

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("To select specific analyzers, use the -NAME flag for each one,")
-		fmt.Println(" or -NAME.enable=false to run all analyzers not explicitly disabled.")
+		fmt.Println(" or -NAME=false to run all analyzers not explicitly disabled.")
 
 		// Show only the core command-line flags.
 		fmt.Println("\nCore flags:")