From 3cb54c86043a92ab080a89c06643d80015a5638e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Mon, 19 Feb 2018 20:48:58 +0000 Subject: [PATCH] text/template: differentiate nil from missing arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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í TryBot-Result: Gobot Gobot Reviewed-by: Rob Pike --- src/text/template/exec.go | 31 ++++++++++++++++++------------- src/text/template/exec_test.go | 5 +++++ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/text/template/exec.go b/src/text/template/exec.go index 2ed52723498fd..099726a180773 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) } diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go index e33e0794dd1fb..8fb4749169738 100644 --- 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` }}-", "->-", 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},