text/template: allow .Field access to parenthesized expressions

Change the grammar so that field access is a proper operator.
This introduces a new node, ChainNode, into the public (but
actually internal) API of text/template/parse. For
compatibility, we only use the new node type for the specific
construct, which was not parseable before. Therefore this
should be backward-compatible.

Before, .X.Y was a token in the lexer; this CL breaks it out
into .Y applied to .X. But for compatibility we mush them
back together before delivering. One day we might remove
that hack; it's the simple TODO in parse.go/operand.

This change also provides grammatical distinction between
        f
and
        (f)
which might permit function values later, but not now.

Fixes #3999.

R=golang-dev, dsymonds, gri, rsc, mikesamuel
CC=golang-dev
https://golang.org/cl/6494119

src/pkg/html/template/escape_test.go

@@ -1539,6 +1539,11 @@ func TestEnsurePipelineContains(t *testing.T) {
 			".X | urlquery | html | print",
 			[]string{"urlquery", "html"},
 		},
+		{
+			"{{($).X | html | print}}",
+			"($).X | urlquery | html | print",
+			[]string{"urlquery", "html"},
+		},
 	}
 	for i, test := range tests {
 		tmpl := template.Must(template.New("test").Parse(test.input))

src/pkg/text/template/doc.go

@@ -148,8 +148,10 @@ An argument is a simple value, denoted by one of the following.
 	  The result is the value of invoking the function, fun(). The return
 	  types and values behave as in methods. Functions and function
 	  names are described below.
-	- Parentheses may be used for grouping, as in
+	- A parenthesized instance of one the above, for grouping. The result
+	  may be accessed by a field or map key invocation.
 		print (.F1 arg1) (.F2 arg2)
+		(.StructValuedMethod "arg").Field
 
 Arguments may evaluate to any type; if they are pointers the implementation
 automatically indirects to the base type when required.

src/pkg/text/template/exec.go

@@ -315,9 +315,15 @@ func (s *state) evalCommand(dot reflect.Value, cmd *parse.CommandNode, final ref
 	switch n := firstWord.(type) {
 	case *parse.FieldNode:
 		return s.evalFieldNode(dot, n, cmd.Args, final)
+	case *parse.ChainNode:
+		return s.evalChainNode(dot, n, cmd.Args, final)
 	case *parse.IdentifierNode:
 		// Must be a function.
 		return s.evalFunction(dot, n.Ident, cmd.Args, final)
+	case *parse.PipeNode:
+		// Parenthesized pipeline. The arguments are all inside the pipeline; final is ignored.
+		// TODO: is this right?
+		return s.evalPipeline(dot, n)
 	case *parse.VariableNode:
 		return s.evalVariableNode(dot, n, cmd.Args, final)
 	}
@@ -367,6 +373,15 @@ func (s *state) evalFieldNode(dot reflect.Value, field *parse.FieldNode, args []
 	return s.evalFieldChain(dot, dot, field.Ident, args, final)
 }
 
+func (s *state) evalChainNode(dot reflect.Value, chain *parse.ChainNode, args []parse.Node, final reflect.Value) reflect.Value {
+	// (pipe).Field1.Field2 has pipe as .Node, fields as .Field. Eval the pipeline, then the fields.
+	pipe := s.evalArg(dot, nil, chain.Node)
+	if len(chain.Field) == 0 {
+		s.errorf("internal error: no fields in evalChainNode")
+	}
+	return s.evalFieldChain(dot, pipe, chain.Field, args, final)
+}
+
 func (s *state) evalVariableNode(dot reflect.Value, v *parse.VariableNode, args []parse.Node, final reflect.Value) reflect.Value {
 	// $x.Field has $x as the first ident, Field as the second. Eval the var, then the fields.
 	value := s.varValue(v.Ident[0])
@@ -521,13 +536,13 @@ func canBeNil(typ reflect.Type) bool {
 // validateType guarantees that the value is valid and assignable to the type.
 func (s *state) validateType(value reflect.Value, typ reflect.Type) reflect.Value {
 	if !value.IsValid() {
-		if canBeNil(typ) {
+		if typ == nil || canBeNil(typ) {
 			// An untyped nil interface{}. Accept as a proper nil value.
 			return reflect.Zero(typ)
 		}
 		s.errorf("invalid value; expected %s", typ)
 	}
-	if !value.Type().AssignableTo(typ) {
+	if typ != nil && !value.Type().AssignableTo(typ) {
 		if value.Kind() == reflect.Interface && !value.IsNil() {
 			value = value.Elem()
 			if value.Type().AssignableTo(typ) {

src/pkg/text/template/exec_test.go

@@ -340,6 +340,12 @@ var execTests = []execTest{
 	// Parenthesized expressions
 	{"parens in pipeline", "{{printf `%d %d %d` (1) (2 | add 3) (add 4 (add 5 6))}}", "1 5 15", tVal, true},
 
+	// Parenthesized expressions with field accesses
+	{"parens: $ in paren", "{{($).X}}", "x", tVal, true},
+	{"parens: $.GetU in paren", "{{($.GetU).V}}", "v", tVal, true},
+	{"parens: $ in paren in pipe", "{{($ | echo).X}}", "x", tVal, true},
+	{"parens: spaces and args", `{{(makemap "up" "down" "left" "right").left}}`, "right", tVal, true},
+
 	// If.
 	{"if true", "{{if true}}TRUE{{end}}", "TRUE", tVal, true},
 	{"if false", "{{if false}}TRUE{{else}}FALSE{{end}}", "FALSE", tVal, true},
@@ -535,6 +541,21 @@ func add(args ...int) int {
 	return sum
 }
 
+func echo(arg interface{}) interface{} {
+	return arg
+}
+
+func makemap(arg ...string) map[string]string {
+	if len(arg)%2 != 0 {
+		panic("bad makemap")
+	}
+	m := make(map[string]string)
+	for i := 0; i < len(arg); i += 2 {
+		m[arg[i]] = arg[i+1]
+	}
+	return m
+}
+
 func stringer(s fmt.Stringer) string {
 	return s.String()
 }
@@ -545,6 +566,8 @@ func testExecute(execTests []execTest, template *Template, t *testing.T) {
 		"add":      add,
 		"count":    count,
 		"dddArg":   dddArg,
+		"echo":     echo,
+		"makemap":  makemap,
 		"oneArg":   oneArg,
 		"typeOf":   typeOf,
 		"vfunc":    vfunc,

src/pkg/text/template/parse/lex.go

@@ -43,8 +43,8 @@ const (
 	itemComplex                      // complex constant (1+2i); imaginary is just a number
 	itemColonEquals                  // colon-equals (':=') introducing a declaration
 	itemEOF
-	itemField      // alphanumeric identifier, starting with '.', possibly chained ('.x.y')
-	itemIdentifier // alphanumeric identifier
+	itemField      // alphanumeric identifier starting with '.'
+	itemIdentifier // alphanumeric identifier not starting with '.'
 	itemLeftDelim  // left action delimiter
 	itemLeftParen  // '(' inside action
 	itemNumber     // simple number, including imaginary
@@ -286,7 +286,7 @@ func lexInsideAction(l *lexer) stateFn {
 	case r == '`':
 		return lexRawQuote
 	case r == '$':
-		return lexIdentifier
+		return lexVariable
 	case r == '\'':
 		return lexChar
 	case r == '.':
@@ -294,7 +294,7 @@ func lexInsideAction(l *lexer) stateFn {
 		if l.pos < len(l.input) {
 			r := l.input[l.pos]
 			if r < '0' || '9' < r {
-				return lexIdentifier // itemDot comes from the keyword table.
+				return lexField
 			}
 		}
 		fallthrough // '.' can start a number.
@@ -334,15 +334,13 @@ func lexSpace(l *lexer) stateFn {
 	return lexInsideAction
 }
 
-// lexIdentifier scans an alphanumeric or field.
+// lexIdentifier scans an alphanumeric.
 func lexIdentifier(l *lexer) stateFn {
 Loop:
 	for {
 		switch r := l.next(); {
 		case isAlphaNumeric(r):
 			// absorb.
-		case r == '.' && (l.input[l.start] == '.' || l.input[l.start] == '$'):
-			// field chaining; absorb into one token.
 		default:
 			l.backup()
 			word := l.input[l.start:l.pos]
@@ -354,8 +352,6 @@ Loop:
 				l.emit(key[word])
 			case word[0] == '.':
 				l.emit(itemField)
-			case word[0] == '$':
-				l.emit(itemVariable)
 			case word == "true", word == "false":
 				l.emit(itemBool)
 			default:
@@ -367,17 +363,59 @@ Loop:
 	return lexInsideAction
 }
 
+// lexField scans a field: .Alphanumeric.
+// The . has been scanned.
+func lexField(l *lexer) stateFn {
+	return lexFieldOrVariable(l, itemField)
+}
+
+// lexVariable scans a Variable: $Alphanumeric.
+// The $ has been scanned.
+func lexVariable(l *lexer) stateFn {
+	if l.atTerminator() { // Nothing interesting follows -> "$".
+		l.emit(itemVariable)
+		return lexInsideAction
+	}
+	return lexFieldOrVariable(l, itemVariable)
+}
+
+// lexVariable scans a field or variable: [.$]Alphanumeric.
+// The . or $ has been scanned.
+func lexFieldOrVariable(l *lexer, typ itemType) stateFn {
+	if l.atTerminator() { // Nothing interesting follows -> "." or "$".
+		if typ == itemVariable {
+			l.emit(itemVariable)
+		} else {
+			l.emit(itemDot)
+		}
+		return lexInsideAction
+	}
+	var r rune
+	for {
+		r = l.next()
+		if !isAlphaNumeric(r) {
+			l.backup()
+			break
+		}
+	}
+	if !l.atTerminator() {
+		return l.errorf("bad character %#U", r)
+	}
+	l.emit(typ)
+	return lexInsideAction
+}
+
 // atTerminator reports whether the input is at valid termination character to
-// appear after an identifier. Mostly to catch cases like "$x+2" not being
-// acceptable without a space, in case we decide one day to implement
-// arithmetic.
+// appear after an identifier. Breaks .X.Y into two pieces. Also catches cases
+// like "$x+2" not being acceptable without a space, in case we decide one
+// day to implement arithmetic.
 func (l *lexer) atTerminator() bool {
 	r := l.peek()
 	if isSpace(r) || isEndOfLine(r) {
 		return true
 	}
 	switch r {
-	case eof, ',', '|', ':', ')', '(':
+	case eof, '.', ',', '|', ':', ')', '(':
 		return true
 	}
 	// Does r start the delimiter? This can be ambiguous (with delim=="//", $x/2 will

src/pkg/text/template/parse/lex_test.go

@@ -61,10 +61,12 @@ var (
 	tEOF      = item{itemEOF, 0, ""}
 	tFor      = item{itemIdentifier, 0, "for"}
 	tLeft     = item{itemLeftDelim, 0, "{{"}
+	tLpar     = item{itemLeftParen, 0, "("}
 	tPipe     = item{itemPipe, 0, "|"}
 	tQuote    = item{itemString, 0, `"abc \n\t\" "`}
 	tRange    = item{itemRange, 0, "range"}
 	tRight    = item{itemRightDelim, 0, "}}"}
+	tRpar     = item{itemRightParen, 0, ")"}
 	tSpace    = item{itemSpace, 0, " "}
 	raw       = "`" + `abc\n\t\" ` + "`"
 	tRawQuote = item{itemRawString, 0, raw}
@@ -90,11 +92,11 @@ var lexTests = []lexTest{
 	}},
 	{"parens", "{{((3))}}", []item{
 		tLeft,
-		{itemLeftParen, 0, "("},
-		{itemLeftParen, 0, "("},
+		tLpar,
+		tLpar,
 		{itemNumber, 0, "3"},
-		{itemRightParen, 0, ")"},
-		{itemRightParen, 0, ")"},
+		tRpar,
+		tRpar,
 		tRight,
 		tEOF,
 	}},
@@ -160,7 +162,7 @@ var lexTests = []lexTest{
 		tRight,
 		tEOF,
 	}},
-	{"dots", "{{.x . .2 .x.y}}", []item{
+	{"dots", "{{.x . .2 .x.y.z}}", []item{
 		tLeft,
 		{itemField, 0, ".x"},
 		tSpace,
@@ -168,7 +170,9 @@ var lexTests = []lexTest{
 		tSpace,
 		{itemNumber, 0, ".2"},
 		tSpace,
-		{itemField, 0, ".x.y"},
+		{itemField, 0, ".x"},
+		{itemField, 0, ".y"},
+		{itemField, 0, ".z"},
 		tRight,
 		tEOF,
 	}},
@@ -202,13 +206,14 @@ var lexTests = []lexTest{
 		tSpace,
 		{itemVariable, 0, "$"},
 		tSpace,
-		{itemVariable, 0, "$var.Field"},
+		{itemVariable, 0, "$var"},
+		{itemField, 0, ".Field"},
 		tSpace,
 		{itemField, 0, ".Method"},
 		tRight,
 		tEOF,
 	}},
-	{"variable invocation ", "{{$x 23}}", []item{
+	{"variable invocation", "{{$x 23}}", []item{
 		tLeft,
 		{itemVariable, 0, "$x"},
 		tSpace,
@@ -261,6 +266,15 @@ var lexTests = []lexTest{
 		tRight,
 		tEOF,
 	}},
+	{"field of parenthesized expression", "{{(.X).Y}}", []item{
+		tLeft,
+		tLpar,
+		{itemField, 0, ".X"},
+		tRpar,
+		{itemField, 0, ".Y"},
+		tRight,
+		tEOF,
+	}},
 	// errors
 	{"badchar", "#{{\x01}}", []item{
 		{itemText, 0, "#"},
@@ -294,14 +308,14 @@ var lexTests = []lexTest{
 	}},
 	{"unclosed paren", "{{(3}}", []item{
 		tLeft,
-		{itemLeftParen, 0, "("},
+		tLpar,
 		{itemNumber, 0, "3"},
 		{itemError, 0, `unclosed left paren`},
 	}},
 	{"extra right paren", "{{3)}}", []item{
 		tLeft,
 		{itemNumber, 0, "3"},
-		{itemRightParen, 0, ")"},
+		tRpar,
 		{itemError, 0, `unexpected right paren U+0029 ')'`},
 	}},
 

src/pkg/text/template/parse/node.go

@@ -34,8 +34,9 @@ func (t NodeType) Type() NodeType {
 
 const (
 	NodeText       NodeType = iota // Plain text.
-	NodeAction                     // A simple action such as field evaluation.
+	NodeAction                     // A non-control action such as a field evaluation.
 	NodeBool                       // A boolean constant.
+	NodeChain                      // A sequence of field accesses.
 	NodeCommand                    // An element of a pipeline.
 	NodeDot                        // The cursor, dot.
 	nodeElse                       // An else action. Not added to tree.
@@ -168,7 +169,7 @@ func (p *PipeNode) Copy() Node {
 
 // ActionNode holds an action (something bounded by delimiters).
 // Control actions have their own nodes; ActionNode represents simple
-// ones such as field evaluations.
+// ones such as field evaluations and parenthesized pipelines.
 type ActionNode struct {
 	NodeType
 	Line int       // The line number in the input.
@@ -248,11 +249,11 @@ func (i *IdentifierNode) Copy() Node {
 	return NewIdentifier(i.Ident)
 }
 
-// VariableNode holds a list of variable names. The dollar sign is
-// part of the name.
+// VariableNode holds a list of variable names, possibly with chained field
+// accesses. The dollar sign is part of the (first) name.
 type VariableNode struct {
 	NodeType
-	Ident []string // Variable names in lexical order.
+	Ident []string // Variable name and fields in lexical order.
 }
 
 func newVariable(ident string) *VariableNode {
@@ -337,6 +338,46 @@ func (f *FieldNode) Copy() Node {
 	return &FieldNode{NodeType: NodeField, Ident: append([]string{}, f.Ident...)}
 }
 
+// ChainNode holds a term followed by a chain of field accesses (identifier starting with '.').
+// The names may be chained ('.x.y').
+// The periods are dropped from each ident.
+type ChainNode struct {
+	NodeType
+	Node  Node
+	Field []string // The identifiers in lexical order.
+}
+
+func newChain(node Node) *ChainNode {
+	return &ChainNode{NodeType: NodeChain, Node: node}
+}
+
+// Add adds the named field (which should start with a period) to the end of the chain.
+func (c *ChainNode) Add(field string) {
+	if len(field) == 0 || field[0] != '.' {
+		panic("no dot in field")
+	}
+	field = field[1:] // Remove leading dot.
+	if field == "" {
+		panic("empty field")
+	}
+	c.Field = append(c.Field, field)
+}
+
+func (c *ChainNode) String() string {
+	s := c.Node.String()
+	if _, ok := c.Node.(*PipeNode); ok {
+		s = "(" + s + ")"
+	}
+	for _, field := range c.Field {
+		s += "." + field
+	}
+	return s
+}
+
+func (c *ChainNode) Copy() Node {
+	return &ChainNode{NodeType: NodeChain, Node: c.Node, Field: append([]string{}, c.Field...)}
+}
+
 // BoolNode holds a boolean constant.
 type BoolNode struct {
 	NodeType

src/pkg/text/template/parse/parse.go

@@ -353,8 +353,7 @@ func (t *Tree) action() (n Node) {
 }
 
 // Pipeline:
-//	field or command
-//	pipeline "|" pipeline
+//	declarations? command ('|' command)*
 func (t *Tree) pipeline(context string) (pipe *PipeNode) {
 	var decl []*VariableNode
 	// Are there declarations?
@@ -369,9 +368,6 @@ func (t *Tree) pipeline(context string) (pipe *PipeNode) {
 			if next := t.peekNonSpace(); next.typ == itemColonEquals || (next.typ == itemChar && next.val == ",") {
 				t.nextNonSpace()
 				variable := newVariable(v.val)
-				if len(variable.Ident) != 1 {
-					t.errorf("illegal variable in declaration: %s", v.val)
-				}
 				decl = append(decl, variable)
 				t.vars = append(t.vars, v.val)
 				if next.typ == itemChar && next.val == "," {
@@ -400,7 +396,7 @@ func (t *Tree) pipeline(context string) (pipe *PipeNode) {
 			}
 			return
 		case itemBool, itemCharConstant, itemComplex, itemDot, itemField, itemIdentifier,
-			itemNumber, itemNil, itemRawString, itemString, itemVariable:
+			itemNumber, itemNil, itemRawString, itemString, itemVariable, itemLeftParen:
 			t.backup()
 			pipe.append(t.command())
 		default:
@@ -494,57 +490,29 @@ func (t *Tree) templateControl() Node {
 }
 
 // command:
+//	operand (space operand)*
 // space-separated arguments up to a pipeline character or right delimiter.
 // we consume the pipe character but leave the right delim to terminate the action.
 func (t *Tree) command() *CommandNode {
 	cmd := newCommand()
-Loop:
 	for {
-		switch token := t.nextNonSpace(); token.typ {
-		case itemRightDelim, itemRightParen:
-			t.backup()
-			break Loop
-		case itemPipe:
-			break Loop
-		case itemLeftParen:
-			p := t.pipeline("parenthesized expression")
-			if t.nextNonSpace().typ != itemRightParen {
-				t.errorf("missing right paren in parenthesized expression")
-			}
-			cmd.append(p)
+		t.peekNonSpace() // skip leading spaces.
+		operand := t.operand()
+		if operand != nil {
+			cmd.append(operand)
+		}
+		switch token := t.next(); token.typ {
+		case itemSpace:
+			continue
 		case itemError:
 			t.errorf("%s", token.val)
-		case itemIdentifier:
-			if !t.hasFunction(token.val) {
-				t.errorf("function %q not defined", token.val)
-			}
-			cmd.append(NewIdentifier(token.val))
-		case itemDot:
-			cmd.append(newDot())
-		case itemNil:
-			cmd.append(newNil())
-		case itemVariable:
-			cmd.append(t.useVar(token.val))
-		case itemField:
-			cmd.append(newField(token.val))
-		case itemBool:
-			cmd.append(newBool(token.val == "true"))
-		case itemCharConstant, itemComplex, itemNumber:
-			number, err := newNumber(token.val, token.typ)
-			if err != nil {
-				t.error(err)
-			}
-			cmd.append(number)
-		case itemString, itemRawString:
-			s, err := strconv.Unquote(token.val)
-			if err != nil {
-				t.error(err)
-			}
-			cmd.append(newString(token.val, s))
+		case itemRightDelim, itemRightParen:
+			t.backup()
+		case itemPipe:
 		default:
-			t.unexpected(token, "command")
+			t.errorf("unexpected %s in operand; missing space?", token)
 		}
-		t.terminate()
+		break
 	}
 	if len(cmd.Args) == 0 {
 		t.errorf("empty command")
@@ -552,15 +520,86 @@ Loop:
 	return cmd
 }
 
-// terminate checks that the next token terminates an argument. This guarantees
-// that arguments are space-separated, for example that (2)3 does not parse.
-func (t *Tree) terminate() {
-	token := t.peek()
-	switch token.typ {
-	case itemChar, itemPipe, itemRightDelim, itemRightParen, itemSpace:
-		return
+// operand:
+//	term .Field*
+// An operand is a space-separated component of a command,
+// a term possibly followed by field accesses.
+// A nil return means the next item is not an operand.
+func (t *Tree) operand() Node {
+	node := t.term()
+	if node == nil {
+		return nil
 	}
-	t.unexpected(token, "argument list (missing space?)")
+	if t.peek().typ == itemField {
+		chain := newChain(node)
+		for t.peek().typ == itemField {
+			chain.Add(t.next().val)
+		}
+		// Compatibility with original API: If the term is of type NodeField
+		// or NodeVariable, just put more fields on the original.
+		// Otherwise, keep the Chain node.
+		// TODO: Switch to Chains always when we can.
+		switch node.Type() {
+		case NodeField:
+			node = newField(chain.String())
+		case NodeVariable:
+			node = newVariable(chain.String())
+		default:
+			node = chain
+		}
+	}
+	return node
+}
+
+// term:
+//	literal (number, string, nil, boolean)
+//	function (identifier)
+//	.
+//	.Field
+//	$
+//	'(' pipeline ')'
+// A term is a simple "expression".
+// A nil return means the next item is not a term.
+func (t *Tree) term() Node {
+	switch token := t.nextNonSpace(); token.typ {
+	case itemError:
+		t.errorf("%s", token.val)
+	case itemIdentifier:
+		if !t.hasFunction(token.val) {
+			t.errorf("function %q not defined", token.val)
+		}
+		return NewIdentifier(token.val)
+	case itemDot:
+		return newDot()
+	case itemNil:
+		return newNil()
+	case itemVariable:
+		return t.useVar(token.val)
+	case itemField:
+		return newField(token.val)
+	case itemBool:
+		return newBool(token.val == "true")
+	case itemCharConstant, itemComplex, itemNumber:
+		number, err := newNumber(token.val, token.typ)
+		if err != nil {
+			t.error(err)
+		}
+		return number
+	case itemLeftParen:
+		pipe := t.pipeline("parenthesized pipeline")
+		if token := t.next(); token.typ != itemRightParen {
+			t.errorf("unclosed right paren: unexpected %s", token)
+		}
+		return pipe
+	case itemString, itemRawString:
+		s, err := strconv.Unquote(token.val)
+		if err != nil {
+			t.error(err)
+		}
+		return newString(token.val, s)
+	}
+	t.backup()
+	return nil
 }
 
 // hasFunction reports if a function name exists in the Tree's maps.

src/pkg/text/template/parse/parse_test.go

@@ -188,6 +188,8 @@ var parseTests = []parseTest{
 		`{{$x := .X | .Y}}`},
 	{"nested pipeline", "{{.X (.Y .Z) (.A | .B .C) (.E)}}", noError,
 		`{{.X (.Y .Z) (.A | .B .C) (.E)}}`},
+	{"field applied to parentheses", "{{(.Y .Z).Field}}", noError,
+		`{{(.Y .Z).Field}}`},
 	{"simple if", "{{if .X}}hello{{end}}", noError,
 		`{{if .X}}"hello"{{end}}`},
 	{"if with else", "{{if .X}}true{{else}}false{{end}}", noError,
@@ -370,8 +372,9 @@ var errorTests = []parseTest{
 		"{{range .X}}",
 		hasError, `unexpected EOF`},
 	{"variable",
-		"{{$a.b := 23}}",
-		hasError, `illegal variable in declaration`},
+		// Declare $x so it's defined, to avoid that error, and then check we don't parse a declaration.
+		"{{$x := 23}}{{with $x.y := 3}}{{$x 23}}{{end}}",
+		hasError, `unexpected ":="`},
 	{"multidecl",
 		"{{$a,$b,$c := 23}}",
 		hasError, `too many declarations`},