cmd/go/internal/test: add an all sentinel to -vet

The vet flag either accepts a list of vets to run, or a distinguished
value, off, to disable vet during test. By default only 100% reliable
checks are run, thus there is no way to run all vets. This change adds
another distinguished value, all, that runs every vet, by passing no
flags.

During development it was discovered that parsing of the -vet flag value
is problematic, in that it accepts deprecated flags like -all. The root
cause is detailed in #47309, but for now passing distinguished values
(all, off) and anything else returns an error.

Fixes #45963

Change-Id: I39fafb7d717dad51b507d560b3f6e604510a2881
Reviewed-on: https://go-review.googlesource.com/c/go/+/334873
Trust: Than McIntosh <thanm@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>

src/cmd/go/alldocs.go

@@ -1405,7 +1405,8 @@
 // used. That subset is: 'atomic', 'bool', 'buildtags', 'errorsas',
 // 'ifaceassert', 'nilfunc', 'printf', and 'stringintconv'. You can see
 // the documentation for these and other vet tests via "go doc cmd/vet".
-// To disable the running of go vet, use the -vet=off flag.
+// To disable the running of go vet, use the -vet=off flag. To run all
+// checks, use the -vet=all flag.
 //
 // All test output and summary lines are printed to the go command's
 // standard output, even if the test printed them to its own standard

src/cmd/go/internal/test/test.go

@@ -78,7 +78,8 @@ binary. Only a high-confidence subset of the default go vet checks are
 used. That subset is: 'atomic', 'bool', 'buildtags', 'errorsas',
 'ifaceassert', 'nilfunc', 'printf', and 'stringintconv'. You can see
 the documentation for these and other vet tests via "go doc cmd/vet".
-To disable the running of go vet, use the -vet=off flag.
+To disable the running of go vet, use the -vet=off flag. To run all
+checks, use the -vet=all flag.
 
 All test output and summary lines are printed to the go command's
 standard output, even if the test printed them to its own standard

src/cmd/go/internal/test/testflag.go

@@ -5,6 +5,10 @@
 package test
 
 import (
+	"cmd/go/internal/base"
+	"cmd/go/internal/cfg"
+	"cmd/go/internal/cmdflag"
+	"cmd/go/internal/work"
 	"errors"
 	"flag"
 	"fmt"
@@ -13,11 +17,6 @@ import (
 	"strconv"
 	"strings"
 	"time"
-
-	"cmd/go/internal/base"
-	"cmd/go/internal/cfg"
-	"cmd/go/internal/cmdflag"
-	"cmd/go/internal/work"
 )
 
 //go:generate go run ./genflags.go
@@ -134,6 +133,7 @@ type outputdirFlag struct {
 func (f *outputdirFlag) String() string {
 	return f.abs
 }
+
 func (f *outputdirFlag) Set(value string) (err error) {
 	if value == "" {
 		f.abs = ""
@@ -142,6 +142,7 @@ func (f *outputdirFlag) Set(value string) (err error) {
 	}
 	return err
 }
+
 func (f *outputdirFlag) getAbs() string {
 	if f.abs == "" {
 		return base.Cwd()
@@ -150,8 +151,12 @@ func (f *outputdirFlag) getAbs() string {
 }
 
 // vetFlag implements the special parsing logic for the -vet flag:
-// a comma-separated list, with a distinguished value "off" and
-// a boolean tracking whether it was set explicitly.
+// a comma-separated list, with distinguished values "all" and
+// "off", plus a boolean tracking whether it was set explicitly.
+//
+// "all" is encoded as vetFlag{true, false, nil}, since it will
+// pass no flags to the vet binary, and by default, it runs all
+// analyzers.
 type vetFlag struct {
 	explicit bool
 	off      bool
@@ -159,7 +164,10 @@ type vetFlag struct {
 }
 
 func (f *vetFlag) String() string {
-	if f.off {
+	switch {
+	case !f.off && !f.explicit && len(f.flags) == 0:
+		return "all"
+	case f.off:
 		return "off"
 	}
 
@@ -174,32 +182,38 @@ func (f *vetFlag) String() string {
 }
 
 func (f *vetFlag) Set(value string) error {
-	if value == "" {
+	switch {
+	case value == "":
 		*f = vetFlag{flags: defaultVetFlags}
 		return nil
+	case strings.Contains(value, "="):
+		return fmt.Errorf("-vet argument cannot contain equal signs")
+	case strings.Contains(value, " "):
+		return fmt.Errorf("-vet argument is comma-separated list, cannot contain spaces")
 	}
-
-	if value == "off" {
+	*f = vetFlag{explicit: true}
+	var single string
+	for _, arg := range strings.Split(value, ",") {
+		switch arg {
+		case "":
+			return fmt.Errorf("-vet argument contains empty list element")
+		case "all":
+			single = arg
+			*f = vetFlag{explicit: true}
+			continue
+		case "off":
+			single = arg
 			*f = vetFlag{
 				explicit: true,
 				off:      true,
 			}
-		return nil
-	}
-
-	if strings.Contains(value, "=") {
-		return fmt.Errorf("-vet argument cannot contain equal signs")
-	}
-	if strings.Contains(value, " ") {
-		return fmt.Errorf("-vet argument is comma-separated list, cannot contain spaces")
-	}
-	*f = vetFlag{explicit: true}
-	for _, arg := range strings.Split(value, ",") {
-		if arg == "" {
-			return fmt.Errorf("-vet argument contains empty list element")
+			continue
 		}
 		f.flags = append(f.flags, "-"+arg)
 	}
+	if len(f.flags) > 1 && single != "" {
+		return fmt.Errorf("-vet does not accept %q in a list with other analyzers", single)
+	}
 	return nil
 }
 

src/cmd/go/testdata/script/test_vet.txt

@@ -16,6 +16,11 @@ go test -vet=off p1.go
 ! stderr '[\\/]vet.*-shift'
 stdout '\[no test files\]'
 
+# ensure all runs non-default vet
+! go test -vet=all ./vetall/...
+stderr 'using resp before checking for errors'
+
+
 # Test issue #22890
 go test m/vetcycle
 stdout 'm/vetcycle.*\[no test files\]'
@@ -46,11 +51,27 @@ func Test(t *testing.T) {
 -- p1.go --
 package p
 
-import "fmt"
+import (
+	"fmt"
+	"net/http"
+)
 
 func F() {
 	fmt.Printf("%d") // oops
 }
+-- vetall/p.go --
+package p
+
+func F() {
+	resp, err := http.Head("example.com")
+	defer resp.Body.Close()
+	if err != nil {
+		panic(err)
+	}
+	// (defer statement belongs here)
+}
+-- vetall/p_test.go --
+package p
 -- vetcycle/p.go --
 package p