From 5824a4ce1a0e47f3093128371c7156b35fe9d806 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Daniel=20Mart=C3=AD?= Date: Fri, 12 Jun 2020 15:14:42 +0100 Subject: [PATCH] cmd/go: error when -c or -i are used with unknown flags MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Other test flags passed to the test binary, such as -run or -count, are equally pointless when -c or -i are used, since the test binary is never run. However, custom flags in that scenario are far more likely to be due to human error, such as: # note the "ldflags" typo, which silently did nothing go test -c -lflags=-w Instead, make this scenario error. It seems unlikely that anyone is using -c along with intended custom-defined test flags, and if they are, removing those extra flags that do nothing is probably a good idea anyway. We don't add this restriction for the flags defined in 'go help testflag', since they are far less likely to be typos or unintended mistakes. Another reason not to do that change is that other commands similarly silently ignore no-op flags, such as: # -d disables the build, so -ldflags is never used go get -d -ldflags=-w Fixes #39484. Change-Id: I6ba2f6866562fe8f8fceaf4cd862d874bf5cd978 Reviewed-on: https://go-review.googlesource.com/c/go/+/237697 Trust: Daniel Martí Run-TryBot: Daniel Martí TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills --- src/cmd/go/internal/test/testflag.go | 16 ++++++++++++++++ src/cmd/go/testdata/script/test_flag.txt | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/cmd/go/internal/test/testflag.go b/src/cmd/go/internal/test/testflag.go index 4f0a8924f1..d2671ff5a7 100644 --- a/src/cmd/go/internal/test/testflag.go +++ b/src/cmd/go/internal/test/testflag.go @@ -212,6 +212,10 @@ func testFlags(args []string) (packageNames, passToTest []string) { } }) + // firstUnknownFlag helps us report an error when flags not known to 'go + // test' are used along with -i or -c. + firstUnknownFlag := "" + explicitArgs := make([]string, 0, len(args)) inPkgList := false afterFlagWithoutValue := false @@ -288,6 +292,10 @@ func testFlags(args []string) (packageNames, passToTest []string) { break } + if firstUnknownFlag == "" { + firstUnknownFlag = nd.RawArg + } + explicitArgs = append(explicitArgs, nd.RawArg) args = remainingArgs if !nd.HasValue { @@ -312,6 +320,14 @@ func testFlags(args []string) (packageNames, passToTest []string) { args = remainingArgs } + if firstUnknownFlag != "" && (testC || cfg.BuildI) { + buildFlag := "-c" + if !testC { + buildFlag = "-i" + } + fmt.Fprintf(os.Stderr, "flag %s is not a 'go test' flag (unknown flags cannot be used with %s)\n", firstUnknownFlag, buildFlag) + exitWithUsage() + } var injectedFlags []string if testJSON { diff --git a/src/cmd/go/testdata/script/test_flag.txt b/src/cmd/go/testdata/script/test_flag.txt index bbcad1c59c..ec88d38cbe 100644 --- a/src/cmd/go/testdata/script/test_flag.txt +++ b/src/cmd/go/testdata/script/test_flag.txt @@ -3,6 +3,22 @@ go test flag_test.go -v -args -v=7 # Two distinct -v flags go test -v flag_test.go -args -v=7 # Two distinct -v flags +# Using a custom flag mixed with regular 'go test' flags should be OK. +go test -count=1 -custom -args -v=7 + +# However, it should be an error to use custom flags when -i or -c are used, +# since we know for sure that no test binary will run at all. +! go test -i -custom +stderr '^flag -custom is not a ''go test'' flag \(unknown flags cannot be used with -i\)$' +! go test -c -custom +stderr '^flag -custom is not a ''go test'' flag \(unknown flags cannot be used with -c\)$' + +# The same should apply even if -c or -i come after a custom flag. +! go test -custom -c +stderr '^flag -custom is not a ''go test'' flag \(unknown flags cannot be used with -c\)$' + +-- go.mod -- +module m -- flag_test.go -- package flag_test @@ -14,6 +30,8 @@ import ( var v = flag.Int("v", 0, "v flag") +var custom = flag.Bool("custom", false, "") + // Run this as go test pkg -v=7 func TestVFlagIsSet(t *testing.T) { if *v != 7 { -- 2.48.1