go.tools/cmd/vet: improvements to static checking of printf calls.

Details:
- added support for complex numbers as distinct from floats:
  %[efg] allows complex; %b does not.
- %p: only Signature, Map, Chan, Slice, unsafe.Pointer allowed.
- %s: allow []byte.
- allow a verb to match map[K]V and []T if it matches K/V/T,
  e.g. %d now matches []int.  i.e. matching is recursive.
- use go/types' constant folding.  literal() is gone.
- group cases together.

Added tests.

R=gri, r
CC=golang-dev
https://golang.org/cl/10895043

cmd/vet/print.go

@@ -14,6 +14,8 @@ import (
 	"strconv"
 	"strings"
 	"unicode/utf8"
+
+	"code.google.com/p/go.tools/go/exact"
 )
 
 var printfuncs = flag.String("printfuncs", "", "comma-separated list of print function names to check")
@@ -58,53 +60,6 @@ func (f *File) checkFmtPrintfCall(call *ast.CallExpr, Name string) {
 	}
 }
 
-// literal returns the literal value represented by the expression, or nil if it is not a literal.
-func (f *File) literal(value ast.Expr) *ast.BasicLit {
-	switch v := value.(type) {
-	case *ast.BasicLit:
-		return v
-	case *ast.ParenExpr:
-		return f.literal(v.X)
-	case *ast.BinaryExpr:
-		if v.Op != token.ADD {
-			break
-		}
-		litX := f.literal(v.X)
-		litY := f.literal(v.Y)
-		if litX != nil && litY != nil {
-			lit := *litX
-			x, errX := strconv.Unquote(litX.Value)
-			y, errY := strconv.Unquote(litY.Value)
-			if errX == nil && errY == nil {
-				return &ast.BasicLit{
-					ValuePos: lit.ValuePos,
-					Kind:     lit.Kind,
-					Value:    strconv.Quote(x + y),
-				}
-			}
-		}
-	case *ast.Ident:
-		// See if it's a constant or initial value (we can't tell the difference).
-		if v.Obj == nil || v.Obj.Decl == nil {
-			return nil
-		}
-		valueSpec, ok := v.Obj.Decl.(*ast.ValueSpec)
-		if ok && len(valueSpec.Names) == len(valueSpec.Values) {
-			// Find the index in the list of names
-			var i int
-			for i = 0; i < len(valueSpec.Names); i++ {
-				if valueSpec.Names[i].Name == v.Name {
-					if lit, ok := valueSpec.Values[i].(*ast.BasicLit); ok {
-						return lit
-					}
-					return nil
-				}
-			}
-		}
-	}
-	return nil
-}
-
 // formatState holds the parsed representation of a printf directive such as "%3.*[4]d".
 // It is constructed by parsePrintfVerb.
 type formatState struct {
@@ -127,23 +82,20 @@ type formatState struct {
 // call.Args[formatIndex] is (well, should be) the format argument.
 func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) {
 	if formatIndex >= len(call.Args) {
+		f.Warn(call.Pos(), "too few arguments in call to", name)
 		return
 	}
-	lit := f.literal(call.Args[formatIndex])
+	lit := f.pkg.values[call.Args[formatIndex]]
 	if lit == nil {
 		if *verbose {
-			f.Warn(call.Pos(), "can't check non-literal format in call to", name)
+			f.Warn(call.Pos(), "can't check non-constant format in call to", name)
 		}
 		return
 	}
-	if lit.Kind != token.STRING {
-		f.Badf(call.Pos(), "literal %v not a string in call to", lit.Value, name)
-	}
-	format, err := strconv.Unquote(lit.Value)
-	if err != nil {
-		// Shouldn't happen if parser returned no errors, but be safe.
-		f.Badf(call.Pos(), "invalid quoted string literal")
+	if lit.Kind() != exact.String {
+		f.Badf(call.Pos(), "constant %v not a string in call to", lit, name)
 	}
+	format := exact.StringVal(lit)
 	firstArg := formatIndex + 1 // Arguments are immediately after format string.
 	if !strings.Contains(format, "%") {
 		if len(call.Args) > firstArg {
@@ -325,6 +277,7 @@ const (
 	argRune
 	argString
 	argFloat
+	argComplex
 	argPointer
 	anyType printfArgType = ^0
 )
@@ -356,12 +309,12 @@ var printVerbs = []printVerb{
 	{'b', numFlag, argInt | argFloat},
 	{'c', "-", argRune | argInt},
 	{'d', numFlag, argInt},
-	{'e', numFlag, argFloat},
-	{'E', numFlag, argFloat},
-	{'f', numFlag, argFloat},
-	{'F', numFlag, argFloat},
-	{'g', numFlag, argFloat},
-	{'G', numFlag, argFloat},
+	{'e', numFlag, argFloat | argComplex},
+	{'E', numFlag, argFloat | argComplex},
+	{'f', numFlag, argFloat | argComplex},
+	{'F', numFlag, argFloat | argComplex},
+	{'g', numFlag, argFloat | argComplex},
+	{'G', numFlag, argFloat | argComplex},
 	{'o', sharpNumFlag, argInt},
 	{'p', "-#", argPointer},
 	{'q', " -+.0#", argRune | argInt | argString},
@@ -410,7 +363,7 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) {
 			return
 		}
 		arg := call.Args[argNum]
-		if !f.matchArgType(argInt, arg) {
+		if !f.matchArgType(argInt, nil, arg) {
 			f.Badf(call.Pos(), "arg %s for * in printf format not of type int", f.gofmt(arg))
 			return false
 		}
@@ -423,7 +376,7 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) {
 		return false
 	}
 	arg := call.Args[argNum]
-	if !f.matchArgType(v.typ, arg) {
+	if !f.matchArgType(v.typ, nil, arg) {
 		typeString := ""
 		if typ := f.pkg.types[arg]; typ != nil {
 			typeString = typ.String()

cmd/vet/testdata/print.go

@@ -53,21 +53,24 @@ func PrintfTests() {
 	var s string
 	var x float64
 	var p *int
+	var imap map[int]int
+	var fslice []float64
+	var c complex64
 	// Some good format/argtypes
 	fmt.Printf("")
-	fmt.Printf("%b %b", 3, i)
+	fmt.Printf("%b %b %b", 3, i, x)
 	fmt.Printf("%c %c %c %c", 3, i, 'x', r)
-	fmt.Printf("%d %d", 3, i)
-	fmt.Printf("%e %e", 3e9, x)
-	fmt.Printf("%E %E", 3e9, x)
-	fmt.Printf("%f %f", 3e9, x)
-	fmt.Printf("%F %F", 3e9, x)
-	fmt.Printf("%g %g", 3e9, x)
-	fmt.Printf("%G %G", 3e9, x)
+	fmt.Printf("%d %d %d", 3, i, imap)
+	fmt.Printf("%e %e %e %e", 3e9, x, fslice, c)
+	fmt.Printf("%E %E %E %E", 3e9, x, fslice, c)
+	fmt.Printf("%f %f %f %f", 3e9, x, fslice, c)
+	fmt.Printf("%F %F %F %F", 3e9, x, fslice, c)
+	fmt.Printf("%g %g %g %g", 3e9, x, fslice, c)
+	fmt.Printf("%G %G %G %G", 3e9, x, fslice, c)
 	fmt.Printf("%o %o", 3, i)
 	fmt.Printf("%p %p", p, nil)
 	fmt.Printf("%q %q %q %q", 3, i, 'x', r)
-	fmt.Printf("%s %s", "hi", s)
+	fmt.Printf("%s %s %s", "hi", s, []byte{65})
 	fmt.Printf("%t %t", true, b)
 	fmt.Printf("%T %T", 3, i)
 	fmt.Printf("%U %U", 3, i)
@@ -78,8 +81,12 @@ func PrintfTests() {
 	fmt.Printf("%s", &stringerv)
 	fmt.Printf("%T", &stringerv)
 	fmt.Printf("%*%", 2) // Ridiculous but allowed.
+
+	fmt.Printf("%g", 1+2i)
 	// Some bad format/argTypes
 	fmt.Printf("%b", "hi")                     // ERROR "arg .hi. for printf verb %b of wrong type"
+	fmt.Printf("%b", c)                        // ERROR "arg c for printf verb %b of wrong type"
+	fmt.Printf("%b", 1+2i)                     // ERROR "arg 1 \+ 2i for printf verb %b of wrong type"
 	fmt.Printf("%c", 2.3)                      // ERROR "arg 2.3 for printf verb %c of wrong type"
 	fmt.Printf("%d", 2.3)                      // ERROR "arg 2.3 for printf verb %d of wrong type"
 	fmt.Printf("%e", "hi")                     // ERROR "arg .hi. for printf verb %e of wrong type"
@@ -87,15 +94,18 @@ func PrintfTests() {
 	fmt.Printf("%f", "hi")                     // ERROR "arg .hi. for printf verb %f of wrong type"
 	fmt.Printf("%F", 'x')                      // ERROR "arg 'x' for printf verb %F of wrong type"
 	fmt.Printf("%g", "hi")                     // ERROR "arg .hi. for printf verb %g of wrong type"
+	fmt.Printf("%g", imap)                     // ERROR "arg imap for printf verb %g of wrong type"
 	fmt.Printf("%G", i)                        // ERROR "arg i for printf verb %G of wrong type"
 	fmt.Printf("%o", x)                        // ERROR "arg x for printf verb %o of wrong type"
 	fmt.Printf("%p", 23)                       // ERROR "arg 23 for printf verb %p of wrong type"
 	fmt.Printf("%q", x)                        // ERROR "arg x for printf verb %q of wrong type"
 	fmt.Printf("%s", b)                        // ERROR "arg b for printf verb %s of wrong type"
+	fmt.Printf("%s", byte(65))                 // ERROR "arg byte\(65\) for printf verb %s of wrong type"
 	fmt.Printf("%t", 23)                       // ERROR "arg 23 for printf verb %t of wrong type"
 	fmt.Printf("%U", x)                        // ERROR "arg x for printf verb %U of wrong type"
 	fmt.Printf("%x", nil)                      // ERROR "arg nil for printf verb %x of wrong type"
 	fmt.Printf("%X", 2.3)                      // ERROR "arg 2.3 for printf verb %X of wrong type"
+	fmt.Printf("%s", stringerv)                // ERROR "arg stringerv for printf verb %s of wrong type"
 	fmt.Printf("%t", stringerv)                // ERROR "arg stringerv for printf verb %t of wrong type"
 	fmt.Printf("%.*s %d %g", 3, "hi", 23, 'x') // ERROR "arg 'x' for printf verb %g of wrong type"
 	fmt.Println()                              // not an error

cmd/vet/types.go

@@ -69,11 +69,21 @@ var (
 	stringerType       = types.New("interface{ String() string }")
 )
 
-func (f *File) matchArgType(t printfArgType, arg ast.Expr) bool {
-	// TODO: for now, we can only test builtin types, untyped constants, and Stringer/Errors.
-	typ := f.pkg.types[arg]
+// matchArgType reports an error if printf verb t is not appropriate
+// for operand arg.
+//
+// typ is used only for recursive calls; external callers must supply nil.
+//
+// (Recursion arises from the compound types {map,chan,slice} which
+// may be printed with %d etc. if that is appropriate for their element
+// types.)
+func (f *File) matchArgType(t printfArgType, typ types.Type, arg ast.Expr) bool {
 	if typ == nil {
-		return true
+		// external call
+		typ = f.pkg.types[arg]
+		if typ == nil {
+			return true // probably a type check problem
+		}
 	}
 	// If we can use a string, does arg implement the Stringer or Error interface?
 	if t&argString != 0 {
@@ -81,48 +91,81 @@ func (f *File) matchArgType(t printfArgType, arg ast.Expr) bool {
 			return true
 		}
 	}
-	basic, ok := typ.Underlying().(*types.Basic)
-	if !ok {
+
+	switch typ := typ.Underlying().(type) {
+	case *types.Signature:
+		return t&argPointer != 0
+
+	case *types.Map:
+		// Recurse: map[int]int matches %d.
+		return t&argPointer != 0 ||
+			(f.matchArgType(t, typ.Key(), arg) && f.matchArgType(t, typ.Elem(), arg))
+
+	case *types.Chan:
+		return t&argPointer != 0
+
+	case *types.Slice:
+		if types.IsIdentical(typ.Elem().Underlying(), types.Typ[types.Byte]) && t&argString != 0 {
+			return true // %s matches []byte
+		}
+		// Recurse: []int matches %d.
+		return t&argPointer != 0 || f.matchArgType(t, typ.Elem(), arg)
+
+	case *types.Basic:
+		switch typ.Kind() {
+		case types.UntypedBool,
+			types.Bool:
+			return t&argBool != 0
+
+		case types.UntypedInt,
+			types.Int,
+			types.Int8,
+			types.Int16,
+			types.Int32,
+			types.Int64,
+			types.Uint,
+			types.Uint8,
+			types.Uint16,
+			types.Uint32,
+			types.Uint64,
+			types.Uintptr:
+			return t&argInt != 0
+
+		case types.UntypedFloat,
+			types.Float32,
+			types.Float64:
+			return t&argFloat != 0
+
+		case types.UntypedComplex,
+			types.Complex64,
+			types.Complex128:
+			return t&argComplex != 0
+
+		case types.UntypedString,
+			types.String:
+			return t&argString != 0
+
+		case types.UnsafePointer:
+			return t&(argPointer|argInt) != 0
+
+		case types.UntypedRune:
+			return t&(argInt|argRune) != 0
+
+		case types.UntypedNil:
+			return t&argPointer != 0 // TODO?
+
+		case types.Invalid:
+			if *verbose {
+				f.Warnf(arg.Pos(), "printf argument %v has invalid or unknown type", f.gofmt(arg))
+			}
+			return true // Probably a type check problem.
+		}
+		panic("unreachable")
+
+	default:
 		return true
 	}
-	switch basic.Kind() {
-	case types.Bool:
-		return t&argBool != 0
-	case types.Int, types.Int8, types.Int16, types.Int32, types.Int64:
-		fallthrough
-	case types.Uint, types.Uint8, types.Uint16, types.Uint32, types.Uint64, types.Uintptr:
-		return t&argInt != 0
-	case types.Float32, types.Float64, types.Complex64, types.Complex128:
-		return t&argFloat != 0
-	case types.String:
-		return t&argString != 0
-	case types.UnsafePointer:
-		return t&(argPointer|argInt) != 0
-	case types.UntypedBool:
-		return t&argBool != 0
-	case types.UntypedComplex:
-		return t&argFloat != 0
-	case types.UntypedFloat:
-		// If it's integral, we can use an int format.
-		switch f.pkg.values[arg].Kind() {
-		case exact.Int:
-			return t&(argInt|argFloat) != 0
-		}
-		return t&argFloat != 0
-	case types.UntypedInt:
-		return t&argInt != 0
-	case types.UntypedRune:
-		return t&(argInt|argRune) != 0
-	case types.UntypedString:
-		return t&argString != 0
-	case types.UntypedNil:
-		return t&argPointer != 0 // TODO?
-	case types.Invalid:
-		if *verbose {
-			f.Warnf(arg.Pos(), "printf argument %v has invalid or unknown type", arg)
-		}
-		return true // Probably a type check problem.
-	}
+
 	return false
 }
 
@@ -186,10 +229,5 @@ func (f *File) isErrorMethodCall(call *ast.CallExpr) bool {
 		return false
 	}
 	// It must have return type "string" from the universe.
-	return isUniverseString(sig.Results().At(0).Type())
-}
-
-// isUniverseString reports whether the type is the predeclared type "string".
-func isUniverseString(typ types.Type) bool {
-	return types.IsIdentical(typ, types.Typ[types.String])
+	return sig.Results().At(0).Type() == types.Typ[types.String]
 }