From: Bryan C. Mills Date: Wed, 11 Dec 2019 14:12:25 +0000 (-0500) Subject: cmd/go/internal/{test,vet}: use a standard flag.FlagSet to parse flags X-Git-Tag: go1.15beta1~1064 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=583419e5d2a893e369095355035f4ebb18bb85f8;p=gostls13.git cmd/go/internal/{test,vet}: use a standard flag.FlagSet to parse flags This removes much of the complexity of the implementation and use of the cmd/go/internal/cmdflag package, and makes the behavior of GOFLAGS in 'go test' and 'go vet' more consistent with other subcommands. Some of the complexity reduction has been offset by code comments and bug fixes, particularly for the handling of GOPATH arguments and flag terminators ('--'). Fixes #32471 Fixes #18682 Change-Id: I1f6e46a7c679062e1e409e44a2b9f03b9172883b Reviewed-on: https://go-review.googlesource.com/c/go/+/211358 Reviewed-by: Jay Conrod --- diff --git a/doc/go1.15.html b/doc/go1.15.html index 5b0459e67a..1eb159c318 100644 --- a/doc/go1.15.html +++ b/doc/go1.15.html @@ -47,6 +47,17 @@ TODO TODO

+

Flag parsing

+ +

+ Various flag parsing issues in go test and + go vet have been fixed. Notably, flags specified + in GOFLAGS are handled more consistently, and + the -outputdir flag now interprets relative paths relative to the + working directory of the go command (rather than the working + directory of each individual test). +

+

Runtime

