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>

diff --git a/src/text/template/exec.go b/src/text/template/exec.go
index 2ed52723498fd50dfb8b14053f321b45cd4b6b4c..099726a180773aa3ebab44d8b0eaac4f85e49575 100644 (file)
--- 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)
        }

diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go
index e33e0794dd1fbfd5d6ab4e867825f41530139639..8fb4749169738b42f9b7fce6c27f5f34c0a5961f 100644 (file)
--- a/src/text/template/exec_test.go
+++ b/src/text/template/exec_test.go
@@ -382,6 +382,11 @@ var execTests = []execTest{
        {"pipeline", "-{{.Method0 | .Method2 .U16}}-", "-Method2: 16 M0-", tVal, true},
        {"pipeline func", "-{{call .VariadicFunc `llo` | call .VariadicFunc `he` }}-", "-<he+<llo>>-", tVal, true},
 
+       // Nil values aren't missing arguments.
+       {"nil pipeline", "{{ .Empty0 | call .NilOKFunc }}", "true", tVal, true},
+       {"nil call arg", "{{ call .NilOKFunc .Empty0 }}", "true", tVal, true},
+       {"bad nil pipeline", "{{ .Empty0 | .VariadicFunc }}", "", tVal, false},
+
        // Parenthesized expressions
        {"parens in pipeline", "{{printf `%d %d %d` (1) (2 | add 3) (add 4 (add 5 6))}}", "1 5 15", tVal, true},