text/template: differentiate nil from missing arg

reflect.Value is a struct and does not have a kind nor any flag for
untyped nils. As a result, it is tricky to differentiate when we're
missing a value, from when we have one but it is untyped nil.

We could start using *reflect.Value instead, to add one level of
indirection, using nil for missing values and new(reflect.Value) for
untyped nils. However, that is a fairly invasive change, and would also
mean unnecessary allocations.

Instead, use a special reflect.Value that depicts when a value is
missing. This is the case for the "final" reflect.Value in multiple
scenarios, such as the start of a pipeline. Give it a specific,
unexported type too, to make sure it cannot be mistaken for any other
valid value.

Finally, replace "final.IsValid()" with "final != missingVal", since
final.IsValid() will be false when final is an untyped nil.

Also add a few test cases, all different variants of the untyped nil
versus missing value scenario.

Fixes #18716.

Change-Id: Ia9257a84660ead5a7007fd1cced7782760b62d9d
Reviewed-on: https://go-review.googlesource.com/95215
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

1 files changed, 18 insertions, 13 deletions

diff --git a/src/text/template/exec.go b/src/text/template/exec.go
index 2ed5272349..099726a180 100644
--- a/src/text/template/exec.go
+++ b/src/text/template/exec.go
@@ -71,6 +71,10 @@ func (s *state) varValue(name string) reflect.Value {
 
 var zero reflect.Value
 
+type missingValType struct{}
+
+var missingVal = reflect.ValueOf(missingValType{})
+
 // at marks the state to be on node n, for error reporting.
 func (s *state) at(node parse.Node) {
 	s.node = node
@@ -401,6 +405,7 @@ func (s *state) evalPipeline(dot reflect.Value, pipe *parse.PipeNode) (value ref
 		return
 	}
 	s.at(pipe)
+	value = missingVal
 	for _, cmd := range pipe.Cmds {
 		value = s.evalCommand(dot, cmd, value) // previous value is this one's final arg.
 		// If the object has type interface{}, dig down one level to the thing inside.
@@ -415,7 +420,7 @@ func (s *state) evalPipeline(dot reflect.Value, pipe *parse.PipeNode) (value ref
 }
 
 func (s *state) notAFunction(args []parse.Node, final reflect.Value) {
-	if len(args) > 1 || final.IsValid() {
+	if len(args) > 1 || final != missingVal {
 		s.errorf("can't give argument to non-function %s", args[0])
 	}
 }
@@ -519,7 +524,7 @@ func (s *state) evalVariableNode(dot reflect.Value, variable *parse.VariableNode
 func (s *state) evalFieldChain(dot, receiver reflect.Value, node parse.Node, ident []string, args []parse.Node, final reflect.Value) reflect.Value {
 	n := len(ident)
 	for i := 0; i < n-1; i++ {
-		receiver = s.evalField(dot, ident[i], node, nil, zero, receiver)
+		receiver = s.evalField(dot, ident[i], node, nil, missingVal, receiver)
 	}
 	// Now if it's a method, it gets the arguments.
 	return s.evalField(dot, ident[n-1], node, args, final, receiver)
@@ -556,7 +561,7 @@ func (s *state) evalField(dot reflect.Value, fieldName string, node parse.Node,
 	if method := ptr.MethodByName(fieldName); method.IsValid() {
 		return s.evalCall(dot, method, node, fieldName, args, final)
 	}
-	hasArgs := len(args) > 1 || final.IsValid()
+	hasArgs := len(args) > 1 || final != missingVal
 	// It's not a method; must be a field of a struct or an element of a map.
 	switch receiver.Kind() {
 	case reflect.Struct:
@@ -618,7 +623,7 @@ func (s *state) evalCall(dot, fun reflect.Value, node parse.Node, name string, a
 	}
 	typ := fun.Type()
 	numIn := len(args)
-	if final.IsValid() {
+	if final != missingVal {
 		numIn++
 	}
 	numFixed := len(args)
@@ -628,7 +633,7 @@ func (s *state) evalCall(dot, fun reflect.Value, node parse.Node, name string, a
 			s.errorf("wrong number of args for %s: want at least %d got %d", name, typ.NumIn()-1, len(args))
 		}
 	} else if numIn != typ.NumIn() {
-		s.errorf("wrong number of args for %s: want %d got %d", name, typ.NumIn(), len(args))
+		s.errorf("wrong number of args for %s: want %d got %d", name, typ.NumIn(), numIn)
 	}
 	if !goodFunc(typ) {
 		// TODO: This could still be a confusing error; maybe goodFunc should provide info.
@@ -649,7 +654,7 @@ func (s *state) evalCall(dot, fun reflect.Value, node parse.Node, name string, a
 		}
 	}
 	// Add final value if necessary.
-	if final.IsValid() {
+	if final != missingVal {
 		t := typ.In(typ.NumIn() - 1)
 		if typ.IsVariadic() {
 			if numIn-1 < numFixed {
@@ -742,15 +747,15 @@ func (s *state) evalArg(dot reflect.Value, typ reflect.Type, n parse.Node) refle
 		}
 		s.errorf("cannot assign nil to %s", typ)
 	case *parse.FieldNode:
-		return s.validateType(s.evalFieldNode(dot, arg, []parse.Node{n}, zero), typ)
+		return s.validateType(s.evalFieldNode(dot, arg, []parse.Node{n}, missingVal), typ)
 	case *parse.VariableNode:
-		return s.validateType(s.evalVariableNode(dot, arg, nil, zero), typ)
+		return s.validateType(s.evalVariableNode(dot, arg, nil, missingVal), typ)
 	case *parse.PipeNode:
 		return s.validateType(s.evalPipeline(dot, arg), typ)
 	case *parse.IdentifierNode:
-		return s.validateType(s.evalFunction(dot, arg, arg, nil, zero), typ)
+		return s.validateType(s.evalFunction(dot, arg, arg, nil, missingVal), typ)
 	case *parse.ChainNode:
-		return s.validateType(s.evalChainNode(dot, arg, nil, zero), typ)
+		return s.validateType(s.evalChainNode(dot, arg, nil, missingVal), typ)
 	}
 	switch typ.Kind() {
 	case reflect.Bool:
@@ -851,9 +856,9 @@ func (s *state) evalEmptyInterface(dot reflect.Value, n parse.Node) reflect.Valu
 	case *parse.DotNode:
 		return dot
 	case *parse.FieldNode:
-		return s.evalFieldNode(dot, n, nil, zero)
+		return s.evalFieldNode(dot, n, nil, missingVal)
 	case *parse.IdentifierNode:
-		return s.evalFunction(dot, n, n, nil, zero)
+		return s.evalFunction(dot, n, n, nil, missingVal)
 	case *parse.NilNode:
 		// NilNode is handled in evalArg, the only place that calls here.
 		s.errorf("evalEmptyInterface: nil (can't happen)")
@@ -862,7 +867,7 @@ func (s *state) evalEmptyInterface(dot reflect.Value, n parse.Node) reflect.Valu
 	case *parse.StringNode:
 		return reflect.ValueOf(n.Text)
 	case *parse.VariableNode:
-		return s.evalVariableNode(dot, n, nil, zero)
+		return s.evalVariableNode(dot, n, nil, missingVal)
 	case *parse.PipeNode:
 		return s.evalPipeline(dot, n)
 	}