diff --git a/src/cmd/go/internal/base/goflags.go b/src/cmd/go/internal/base/goflags.go index 187c2a1472..34766134b0 100644 --- a/src/cmd/go/internal/base/goflags.go +++ b/src/cmd/go/internal/base/goflags.go @@ -102,7 +102,7 @@ type boolFlag interface { } // SetFromGOFLAGS sets the flags in the given flag set using settings in $GOFLAGS. -func SetFromGOFLAGS(flags flag.FlagSet) { +func SetFromGOFLAGS(flags *flag.FlagSet) { InitGOFLAGS() // This loop is similar to flag.Parse except that it ignores @@ -125,14 +125,18 @@ func SetFromGOFLAGS(flags flag.FlagSet) { if f == nil { continue } + + // Use flags.Set consistently (instead of f.Value.Set) so that a subsequent + // call to flags.Visit will correctly visit the flags that have been set. + if fb, ok := f.Value.(boolFlag); ok && fb.IsBoolFlag() { if hasValue { - if err := fb.Set(value); err != nil { + if err := flags.Set(f.Name, value); err != nil { fmt.Fprintf(flags.Output(), "go: invalid boolean value %q for flag %s (from %s): %v\n", value, name, where, err) flags.Usage() } } else { - if err := fb.Set("true"); err != nil { + if err := flags.Set(f.Name, "true"); err != nil { fmt.Fprintf(flags.Output(), "go: invalid boolean flag %s (from %s): %v\n", name, where, err) flags.Usage() } @@ -142,7 +146,7 @@ func SetFromGOFLAGS(flags flag.FlagSet) { fmt.Fprintf(flags.Output(), "go: flag needs an argument: %s (from %s)\n", name, where) flags.Usage() } - if err := f.Value.Set(value); err != nil { + if err := flags.Set(f.Name, value); err != nil { fmt.Fprintf(flags.Output(), "go: invalid value %q for flag %s (from %s): %v\n", value, name, where, err) flags.Usage() } diff --git a/src/cmd/go/internal/cmdflag/flag.go b/src/cmd/go/internal/cmdflag/flag.go index 3f934328fe..8abb7e559f 100644 --- a/src/cmd/go/internal/cmdflag/flag.go +++ b/src/cmd/go/internal/cmdflag/flag.go @@ -6,13 +6,10 @@ package cmdflag import ( + "errors" "flag" "fmt" - "os" - "strconv" "strings" - - "cmd/go/internal/base" ) // The flag handling part of go commands such as test is large and distracting. @@ -20,141 +17,113 @@ import ( // our command line are for us, and some are for the binary we're running, // and some are for both. -// Defn defines a flag we know about. -type Defn struct { - Name string // Name on command line. - BoolVar *bool // If it's a boolean flag, this points to it. - Value flag.Value // The flag.Value represented. - PassToTest bool // Pass to the test binary? Used only by go test. - Present bool // Flag has been seen. -} +// ErrFlagTerminator indicates the distinguished token "--", which causes the +// flag package to treat all subsequent arguments as non-flags. +var ErrFlagTerminator = errors.New("flag terminator") -// IsBool reports whether v is a bool flag. -func IsBool(v flag.Value) bool { - vv, ok := v.(interface { - IsBoolFlag() bool - }) - if ok { - return vv.IsBoolFlag() - } - return false +// A FlagNotDefinedError indicates a flag-like argument that does not correspond +// to any registered flag in a FlagSet. +type FlagNotDefinedError struct { + RawArg string // the original argument, like --foo or -foo=value + Name string + HasValue bool // is this the -foo=value or --foo=value form? + Value string // only provided if HasValue is true } -// SetBool sets the addressed boolean to the value. -func SetBool(cmd string, flag *bool, value string) { - x, err := strconv.ParseBool(value) - if err != nil { - SyntaxError(cmd, "illegal bool flag value "+value) - } - *flag = x +func (e FlagNotDefinedError) Error() string { + return fmt.Sprintf("flag provided but not defined: -%s", e.Name) } -// SetInt sets the addressed integer to the value. -func SetInt(cmd string, flag *int, value string) { - x, err := strconv.Atoi(value) - if err != nil { - SyntaxError(cmd, "illegal int flag value "+value) - } - *flag = x +// A NonFlagError indicates an argument that is not a syntactically-valid flag. +type NonFlagError struct { + RawArg string } -// SyntaxError reports an argument syntax error and exits the program. -func SyntaxError(cmd, msg string) { - fmt.Fprintf(os.Stderr, "go %s: %s\n", cmd, msg) - if cmd == "test" { - fmt.Fprintf(os.Stderr, `run "go help %s" or "go help testflag" for more information`+"\n", cmd) - } else { - fmt.Fprintf(os.Stderr, `run "go help %s" for more information`+"\n", cmd) - } - base.SetExitStatus(2) - base.Exit() +func (e NonFlagError) Error() string { + return fmt.Sprintf("not a flag: %q", e.RawArg) } -// AddKnownFlags registers the flags in defns with base.AddKnownFlag. -func AddKnownFlags(cmd string, defns []*Defn) { - for _, f := range defns { - base.AddKnownFlag(f.Name) - base.AddKnownFlag(cmd + "." + f.Name) - } -} +// ParseOne sees if args[0] is present in the given flag set and if so, +// sets its value and returns the flag along with the remaining (unused) arguments. +// +// ParseOne always returns either a non-nil Flag or a non-nil error, +// and always consumes at least one argument (even on error). +// +// Unlike (*flag.FlagSet).Parse, ParseOne does not log its own errors. +func ParseOne(fs *flag.FlagSet, args []string) (f *flag.Flag, remainingArgs []string, err error) { + // This function is loosely derived from (*flag.FlagSet).parseOne. -// Parse sees if argument i is present in the definitions and if so, -// returns its definition, value, and whether it consumed an extra word. -// If the flag begins (cmd.Name()+".") it is ignored for the purpose of this function. -func Parse(cmd string, usage func(), defns []*Defn, args []string, i int) (f *Defn, value string, extra bool) { - arg := args[i] - if strings.HasPrefix(arg, "--") { // reduce two minuses to one - arg = arg[1:] + raw, args := args[0], args[1:] + arg := raw + if strings.HasPrefix(arg, "--") { + if arg == "--" { + return nil, args, ErrFlagTerminator + } + arg = arg[1:] // reduce two minuses to one } + switch arg { case "-?", "-h", "-help": - usage() + return nil, args, flag.ErrHelp } - if arg == "" || arg[0] != '-' { - return + if len(arg) < 2 || arg[0] != '-' || arg[1] == '-' || arg[1] == '=' { + return nil, args, NonFlagError{RawArg: raw} } + name := arg[1:] - // If there's already a prefix such as "test.", drop it for now. - name = strings.TrimPrefix(name, cmd+".") - equals := strings.Index(name, "=") - if equals >= 0 { - value = name[equals+1:] - name = name[:equals] + hasValue := false + value := "" + if i := strings.Index(name, "="); i >= 0 { + value = name[i+1:] + hasValue = true + name = name[0:i] } - for _, f = range defns { - if name == f.Name { - // Booleans are special because they have modes -x, -x=true, -x=false. - if f.BoolVar != nil || IsBool(f.Value) { - if equals < 0 { // Otherwise, it's been set and will be verified in SetBool. - value = "true" - } else { - // verify it parses - SetBool(cmd, new(bool), value) - } - } else { // Non-booleans must have a value. - extra = equals < 0 - if extra { - if i+1 >= len(args) { - SyntaxError(cmd, "missing argument for flag "+f.Name) - } - value = args[i+1] - } - } - if f.Present { - SyntaxError(cmd, f.Name+" flag may be set only once") - } - f.Present = true - return + + f = fs.Lookup(name) + if f == nil { + return nil, args, FlagNotDefinedError{ + RawArg: raw, + Name: name, + HasValue: hasValue, + Value: value, } } - f = nil - return -} -// FindGOFLAGS extracts and returns the flags matching defns from GOFLAGS. -// Ideally the caller would mention that the flags were from GOFLAGS -// when reporting errors, but that's too hard for now. -func FindGOFLAGS(defns []*Defn) []string { - var flags []string - for _, flag := range base.GOFLAGS() { - // Flags returned by base.GOFLAGS are well-formed, one of: - // -x - // --x - // -x=value - // --x=value - if strings.HasPrefix(flag, "--") { - flag = flag[1:] + // Use fs.Set instead of f.Value.Set below so that any subsequent call to + // fs.Visit will correctly visit the flags that have been set. + + failf := func(format string, a ...interface{}) (*flag.Flag, []string, error) { + return f, args, fmt.Errorf(format, a...) + } + + if fv, ok := f.Value.(boolFlag); ok && fv.IsBoolFlag() { // special case: doesn't need an arg + if hasValue { + if err := fs.Set(name, value); err != nil { + return failf("invalid boolean value %q for -%s: %v", value, name, err) + } + } else { + if err := fs.Set(name, "true"); err != nil { + return failf("invalid boolean flag %s: %v", name, err) + } } - name := flag[1:] - if i := strings.Index(name, "="); i >= 0 { - name = name[:i] + } else { + // It must have a value, which might be the next argument. + if !hasValue && len(args) > 0 { + // value is the next arg + hasValue = true + value, args = args[0], args[1:] } - for _, f := range defns { - if name == f.Name { - flags = append(flags, flag) - break - } + if !hasValue { + return failf("flag needs an argument: -%s", name) + } + if err := fs.Set(name, value); err != nil { + return failf("invalid value %q for flag -%s: %v", value, name, err) } } - return flags + + return f, args, nil +} + +type boolFlag interface { + IsBoolFlag() bool } diff --git a/src/cmd/go/internal/test/flagdefs.go b/src/cmd/go/internal/test/flagdefs.go new file mode 100644 index 0000000000..8a0a07683b --- /dev/null +++ b/src/cmd/go/internal/test/flagdefs.go @@ -0,0 +1,34 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Code generated by genflags.go — DO NOT EDIT. + +package test + +// passFlagToTest contains the flags that should be forwarded to +// the test binary with the prefix "test.". +var passFlagToTest = map[string]bool{ + "bench": true, + "benchmem": true, + "benchtime": true, + "blockprofile": true, + "blockprofilerate": true, + "count": true, + "coverprofile": true, + "cpu": true, + "cpuprofile": true, + "failfast": true, + "list": true, + "memprofile": true, + "memprofilerate": true, + "mutexprofile": true, + "mutexprofilefraction": true, + "outputdir": true, + "parallel": true, + "run": true, + "short": true, + "timeout": true, + "trace": true, + "v": true, +} diff --git a/src/cmd/go/internal/test/flagdefs_test.go b/src/cmd/go/internal/test/flagdefs_test.go new file mode 100644 index 0000000000..7562415298 --- /dev/null +++ b/src/cmd/go/internal/test/flagdefs_test.go @@ -0,0 +1,34 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package test + +import ( + "flag" + "strings" + "testing" +) + +func TestPassFlagToTestIncludesAllTestFlags(t *testing.T) { + flag.VisitAll(func(f *flag.Flag) { + if !strings.HasPrefix(f.Name, "test.") { + return + } + name := strings.TrimPrefix(f.Name, "test.") + if name != "testlogfile" && !passFlagToTest[name] { + t.Errorf("passFlagToTest missing entry for %q (flag test.%s)", name, name) + t.Logf("(Run 'go generate cmd/go/internal/test' if it should be added.)") + } + }) + + for name := range passFlagToTest { + if flag.Lookup("test."+name) == nil { + t.Errorf("passFlagToTest contains %q, but flag -test.%s does not exist in test binary", name, name) + } + + if CmdTest.Flag.Lookup(name) == nil { + t.Errorf("passFlagToTest contains %q, but flag -%s does not exist in 'go test' subcommand", name, name) + } + } +} diff --git a/src/cmd/go/internal/test/genflags.go b/src/cmd/go/internal/test/genflags.go new file mode 100644 index 0000000000..512fa1671e --- /dev/null +++ b/src/cmd/go/internal/test/genflags.go @@ -0,0 +1,90 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build ignore + +package main + +import ( + "bytes" + "flag" + "log" + "os" + "os/exec" + "strings" + "testing" + "text/template" +) + +func main() { + if err := regenerate(); err != nil { + log.Fatal(err) + } +} + +func regenerate() error { + t := template.Must(template.New("fileTemplate").Parse(fileTemplate)) + buf := bytes.NewBuffer(nil) + if err := t.Execute(buf, testFlags()); err != nil { + return err + } + + f, err := os.Create("flagdefs.go") + if err != nil { + return err + } + + cmd := exec.Command("gofmt") + cmd.Stdin = buf + cmd.Stdout = f + cmd.Stderr = os.Stderr + cmdErr := cmd.Run() + + if err := f.Close(); err != nil { + return err + } + if cmdErr != nil { + os.Remove(f.Name()) + return cmdErr + } + + return nil +} + +func testFlags() []string { + testing.Init() + + var names []string + flag.VisitAll(func(f *flag.Flag) { + if !strings.HasPrefix(f.Name, "test.") { + return + } + name := strings.TrimPrefix(f.Name, "test.") + + if name == "testlogfile" { + // test.testlogfile is “for use only by cmd/go” + } else { + names = append(names, name) + } + }) + + return names +} + +const fileTemplate = `// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Code generated by genflags.go — DO NOT EDIT. + +package test + +// passFlagToTest contains the flags that should be forwarded to +// the test binary with the prefix "test.". +var passFlagToTest = map[string]bool { +{{- range .}} + "{{.}}": true, +{{- end }} +} +` diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index fb011d4c03..600f76df4c 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -466,37 +466,76 @@ See the documentation of the testing package for more information. } var ( - testC bool // -c flag - testCover bool // -cover flag - testCoverMode string // -covermode flag - testCoverPaths []string // -coverpkg flag - testCoverPkgs []*load.Package // -coverpkg flag - testCoverProfile string // -coverprofile flag - testOutputDir string // -outputdir flag - testO string // -o flag - testProfile string // profiling flag that limits test to one package - testNeedBinary bool // profile needs to keep binary around - testJSON bool // -json flag - testV bool // -v flag - testTimeout string // -timeout flag - testArgs []string - testBench bool - testList bool - testShowPass bool // show passing output - testVetList string // -vet flag - pkgArgs []string - pkgs []*load.Package - - testActualTimeout = 10 * time.Minute // actual timeout which is passed to tests - testKillTimeout = testActualTimeout + 1*time.Minute // backup alarm - testCacheExpire time.Time // ignore cached test results before this time + testBench string // -bench flag + testC bool // -c flag + testCover bool // -cover flag + testCoverMode string // -covermode flag + testCoverPaths []string // -coverpkg flag + testCoverPkgs []*load.Package // -coverpkg flag + testCoverProfile string // -coverprofile flag + testJSON bool // -json flag + testList string // -list flag + testO string // -o flag + testOutputDir = base.Cwd // -outputdir flag + testTimeout time.Duration // -timeout flag + testV bool // -v flag + testVet = vetFlag{flags: defaultVetFlags} // -vet flag ) -// testVetExplicit records whether testVetFlags were set by an explicit -vet. -var testVetExplicit = false +var ( + testArgs []string + pkgArgs []string + pkgs []*load.Package + + testKillTimeout = 100 * 365 * 24 * time.Hour // backup alarm; defaults to about a century if no timeout is set + testCacheExpire time.Time // ignore cached test results before this time + + testBlockProfile, testCPUProfile, testMemProfile, testMutexProfile, testTrace string // profiling flag that limits test to one package +) + +// testProfile returns the name of an arbitrary single-package profiling flag +// that is set, if any. +func testProfile() string { + switch { + case testBlockProfile != "": + return "-blockprofile" + case testCPUProfile != "": + return "-cpuprofile" + case testMemProfile != "": + return "-memprofile" + case testMutexProfile != "": + return "-mutexprofile" + case testTrace != "": + return "-trace" + default: + return "" + } +} + +// testNeedBinary reports whether the test needs to keep the binary around. +func testNeedBinary() bool { + switch { + case testBlockProfile != "": + return true + case testCPUProfile != "": + return true + case testMemProfile != "": + return true + case testMutexProfile != "": + return true + case testO != "": + return true + default: + return false + } +} + +// testShowPass reports whether the output for a passing test should be shown. +func testShowPass() bool { + return testV || (testList != "") +} -// testVetFlags is the list of flags to pass to vet when invoked automatically during go test. -var testVetFlags = []string{ +var defaultVetFlags = []string{ // TODO(rsc): Decide which tests are enabled by default. // See golang.org/issue/18085. // "-asmdecl", @@ -522,22 +561,16 @@ var testVetFlags = []string{ // "-unusedresult", } -func testCmdUsage() { - fmt.Fprintf(os.Stderr, "usage: %s\n", CmdTest.UsageLine) - fmt.Fprintf(os.Stderr, "Run 'go help %s' and 'go help %s' for details.\n", CmdTest.LongName(), HelpTestflag.LongName()) - os.Exit(2) -} - func runTest(cmd *base.Command, args []string) { modload.LoadTests = true - pkgArgs, testArgs = testFlags(testCmdUsage, args) + pkgArgs, testArgs = testFlags(args) work.FindExecCmd() // initialize cached result work.BuildInit() - work.VetFlags = testVetFlags - work.VetExplicit = testVetExplicit + work.VetFlags = testVet.flags + work.VetExplicit = testVet.explicit pkgs = load.PackagesForBuild(pkgArgs) if len(pkgs) == 0 { @@ -550,38 +583,20 @@ func runTest(cmd *base.Command, args []string) { if testO != "" && len(pkgs) != 1 { base.Fatalf("cannot use -o flag with multiple packages") } - if testProfile != "" && len(pkgs) != 1 { - base.Fatalf("cannot use %s flag with multiple packages", testProfile) + if testProfile() != "" && len(pkgs) != 1 { + base.Fatalf("cannot use %s flag with multiple packages", testProfile()) } initCoverProfile() defer closeCoverProfile() - // If a test timeout was given and is parseable, set our kill timeout + // If a test timeout is finite, set our kill timeout // to that timeout plus one minute. This is a backup alarm in case // the test wedges with a goroutine spinning and its background // timer does not get a chance to fire. - if dt, err := time.ParseDuration(testTimeout); err == nil && dt > 0 { - testActualTimeout = dt - testKillTimeout = testActualTimeout + 1*time.Minute - } else if err == nil && dt == 0 { - // An explicit zero disables the test timeout. - // No timeout is passed to tests. - // Let it have one century (almost) before we kill it. - testActualTimeout = -1 - testKillTimeout = 100 * 365 * 24 * time.Hour + if testTimeout > 0 { + testKillTimeout = testTimeout + 1*time.Minute } - // Pass timeout to tests if it exists. - // Prepend rather than appending so that it appears before positional arguments. - if testActualTimeout > 0 { - testArgs = append([]string{"-test.timeout=" + testActualTimeout.String()}, testArgs...) - } - - // show passing test output (after buffering) with -v flag. - // must buffer because tests are running in parallel, and - // otherwise the output will get mixed. - testShowPass = testV || testList - // For 'go test -i -o x.test', we want to build x.test. Imply -c to make the logic easier. if cfg.BuildI && testO != "" { testC = true @@ -755,7 +770,7 @@ func runTest(cmd *base.Command, args []string) { } // Force benchmarks to run in serial. - if !testC && testBench { + if !testC && (testBench != "") { // The first run must wait for all builds. // Later runs must wait for the previous run's print. for i, run := range runs { @@ -839,7 +854,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin } pmain.Dir = testDir - pmain.Internal.OmitDebug = !testC && !testNeedBinary + pmain.Internal.OmitDebug = !testC && !testNeedBinary() if !cfg.BuildN { // writeTestmain writes _testmain.go, @@ -887,7 +902,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin } buildAction = a var installAction, cleanAction *work.Action - if testC || testNeedBinary { + if testC || testNeedBinary() { // -c or profiling flag: create action to copy binary to ./test.out. target := filepath.Join(base.Cwd, testBinary+cfg.ExeSuffix) if testO != "" { @@ -964,7 +979,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin } func addTestVet(b *work.Builder, p *load.Package, runAction, installAction *work.Action) { - if testVetList == "off" { + if testVet.off { return } @@ -1067,7 +1082,7 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error { } var buf bytes.Buffer - if len(pkgArgs) == 0 || testBench { + if len(pkgArgs) == 0 || (testBench != "") { // Stream test output (no buffering) when no package has // been given on the command line (implicit current directory) // or when benchmarking. @@ -1087,7 +1102,7 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error { // possible even when multiple tests are being run: the JSON output // events are attributed to specific package tests, so interlacing them // is OK. - if testShowPass && (len(pkgs) == 1 || cfg.BuildP == 1) || testJSON { + if testShowPass() && (len(pkgs) == 1 || cfg.BuildP == 1) || testJSON { // Write both to stdout and buf, for possible saving // to cache, and for looking for the "no tests to run" message. stdout = io.MultiWriter(stdout, &buf) @@ -1209,7 +1224,7 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error { if err == nil { norun := "" - if !testShowPass && !testJSON { + if !testShowPass() && !testJSON { buf.Reset() } if bytes.HasPrefix(out, noTestsToRun[1:]) || bytes.Contains(out, noTestsToRun) { diff --git a/src/cmd/go/internal/test/testflag.go b/src/cmd/go/internal/test/testflag.go index e214b1532b..9a3042bfe7 100644 --- a/src/cmd/go/internal/test/testflag.go +++ b/src/cmd/go/internal/test/testflag.go @@ -5,81 +5,197 @@ package test import ( + "errors" "flag" + "fmt" "os" + "path/filepath" "strings" + "time" "cmd/go/internal/base" "cmd/go/internal/cfg" "cmd/go/internal/cmdflag" - "cmd/go/internal/str" "cmd/go/internal/work" ) -const cmd = "test" +//go:generate go run ./genflags.go // The flag handling part of go test is large and distracting. -// We can't use the flag package because some of the flags from -// our command line are for us, and some are for 6.out, and +// We can't use (*flag.FlagSet).Parse because some of the flags from +// our command line are for us, and some are for the test binary, and // some are for both. -// testFlagDefn is the set of flags we process. -var testFlagDefn = []*cmdflag.Defn{ - // local. - {Name: "c", BoolVar: &testC}, - {Name: "i", BoolVar: &cfg.BuildI}, - {Name: "o"}, - {Name: "cover", BoolVar: &testCover}, - {Name: "covermode"}, - {Name: "coverpkg"}, - {Name: "exec"}, - {Name: "json", BoolVar: &testJSON}, - {Name: "vet"}, - - // Passed to 6.out, adding a "test." prefix to the name if necessary: -v becomes -test.v. - {Name: "bench", PassToTest: true}, - {Name: "benchmem", BoolVar: new(bool), PassToTest: true}, - {Name: "benchtime", PassToTest: true}, - {Name: "blockprofile", PassToTest: true}, - {Name: "blockprofilerate", PassToTest: true}, - {Name: "count", PassToTest: true}, - {Name: "coverprofile", PassToTest: true}, - {Name: "cpu", PassToTest: true}, - {Name: "cpuprofile", PassToTest: true}, - {Name: "failfast", BoolVar: new(bool), PassToTest: true}, - {Name: "list", PassToTest: true}, - {Name: "memprofile", PassToTest: true}, - {Name: "memprofilerate", PassToTest: true}, - {Name: "mutexprofile", PassToTest: true}, - {Name: "mutexprofilefraction", PassToTest: true}, - {Name: "outputdir", PassToTest: true}, - {Name: "parallel", PassToTest: true}, - {Name: "run", PassToTest: true}, - {Name: "short", BoolVar: new(bool), PassToTest: true}, - {Name: "timeout", PassToTest: true}, - {Name: "trace", PassToTest: true}, - {Name: "v", BoolVar: &testV, PassToTest: true}, +func init() { + work.AddBuildFlags(CmdTest, work.OmitVFlag) + + cf := CmdTest.Flag + cf.BoolVar(&testC, "c", false, "") + cf.BoolVar(&cfg.BuildI, "i", false, "") + cf.StringVar(&testO, "o", "", "") + + cf.BoolVar(&testCover, "cover", false, "") + cf.Var(coverFlag{(*coverModeFlag)(&testCoverMode)}, "covermode", "") + cf.Var(coverFlag{commaListFlag{&testCoverPaths}}, "coverpkg", "") + + cf.Var((*base.StringsFlag)(&work.ExecCmd), "exec", "") + cf.BoolVar(&testJSON, "json", false, "") + cf.Var(&testVet, "vet", "") + + // Register flags to be forwarded to the test binary. We retain variables for + // some of them so that cmd/go knows what to do with the test output, or knows + // to build the test in a way that supports the use of the flag. + + cf.StringVar(&testBench, "bench", "", "") + cf.Bool("benchmem", false, "") + cf.String("benchtime", "", "") + cf.StringVar(&testBlockProfile, "blockprofile", "", "") + cf.String("blockprofilerate", "", "") + cf.Int("count", 0, "") + cf.Var(coverFlag{stringFlag{&testCoverProfile}}, "coverprofile", "") + cf.String("cpu", "", "") + cf.StringVar(&testCPUProfile, "cpuprofile", "", "") + cf.Bool("failfast", false, "") + cf.StringVar(&testList, "list", "", "") + cf.StringVar(&testMemProfile, "memprofile", "", "") + cf.String("memprofilerate", "", "") + cf.StringVar(&testMutexProfile, "mutexprofile", "", "") + cf.String("mutexprofilefraction", "", "") + cf.Var(outputdirFlag{&testOutputDir}, "outputdir", "") + cf.Int("parallel", 0, "") + cf.String("run", "", "") + cf.Bool("short", false, "") + cf.DurationVar(&testTimeout, "timeout", 10*time.Minute, "") + cf.StringVar(&testTrace, "trace", "", "") + cf.BoolVar(&testV, "v", false, "") + + for name, _ := range passFlagToTest { + cf.Var(cf.Lookup(name).Value, "test."+name, "") + } } -// add build flags to testFlagDefn -func init() { - cmdflag.AddKnownFlags("test", testFlagDefn) - var cmd base.Command - work.AddBuildFlags(&cmd, work.DefaultBuildFlags) - cmd.Flag.VisitAll(func(f *flag.Flag) { - if f.Name == "v" { - // test overrides the build -v flag - return +// A coverFlag is a flag.Value that also implies -cover. +type coverFlag struct{ v flag.Value } + +func (f coverFlag) String() string { return f.v.String() } + +func (f coverFlag) Set(value string) error { + if err := f.v.Set(value); err != nil { + return err + } + testCover = true + return nil +} + +type coverModeFlag string + +func (f *coverModeFlag) String() string { return string(*f) } +func (f *coverModeFlag) Set(value string) error { + switch value { + case "", "set", "count", "atomic": + *f = coverModeFlag(value) + return nil + default: + return errors.New(`valid modes are "set", "count", or "atomic"`) + } +} + +// A commaListFlag is a flag.Value representing a comma-separated list. +type commaListFlag struct{ vals *[]string } + +func (f commaListFlag) String() string { return strings.Join(*f.vals, ",") } + +func (f commaListFlag) Set(value string) error { + if value == "" { + *f.vals = nil + } else { + *f.vals = strings.Split(value, ",") + } + return nil +} + +// A stringFlag is a flag.Value representing a single string. +type stringFlag struct{ val *string } + +func (f stringFlag) String() string { return *f.val } +func (f stringFlag) Set(value string) error { + *f.val = value + return nil +} + +// outputdirFlag implements the -outputdir flag. +// It interprets an empty value as the working directory of the 'go' command. +type outputdirFlag struct { + resolved *string +} + +func (f outputdirFlag) String() string { return *f.resolved } +func (f outputdirFlag) Set(value string) (err error) { + if value == "" { + // The empty string implies the working directory of the 'go' command. + *f.resolved = base.Cwd + } else { + *f.resolved, err = filepath.Abs(value) + } + return err +} + +// 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. +type vetFlag struct { + explicit bool + off bool + flags []string // passed to vet when invoked automatically during 'go test' +} + +func (f *vetFlag) String() string { + if f.off { + return "off" + } + + var buf strings.Builder + for i, f := range f.flags { + if i > 0 { + buf.WriteByte(',') } - testFlagDefn = append(testFlagDefn, &cmdflag.Defn{ - Name: f.Name, - Value: f.Value, - }) - }) + buf.WriteString(f) + } + return buf.String() +} + +func (f *vetFlag) Set(value string) error { + if value == "" { + *f = vetFlag{flags: defaultVetFlags} + return nil + } + + if value == "off" { + *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") + } + f.flags = append(f.flags, "-"+arg) + } + return nil } // testFlags processes the command line, grabbing -x and -c, rewriting known flags -// to have "test" before them, and reading the command line for the 6.out. +// to have "test" before them, and reading the command line for the test binary. // Unfortunately for us, we need to do our own flag processing because go test // grabs some flags but otherwise its command line is just a holding place for // pkg.test's arguments. @@ -87,117 +203,137 @@ func init() { // to allow both // go test fmt -custom-flag-for-fmt-test // go test -x math -func testFlags(usage func(), args []string) (packageNames, passToTest []string) { - goflags := cmdflag.FindGOFLAGS(testFlagDefn) - args = str.StringList(goflags, args) - inPkg := false - var explicitArgs []string - for i := 0; i < len(args); i++ { - if !strings.HasPrefix(args[i], "-") { - if !inPkg && packageNames == nil { - // First package name we've seen. - inPkg = true - } - if inPkg { - packageNames = append(packageNames, args[i]) - continue +func testFlags(args []string) (packageNames, passToTest []string) { + base.SetFromGOFLAGS(&CmdTest.Flag) + addFromGOFLAGS := map[string]bool{} + CmdTest.Flag.Visit(func(f *flag.Flag) { + if short := strings.TrimPrefix(f.Name, "test."); passFlagToTest[short] { + addFromGOFLAGS[f.Name] = true + } + }) + + explicitArgs := make([]string, 0, len(args)) + inPkgList := false + for len(args) > 0 { + f, remainingArgs, err := cmdflag.ParseOne(&CmdTest.Flag, args) + + if errors.Is(err, flag.ErrHelp) { + exitWithUsage() + } + + if errors.Is(err, cmdflag.ErrFlagTerminator) { + // 'go list' allows package arguments to be named either before or after + // the terminator, but 'go test' has historically allowed them only + // before. Preserve that behavior and treat all remaining arguments — + // including the terminator itself! — as arguments to the test. + explicitArgs = append(explicitArgs, args...) + break + } + + if nf := (cmdflag.NonFlagError{}); errors.As(err, &nf) { + if !inPkgList && packageNames != nil { + // We already saw the package list previously, and this argument is not + // a flag, so it — and everything after it — must be a literal argument + // to the test binary. + explicitArgs = append(explicitArgs, args...) + break } + + inPkgList = true + packageNames = append(packageNames, nf.RawArg) + args = remainingArgs // Consume the package name. + continue } - if inPkg { - // Found an argument beginning with "-"; end of package list. - inPkg = false + if inPkgList { + // This argument is syntactically a flag, so if we were in the package + // list we're not anymore. + inPkgList = false } - f, value, extraWord := cmdflag.Parse(cmd, usage, testFlagDefn, args, i) - if f == nil { - // This is a flag we do not know; we must assume - // that any args we see after this might be flag - // arguments, not package names. - inPkg = false + if nd := (cmdflag.FlagNotDefinedError{}); errors.As(err, &nd) { + // This is a flag we do not know. We must assume that any args we see + // after this might be flag arguments, not package names, so make + // packageNames non-nil to indicate that the package list is complete. + // + // (Actually, we only strictly need to assume that if the flag is not of + // the form -x=value, but making this more precise would be a breaking + // change in the command line API.) if packageNames == nil { - // make non-nil: we have seen the empty package list packageNames = []string{} } - if args[i] == "-args" || args[i] == "--args" { + + if nd.RawArg == "-args" || nd.RawArg == "--args" { // -args or --args signals that everything that follows // should be passed to the test. - explicitArgs = args[i+1:] + explicitArgs = append(explicitArgs, remainingArgs...) break } - passToTest = append(passToTest, args[i]) + + explicitArgs = append(explicitArgs, nd.RawArg) + args = remainingArgs continue } - if i < len(goflags) { - f.Present = false // Not actually present on the command line. + + if err != nil { + fmt.Fprintln(os.Stderr, err) + exitWithUsage() } - if f.Value != nil { - if err := f.Value.Set(value); err != nil { - base.Fatalf("invalid flag argument for -%s: %v", f.Name, err) - } - } else { - // Test-only flags. - // Arguably should be handled by f.Value, but aren't. - switch f.Name { - // bool flags. - case "c", "i", "v", "cover", "json": - cmdflag.SetBool(cmd, f.BoolVar, value) - if f.Name == "json" && testJSON { - passToTest = append(passToTest, "-test.v=true") - } - case "o": - testO = value - testNeedBinary = true - case "exec": - xcmd, err := str.SplitQuotedFields(value) - if err != nil { - base.Fatalf("invalid flag argument for -%s: %v", f.Name, err) - } - work.ExecCmd = xcmd - case "bench": - // record that we saw the flag; don't care about the value - testBench = true - case "list": - testList = true - case "timeout": - testTimeout = value - case "blockprofile", "cpuprofile", "memprofile", "mutexprofile": - testProfile = "-" + f.Name - testNeedBinary = true - case "trace": - testProfile = "-trace" - case "coverpkg": - testCover = true - if value == "" { - testCoverPaths = nil - } else { - testCoverPaths = strings.Split(value, ",") - } - case "coverprofile": - testCover = true - testCoverProfile = value - case "covermode": - switch value { - case "set", "count", "atomic": - testCoverMode = value - default: - base.Fatalf("invalid flag argument for -covermode: %q", value) - } - testCover = true - case "outputdir": - testOutputDir = value - case "vet": - testVetList = value - } + + if short := strings.TrimPrefix(f.Name, "test."); passFlagToTest[short] { + explicitArgs = append(explicitArgs, fmt.Sprintf("-test.%s=%v", short, f.Value)) + + // This flag has been overridden explicitly, so don't forward its implicit + // value from GOFLAGS. + delete(addFromGOFLAGS, short) + delete(addFromGOFLAGS, "test."+short) } - if extraWord { - i++ + + args = remainingArgs + } + + var injectedFlags []string + if testJSON { + // If converting to JSON, we need the full output in order to pipe it to + // test2json. + injectedFlags = append(injectedFlags, "-test.v=true") + delete(addFromGOFLAGS, "v") + delete(addFromGOFLAGS, "test.v") + } + + // Inject flags from GOFLAGS before the explicit command-line arguments. + // (They must appear before the flag terminator or first non-flag argument.) + // Also determine whether flags with awkward defaults have already been set. + var timeoutSet, outputDirSet bool + CmdTest.Flag.Visit(func(f *flag.Flag) { + short := strings.TrimPrefix(f.Name, "test.") + if addFromGOFLAGS[f.Name] { + injectedFlags = append(injectedFlags, fmt.Sprintf("-test.%s=%v", short, f.Value)) } - if f.PassToTest { - passToTest = append(passToTest, "-test."+f.Name+"="+value) + switch short { + case "timeout": + timeoutSet = true + case "outputdir": + outputDirSet = true } + }) + + // 'go test' has a default timeout, but the test binary itself does not. + // If the timeout wasn't set (and forwarded) explicitly, add the default + // timeout to the command line. + if testTimeout > 0 && !timeoutSet { + injectedFlags = append(injectedFlags, fmt.Sprintf("-test.timeout=%v", testTimeout)) + } + + // Similarly, the test binary defaults -test.outputdir to its own working + // directory, but 'go test' defaults it to the working directory of the 'go' + // command. Set it explicitly if it is needed due to some other flag that + // requests output. + if testProfile() != "" && !outputDirSet { + injectedFlags = append(injectedFlags, "-test.outputdir="+testOutputDir) } + // Ensure that -race and -covermode are compatible. if testCoverMode == "" { testCoverMode = "set" if cfg.BuildRace { @@ -205,35 +341,18 @@ func testFlags(usage func(), args []string) (packageNames, passToTest []string) testCoverMode = "atomic" } } - - testVetExplicit = testVetList != "" - if testVetList != "" && testVetList != "off" { - if strings.Contains(testVetList, "=") { - base.Fatalf("-vet argument cannot contain equal signs") - } - if strings.Contains(testVetList, " ") { - base.Fatalf("-vet argument is comma-separated list, cannot contain spaces") - } - list := strings.Split(testVetList, ",") - for i, arg := range list { - list[i] = "-" + arg - } - testVetFlags = list - } - if cfg.BuildRace && testCoverMode != "atomic" { base.Fatalf(`-covermode must be "atomic", not %q, when -race is enabled`, testCoverMode) } - // Tell the test what directory we're running in, so it can write the profiles there. - if testProfile != "" && testOutputDir == "" { - dir, err := os.Getwd() - if err != nil { - base.Fatalf("error from os.Getwd: %s", err) - } - passToTest = append(passToTest, "-test.outputdir", dir) - } + // Forward any unparsed arguments (following --args) to the test binary. + return packageNames, append(injectedFlags, explicitArgs...) +} + +func exitWithUsage() { + fmt.Fprintf(os.Stderr, "usage: %s\n", CmdTest.UsageLine) + fmt.Fprintf(os.Stderr, "Run 'go help %s' and 'go help %s' for details.\n", CmdTest.LongName(), HelpTestflag.LongName()) - passToTest = append(passToTest, explicitArgs...) - return + base.SetExitStatus(2) + base.Exit() } diff --git a/src/cmd/go/internal/vet/vet.go b/src/cmd/go/internal/vet/vet.go index 4e09c0fb9c..4ec58de785 100644 --- a/src/cmd/go/internal/vet/vet.go +++ b/src/cmd/go/internal/vet/vet.go @@ -13,8 +13,12 @@ import ( "path/filepath" ) +// Break init loop. +func init() { + CmdVet.Run = runVet +} + var CmdVet = &base.Command{ - Run: runVet, CustomFlags: true, UsageLine: "go vet [-n] [-x] [-vettool prog] [build flags] [vet flags] [packages]", Short: "report likely mistakes in packages", @@ -47,7 +51,7 @@ See also: go fmt, go fix. func runVet(cmd *base.Command, args []string) { modload.LoadTests = true - vetFlags, pkgArgs := vetFlags(vetUsage, args) + vetFlags, pkgArgs := vetFlags(args) work.BuildInit() work.VetFlags = vetFlags diff --git a/src/cmd/go/internal/vet/vetflag.go b/src/cmd/go/internal/vet/vetflag.go index e3de48bbff..ef995ef835 100644 --- a/src/cmd/go/internal/vet/vetflag.go +++ b/src/cmd/go/internal/vet/vetflag.go @@ -7,6 +7,7 @@ package vet import ( "bytes" "encoding/json" + "errors" "flag" "fmt" "log" @@ -17,7 +18,6 @@ import ( "cmd/go/internal/base" "cmd/go/internal/cmdflag" - "cmd/go/internal/str" "cmd/go/internal/work" ) @@ -39,37 +39,44 @@ import ( var vetTool string // -vettool func init() { + work.AddBuildFlags(CmdVet, work.DefaultBuildFlags) + CmdVet.Flag.StringVar(&vetTool, "vettool", "", "") +} + +func parseVettoolFlag(args []string) { // Extract -vettool by ad hoc flag processing: // its value is needed even before we can declare // the flags available during main flag processing. - for i, arg := range os.Args { + for i, arg := range args { if arg == "-vettool" || arg == "--vettool" { - if i+1 >= len(os.Args) { + if i+1 >= len(args) { log.Fatalf("%s requires a filename", arg) } - vetTool = os.Args[i+1] - break + vetTool = args[i+1] + return } else if strings.HasPrefix(arg, "-vettool=") || strings.HasPrefix(arg, "--vettool=") { vetTool = arg[strings.IndexByte(arg, '=')+1:] - break + return } } } // vetFlags processes the command line, splitting it at the first non-flag // into the list of flags and list of packages. -func vetFlags(usage func(), args []string) (passToVet, packageNames []string) { +func vetFlags(args []string) (passToVet, packageNames []string) { + parseVettoolFlag(args) + // Query the vet command for its flags. - tool := vetTool - if tool != "" { + var tool string + if vetTool == "" { + tool = base.Tool("vet") + } else { var err error - tool, err = filepath.Abs(tool) + tool, err = filepath.Abs(vetTool) if err != nil { log.Fatal(err) } - } else { - tool = base.Tool("vet") } out := new(bytes.Buffer) vetcmd := exec.Command(tool, "-flags") @@ -90,95 +97,85 @@ func vetFlags(usage func(), args []string) (passToVet, packageNames []string) { base.Exit() } - // Add vet's flags to vetflagDefn. + // Add vet's flags to CmdVet.Flag. // // Some flags, in particular -tags and -v, are known to vet but - // also defined as build flags. This works fine, so we don't - // define them here but use AddBuildFlags to init them. + // also defined as build flags. This works fine, so we omit duplicates here. // However some, like -x, are known to the build but not to vet. - var vetFlagDefn []*cmdflag.Defn + isVetFlag := make(map[string]bool, len(analysisFlags)) + cf := CmdVet.Flag for _, f := range analysisFlags { - switch f.Name { - case "tags", "v": - continue - } - defn := &cmdflag.Defn{ - Name: f.Name, - PassToTest: true, - } - if f.Bool { - defn.BoolVar = new(bool) + isVetFlag[f.Name] = true + if cf.Lookup(f.Name) == nil { + if f.Bool { + cf.Bool(f.Name, false, "") + } else { + cf.String(f.Name, "", "") + } } - vetFlagDefn = append(vetFlagDefn, defn) } - // Add build flags to vetFlagDefn. - var cmd base.Command - work.AddBuildFlags(&cmd, work.DefaultBuildFlags) - // This flag declaration is a placeholder: - // -vettool is actually parsed by the init function above. - cmd.Flag.StringVar(new(string), "vettool", "", "path to vet tool binary") - cmd.Flag.VisitAll(func(f *flag.Flag) { - vetFlagDefn = append(vetFlagDefn, &cmdflag.Defn{ - Name: f.Name, - Value: f.Value, - }) + // Record the set of vet tool flags set by GOFLAGS. We want to pass them to + // the vet tool, but only if they aren't overridden by an explicit argument. + base.SetFromGOFLAGS(&CmdVet.Flag) + addFromGOFLAGS := map[string]bool{} + CmdVet.Flag.Visit(func(f *flag.Flag) { + if isVetFlag[f.Name] { + addFromGOFLAGS[f.Name] = true + } }) - // Process args. - goflags := cmdflag.FindGOFLAGS(vetFlagDefn) - args = str.StringList(goflags, args) - for i := 0; i < len(args); i++ { - if !strings.HasPrefix(args[i], "-") { - return args[:i], args[i:] + explicitFlags := make([]string, 0, len(args)) + for len(args) > 0 { + f, remainingArgs, err := cmdflag.ParseOne(&CmdVet.Flag, args) + + if errors.Is(err, flag.ErrHelp) { + exitWithUsage() } - f, value, extraWord := cmdflag.Parse("vet", usage, vetFlagDefn, args, i) - if f == nil { - fmt.Fprintf(os.Stderr, "vet: flag %q not defined\n", args[i]) - fmt.Fprintf(os.Stderr, "Run \"go help vet\" for more information\n") - base.SetExitStatus(2) - base.Exit() + if errors.Is(err, cmdflag.ErrFlagTerminator) { + // All remaining args must be package names, but the flag terminator is + // not included. + packageNames = remainingArgs + break } - if i < len(goflags) { - f.Present = false // Not actually present on the command line. + + if nf := (cmdflag.NonFlagError{}); errors.As(err, &nf) { + // Everything from here on out — including the argument we just consumed — + // must be a package name. + packageNames = args + break } - if f.Value != nil { - if err := f.Value.Set(value); err != nil { - base.Fatalf("invalid flag argument for -%s: %v", f.Name, err) - } - keep := f.PassToTest - if !keep { - // A build flag, probably one we don't want to pass to vet. - // Can whitelist. - switch f.Name { - case "tags", "v": - keep = true - } - } - if !keep { - // Flags known to the build but not to vet, so must be dropped. - if extraWord { - args = append(args[:i], args[i+2:]...) - extraWord = false - } else { - args = append(args[:i], args[i+1:]...) - } - i-- - } + + if err != nil { + fmt.Fprintln(os.Stderr, err) + exitWithUsage() } - if extraWord { - i++ + + if isVetFlag[f.Name] { + // Forward the raw arguments rather than cleaned equivalents, just in + // case the vet tool parses them idiosyncratically. + explicitFlags = append(explicitFlags, args[:len(args)-len(remainingArgs)]...) + + // This flag has been overridden explicitly, so don't forward its implicit + // value from GOFLAGS. + delete(addFromGOFLAGS, f.Name) } - } - return args, nil -} -var vetUsage func() + args = remainingArgs + } -func init() { vetUsage = usage } // break initialization cycle + // Prepend arguments from GOFLAGS before other arguments. + CmdVet.Flag.Visit(func(f *flag.Flag) { + if addFromGOFLAGS[f.Name] { + passToVet = append(passToVet, fmt.Sprintf("-%s=%s", f.Name, f.Value)) + } + }) + passToVet = append(passToVet, explicitFlags...) + return passToVet, packageNames +} -func usage() { +func exitWithUsage() { fmt.Fprintf(os.Stderr, "usage: %s\n", CmdVet.UsageLine) fmt.Fprintf(os.Stderr, "Run 'go help %s' for details.\n", CmdVet.LongName()) diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go index e3b25c937c..7146c9ce00 100644 --- a/src/cmd/go/internal/work/build.go +++ b/src/cmd/go/internal/work/build.go @@ -232,6 +232,7 @@ const ( DefaultBuildFlags BuildFlagMask = 0 OmitModFlag BuildFlagMask = 1 << iota OmitModCommonFlags + OmitVFlag ) // AddBuildFlags adds the flags common to the build, clean, get, @@ -240,7 +241,9 @@ func AddBuildFlags(cmd *base.Command, mask BuildFlagMask) { cmd.Flag.BoolVar(&cfg.BuildA, "a", false, "") cmd.Flag.BoolVar(&cfg.BuildN, "n", false, "") cmd.Flag.IntVar(&cfg.BuildP, "p", cfg.BuildP, "") - cmd.Flag.BoolVar(&cfg.BuildV, "v", false, "") + if mask&OmitVFlag == 0 { + cmd.Flag.BoolVar(&cfg.BuildV, "v", false, "") + } cmd.Flag.BoolVar(&cfg.BuildX, "x", false, "") cmd.Flag.Var(&load.BuildAsmflags, "asmflags", "") diff --git a/src/cmd/go/main.go b/src/cmd/go/main.go index 4882375f4e..2112defa6a 100644 --- a/src/cmd/go/main.go +++ b/src/cmd/go/main.go @@ -182,7 +182,7 @@ BigCmdLoop: if cmd.CustomFlags { args = args[1:] } else { - base.SetFromGOFLAGS(cmd.Flag) + base.SetFromGOFLAGS(&cmd.Flag) cmd.Flag.Parse(args[1:]) args = cmd.Flag.Args() } diff --git a/src/cmd/go/testdata/script/test_flags.txt b/src/cmd/go/testdata/script/test_flags.txt new file mode 100644 index 0000000000..a335aec1c8 --- /dev/null +++ b/src/cmd/go/testdata/script/test_flags.txt @@ -0,0 +1,103 @@ +env GO111MODULE=on + +[short] skip + +# Arguments after the flag terminator should be ignored. +# If we pass '-- -test.v', we should not get verbose output +# *and* output from the test should not be echoed. +go test ./x -- -test.v +stdout '\Aok\s+example.com/x\s+[0-9.s]+\n\z' +! stderr . + +# For backward-compatibility with previous releases of the 'go' command, +# arguments that appear after unrecognized flags should not be treated +# as packages, even if they are unambiguously not arguments to flags. +# Even though ./x looks like a package path, the real package should be +# the implicit '.'. +! go test --answer=42 ./x +stderr 'can''t load package: package \.: ' +! stderr '/x' + +# An explicit '-outputdir=' argument should set test.outputdir +# to the 'go' command's working directory, not zero it out +# for the test binary. +go test -x -coverprofile=cover.out '-outputdir=' ./x +stderr '-test.outputdir=[^ ]' +exists ./cover.out +! exists ./x/cover.out + +# Test flags from GOFLAGS should be forwarded to the test binary, +# with the 'test.' prefix in the GOFLAGS entry... +env GOFLAGS='-test.timeout=24h0m0s -count=1' +go test -v -x ./x +stdout 'TestLogTimeout: .*: 24h0m0s$' +stderr '-test.count=1' + +# ...or without. +env GOFLAGS='-timeout=24h0m0s -count=1' +go test -v -x ./x +stdout 'TestLogTimeout: .*: 24h0m0s$' +stderr '-test.count=1' + +# Arguments from the command line should override GOFLAGS... +go test -v -x -timeout=25h0m0s ./x +stdout 'TestLogTimeout: .*: 25h0m0s$' +stderr '-test.count=1' + +# ...even if they use a different flag name. +go test -v -x -test.timeout=26h0m0s ./x +stdout 'TestLogTimeout: .*: 26h0m0s$' +stderr '-test\.timeout=26h0m0s' +! stderr 'timeout=24h0m0s' +stderr '-test.count=1' + +# Invalid flags should be reported exactly once. +! go test -covermode=walrus ./x +stderr -count=1 'invalid value "walrus" for flag -covermode: valid modes are .*$' +stderr '^usage: go test .*$' +stderr '^Run ''go help test'' and ''go help testflag'' for details.$' + +# -covermode, -coverpkg, and -coverprofile should imply -cover +go test -covermode=set ./x +stdout '\s+coverage:\s+' + +go test -coverpkg=encoding/binary ./x +stdout '\s+coverage:\s+' + +go test -coverprofile=cover.out ./x +stdout '\s+coverage:\s+' +exists ./cover.out +rm ./cover.out + +# -*profile and -trace flags should force output to the current working directory +# or -outputdir, not the directory containing the test. + +go test -memprofile=mem.out ./x +exists ./mem.out +rm ./mem.out + +go test -trace=trace.out ./x +exists ./trace.out +rm ./trace.out + +# Relative paths with -outputdir should be relative to the go command's working +# directory, not the directory containing the test. +mkdir profiles +go test -memprofile=mem.out -outputdir=./profiles ./x +exists ./profiles/mem.out +rm profiles + +-- go.mod -- +module example.com +go 1.14 +-- x/x_test.go -- +package x + +import ( + "flag" + "testing" +) + +func TestLogTimeout(t *testing.T) { + t.Log(flag.Lookup("test.timeout").Value) +} diff --git a/src/cmd/go/testdata/script/vet_flags.txt b/src/cmd/go/testdata/script/vet_flags.txt index f2cf021f62..b85b133c19 100644 --- a/src/cmd/go/testdata/script/vet_flags.txt +++ b/src/cmd/go/testdata/script/vet_flags.txt @@ -1,7 +1,7 @@ env GO111MODULE=on -# Regression test for issue 35837: "go vet - " -# did not apply the requested analyzer. +# Issue 35837: "go vet - " should use the requested +# analyzers, not the default analyzers for 'go test'. go vet -n -unreachable=false encoding/binary stderr '-unreachable=false' ! stderr '-unsafeptr=false' @@ -17,20 +17,54 @@ go vet -n -unsafeptr encoding/binary stderr '-unsafeptr' ! stderr '-unsafeptr=false' +# A flag terminator should be allowed before the package list. +go vet -n -- . + [short] stop + +# Analyzer flags should be included from GOFLAGS, and should override +# the defaults. +go vet . +env GOFLAGS='-tags=buggy' +! go vet . +stderr 'possible formatting directive' + +# Enabling one analyzer in GOFLAGS should disable the rest implicitly... +env GOFLAGS='-tags=buggy -unsafeptr' +go vet . + +# ...but enabling one on the command line should not disable the analyzers +# enabled via GOFLAGS. +env GOFLAGS='-tags=buggy -printf' +! go vet -unsafeptr +stderr 'possible formatting directive' + +# Analyzer flags don't exist unless we're running 'go vet', +# and we shouldn't run the vet tool to discover them otherwise. +# (Maybe someday we'll hard-code the analyzer flags for the default vet +# tool to make this work, but not right now.) +env GOFLAGS='-unsafeptr' +! go list . +stderr 'go: parsing \$GOFLAGS: unknown flag -unsafeptr' +env GOFLAGS= + env GOCACHE=$WORK/gocache -env GOTMPDIR=$WORK/tmp -go env GOTMPDIR -stdout '/tmp' -# "go test" on a user package should by default enable an explicit whitelist of analyzers. +# "go test" on a user package should by default enable an explicit list of analyzers. go test -x -run=none . stderr '[/\\]vet'$GOEXE'["]? .* -errorsas .* ["]?\$WORK[/\\][^ ]*[/\\]vet\.cfg' -# "go test" on a standard package should by default disable an explicit blacklist. +# An explicitly-empty -vet argument should imply the default analyzers. +go test -x -vet= -run=none . +stderr '[/\\]vet'$GOEXE'["]? .* -errorsas .* ["]?\$WORK[/\\][^ ]*[/\\]vet\.cfg' + +# "go test" on a standard package should by default disable an explicit list. go test -x -run=none encoding/binary stderr '[/\\]vet'$GOEXE'["]? -unsafeptr=false ["]?\$WORK[/\\][^ ]*[/\\]vet\.cfg' +go test -x -vet= -run=none encoding/binary +stderr '[/\\]vet'$GOEXE'["]? -unsafeptr=false ["]?\$WORK[/\\][^ ]*[/\\]vet\.cfg' + # Both should allow users to override via the -vet flag. go test -x -vet=unreachable -run=none . stderr '[/\\]vet'$GOEXE'["]? -unreachable ["]?\$WORK[/\\][^ ]*[/\\]vet\.cfg' @@ -43,3 +77,13 @@ module example.com/x package x -- x_test.go -- package x +-- x_tagged.go -- +// +build buggy + +package x + +import "fmt" + +func init() { + fmt.Sprint("%s") // oops! +}