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>

diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index ddf7bb531897d768d9507e3d8e372d0c68d48030..592ea4932e2ef1ba8db3e6c5e378f2ea2eb1ce5d 100644 (file)
--- a/src/cmd/go/alldocs.go
+++ b/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

diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index e8bf0e198ed21b94d5fe6aaa002ef2407aa4ed55..efebc12363fdecbc5e79bec239b9e71338850ddd 100644 (file)
--- a/src/cmd/go/internal/test/test.go
+++ b/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

diff --git a/src/cmd/go/internal/test/testflag.go b/src/cmd/go/internal/test/testflag.go
index 08f1efa2c0d26a0cf398f2b778c0adde90fc97ae..45118cb6380065809122352c6fa7ef09047c8aa7 100644 (file)
--- a/src/cmd/go/internal/test/testflag.go
+++ b/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
-       }
-
-       if value == "off" {
-               *f = vetFlag{
-                       explicit: true,
-                       off:      true,
-               }
-               return nil
-       }
-
-       if strings.Contains(value, "=") {
+       case strings.Contains(value, "="):
                return fmt.Errorf("-vet argument cannot contain equal signs")
-       }
-       if strings.Contains(value, " ") {
+       case strings.Contains(value, " "):
                return fmt.Errorf("-vet argument is comma-separated list, cannot contain spaces")
        }
        *f = vetFlag{explicit: true}
+       var single string
        for _, arg := range strings.Split(value, ",") {
-               if arg == "" {
+               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,
+                       }
+                       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
 }
 

diff --git a/src/cmd/go/testdata/script/test_vet.txt b/src/cmd/go/testdata/script/test_vet.txt
index 5af26b54f9141bb189bbf15b55edef6fa09c0919..239a477a1809898b7d646c74c482191e5a510bad 100644 (file)
--- a/src/cmd/go/testdata/script/test_vet.txt
+++ b/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