html/template, text/template: implement break and continue for range loops

Break and continue for range loops was accepted as a proposal in June 2017.
It was implemented in CL 66410 (Oct 2017)
but then rolled back in CL 92155 (Feb 2018)
because html/template changes had not been implemented.

This CL reimplements break and continue in text/template
and then adds support for them in html/template as well.

Fixes #20531.

Change-Id: I05330482a976f1c078b4b49c2287bd9031bb7616
Reviewed-on: https://go-review.googlesource.com/c/go/+/321491
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

4 files changed, 100 insertions, 1 deletions

diff --git a/src/html/template/context.go b/src/html/template/context.go
index f7d4849928..aaa7d08359 100644
--- a/src/html/template/context.go
+++ b/src/html/template/context.go
@@ -6,6 +6,7 @@ package template
 
 import (
 	"fmt"
+	"text/template/parse"
 )
 
 // context describes the state an HTML parser must be in when it reaches the
@@ -22,6 +23,7 @@ type context struct {
 	jsCtx   jsCtx
 	attr    attr
 	element element
+	n       parse.Node // for range break/continue
 	err     *Error
 }
 
@@ -141,6 +143,8 @@ const (
 	// stateError is an infectious error state outside any valid
 	// HTML/CSS/JS construct.
 	stateError
+	// stateDead marks unreachable code after a {{break}} or {{continue}}.
+	stateDead
 )
 
 // isComment is true for any state that contains content meant for template
diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index 8739735cb7..6dea79c7b5 100644
--- a/src/html/template/escape.go
+++ b/src/html/template/escape.go
@@ -97,6 +97,15 @@ type escaper struct {
 	actionNodeEdits   map[*parse.ActionNode][]string
 	templateNodeEdits map[*parse.TemplateNode]string
 	textNodeEdits     map[*parse.TextNode][]byte
+	// rangeContext holds context about the current range loop.
+	rangeContext *rangeContext
+}
+
+// rangeContext holds information about the current range loop.
+type rangeContext struct {
+	outer     *rangeContext // outer loop
+	breaks    []context     // context at each break action
+	continues []context     // context at each continue action
 }
 
 // makeEscaper creates a blank escaper for the given set.
@@ -109,6 +118,7 @@ func makeEscaper(n *nameSpace) escaper {
 		map[*parse.ActionNode][]string{},
 		map[*parse.TemplateNode]string{},
 		map[*parse.TextNode][]byte{},
+		nil,
 	}
 }
 
