text/template/parse: error on bad range variables

The package used to accept invalid range pipelines, such as:

	{{range $k, .}}
	{{range $k, 123 := .}}

This is because the logic that allowed a range pipeline to declare
multiple variables was broken. When encountering a single comma inside a
range pipeline, it would happily continue parsing a second variable,
even if we didn't have a variable token at all.

Then, the loop would immediately break, and we'd parse the pipeline we'd
be ranging over. That is, we'd parse {{range $k, .}} as if it were
{{range $k = .}}.

To fix this, only allow the loop to continue if we know we're going to
parse another variable or a token that would end the pipeline. Also add
a few test cases for these error edge cases.

While at it, make use of T.Run, which was useful in debugging
Tree.pipeline via print statements.

Fixes #28437.

Change-Id: Idc9966bf643f0f3bc1b052620357e5b0aa2022ea
Reviewed-on: https://go-review.googlesource.com/c/145282
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>

src/text/template/parse/parse.go

@@ -385,6 +385,7 @@ func (t *Tree) pipeline(context string) (pipe *PipeNode) {
 	token := t.peekNonSpace()
 	pos := token.pos
 	// Are there declarations or assignments?
+	// TODO(mvdan): simplify the loop break/continue logic
 	for {
 		if v := t.peekNonSpace(); v.typ == itemVariable {
 			t.next()
@@ -406,7 +407,12 @@ func (t *Tree) pipeline(context string) (pipe *PipeNode) {
 				}
 				if next.typ == itemChar && next.val == "," {
 					if context == "range" && len(vars) < 2 {
-						continue
+						switch t.peekNonSpace().typ {
+						case itemVariable, itemRightDelim, itemRightParen:
+							continue
+						default:
+							t.errorf("range can only initialize variables")
+						}
 					}
 					t.errorf("too many declarations in %s", context)
 				}

src/text/template/parse/parse_test.go

@@ -450,18 +450,37 @@ var errorTests = []parseTest{
 	{"multilinerawstring",
 		"{{ $v := `\n` }} {{",
 		hasError, `multilinerawstring:2: unexpected unclosed action`},
+	{"rangeundefvar",
+		"{{range $k}}{{end}}",
+		hasError, `undefined variable`},
+	{"rangeundefvars",
+		"{{range $k, $v}}{{end}}",
+		hasError, `undefined variable`},
+	{"rangemissingvalue1",
+		"{{range $k,}}{{end}}",
+		hasError, `missing value for range`},
+	{"rangemissingvalue2",
+		"{{range $k, $v := }}{{end}}",
+		hasError, `missing value for range`},
+	{"rangenotvariable1",
+		"{{range $k, .}}{{end}}",
+		hasError, `range can only initialize variables`},
+	{"rangenotvariable2",
+		"{{range $k, 123 := .}}{{end}}",
+		hasError, `range can only initialize variables`},
 }
 
 func TestErrors(t *testing.T) {
 	for _, test := range errorTests {
-		_, err := New(test.name).Parse(test.input, "", "", make(map[string]*Tree))
-		if err == nil {
-			t.Errorf("%q: expected error", test.name)
-			continue
-		}
-		if !strings.Contains(err.Error(), test.result) {
-			t.Errorf("%q: error %q does not contain %q", test.name, err, test.result)
-		}
+		t.Run(test.name, func(t *testing.T) {
+			_, err := New(test.name).Parse(test.input, "", "", make(map[string]*Tree))
+			if err == nil {
+				t.Fatalf("expected error %q, got nil", test.result)
+			}
+			if !strings.Contains(err.Error(), test.result) {
+				t.Fatalf("error %q does not contain %q", err, test.result)
+			}
+		})
 	}
 }