Commit 662d2535 authored by Dhananjay Nakrani's avatar Dhananjay Nakrani Committed by Rob Pike

cmd/vet: ignore unrecognized verbs for fmt.Formatter

Updates #17057.

Change-Id: I54c838d3a44007d4023754e42971e91bfb5e8612
Reviewed-on: https://go-review.googlesource.com/32851
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarRob Pike <r@golang.org>
parent 524cd485
...@@ -192,6 +192,12 @@ func isStringer(f *File, d *ast.FuncDecl) bool { ...@@ -192,6 +192,12 @@ func isStringer(f *File, d *ast.FuncDecl) bool {
f.pkg.types[d.Type.Results.List[0].Type].Type == types.Typ[types.String] f.pkg.types[d.Type.Results.List[0].Type].Type == types.Typ[types.String]
} }
// isFormatter reports whether t satisfies fmt.Formatter.
// Unlike fmt.Stringer, it's impossible to satisfy fmt.Formatter without importing fmt.
func (f *File) isFormatter(t types.Type) bool {
return formatterType != nil && types.Implements(t, formatterType)
}
// formatState holds the parsed representation of a printf directive such as "%3.*[4]d". // formatState holds the parsed representation of a printf directive such as "%3.*[4]d".
// It is constructed by parsePrintfVerb. // It is constructed by parsePrintfVerb.
type formatState struct { type formatState struct {
...@@ -423,8 +429,6 @@ const ( ...@@ -423,8 +429,6 @@ const (
) )
// printVerbs identifies which flags are known to printf for each verb. // printVerbs identifies which flags are known to printf for each verb.
// TODO: A type that implements Formatter may do what it wants, and vet
// will complain incorrectly.
var printVerbs = []printVerb{ var printVerbs = []printVerb{
// '-' is a width modifier, always valid. // '-' is a width modifier, always valid.
// '.' is a precision for float, max width for strings. // '.' is a precision for float, max width for strings.
...@@ -466,7 +470,16 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) { ...@@ -466,7 +470,16 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) {
break break
} }
} }
if !found {
// Does current arg implement fmt.Formatter?
formatter := false
if state.argNum < len(call.Args) {
if tv, ok := f.pkg.types[call.Args[state.argNum]]; ok {
formatter = f.isFormatter(tv.Type)
}
}
if !found && !formatter {
f.Badf(call.Pos(), "unrecognized printf verb %q", state.verb) f.Badf(call.Pos(), "unrecognized printf verb %q", state.verb)
return false return false
} }
...@@ -494,7 +507,7 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) { ...@@ -494,7 +507,7 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) {
return false return false
} }
} }
if state.verb == '%' { if state.verb == '%' || formatter {
return true return true
} }
argNum := state.argNums[len(state.argNums)-1] argNum := state.argNums[len(state.argNums)-1]
......
...@@ -128,8 +128,10 @@ func PrintfTests() { ...@@ -128,8 +128,10 @@ func PrintfTests() {
fmt.Printf("%t", stringerarrayv) // ERROR "arg stringerarrayv for printf verb %t of wrong type" fmt.Printf("%t", stringerarrayv) // ERROR "arg stringerarrayv for printf verb %t of wrong type"
fmt.Printf("%t", notstringerarrayv) // ERROR "arg notstringerarrayv for printf verb %t of wrong type" fmt.Printf("%t", notstringerarrayv) // ERROR "arg notstringerarrayv for printf verb %t of wrong type"
fmt.Printf("%q", notstringerarrayv) // ERROR "arg notstringerarrayv for printf verb %q of wrong type" fmt.Printf("%q", notstringerarrayv) // ERROR "arg notstringerarrayv for printf verb %q of wrong type"
fmt.Printf("%d", Formatter(true)) // correct (the type is responsible for formatting) fmt.Printf("%d", Formatter(true)) // ERROR "arg Formatter\(true\) for printf verb %d of wrong type: testdata.Formatter"
fmt.Printf("%s", nonemptyinterface) // correct (the dynamic type of nonemptyinterface may be a stringer) fmt.Printf("%z", FormatterVal(true)) // correct (the type is responsible for formatting)
fmt.Printf("%d", FormatterVal(true)) // correct (the type is responsible for formatting)
fmt.Printf("%s", nonemptyinterface) // correct (the type is responsible for formatting)
fmt.Printf("%.*s %d %g", 3, "hi", 23, 'x') // ERROR "arg 'x' for printf verb %g 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 fmt.Println() // not an error
fmt.Println("%s", "hi") // ERROR "possible formatting directive in Println call" fmt.Println("%s", "hi") // ERROR "possible formatting directive in Println call"
...@@ -416,6 +418,12 @@ type Formatter bool ...@@ -416,6 +418,12 @@ type Formatter bool
func (*Formatter) Format(fmt.State, rune) { func (*Formatter) Format(fmt.State, rune) {
} }
// Formatter with value receiver
type FormatterVal bool
func (FormatterVal) Format(fmt.State, rune) {
}
type RecursiveSlice []RecursiveSlice type RecursiveSlice []RecursiveSlice
var recursiveSliceV = &RecursiveSlice{} var recursiveSliceV = &RecursiveSlice{}
......
...@@ -113,8 +113,7 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp ...@@ -113,8 +113,7 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp
} }
} }
// If the type implements fmt.Formatter, we have nothing to check. // If the type implements fmt.Formatter, we have nothing to check.
// formatterTyp may be nil - be conservative and check for Format method in that case. if f.isFormatter(typ) {
if formatterType != nil && types.Implements(typ, formatterType) || f.hasMethod(typ, "Format") {
return true return true
} }
// If we can use a string, might arg (dynamically) implement the Stringer or Error interface? // If we can use a string, might arg (dynamically) implement the Stringer or Error interface?
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment