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 <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/text/template/parse/parse.go b/src/text/template/parse/parse.go
index 5195694d421947575e963064c2165895ded5caaf..7c35b0ff3d89169d7b249cc55003670b36644725 100644 (file)
--- a/src/text/template/parse/parse.go
+++ b/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: