From d0dd26a88c019d54f22463daae81e785f5867565 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 20 May 2021 12:46:33 -0400 Subject: [PATCH] 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 Run-TryBot: Russ Cox TryBot-Result: Go Bot Reviewed-by: Rob Pike --- src/html/template/context.go | 4 ++ src/html/template/escape.go | 71 ++++++++++++++++++++++++++- src/html/template/escape_test.go | 24 +++++++++ src/html/template/exec_test.go | 2 + src/text/template/doc.go | 8 +++ src/text/template/exec.go | 24 ++++++++- src/text/template/exec_test.go | 2 + src/text/template/parse/lex.go | 13 ++++- src/text/template/parse/lex_test.go | 2 + src/text/template/parse/node.go | 36 ++++++++++++++ src/text/template/parse/parse.go | 42 +++++++++++++++- src/text/template/parse/parse_test.go | 8 +++ 12 files changed, 232 insertions(+), 4 deletions(-) diff --git a/src/html/template/context.go b/src/html/template/context.go index f7d4849928d..aaa7d083593 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 8739735cb7b..6dea79c7b55 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 fbc84a75928..3b0aa8c8466 100644 --- a/src/html/template/escape_test.go +++ b/src/html/template/escape_test.go @@ -920,6 +920,22 @@ func TestErrors(t *testing.T) { "", "", }, + { + "{{range .Items}}{{end}}", + "", + }, + { + "{{range .Items}}{{continue}}{{end}}", + "", + }, + { + "{{range .Items}}{{break}}{{end}}", + "", + }, + { + "{{range .Items}}{{if .X}}{{break}}{{end}}{{end}}", + "", + }, // Error cases. { "{{if .Cond}} itemKeyword: - l.emit(key[word]) + item := key[word] + if item == itemBreak && !l.breakOK || item == itemContinue && !l.continueOK { + l.emit(itemIdentifier) + } else { + l.emit(item) + } case word[0] == '.': l.emit(itemField) case word == "true", word == "false": diff --git a/src/text/template/parse/lex_test.go b/src/text/template/parse/lex_test.go index 6510eed674d..df6aabffb20 100644 --- a/src/text/template/parse/lex_test.go +++ b/src/text/template/parse/lex_test.go @@ -35,6 +35,8 @@ var itemName = map[itemType]string{ // keywords itemDot: ".", itemBlock: "block", + itemBreak: "break", + itemContinue: "continue", itemDefine: "define", itemElse: "else", itemIf: "if", diff --git a/src/text/template/parse/node.go b/src/text/template/parse/node.go index 177482f9b26..47268225c8c 100644 --- a/src/text/template/parse/node.go +++ b/src/text/template/parse/node.go @@ -71,6 +71,8 @@ const ( NodeVariable // A $ variable. NodeWith // A with action. NodeComment // A comment. + NodeBreak // A break action. + NodeContinue // A continue action. ) // Nodes. @@ -907,6 +909,40 @@ func (i *IfNode) Copy() Node { return i.tr.newIf(i.Pos, i.Line, i.Pipe.CopyPipe(), i.List.CopyList(), i.ElseList.CopyList()) } +// BreakNode represents a {{break}} action. +type BreakNode struct { + tr *Tree + NodeType + Pos + Line int +} + +func (t *Tree) newBreak(pos Pos, line int) *BreakNode { + return &BreakNode{tr: t, NodeType: NodeBreak, Pos: pos, Line: line} +} + +func (b *BreakNode) Copy() Node { return b.tr.newBreak(b.Pos, b.Line) } +func (b *BreakNode) String() string { return "{{break}}" } +func (b *BreakNode) tree() *Tree { return b.tr } +func (b *BreakNode) writeTo(sb *strings.Builder) { sb.WriteString("{{break}}") } + +// ContinueNode represents a {{continue}} action. +type ContinueNode struct { + tr *Tree + NodeType + Pos + Line int +} + +func (t *Tree) newContinue(pos Pos, line int) *ContinueNode { + return &ContinueNode{tr: t, NodeType: NodeContinue, Pos: pos, Line: line} +} + +func (c *ContinueNode) Copy() Node { return c.tr.newContinue(c.Pos, c.Line) } +func (c *ContinueNode) String() string { return "{{continue}}" } +func (c *ContinueNode) tree() *Tree { return c.tr } +func (c *ContinueNode) writeTo(sb *strings.Builder) { sb.WriteString("{{continue}}") } + // RangeNode represents a {{range}} action and its commands. type RangeNode struct { BranchNode diff --git a/src/text/template/parse/parse.go b/src/text/template/parse/parse.go index 1a63961c13e..d92bed5d3d4 100644 --- a/src/text/template/parse/parse.go +++ b/src/text/template/parse/parse.go @@ -31,6 +31,7 @@ type Tree struct { vars []string // variables defined at the moment. treeSet map[string]*Tree actionLine int // line of left delim starting action + rangeDepth int mode Mode } @@ -224,6 +225,8 @@ func (t *Tree) startParse(funcs []map[string]interface{}, lex *lexer, treeSet ma t.vars = []string{"$"} t.funcs = funcs t.treeSet = treeSet + lex.breakOK = !t.hasFunction("break") + lex.continueOK = !t.hasFunction("continue") } // stopParse terminates parsing. @@ -386,6 +389,10 @@ func (t *Tree) action() (n Node) { switch token := t.nextNonSpace(); token.typ { case itemBlock: return t.blockControl() + case itemBreak: + return t.breakControl(token.pos, token.line) + case itemContinue: + return t.continueControl(token.pos, token.line) case itemElse: return t.elseControl() case itemEnd: @@ -405,6 +412,32 @@ func (t *Tree) action() (n Node) { return t.newAction(token.pos, token.line, t.pipeline("command", itemRightDelim)) } +// Break: +// {{break}} +// Break keyword is past. +func (t *Tree) breakControl(pos Pos, line int) Node { + if token := t.next(); token.typ != itemRightDelim { + t.unexpected(token, "in {{break}}") + } + if t.rangeDepth == 0 { + t.errorf("{{break}} outside {{range}}") + } + return t.newBreak(pos, line) +} + +// Continue: +// {{continue}} +// Continue keyword is past. +func (t *Tree) continueControl(pos Pos, line int) Node { + if token := t.next(); token.typ != itemRightDelim { + t.unexpected(token, "in {{continue}}") + } + if t.rangeDepth == 0 { + t.errorf("{{continue}} outside {{range}}") + } + return t.newContinue(pos, line) +} + // Pipeline: // declarations? command ('|' command)* func (t *Tree) pipeline(context string, end itemType) (pipe *PipeNode) { @@ -480,8 +513,14 @@ func (t *Tree) checkPipeline(pipe *PipeNode, context string) { func (t *Tree) parseControl(allowElseIf bool, context string) (pos Pos, line int, pipe *PipeNode, list, elseList *ListNode) { defer t.popVars(len(t.vars)) pipe = t.pipeline(context, itemRightDelim) + if context == "range" { + t.rangeDepth++ + } var next Node list, next = t.itemList() + if context == "range" { + t.rangeDepth-- + } switch next.Type() { case nodeEnd: //done case nodeElse: @@ -523,7 +562,8 @@ func (t *Tree) ifControl() Node { // {{range pipeline}} itemList {{else}} itemList {{end}} // Range keyword is past. func (t *Tree) rangeControl() Node { - return t.newRange(t.parseControl(false, "range")) + r := t.newRange(t.parseControl(false, "range")) + return r } // With: diff --git a/src/text/template/parse/parse_test.go b/src/text/template/parse/parse_test.go index 9b1be272e57..c3679a08dec 100644 --- a/src/text/template/parse/parse_test.go +++ b/src/text/template/parse/parse_test.go @@ -230,6 +230,10 @@ var parseTests = []parseTest{ `{{range $x := .SI}}{{.}}{{end}}`}, {"range 2 vars", "{{range $x, $y := .SI}}{{.}}{{end}}", noError, `{{range $x, $y := .SI}}{{.}}{{end}}`}, + {"range with break", "{{range .SI}}{{.}}{{break}}{{end}}", noError, + `{{range .SI}}{{.}}{{break}}{{end}}`}, + {"range with continue", "{{range .SI}}{{.}}{{continue}}{{end}}", noError, + `{{range .SI}}{{.}}{{continue}}{{end}}`}, {"constants", "{{range .SI 1 -3.2i true false 'a' nil}}{{end}}", noError, `{{range .SI 1 -3.2i true false 'a' nil}}{{end}}`}, {"template", "{{template `x`}}", noError, @@ -279,6 +283,10 @@ var parseTests = []parseTest{ {"adjacent args", "{{printf 3`x`}}", hasError, ""}, {"adjacent args with .", "{{printf `x`.}}", hasError, ""}, {"extra end after if", "{{if .X}}a{{else if .Y}}b{{end}}{{end}}", hasError, ""}, + {"break outside range", "{{range .}}{{end}} {{break}}", hasError, ""}, + {"continue outside range", "{{range .}}{{end}} {{continue}}", hasError, ""}, + {"break in range else", "{{range .}}{{else}}{{break}}{{end}}", hasError, ""}, + {"continue in range else", "{{range .}}{{else}}{{continue}}{{end}}", hasError, ""}, // Other kinds of assignments and operators aren't available yet. {"bug0a", "{{$x := 0}}{{$x}}", noError, "{{$x := 0}}{{$x}}"}, {"bug0b", "{{$x += 1}}{{$x}}", hasError, ""},