text/template/parse: simplify Tree.pipeline

The pipeline parsing code was unnecessarily complex. It used a for loop
with a trailing break, a complex switch, and up to seven levels of
indentation.

Instead, drop the loop in favor of a single named goto with a comment,
and flatten out the complex switch to be easier to follow. Two lines of
code are now duplicated, but they're simple and only three lines apart.

While at it, move the pipe initialization further up to remove the need
for three variables.

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

Author: mvdan

src/text/template/parse/parse.go

@@ -380,52 +380,44 @@ func (t *Tree) action() (n Node) {
 // Pipeline:
 //	declarations? command ('|' command)*
 func (t *Tree) pipeline(context string) (pipe *PipeNode) {
-	decl := false
-	var vars []*VariableNode
 	token := t.peekNonSpace()
-	pos := token.pos
+	pipe = t.newPipeline(token.pos, token.line, nil)
 	// Are there declarations or assignments?
-	// TODO(mvdan): simplify the loop break/continue logic
-	for {
-		if v := t.peekNonSpace(); v.typ == itemVariable {
-			t.next()
-			// Since space is a token, we need 3-token look-ahead here in the worst case:
-			// in "$x foo" we need to read "foo" (as opposed to ":=") to know that $x is an
-			// argument variable rather than a declaration. So remember the token
-			// adjacent to the variable so we can push it back if necessary.
-			tokenAfterVariable := t.peek()
-			next := t.peekNonSpace()
-			switch {
-			case next.typ == itemAssign, next.typ == itemDeclare,
-				next.typ == itemChar && next.val == ",":
-				t.nextNonSpace()
-				variable := t.newVariable(v.pos, v.val)
-				vars = append(vars, variable)
-				t.vars = append(t.vars, v.val)
-				if next.typ == itemDeclare {
-					decl = true
-				}
-				if next.typ == itemChar && next.val == "," {
-					if context == "range" && len(vars) < 2 {
-						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)
+decls:
+	if v := t.peekNonSpace(); v.typ == itemVariable {
+		t.next()
+		// Since space is a token, we need 3-token look-ahead here in the worst case:
+		// in "$x foo" we need to read "foo" (as opposed to ":=") to know that $x is an
+		// argument variable rather than a declaration. So remember the token
+		// adjacent to the variable so we can push it back if necessary.
+		tokenAfterVariable := t.peek()
+		next := t.peekNonSpace()
+		switch {
+		case next.typ == itemAssign, next.typ == itemDeclare:
+			pipe.IsAssign = next.typ == itemAssign
+			t.nextNonSpace()
+			pipe.Decl = append(pipe.Decl, t.newVariable(v.pos, v.val))
+			t.vars = append(t.vars, v.val)
+		case next.typ == itemChar && next.val == ",":
+			t.nextNonSpace()
+			pipe.Decl = append(pipe.Decl, t.newVariable(v.pos, v.val))
+			t.vars = append(t.vars, v.val)
+			if context == "range" && len(pipe.Decl) < 2 {
+				switch t.peekNonSpace().typ {
+				case itemVariable, itemRightDelim, itemRightParen:
+					// second initialized variable in a range pipeline
+					goto decls
+				default:
+					t.errorf("range can only initialize variables")
 				}
-			case tokenAfterVariable.typ == itemSpace:
-				t.backup3(v, tokenAfterVariable)
-			default:
-				t.backup2(v)
 			}
+			t.errorf("too many declarations in %s", context)
+		case tokenAfterVariable.typ == itemSpace:
+			t.backup3(v, tokenAfterVariable)
+		default:
+			t.backup2(v)
 		}
-		break
 	}
-	pipe = t.newPipeline(pos, token.line, vars)
-	pipe.IsAssign = !decl
 	for {
 		switch token := t.nextNonSpace(); token.typ {
 		case itemRightDelim, itemRightParen: