cmd/go: error when -c or -i are used with unknown flags

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í <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

2 files changed, 34 insertions, 0 deletions

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 {