@@ -124,8 +134,16 @@ func (e *escaper) escape(c context, n parse.Node) context {
 	switch n := n.(type) {
 	case *parse.ActionNode:
 		return e.escapeAction(c, n)
+	case *parse.BreakNode:
+		c.n = n
+		e.rangeContext.breaks = append(e.rangeContext.breaks, c)
+		return context{state: stateDead}
 	case *parse.CommentNode:
 		return c
+	case *parse.ContinueNode:
+		c.n = n
+		e.rangeContext.continues = append(e.rangeContext.breaks, c)
+		return context{state: stateDead}
 	case *parse.IfNode:
 		return e.escapeBranch(c, &n.BranchNode, "if")
 	case *parse.ListNode:
@@ -427,6 +445,12 @@ func join(a, b context, node parse.Node, nodeName string) context {
 	if b.state == stateError {
 		return b
 	}
+	if a.state == stateDead {
+		return b
+	}
+	if b.state == stateDead {
+		return a
+	}
 	if a.eq(b) {
 		return a
 	}
@@ -466,14 +490,27 @@ func join(a, b context, node parse.Node, nodeName string) context {
 
 // escapeBranch escapes a branch template node: "if", "range" and "with".
 func (e *escaper) escapeBranch(c context, n *parse.BranchNode, nodeName string) context {
+	if nodeName == "range" {
+		e.rangeContext = &rangeContext{outer: e.rangeContext}
+	}
 	c0 := e.escapeList(c, n.List)
-	if nodeName == "range" && c0.state != stateError {
+	if nodeName == "range" {
+		if c0.state != stateError {
+			c0 = joinRange(c0, e.rangeContext)
+		}
+		e.rangeContext = e.rangeContext.outer
+		if c0.state == stateError {
+			return c0
+		}
+
 		// The "true" branch of a "range" node can execute multiple times.
 		// We check that executing n.List once results in the same context
 		// as executing n.List twice.
+		e.rangeContext = &rangeContext{outer: e.rangeContext}
 		c1, _ := e.escapeListConditionally(c0, n.List, nil)
 		c0 = join(c0, c1, n, nodeName)
 		if c0.state == stateError {
+			e.rangeContext = e.rangeContext.outer
 			// Make clear that this is a problem on loop re-entry
 			// since developers tend to overlook that branch when
 			// debugging templates.
@@ -481,11 +518,39 @@ func (e *escaper) escapeBranch(c context, n *parse.BranchNode, nodeName string)
 			c0.err.Description = "on range loop re-entry: " + c0.err.Description
 			return c0
 		}
+		c0 = joinRange(c0, e.rangeContext)
+		e.rangeContext = e.rangeContext.outer
+		if c0.state == stateError {
+			return c0
+		}
 	}
 	c1 := e.escapeList(c, n.ElseList)
 	return join(c0, c1, n, nodeName)
 }
 
+func joinRange(c0 context, rc *rangeContext) context {
+	// Merge contexts at break and continue statements into overall body context.
+	// In theory we could treat breaks differently from continues, but for now it is
+	// enough to treat them both as going back to the start of the loop (which may then stop).
+	for _, c := range rc.breaks {
+		c0 = join(c0, c, c.n, "range")
+		if c0.state == stateError {
+			c0.err.Line = c.n.(*parse.BreakNode).Line
+			c0.err.Description = "at range loop break: " + c0.err.Description
+			return c0
+		}
+	}
+	for _, c := range rc.continues {
+		c0 = join(c0, c, c.n, "range")
+		if c0.state == stateError {
+			c0.err.Line = c.n.(*parse.ContinueNode).Line
+			c0.err.Description = "at range loop continue: " + c0.err.Description
+			return c0
+		}
+	}
+	return c0
+}
+
 // escapeList escapes a list template node.
 func (e *escaper) escapeList(c context, n *parse.ListNode) context {
 	if n == nil {
@@ -493,6 +558,9 @@ func (e *escaper) escapeList(c context, n *parse.ListNode) context {
 	}
 	for _, m := range n.Nodes {
 		c = e.escape(c, m)
+		if c.state == stateDead {
+			break
+		}
 	}
 	return c
 }
@@ -503,6 +571,7 @@ func (e *escaper) escapeList(c context, n *parse.ListNode) context {
 // which is the same as whether e was updated.
 func (e *escaper) escapeListConditionally(c context, n *parse.ListNode, filter func(*escaper, context) bool) (context, bool) {
 	e1 := makeEscaper(e.ns)
+	e1.rangeContext = e.rangeContext
 	// Make type inferences available to f.
 	for k, v := range e.output {
 		e1.output[k] = v
diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index fbc84a7592..3b0aa8c846 100644
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -920,6 +920,22 @@ func TestErrors(t *testing.T) {
 			"<a href='/foo?{{range .Items}}&{{.K}}={{.V}}{{end}}'>",
 			"",
 		},
+		{
+			"{{range .Items}}<a{{if .X}}{{end}}>{{end}}",
+			"",
+		},
+		{
+			"{{range .Items}}<a{{if .X}}{{end}}>{{continue}}{{end}}",
+			"",
+		},
+		{
+			"{{range .Items}}<a{{if .X}}{{end}}>{{break}}{{end}}",
+			"",
+		},
+		{
+			"{{range .Items}}<a{{if .X}}{{end}}>{{if .X}}{{break}}{{end}}{{end}}",
+			"",
+		},
 		// Error cases.
 		{
 			"{{if .Cond}}<a{{end}}",
@@ -956,6 +972,14 @@ func TestErrors(t *testing.T) {
 			"z:2:8: on range loop re-entry: {{range}} branches",
 		},
 		{
+			"{{range .Items}}<a{{if .X}}{{break}}{{end}}>{{end}}",
+			"z:1:29: at range loop break: {{range}} branches end in different contexts",
+		},
+		{
+			"{{range .Items}}<a{{if .X}}{{continue}}{{end}}>{{end}}",
+			"z:1:29: at range loop continue: {{range}} branches end in different contexts",
+		},
+		{
 			"<a b=1 c={{.H}}",
 			"z: ends in a non-text context: {stateAttr delimSpaceOrTagEnd",
 		},
diff --git a/src/html/template/exec_test.go b/src/html/template/exec_test.go
index 888587335d..523340bac9 100644
--- a/src/html/template/exec_test.go
+++ b/src/html/template/exec_test.go
@@ -567,6 +567,8 @@ var execTests = []execTest{
 	{"range empty no else", "{{range .SIEmpty}}-{{.}}-{{end}}", "", tVal, true},
 	{"range []int else", "{{range .SI}}-{{.}}-{{else}}EMPTY{{end}}", "-3--4--5-", tVal, true},
 	{"range empty else", "{{range .SIEmpty}}-{{.}}-{{else}}EMPTY{{end}}", "EMPTY", tVal, true},
+	{"range []int break else", "{{range .SI}}-{{.}}-{{break}}NOTREACHED{{else}}EMPTY{{end}}", "-3-", tVal, true},
+	{"range []int continue else", "{{range .SI}}-{{.}}-{{continue}}NOTREACHED{{else}}EMPTY{{end}}", "-3--4--5-", tVal, true},
 	{"range []bool", "{{range .SB}}-{{.}}-{{end}}", "-true--false-", tVal, true},
 	{"range []int method", "{{range .SI | .MAdd .I}}-{{.}}-{{end}}", "-20--21--22-", tVal, true},
 	{"range map", "{{range .MSI}}-{{.}}-{{end}}", "-1--3--2-", tVal, true},