Anonymous avatar Anonymous committed 29665c4 Draft

vet: check values for named constants as well as literals.
As in:
const format = "%s"
fmt.Printf(format, "hi")
Also fix a couple of bugs by rewriting the routine.

R=golang-dev, rsc
CC=golang-dev
http://codereview.appspot.com/6099057

Comments (0)

Files changed (1)

src/cmd/vet/print.go

 	}
 }
 
+// literal returns the literal value represented by the expression, or nil if it is not a literal.
+func (f *File) literal(value ast.Expr) *ast.BasicLit {
+	switch v := value.(type) {
+	case *ast.BasicLit:
+		return v
+	case *ast.Ident:
+		// See if it's a constant or initial value (we can't tell the difference).
+		if v.Obj == nil || v.Obj.Decl == nil {
+			return nil
+		}
+		valueSpec, ok := v.Obj.Decl.(*ast.ValueSpec)
+		if ok && len(valueSpec.Names) == len(valueSpec.Values) {
+			// Find the index in the list of names
+			var i int
+			for i = 0; i < len(valueSpec.Names); i++ {
+				if valueSpec.Names[i].Name == v.Name {
+					if lit, ok := valueSpec.Values[i].(*ast.BasicLit); ok {
+						return lit
+					}
+					return nil
+				}
+			}
+		}
+	}
+	return nil
+}
+
 // checkPrintf checks a call to a formatted print routine such as Printf.
 // The skip argument records how many arguments to ignore; that is,
 // call.Args[skip] is (well, should be) the format argument.
 	if len(call.Args) <= skip {
 		return
 	}
-	// Common case: literal is first argument.
-	arg := call.Args[skip]
-	lit, ok := arg.(*ast.BasicLit)
-	if !ok {
-		// Too hard to check.
+	lit := f.literal(call.Args[skip])
+	if lit == nil {
 		if *verbose {
 			f.Warn(call.Pos(), "can't check non-literal format in call to", name)
 		}
 		return
 	}
-	if lit.Kind == token.STRING {
-		if !strings.Contains(lit.Value, "%") {
-			if len(call.Args) > skip+1 {
-				f.Badf(call.Pos(), "no formatting directive in %s call", name)
-			}
-			return
+	if lit.Kind != token.STRING {
+		f.Badf(call.Pos(), "literal %v not a string in call to", lit.Value, name)
+	}
+	format := lit.Value
+	if !strings.Contains(format, "%") {
+		if len(call.Args) > skip+1 {
+			f.Badf(call.Pos(), "no formatting directive in %s call", name)
 		}
+		return
 	}
 	// Hard part: check formats against args.
 	// Trivial but useful test: count.
 	numArgs := 0
-	for i, w := 0, 0; i < len(lit.Value); i += w {
+	for i, w := 0, 0; i < len(format); i += w {
 		w = 1
-		if lit.Value[i] == '%' {
-			nbytes, nargs := f.parsePrintfVerb(call, lit.Value[i:])
+		if format[i] == '%' {
+			nbytes, nargs := f.parsePrintfVerb(call, format[i:])
 			w = nbytes
 			numArgs += nargs
 		}
 	printf("now is the time", "buddy") // ERROR "no formatting directive"
 	Printf("now is the time", "buddy") // ERROR "no formatting directive"
 	Printf("hi")                       // ok
+	const format = "%s %s\n"
+	Printf(format, "hi", "there")
+	Printf(format, "hi") // ERROR "wrong number of args in Printf call"
 	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"
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.