cmd/vet: better align print warnings with fmt

fmt's %d, %x, and %X all accept pointer arguments. However, in cmd/vet's
printVerbs table, they were defined as if they did not accept pointer
arguments.

This inconsistency with fmt did not manifest to users since the vet
codebase worked around it. In particular, pointer arguments were usually
allowed for verbs that accepted integers, as the *types.Pointer argument
type case read the following:

	t&(argInt|argPointer) != 0

As a result, using the %q verb with a pointer resulted in a bug in
cmd/vet:

	$ go run f.go
	%!q(*int=0xc000014140)
	$ go vet f.go
	[no warning]

As documented, fmt's %q verb only accepts runes (integers), strings, and
byte slices. It should not accept pointers, and it does not. But since
vet mixed integers and pointers, it wasn't properly warning about the
misuse of fmt.

This patch surfaced another bug with fmt.Printf("%p", nil):

	$ go run f.go
	%!p(<nil>)
	$ go vet f.go
	[no warning]

As documented, fmt's %p verb only accepts pointers, and untyped nil is
not a valid pointer. But vet did not warn about it, which is another
inconsistency with fmt's documented rules. Fix that too, with a test,
also getting rid of the TODO associated with the code.

As a result of those changes, fix a wrong use of the fmt format verbs in
the standard library, now correctly spotted by vet.

Fixes #25233.

Change-Id: Id0ad31fbc25adfe1c46c6b6879b8d02b23633b3a
Reviewed-on: https://go-review.googlesource.com/111284
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/cmd/vet/print.go b/src/cmd/vet/print.go
index 294688f4ea629f08365ad150f0f7028409f6fe27..e4e99641ef4290e3af97e18d56c81a61bb840cb1 100644 (file)
--- a/src/cmd/vet/print.go
+++ b/src/cmd/vet/print.go
@@ -534,7 +534,7 @@ var printVerbs = []printVerb{
        {'%', noFlag, 0},
        {'b', numFlag, argInt | argFloat | argComplex},
        {'c', "-", argRune | argInt},
-       {'d', numFlag, argInt},
+       {'d', numFlag, argInt | argPointer},
        {'e', sharpNumFlag, argFloat | argComplex},
        {'E', sharpNumFlag, argFloat | argComplex},
        {'f', sharpNumFlag, argFloat | argComplex},
@@ -549,8 +549,8 @@ var printVerbs = []printVerb{
        {'T', "-", anyType},
        {'U', "-#", argRune | argInt},
        {'v', allFlags, anyType},
-       {'x', sharpNumFlag, argRune | argInt | argString},
-       {'X', sharpNumFlag, argRune | argInt | argString},
+       {'x', sharpNumFlag, argRune | argInt | argString | argPointer},
+       {'X', sharpNumFlag, argRune | argInt | argString | argPointer},
 }
 
 // okPrintfArg compares the formatState to the arguments actually present,

diff --git a/src/cmd/vet/testdata/print.go b/src/cmd/vet/testdata/print.go
index 1669a047daf0afc89fce56daaa7bd9edc8edcb4d..459b08141b4996dd2c7fff5bf9a110a392b3815b 100644 (file)
--- a/src/cmd/vet/testdata/print.go
+++ b/src/cmd/vet/testdata/print.go
@@ -79,7 +79,7 @@ func PrintfTests() {
        fmt.Printf("%G %G %G %G", 3e9, x, fslice, c)
        fmt.Printf("%b %b %b %b", 3e9, x, fslice, c)
        fmt.Printf("%o %o", 3, i)
-       fmt.Printf("%p %p", p, nil)
+       fmt.Printf("%p", p)
        fmt.Printf("%q %q %q %q", 3, i, 'x', r)
        fmt.Printf("%s %s %s", "hi", s, []byte{65})
        fmt.Printf("%t %t", true, b)
@@ -122,6 +122,7 @@ func PrintfTests() {
        fmt.Printf("%g", imap)                      // ERROR "Printf format %g has arg imap of wrong type map\[int\]int"
        fmt.Printf("%G", i)                         // ERROR "Printf format %G has arg i of wrong type int"
        fmt.Printf("%o", x)                         // ERROR "Printf format %o has arg x of wrong type float64"
+       fmt.Printf("%p", nil)                       // ERROR "Printf format %p has arg nil of wrong type untyped nil"
        fmt.Printf("%p", 23)                        // ERROR "Printf format %p has arg 23 of wrong type int"
        fmt.Printf("%q", x)                         // ERROR "Printf format %q has arg x of wrong type float64"
        fmt.Printf("%s", b)                         // ERROR "Printf format %s has arg b of wrong type bool"
@@ -180,6 +181,7 @@ func PrintfTests() {
        Printf("%d", notPercentDV)  // ERROR "Printf format %d has arg notPercentDV of wrong type testdata.notPercentDStruct"
        Printf("%d", &notPercentDV) // ERROR "Printf format %d has arg &notPercentDV of wrong type \*testdata.notPercentDStruct"
        Printf("%p", &notPercentDV) // Works regardless: we print it as a pointer.
+       Printf("%q", &percentDV)    // ERROR "Printf format %q has arg &percentDV of wrong type \*testdata.percentDStruct"
        Printf("%s", percentSV)
        Printf("%s", &percentSV)
        // Good argument reorderings.

diff --git a/src/cmd/vet/types.go b/src/cmd/vet/types.go
index ea4269a7f907ea51cd1eff67c0b996ebc17ff7bb..5f8e481e01b68115f603557e3cfc4bd82ed567a5 100644 (file)
--- a/src/cmd/vet/types.go
+++ b/src/cmd/vet/types.go
@@ -201,8 +201,8 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp
                if str, ok := typ.Elem().Underlying().(*types.Struct); ok {
                        return f.matchStructArgType(t, str, arg, inProgress)
                }
-               // The rest can print with %p as pointers, or as integers with %x etc.
-               return t&(argInt|argPointer) != 0
+               // Check whether the rest can print pointers.
+               return t&argPointer != 0
 
        case *types.Struct:
                return f.matchStructArgType(t, typ, arg, inProgress)
@@ -254,7 +254,7 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp
                        return t&(argInt|argRune) != 0
 
                case types.UntypedNil:
-                       return t&argPointer != 0 // TODO?
+                       return false
 
                case types.Invalid:
                        if *verbose {

diff --git a/src/time/zoneinfo_test.go b/src/time/zoneinfo_test.go
index 7a55d4f618dcbfcf5b7759054683ee67521fe700..450f5aa114aaa28caa8aeab550e015cda85baca6 100644 (file)
--- a/src/time/zoneinfo_test.go
+++ b/src/time/zoneinfo_test.go
@@ -32,7 +32,7 @@ func TestEnvVarUsage(t *testing.T) {
        defer time.ResetZoneinfoForTesting()
 
        if zoneinfo := time.ZoneinfoForTesting(); testZoneinfo != *zoneinfo {
-               t.Errorf("zoneinfo does not match env variable: got %q want %q", zoneinfo, testZoneinfo)
+               t.Errorf("zoneinfo does not match env variable: got %q want %q", *zoneinfo, testZoneinfo)
        }
 }