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") + +// 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 } -// 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 +func (e FlagNotDefinedError) Error() string { + return fmt.Sprintf("flag provided but not defined: -%s", e.Name) } -// 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 +// A NonFlagError indicates an argument that is not a syntactically-valid flag. +type NonFlagError struct { + RawArg string } -// 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 +func (e NonFlagError) Error() string { + return fmt.Sprintf("not a flag: %q", e.RawArg) } -// 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() -} +// 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. -// 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) + 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 } -} -// 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:] - } 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 + + // 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) + } + } + } 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:] + } + 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 f, args, nil } -// 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:] - } - name := flag[1:] - if i := strings.Index(name, "="); i >= 0 { - name = name[:i] - } - for _, f := range defns { - if name == f.Name { - flags = append(flags, flag) - break - } - } - } - return flags +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 -// testVetFlags is the list of flags to pass to vet when invoked automatically during go test. -var testVetFlags = []string{ + 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 != "") +} + +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"}, +func init() { + work.AddBuildFlags(CmdTest, work.OmitVFlag) - // 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}, + 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 inPkg { - // Found an argument beginning with "-"; end of package list. - inPkg = false + 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 } - 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 packageNames == nil { - // make non-nil: we have seen the empty package list - packageNames = []string{} - } - if args[i] == "-args" || args[i] == "--args" { - // -args or --args signals that everything that follows - // should be passed to the test. - explicitArgs = args[i+1:] + 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 } - passToTest = append(passToTest, args[i]) + + inPkgList = true + packageNames = append(packageNames, nf.RawArg) + args = remainingArgs // Consume the package name. continue } - if i < len(goflags) { - f.Present = false // Not actually present on the command line. + + if inPkgList { + // This argument is syntactically a flag, so if we were in the package + // list we're not anymore. + inPkgList = false } - if f.Value != nil { - if err := f.Value.Set(value); err != nil { - base.Fatalf("invalid flag argument for -%s: %v", f.Name, err) + + 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 { + packageNames = []string{} } - } 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 nd.RawArg == "-args" || nd.RawArg == "--args" { + // -args or --args signals that everything that follows + // should be passed to the test. + explicitArgs = append(explicitArgs, remainingArgs...) + break } + + explicitArgs = append(explicitArgs, nd.RawArg) + args = remainingArgs + continue } - if extraWord { - i++ + + if err != nil { + fmt.Fprintln(os.Stderr, err) + exitWithUsage() } - if f.PassToTest { - passToTest = append(passToTest, "-test."+f.Name+"="+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) } + + 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)) + } + 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) - } - - passToTest = append(passToTest, explicitArgs...) - return + // 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()) + + 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 + isVetFlag[f.Name] = true + if cf.Lookup(f.Name) == nil { + if f.Bool { + cf.Bool(f.Name, false, "") + } else { + cf.String(f.Name, "", "") + } } - defn := &cmdflag.Defn{ - Name: f.Name, - PassToTest: true, - } - if f.Bool { - defn.BoolVar = new(bool) - } - 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) } + + args = remainingArgs } - return args, nil + + // 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 } -var vetUsage func() - -func init() { vetUsage = usage } // break initialization cycle - -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' -[short] stop -env GOCACHE=$WORK/gocache -env GOTMPDIR=$WORK/tmp -go env GOTMPDIR -stdout '/tmp' +# A flag terminator should be allowed before the package list. +go vet -n -- . -# "go test" on a user package should by default enable an explicit whitelist of analyzers. +[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 + +# "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! +}