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

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 a5f075c30009 https://code.google.com/p/go.tools Created 10 years, 8 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:
Left: Side by side diff | Download
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
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 13 matching lines...) Expand all
40 "panic": 0, "panicln": 0, 42 "panic": 0, "panicln": 0,
41 "print": 0, "println": 0, 43 "print": 0, "println": 0,
42 "sprint": 0, "sprintln": 0, 44 "sprint": 0, "sprintln": 0,
43 } 45 }
44 46
45 // checkCall triggers the print-specific checks if the call invokes a print func tion. 47 // checkCall triggers the print-specific checks if the call invokes a print func tion.
46 func (f *File) checkFmtPrintfCall(call *ast.CallExpr, Name string) { 48 func (f *File) checkFmtPrintfCall(call *ast.CallExpr, Name string) {
47 if !vet("printf") { 49 if !vet("printf") {
48 return 50 return
49 } 51 }
50
51 name := strings.ToLower(Name) 52 name := strings.ToLower(Name)
52 if skip, ok := printfList[name]; ok { 53 if skip, ok := printfList[name]; ok {
53 f.checkPrintf(call, Name, skip) 54 f.checkPrintf(call, Name, skip)
54 return 55 return
55 } 56 }
56 if skip, ok := printList[name]; ok { 57 if skip, ok := printList[name]; ok {
57 f.checkPrint(call, Name, skip) 58 f.checkPrint(call, Name, skip)
58 return 59 return
59 } 60 }
60 }
61
62 // literal returns the literal value represented by the expression, or nil if it is not a literal.
63 // TODO(adonovan): the typechecker provides this information directly; use it.
64 func (f *File) literal(value ast.Expr) *ast.BasicLit {
65 switch v := value.(type) {
66 case *ast.BasicLit:
67 return v
68 case *ast.ParenExpr:
69 return f.literal(v.X)
70 case *ast.BinaryExpr:
71 if v.Op != token.ADD {
72 break
73 }
74 litX := f.literal(v.X)
75 litY := f.literal(v.Y)
76 if litX != nil && litY != nil {
77 lit := *litX
78 x, errX := strconv.Unquote(litX.Value)
79 y, errY := strconv.Unquote(litY.Value)
80 if errX == nil && errY == nil {
81 return &ast.BasicLit{
82 ValuePos: lit.ValuePos,
83 Kind: lit.Kind,
84 Value: strconv.Quote(x + y),
85 }
86 }
87 }
88 case *ast.Ident:
89 // See if it's a constant or initial value (we can't tell the di fference).
90 if v.Obj == nil || v.Obj.Decl == nil {
91 return nil
92 }
93 valueSpec, ok := v.Obj.Decl.(*ast.ValueSpec)
94 if ok && len(valueSpec.Names) == len(valueSpec.Values) {
95 // Find the index in the list of names
96 var i int
97 for i = 0; i < len(valueSpec.Names); i++ {
98 if valueSpec.Names[i].Name == v.Name {
99 if lit, ok := valueSpec.Values[i].(*ast. BasicLit); ok {
100 return lit
101 }
102 return nil
103 }
104 }
105 }
106 }
107 return nil
108 } 61 }
109 62
110 // 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".
111 // It is constructed by parsePrintfVerb. 64 // It is constructed by parsePrintfVerb.
112 type formatState struct { 65 type formatState struct {
113 verb rune // the format verb: 'd' for "%d" 66 verb rune // the format verb: 'd' for "%d"
114 format string // the full format string 67 format string // the full format string
115 name string // Printf, Sprintf etc. 68 name string // Printf, Sprintf etc.
116 flags []byte // the list of # + etc. 69 flags []byte // the list of # + etc.
117 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
118 nbytes int // number of bytes of the format string consumed. 71 nbytes int // number of bytes of the format string consumed.
119 indexed bool // whether an indexing expression appears: %[1]d. 72 indexed bool // whether an indexing expression appears: %[1]d.
120 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.
121 // Used only during parse. 74 // Used only during parse.
122 file *File 75 file *File
123 call *ast.CallExpr 76 call *ast.CallExpr
124 argNum int // Which argument we're expecting to format now. 77 argNum int // Which argument we're expecting to format now.
125 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.
126 } 79 }
127 80
128 // 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.
129 // call.Args[formatIndex] is (well, should be) the format argument. 82 // call.Args[formatIndex] is (well, should be) the format argument.
130 func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) { 83 func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) {
131 if formatIndex >= len(call.Args) { 84 if formatIndex >= len(call.Args) {
132 f.Warn(call.Pos(), "too few arguments in call to", name) 85 f.Warn(call.Pos(), "too few arguments in call to", name)
133 return 86 return
134 } 87 }
135 » // TODO(adonovan): use typechecker constant folding. 88 » lit := f.pkg.values[call.Args[formatIndex]]
136 » lit := f.literal(call.Args[formatIndex])
137 if lit == nil { 89 if lit == nil {
138 if *verbose { 90 if *verbose {
139 » » » 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)
140 » » } 92 » » }
141 » » return 93 » » return
142 » } 94 » }
143 » if lit.Kind != token.STRING { 95 » if lit.Kind() != exact.String {
144 » » 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)
145 » } 97 » }
146 » format, err := strconv.Unquote(lit.Value) 98 » format := exact.StringVal(lit)
147 » if err != nil {
148 » » // Shouldn't happen if parser returned no errors, but be safe.
149 » » f.Badf(call.Pos(), "invalid quoted string literal")
150 » }
151 firstArg := formatIndex + 1 // Arguments are immediately after format st ring. 99 firstArg := formatIndex + 1 // Arguments are immediately after format st ring.
152 if !strings.Contains(format, "%") { 100 if !strings.Contains(format, "%") {
153 if len(call.Args) > firstArg { 101 if len(call.Args) > firstArg {
154 f.Badf(call.Pos(), "no formatting directive in %s call", name) 102 f.Badf(call.Pos(), "no formatting directive in %s call", name)
155 } 103 }
156 return 104 return
157 } 105 }
158 // Hard part: check formats against args. 106 // Hard part: check formats against args.
159 argNum := firstArg 107 argNum := firstArg
160 indexed := false 108 indexed := false
(...skipping 348 matching lines...) Expand 10 before | Expand all | Expand 10 after
509 if isLn { 457 if isLn {
510 // The last item, if a string, should not have a newline. 458 // The last item, if a string, should not have a newline.
511 arg = args[len(call.Args)-1] 459 arg = args[len(call.Args)-1]
512 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 {
513 if strings.HasSuffix(lit.Value, `\n"`) { 461 if strings.HasSuffix(lit.Value, `\n"`) {
514 f.Badf(call.Pos(), "%s call ends with newline", name) 462 f.Badf(call.Pos(), "%s call ends with newline", name)
515 } 463 }
516 } 464 }
517 } 465 }
518 } 466 }
LEFTRIGHT

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