text/template/parse: fix interaction of break/continue keywords and functions

Fixes a bug that was introduced in CL 410414; in that CL, to avoid
a race condition in the initialization of the lexer, the setting
of the breakOK and continueOK options was moved to before
Tree.funcs was populated from parameters. As a result, the parser
missed the fact that 'break' and 'continue' were defined as functions.

Following CL 421883 race conditions are no longer an issue, so
the simplest fix is just to move the initialization where it was
before - in startParse, after t.funcs has been set.

Fixes #56538

Change-Id: I3b99fe9ad12255a4f6eb9a00eb3f64529ff055c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/447775
Run-TryBot: Eli Bendersky‎ <eliben@golang.org>
Auto-Submit: Eli Bendersky‎ <eliben@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Eli Bendersky <eliben@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>

src/text/template/parse/parse.go

@@ -223,6 +223,11 @@ func (t *Tree) startParse(funcs []map[string]any, lex *lexer, treeSet map[string
 	t.vars = []string{"$"}
 	t.funcs = funcs
 	t.treeSet = treeSet
+	lex.options = lexOptions{
+		emitComment: t.Mode&ParseComments != 0,
+		breakOK:     !t.hasFunction("break"),
+		continueOK:  !t.hasFunction("continue"),
+	}
 }
 
 // stopParse terminates parsing.
@@ -241,11 +246,6 @@ func (t *Tree) Parse(text, leftDelim, rightDelim string, treeSet map[string]*Tre
 	defer t.recover(&err)
 	t.ParseName = t.Name
 	lexer := lex(t.Name, text, leftDelim, rightDelim)
-	lexer.options = lexOptions{
-		emitComment: t.Mode&ParseComments != 0,
-		breakOK:     !t.hasFunction("break"),
-		continueOK:  !t.hasFunction("continue"),
-	}
 	t.startParse(funcs, lexer, treeSet)
 	t.text = text
 	t.parse()

src/text/template/parse/parse_test.go

@@ -391,6 +391,36 @@ func TestParseWithComments(t *testing.T) {
 	}
 }
 
+func TestKeywordsAndFuncs(t *testing.T) {
+	// Check collisions between functions and new keywords like 'break'. When a
+	// break function is provided, the parser should treat 'break' as a function,
+	// not a keyword.
+	textFormat = "%q"
+	defer func() { textFormat = "%s" }()
+
+	inp := `{{range .X}}{{break 20}}{{end}}`
+	{
+		// 'break' is a defined function, don't treat it as a keyword: it should
+		// accept an argument successfully.
+		var funcsWithKeywordFunc = map[string]any{
+			"break": func(in any) any { return in },
+		}
+		tmpl, err := New("").Parse(inp, "", "", make(map[string]*Tree), funcsWithKeywordFunc)
+		if err != nil || tmpl == nil {
+			t.Errorf("with break func: unexpected error: %v", err)
+		}
+	}
+
+	{
+		// No function called 'break'; treat it as a keyword. Results in a parse
+		// error.
+		tmpl, err := New("").Parse(inp, "", "", make(map[string]*Tree), make(map[string]any))
+		if err == nil || tmpl != nil {
+			t.Errorf("without break func: expected error; got none")
+		}
+	}
+}
+
 func TestSkipFuncCheck(t *testing.T) {
 	oldTextFormat := textFormat
 	textFormat = "%q"