diff --git a/src/cmd/govet/Makefile b/src/cmd/govet/Makefile index 1713ea9bb1..dae3ae51d5 100644 --- a/src/cmd/govet/Makefile +++ b/src/cmd/govet/Makefile @@ -14,4 +14,4 @@ GOFILES=\ include ../../Make.cmd test testshort: $(TARG) - ../../../test/errchk $(TARG) -printfuncs='Warn:1,Warnf:1' govet.go + ../../../test/errchk $(TARG) -printfuncs='Warn:1,Warnf:1' print.go diff --git a/src/cmd/govet/print.go b/src/cmd/govet/print.go index 116d2d670d..861a337c6f 100644 --- a/src/cmd/govet/print.go +++ b/src/cmd/govet/print.go @@ -67,7 +67,7 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string, skip int) { if !ok { // Too hard to check. if *verbose { - f.Warn(call.Pos(), "can't check args for call to", name) + f.Warn(call.Pos(), "can't check non-literal format in call to", name) } return } @@ -85,7 +85,7 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string, skip int) { for i, w := 0, 0; i < len(lit.Value); i += w { w = 1 if lit.Value[i] == '%' { - nbytes, nargs := parsePrintfVerb(lit.Value[i:]) + nbytes, nargs := f.parsePrintfVerb(call, lit.Value[i:]) w = nbytes numArgs += nargs } @@ -99,8 +99,9 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string, skip int) { // parsePrintfVerb returns the number of bytes and number of arguments // consumed by the Printf directive that begins s, including its percent sign // and verb. -func parsePrintfVerb(s string) (nbytes, nargs int) { +func (f *File) parsePrintfVerb(call *ast.CallExpr, s string) (nbytes, nargs int) { // There's guaranteed a percent sign. + flags := make([]byte, 0, 5) nbytes = 1 end := len(s) // There may be flags. @@ -108,6 +109,7 @@ FlagLoop: for nbytes < end { switch s[nbytes] { case '#', '0', '+', '-', ' ': + flags = append(flags, s[nbytes]) nbytes++ default: break FlagLoop @@ -127,6 +129,7 @@ FlagLoop: getNum() // If there's a period, there may be a precision. if nbytes < end && s[nbytes] == '.' { + flags = append(flags, '.') // Treat precision as a flag. nbytes++ getNum() } @@ -135,10 +138,70 @@ FlagLoop: nbytes += w if c != '%' { nargs++ + f.checkPrintfVerb(call, c, flags) } return } +type printVerb struct { + verb rune + flags string // known flags are all ASCII +} + +// Common flag sets for printf verbs. +const ( + numFlag = " -+.0" + sharpNumFlag = " -+.0#" + allFlags = " -+.0#" +) + +// printVerbs identifies which flags are known to printf for each verb. +// TODO: A type that implements Formatter may do what it wants, and govet +// will complain incorrectly. +var printVerbs = []printVerb{ + // '-' is a width modifier, always valid. + // '.' is a precision for float, max width for strings. + // '+' is required sign for numbers, Go format for %v. + // '#' is alternate format for several verbs. + // ' ' is spacer for numbers + {'b', numFlag}, + {'c', "-"}, + {'d', numFlag}, + {'e', "-."}, + {'E', numFlag}, + {'f', numFlag}, + {'F', numFlag}, + {'g', numFlag}, + {'G', numFlag}, + {'o', sharpNumFlag}, + {'p', "-#"}, + {'q', "-+#."}, + {'s', "-."}, + {'t', "-"}, + {'T', "-"}, + {'U', "-#"}, + {'v', allFlags}, + {'x', sharpNumFlag}, + {'X', sharpNumFlag}, +} + +const printfVerbs = "bcdeEfFgGopqstTvxUX" + +func (f *File) checkPrintfVerb(call *ast.CallExpr, verb rune, flags []byte) { + // Linear scan is fast enough for a small list. + for _, v := range printVerbs { + if v.verb == verb { + for _, flag := range flags { + if !strings.ContainsRune(v.flags, rune(flag)) { + f.Badf(call.Pos(), "unrecognized printf flag for verb %q: %q", verb, flag) + } + } + return + } + } + f.Badf(call.Pos(), "unrecognized printf verb %q", verb) +} + // checkPrint checks a call to an unformatted print routine such as Println. // The skip argument records how many arguments to ignore; that is, // call.Args[skip] is the first argument to be printed. @@ -183,6 +246,8 @@ func BadFunctionUsedInTests() { f := new(File) f.Warn(0, "%s", "hello", 3) // ERROR "possible formatting directive in Warn call" f.Warnf(0, "%s", "hello", 3) // ERROR "wrong number of args in Warnf call" + f.Warnf(0, "%r", "hello") // ERROR "unrecognized printf verb" + f.Warnf(0, "%#s", "hello") // ERROR "unrecognized printf flag" } type BadTypeUsedInTests struct { diff --git a/src/pkg/encoding/xml/marshal_test.go b/src/pkg/encoding/xml/marshal_test.go index 8040765801..6a241694ba 100644 --- a/src/pkg/encoding/xml/marshal_test.go +++ b/src/pkg/encoding/xml/marshal_test.go @@ -394,7 +394,7 @@ func TestUnmarshal(t *testing.T) { if err != nil { t.Errorf("#%d: unexpected error: %#v", i, err) } else if got, want := dest, test.Value; !reflect.DeepEqual(got, want) { - t.Errorf("#%d: unmarshal(%#s) = %#v, want %#v", i, test.ExpectXML, got, want) + t.Errorf("#%d: unmarshal(%q) = %#v, want %#v", i, test.ExpectXML, got, want) } } } diff --git a/src/pkg/net/http/readrequest_test.go b/src/pkg/net/http/readrequest_test.go index c64fff6109..ad7e3c02b0 100644 --- a/src/pkg/net/http/readrequest_test.go +++ b/src/pkg/net/http/readrequest_test.go @@ -219,7 +219,7 @@ func TestReadRequest(t *testing.T) { t.Errorf("#%d: Body = %q want %q", i, body, tt.Body) } if !reflect.DeepEqual(tt.Trailer, req.Trailer) { - t.Errorf("%#d. Trailers differ.\n got: %v\nwant: %v", i, req.Trailer, tt.Trailer) + t.Errorf("#%d. Trailers differ.\n got: %v\nwant: %v", i, req.Trailer, tt.Trailer) } } } diff --git a/src/pkg/net/textproto/reader_test.go b/src/pkg/net/textproto/reader_test.go index 5aefe39867..0460c1c8de 100644 --- a/src/pkg/net/textproto/reader_test.go +++ b/src/pkg/net/textproto/reader_test.go @@ -203,7 +203,7 @@ func TestRFC959Lines(t *testing.T) { t.Errorf("#%d: code=%d, want %d", i, code, tt.wantCode) } if msg != tt.wantMsg { - t.Errorf("%#d: msg=%q, want %q", i, msg, tt.wantMsg) + t.Errorf("#%d: msg=%q, want %q", i, msg, tt.wantMsg) } } } diff --git a/src/pkg/os/os_test.go b/src/pkg/os/os_test.go index d107020449..2a666f780e 100644 --- a/src/pkg/os/os_test.go +++ b/src/pkg/os/os_test.go @@ -919,7 +919,7 @@ func TestReadAt(t *testing.T) { b := make([]byte, 5) n, err := f.ReadAt(b, 7) if err != nil || n != len(b) { - t.Fatalf("ReadAt 7: %d, %r", n, err) + t.Fatalf("ReadAt 7: %d, %v", n, err) } if string(b) != "world" { t.Fatalf("ReadAt 7: have %q want %q", string(b), "world")