]> Cypherpunks repositories - gostls13.git/commitdiff
govet: add checking for printf verbs
authorRob Pike <r@golang.org>
Thu, 15 Dec 2011 23:17:52 +0000 (15:17 -0800)
committerRob Pike <r@golang.org>
Thu, 15 Dec 2011 23:17:52 +0000 (15:17 -0800)
Also fix the errors it catches.

Fixes #1654.

R=rsc
CC=golang-dev
https://golang.org/cl/5489060

src/cmd/govet/Makefile
src/cmd/govet/print.go
src/pkg/encoding/xml/marshal_test.go
src/pkg/net/http/readrequest_test.go
src/pkg/net/textproto/reader_test.go
src/pkg/os/os_test.go

index 1713ea9bb13c06b95e22fecc1b0e4ded8f3290a4..dae3ae51d53c8234bfc48d695608b0a27f3fbc51 100644 (file)
@@ -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
index 116d2d670d3678f8b9b6192396928afa045f0f4a..861a337c6f9e373a59f0d058d7a8e2e310c5020b 100644 (file)
@@ -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 {
index 804076580155a77dd22ea07bcf028905a2be4ac4..6a241694baf85651ad1082c7f354975fa599e4d0 100644 (file)
@@ -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)
                }
        }
 }
index c64fff6109f2270564b11878fce42505d09129f2..ad7e3c02b0ca0f382e3746319cefdcb725d23407 100644 (file)
@@ -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)
                }
        }
 }
index 5aefe39867df4479497e18882bb1a7e5ffe3b84a..0460c1c8deeb5557c07524ebed9a39f1ee9e7dd3 100644 (file)
@@ -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)
                }
        }
 }
index d107020449a92ace8ca4f5912fb880f887a86bc2..2a666f780e699723e83c157e92fccb1865d25109 100644 (file)
@@ -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")