Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(779)

Delta Between Two Patch Sets: cmd/vet/print.go

Issue 10895043: code review 10895043: go.tools/cmd/vet: improvements to static checking of pr... (Closed)
Left Patch Set: diff -r f6fb747a944e https://code.google.com/p/go.tools Created 10 years, 9 months ago
Right Patch Set: diff -r c13a5b840d50 https://code.google.com/p/go.tools Created 10 years, 8 months ago
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | cmd/vet/testdata/print.go » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
(no file at all)
1 // Copyright 2010 The Go Authors. All rights reserved. 1 // Copyright 2010 The Go Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style 2 // Use of this source code is governed by a BSD-style
3 // license that can be found in the LICENSE file. 3 // license that can be found in the LICENSE file.
4 4
5 // This file contains the printf-checker. 5 // This file contains the printf-checker.
6 6
7 package main 7 package main
8 8
9 import ( 9 import (
10 "flag" 10 "flag"
11 "fmt" 11 "fmt"
12 "go/ast" 12 "go/ast"
13 "go/token" 13 "go/token"
14 "strconv" 14 "strconv"
15 "strings" 15 "strings"
16 "unicode/utf8" 16 "unicode/utf8"
17
18 "code.google.com/p/go.tools/go/exact"
17 ) 19 )
18 20
19 var printfuncs = flag.String("printfuncs", "", "comma-separated list of print fu nction names to check") 21 var printfuncs = flag.String("printfuncs", "", "comma-separated list of print fu nction names to check")
20 22
21 // printfList records the formatted-print functions. The value is the location 23 // printfList records the formatted-print functions. The value is the location
22 // of the format parameter. Names are lower-cased so the lookup is 24 // of the format parameter. Names are lower-cased so the lookup is
23 // case insensitive. 25 // case insensitive.
24 var printfList = map[string]int{ 26 var printfList = map[string]int{
25 "errorf": 0, 27 "errorf": 0,
26 "fatalf": 0, 28 "fatalf": 0,
(...skipping 22 matching lines...) Expand all
49 } 51 }
50 name := strings.ToLower(Name) 52 name := strings.ToLower(Name)
51 if skip, ok := printfList[name]; ok { 53 if skip, ok := printfList[name]; ok {
52 f.checkPrintf(call, Name, skip) 54 f.checkPrintf(call, Name, skip)
53 return 55 return
54 } 56 }
55 if skip, ok := printList[name]; ok { 57 if skip, ok := printList[name]; ok {
56 f.checkPrint(call, Name, skip) 58 f.checkPrint(call, Name, skip)
57 return 59 return
58 } 60 }
59 }
60
61 // literal returns the literal value represented by the expression, or nil if it is not a literal.
62 func (f *File) literal(value ast.Expr) *ast.BasicLit {
63 switch v := value.(type) {
64 case *ast.BasicLit:
65 return v
66 case *ast.ParenExpr:
67 return f.literal(v.X)
68 case *ast.BinaryExpr:
69 if v.Op != token.ADD {
70 break
71 }
72 litX := f.literal(v.X)
73 litY := f.literal(v.Y)
74 if litX != nil && litY != nil {
75 lit := *litX
76 x, errX := strconv.Unquote(litX.Value)
77 y, errY := strconv.Unquote(litY.Value)
78 if errX == nil && errY == nil {
79 return &ast.BasicLit{
80 ValuePos: lit.ValuePos,
81 Kind: lit.Kind,
82 Value: strconv.Quote(x + y),
83 }
84 }
85 }
86 case *ast.Ident:
87 // See if it's a constant or initial value (we can't tell the di fference).
88 if v.Obj == nil || v.Obj.Decl == nil {
89 return nil
90 }
91 valueSpec, ok := v.Obj.Decl.(*ast.ValueSpec)
92 if ok && len(valueSpec.Names) == len(valueSpec.Values) {
93 // Find the index in the list of names
94 var i int
95 for i = 0; i < len(valueSpec.Names); i++ {
96 if valueSpec.Names[i].Name == v.Name {
97 if lit, ok := valueSpec.Values[i].(*ast. BasicLit); ok {
98 return lit
99 }
100 return nil
101 }
102 }
103 }
104 }
105 return nil
106 } 61 }
107 62
108 // formatState holds the parsed representation of a printf directive such as "%3 .*[4]d". 63 // formatState holds the parsed representation of a printf directive such as "%3 .*[4]d".
109 // It is constructed by parsePrintfVerb. 64 // It is constructed by parsePrintfVerb.
110 type formatState struct { 65 type formatState struct {
111 verb rune // the format verb: 'd' for "%d" 66 verb rune // the format verb: 'd' for "%d"
112 format string // the full format string 67 format string // the full format string
113 name string // Printf, Sprintf etc. 68 name string // Printf, Sprintf etc.
114 flags []byte // the list of # + etc. 69 flags []byte // the list of # + etc.
115 argNums []int // the successive argument numbers that are consumed, ad justed to refer to actual arg in call 70 argNums []int // the successive argument numbers that are consumed, ad justed to refer to actual arg in call
116 nbytes int // number of bytes of the format string consumed. 71 nbytes int // number of bytes of the format string consumed.
117 indexed bool // whether an indexing expression appears: %[1]d. 72 indexed bool // whether an indexing expression appears: %[1]d.
118 firstArg int // Index of first argument after the format in the Print f call. 73 firstArg int // Index of first argument after the format in the Print f call.
119 // Used only during parse. 74 // Used only during parse.
120 file *File 75 file *File
121 call *ast.CallExpr 76 call *ast.CallExpr
122 argNum int // Which argument we're expecting to format now. 77 argNum int // Which argument we're expecting to format now.
123 indexPending bool // Whether we have an indexed argument that has not re solved. 78 indexPending bool // Whether we have an indexed argument that has not re solved.
124 } 79 }
125 80
126 // checkPrintf checks a call to a formatted print routine such as Printf. 81 // checkPrintf checks a call to a formatted print routine such as Printf.
127 // call.Args[formatIndex] is (well, should be) the format argument. 82 // call.Args[formatIndex] is (well, should be) the format argument.
128 func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) { 83 func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) {
129 if formatIndex >= len(call.Args) { 84 if formatIndex >= len(call.Args) {
130 » » return 85 » » f.Warn(call.Pos(), "too few arguments in call to", name)
131 » } 86 » » return
132 » lit := f.literal(call.Args[formatIndex]) 87 » }
88 » lit := f.pkg.values[call.Args[formatIndex]]
133 if lit == nil { 89 if lit == nil {
134 if *verbose { 90 if *verbose {
135 » » » f.Warn(call.Pos(), "can't check non-literal format in ca ll to", name) 91 » » » f.Warn(call.Pos(), "can't check non-constant format in c all to", name)
136 » » } 92 » » }
137 » » return 93 » » return
138 » } 94 » }
139 » if lit.Kind != token.STRING { 95 » if lit.Kind() != exact.String {
140 » » f.Badf(call.Pos(), "literal %v not a string in call to", lit.Val ue, name) 96 » » f.Badf(call.Pos(), "constant %v not a string in call to", lit, n ame)
141 » } 97 » }
142 » format, err := strconv.Unquote(lit.Value) 98 » format := exact.StringVal(lit)
143 » if err != nil {
144 » » // Shouldn't happen if parser returned no errors, but be safe.
145 » » f.Badf(call.Pos(), "invalid quoted string literal")
146 » }
147 firstArg := formatIndex + 1 // Arguments are immediately after format st ring. 99 firstArg := formatIndex + 1 // Arguments are immediately after format st ring.
148 if !strings.Contains(format, "%") { 100 if !strings.Contains(format, "%") {
149 if len(call.Args) > firstArg { 101 if len(call.Args) > firstArg {
150 f.Badf(call.Pos(), "no formatting directive in %s call", name) 102 f.Badf(call.Pos(), "no formatting directive in %s call", name)
151 } 103 }
152 return 104 return
153 } 105 }
154 // Hard part: check formats against args. 106 // Hard part: check formats against args.
155 argNum := firstArg 107 argNum := firstArg
156 indexed := false 108 indexed := false
(...skipping 161 matching lines...) Expand 10 before | Expand all | Expand 10 after
318 270
319 // printfArgType encodes the types of expressions a printf verb accepts. It is a bitmask. 271 // printfArgType encodes the types of expressions a printf verb accepts. It is a bitmask.
320 type printfArgType int 272 type printfArgType int
321 273
322 const ( 274 const (
323 argBool printfArgType = 1 << iota 275 argBool printfArgType = 1 << iota
324 argInt 276 argInt
325 argRune 277 argRune
326 argString 278 argString
327 argFloat 279 argFloat
280 argComplex
328 argPointer 281 argPointer
329 anyType printfArgType = ^0 282 anyType printfArgType = ^0
330 ) 283 )
331 284
332 type printVerb struct { 285 type printVerb struct {
333 verb rune // User may provide verb through Formatter; could be a rune . 286 verb rune // User may provide verb through Formatter; could be a rune .
334 flags string // known flags are all ASCII 287 flags string // known flags are all ASCII
335 typ printfArgType 288 typ printfArgType
336 } 289 }
337 290
(...skipping 11 matching lines...) Expand all
349 var printVerbs = []printVerb{ 302 var printVerbs = []printVerb{
350 // '-' is a width modifier, always valid. 303 // '-' is a width modifier, always valid.
351 // '.' is a precision for float, max width for strings. 304 // '.' is a precision for float, max width for strings.
352 // '+' is required sign for numbers, Go format for %v. 305 // '+' is required sign for numbers, Go format for %v.
353 // '#' is alternate format for several verbs. 306 // '#' is alternate format for several verbs.
354 // ' ' is spacer for numbers 307 // ' ' is spacer for numbers
355 {'%', noFlag, 0}, 308 {'%', noFlag, 0},
356 {'b', numFlag, argInt | argFloat}, 309 {'b', numFlag, argInt | argFloat},
357 {'c', "-", argRune | argInt}, 310 {'c', "-", argRune | argInt},
358 {'d', numFlag, argInt}, 311 {'d', numFlag, argInt},
359 » {'e', numFlag, argFloat}, 312 » {'e', numFlag, argFloat | argComplex},
360 » {'E', numFlag, argFloat}, 313 » {'E', numFlag, argFloat | argComplex},
361 » {'f', numFlag, argFloat}, 314 » {'f', numFlag, argFloat | argComplex},
362 » {'F', numFlag, argFloat}, 315 » {'F', numFlag, argFloat | argComplex},
363 » {'g', numFlag, argFloat}, 316 » {'g', numFlag, argFloat | argComplex},
364 » {'G', numFlag, argFloat}, 317 » {'G', numFlag, argFloat | argComplex},
365 {'o', sharpNumFlag, argInt}, 318 {'o', sharpNumFlag, argInt},
366 {'p', "-#", argPointer}, 319 {'p', "-#", argPointer},
367 {'q', " -+.0#", argRune | argInt | argString}, 320 {'q', " -+.0#", argRune | argInt | argString},
368 {'s', " -+.0", argString}, 321 {'s', " -+.0", argString},
369 {'t', "-", argBool}, 322 {'t', "-", argBool},
370 {'T', "-", anyType}, 323 {'T', "-", anyType},
371 {'U', "-#", argRune | argInt}, 324 {'U', "-#", argRune | argInt},
372 {'v', allFlags, anyType}, 325 {'v', allFlags, anyType},
373 {'x', sharpNumFlag, argRune | argInt | argString}, 326 {'x', sharpNumFlag, argRune | argInt | argString},
374 {'X', sharpNumFlag, argRune | argInt | argString}, 327 {'X', sharpNumFlag, argRune | argInt | argString},
(...skipping 28 matching lines...) Expand all
403 if state.verb == '%' { 356 if state.verb == '%' {
404 trueArgs = 0 357 trueArgs = 0
405 } 358 }
406 nargs := len(state.argNums) 359 nargs := len(state.argNums)
407 for i := 0; i < nargs-trueArgs; i++ { 360 for i := 0; i < nargs-trueArgs; i++ {
408 argNum := state.argNums[i] 361 argNum := state.argNums[i]
409 if !f.argCanBeChecked(call, i, true, state) { 362 if !f.argCanBeChecked(call, i, true, state) {
410 return 363 return
411 } 364 }
412 arg := call.Args[argNum] 365 arg := call.Args[argNum]
413 » » if !f.matchArgType(argInt, arg) { 366 » » if !f.matchArgType(argInt, nil, arg) {
414 f.Badf(call.Pos(), "arg %s for * in printf format not of type int", f.gofmt(arg)) 367 f.Badf(call.Pos(), "arg %s for * in printf format not of type int", f.gofmt(arg))
415 return false 368 return false
416 } 369 }
417 } 370 }
418 if state.verb == '%' { 371 if state.verb == '%' {
419 return true 372 return true
420 } 373 }
421 argNum := state.argNums[len(state.argNums)-1] 374 argNum := state.argNums[len(state.argNums)-1]
422 if !f.argCanBeChecked(call, len(state.argNums)-1, false, state) { 375 if !f.argCanBeChecked(call, len(state.argNums)-1, false, state) {
423 return false 376 return false
424 } 377 }
425 arg := call.Args[argNum] 378 arg := call.Args[argNum]
426 » if !f.matchArgType(v.typ, arg) { 379 » if !f.matchArgType(v.typ, nil, arg) {
427 typeString := "" 380 typeString := ""
428 if typ := f.pkg.types[arg]; typ != nil { 381 if typ := f.pkg.types[arg]; typ != nil {
429 typeString = typ.String() 382 typeString = typ.String()
430 } 383 }
431 f.Badf(call.Pos(), "arg %s for printf verb %%%c of wrong type: % s", f.gofmt(arg), state.verb, typeString) 384 f.Badf(call.Pos(), "arg %s for printf verb %%%c of wrong type: % s", f.gofmt(arg), state.verb, typeString)
432 return false 385 return false
433 } 386 }
434 return true 387 return true
435 } 388 }
436 389
(...skipping 67 matching lines...) Expand 10 before | Expand all | Expand 10 after
504 if isLn { 457 if isLn {
505 // The last item, if a string, should not have a newline. 458 // The last item, if a string, should not have a newline.
506 arg = args[len(call.Args)-1] 459 arg = args[len(call.Args)-1]
507 if lit, ok := arg.(*ast.BasicLit); ok && lit.Kind == token.STRIN G { 460 if lit, ok := arg.(*ast.BasicLit); ok && lit.Kind == token.STRIN G {
508 if strings.HasSuffix(lit.Value, `\n"`) { 461 if strings.HasSuffix(lit.Value, `\n"`) {
509 f.Badf(call.Pos(), "%s call ends with newline", name) 462 f.Badf(call.Pos(), "%s call ends with newline", name)
510 } 463 }
511 } 464 }
512 } 465 }
513 } 466 }
LEFTRIGHT

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b