cmd/vet: properly handle indexed arguments in printf

Fixes #15884

Change-Id: I33d98db861d74e3c37a546efaf83ce6f2f76d335
Reviewed-on: https://go-review.googlesource.com/24391
Run-TryBot: Rob Pike <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rob Pike <[email protected]>

Author: valyala

src/cmd/vet/print.go

@@ -194,7 +194,6 @@ type formatState struct {
 	name     string // Printf, Sprintf etc.
 	flags    []byte // the list of # + etc.
 	argNums  []int  // the successive argument numbers that are consumed, adjusted to refer to actual arg in call
-	indexed  bool   // whether an indexing expression appears: %[1]d.
 	firstArg int    // Index of first argument after the format in the Printf call.
 	// Used only during parse.
 	file         *File
@@ -223,7 +222,7 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string) {
 	}
 	// Hard part: check formats against args.
 	argNum := firstArg
-	indexed := false
+	maxArgNum := firstArg
 	for i, w := 0, 0; i < len(format); i += w {
 		w = 1
 		if format[i] == '%' {
@@ -232,26 +231,27 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string) {
 				return
 			}
 			w = len(state.format)
-			if state.indexed {
-				indexed = true
-			}
 			if !f.okPrintfArg(call, state) { // One error per format is enough.
 				return
 			}
 			if len(state.argNums) > 0 {
 				// Continue with the next sequential argument.
 				argNum = state.argNums[len(state.argNums)-1] + 1
 			}
+			for _, n := range state.argNums {
+				if n >= maxArgNum {
+					maxArgNum = n + 1
+				}
+			}
 		}
 	}
 	// Dotdotdot is hard.
-	if call.Ellipsis.IsValid() && argNum >= len(call.Args)-1 {
+	if call.Ellipsis.IsValid() && maxArgNum >= len(call.Args)-1 {
 		return
 	}
-	// If the arguments were direct indexed, we assume the programmer knows what's up.
-	// Otherwise, there should be no leftover arguments.
-	if !indexed && argNum != len(call.Args) {
-		expect := argNum - firstArg
+	// There should be no leftover arguments.
+	if maxArgNum != len(call.Args) {
+		expect := maxArgNum - firstArg
 		numArgs := len(call.Args) - firstArg
 		f.Badf(call.Pos(), "wrong number of args for format in %s call: %d needed but %d args", name, expect, numArgs)
 	}
@@ -286,17 +286,20 @@ func (s *formatState) parseIndex() bool {
 		return true
 	}
 	// Argument index present.
-	s.indexed = true
 	s.nbytes++ // skip '['
 	start := s.nbytes
 	s.scanNum()
 	if s.nbytes == len(s.format) || s.nbytes == start || s.format[s.nbytes] != ']' {
-		s.file.Badf(s.call.Pos(), "illegal syntax for printf argument index")
+		end := strings.Index(s.format, "]")
+		if end < 0 {
+			end = len(s.format)
+		}
+		s.file.Badf(s.call.Pos(), "bad syntax for printf argument index: [%s]", s.format[start:end])
 		return false
 	}
 	arg32, err := strconv.ParseInt(s.format[start:s.nbytes], 10, 32)
 	if err != nil {
-		s.file.Badf(s.call.Pos(), "illegal syntax for printf argument index: %s", err)
+		s.file.Badf(s.call.Pos(), "bad syntax for printf argument index: %s", err)
 		return false
 	}
 	s.nbytes++ // skip ']'
@@ -349,14 +352,12 @@ func (f *File) parsePrintfVerb(call *ast.CallExpr, name, format string, firstArg
 		argNum:   argNum,
 		argNums:  make([]int, 0, 1),
 		nbytes:   1, // There's guaranteed to be a percent sign.
-		indexed:  false,
 		firstArg: firstArg,
 		file:     f,
 		call:     call,
 	}
 	// There may be flags.
 	state.parseFlags()
-	indexPending := false
 	// There may be an index.
 	if !state.parseIndex() {
 		return nil
@@ -370,7 +371,7 @@ func (f *File) parsePrintfVerb(call *ast.CallExpr, name, format string, firstArg
 		return nil
 	}
 	// Now a verb, possibly prefixed by an index (which we may already have).
-	if !indexPending && !state.parseIndex() {
+	if !state.indexPending && !state.parseIndex() {
 		return nil
 	}
 	if state.nbytes == len(state.format) {

src/cmd/vet/testdata/print.go

@@ -174,8 +174,8 @@ func PrintfTests() {
 	Printf("%[2]*.[1]*[3]d", 2, 3, 4)
 	fmt.Fprintf(os.Stderr, "%[2]*.[1]*[3]d", 2, 3, 4) // Use Fprintf to make sure we count arguments correctly.
 	// Bad argument reorderings.
-	Printf("%[xd", 3)                    // ERROR "illegal syntax for printf argument index"
-	Printf("%[x]d", 3)                   // ERROR "illegal syntax for printf argument index"
+	Printf("%[xd", 3)                    // ERROR "bad syntax for printf argument index: \[xd\]"
+	Printf("%[x]d", 3)                   // ERROR "bad syntax for printf argument index: \[x\]"
 	Printf("%[3]*s", "hi", 2)            // ERROR "missing argument for Printf.* reads arg 3, have only 2"
 	_ = fmt.Sprintf("%[3]d", 2)          // ERROR "missing argument for Sprintf.* reads arg 3, have only 1"
 	Printf("%[2]*.[1]*[3]d", 2, "hi", 4) // ERROR "arg .hi. for \* in printf format not of type int"
@@ -249,6 +249,15 @@ func PrintfTests() {
 	ss.log(someFunction)                 // OK
 	ss.log(someFunction, "bar", 1.33)    // OK
 	ss.log(someFunction, someFunction)   // ERROR "arg someFunction in log call is a function value, not a function call"
+
+	// indexed arguments
+	Printf("%d %[3]d %d %[2]d", 1, 2, 3, 4)             // OK
+	Printf("%d %[0]d %d %[2]d", 1, 2, 3, 4)             // ERROR "indexes start at 1"
+	Printf("%d %[3]d %d %[-2]d", 1, 2, 3, 4)            // ERROR "bad syntax for printf argument index: \[-2\]"
+	Printf("%d %[3]d %d %[2234234234234]d", 1, 2, 3, 4) // ERROR "bad syntax for printf argument index: .+ value out of range"
+	Printf("%d %[3]d %d %[2]d", 1, 2, 3)                // ERROR "format reads arg 4, have only 3 args"
+	Printf("%d %[3]d %d %[2]d", 1, 2, 3, 4, 5)          // ERROR "wrong number of args for format in Printf call: 4 needed but 5 args"
+	Printf("%[1][3]d", 1, 2)                            // ERROR "unrecognized printf verb '\['"
 }
 
 type someStruct struct{}