text/template: clean up function values

The recent addition of automatic function invocation generated
some troublesome ambiguities. Restore the previous behavior
and compensate by providing a "call" builtin to make it easy to
do what the automatic invocation did, but in a clear and explicit
manner.

Fixes #3140.

At least for now.

R=golang-dev, dsymonds, r
CC=golang-dev
https://golang.org/cl/5720065

4 files changed, 86 insertions, 18 deletions

diff --git a/src/pkg/text/template/doc.go b/src/pkg/text/template/doc.go
index ae91f4a541..10e0f7fc37 100644
--- a/src/pkg/text/template/doc.go
+++ b/src/pkg/text/template/doc.go
@@ -142,11 +142,6 @@ An argument is a simple value, denoted by one of the following.
 	    .Field1.Key1.Method1.Field2.Key2.Method2
 	  Methods can also be evaluated on variables, including chaining:
 	    $x.Method1.Field
-	- The name of a niladic function-valued struct field of the data,
-	  preceded by a period, such as
-		.Function
-	  Function-valued fields behave like methods (of structs) but do not
-	  pass a receiver.
 	- The name of a niladic function, such as
 		fun
 	  The result is the value of invoking the function, fun(). The return
@@ -155,6 +150,10 @@ An argument is a simple value, denoted by one of the following.
 
 Arguments may evaluate to any type; if they are pointers the implementation
 automatically indirects to the base type when required.
+If an evaluation yields a function value, such as a function-valued
+field of a struct, the function is not invoked automatically, but it
+can be used as a truth value for an if action and the like. To invoke
+it, use the call function, defined below.
 
 A pipeline is a possibly chained sequence of "commands". A command is a simple
 value (argument) or a function or method call, possibly with multiple arguments:
@@ -167,9 +166,6 @@ value (argument) or a function or method call, possibly with multiple arguments:
 		The result is the value of calling the method with the
 		arguments:
 			dot.Method(Argument1, etc.)
-	.Function [Argument...]
-		A function-valued field of a struct works like a method but does
-		not pass the receiver.
 	functionName [Argument...]
 		The result is the value of calling the function associated
 		with the name:
@@ -257,6 +253,17 @@ Predefined global functions are named as follows.
 		first empty argument or the last argument, that is,
 		"and x y" behaves as "if x then y else x". All the
 		arguments are evaluated.
+	call
+		Returns the result of calling the first argument, which
+		must be a function, with the remaining arguments as parameters.
+		Thus "call .X.Y 1 2" is, in Go notation, dot.X.Y(1, 2) where
+		Y is a func-valued field, map entry, or the like.
+		The first argument must be the result of an evaluation
+		that yields a value of function type (as distinct from
+		a predefined function such as print). The function must
+		return either one or two result values, the second of which
+		is of type error. If the arguments don't match the function
+		or the returned error value is non-nil, execution stops.
 	html
 		Returns the escaped HTML equivalent of the textual
 		representation of its arguments.
diff --git a/src/pkg/text/template/exec.go b/src/pkg/text/template/exec.go
index af745286c0..ad0118e4e6 100644
--- a/src/pkg/text/template/exec.go
+++ b/src/pkg/text/template/exec.go
@@ -421,11 +421,8 @@ func (s *state) evalField(dot reflect.Value, fieldName string, args []parse.Node
 			field := receiver.FieldByIndex(tField.Index)
 			if tField.PkgPath == "" { // field is exported
 				// If it's a function, we must call it.
-				if field.Type().Kind() == reflect.Func {
-					return s.evalCall(dot, field, fieldName, args, final)
-				}
 				if hasArgs {
-					s.errorf("%s is not a method or function but has arguments", fieldName)
+					s.errorf("%s has arguments but cannot be invoked as function", fieldName)
 				}
 				return field
 			}
diff --git a/src/pkg/text/template/exec_test.go b/src/pkg/text/template/exec_test.go
index 159cf5100d..83ca0022ba 100644
--- a/src/pkg/text/template/exec_test.go
+++ b/src/pkg/text/template/exec_test.go
@@ -60,7 +60,9 @@ type T struct {
 	PSI *[]int
 	NIL *int
 	// Function (not method)
-	Func func(...string) string
+	BinaryFunc      func(string, string) string
+	VariadicFunc    func(...string) string
+	VariadicFuncInt func(int, ...string) string
 	// Template to test evaluation of templates.
 	Tmpl *Template
 }
@@ -120,7 +122,9 @@ var tVal = &T{
 	Err:               errors.New("erroozle"),
 	PI:                newInt(23),
 	PSI:               newIntSlice(21, 22, 23),
-	Func:              func(s ...string) string { return fmt.Sprint("<", strings.Join(s, "+"), ">") },
+	BinaryFunc:        func(a, b string) string { return fmt.Sprintf("[%s=%s]", a, b) },
+	VariadicFunc:      func(s ...string) string { return fmt.Sprint("<", strings.Join(s, "+"), ">") },
+	VariadicFuncInt:   func(a int, s ...string) string { return fmt.Sprint(a, "=<", strings.Join(s, "+"), ">") },
 	Tmpl:              Must(New("x").Parse("test template")), // "x" is the value of .X
 }
 
@@ -300,13 +304,25 @@ var execTests = []execTest{
 		"{{with $x := .}}{{with .SI}}{{$.GetU.TrueFalse $.True}}{{end}}{{end}}",
 		"true", tVal, true},
 
-	// Function call
-	{".Func", "-{{.Func}}-", "-<>-", tVal, true},
-	{".Func2", "-{{.Func `he` `llo`}}-", "-<he+llo>-", tVal, true},
+	// Function call builtin.
+	{".BinaryFunc", "{{call .BinaryFunc `1` `2`}}", "[1=2]", tVal, true},
+	{".VariadicFunc0", "{{call .VariadicFunc}}", "<>", tVal, true},
+	{".VariadicFunc2", "{{call .VariadicFunc `he` `llo`}}", "<he+llo>", tVal, true},
+	{".VariadicFuncInt", "{{call .VariadicFuncInt 33 `he` `llo`}}", "33=<he+llo>", tVal, true},
+	{"if .BinaryFunc call", "{{ if .BinaryFunc}}{{call .BinaryFunc `1` `2`}}{{end}}", "[1=2]", tVal, true},
+	{"if not .BinaryFunc call", "{{ if not .BinaryFunc}}{{call .BinaryFunc `1` `2`}}{{else}}No{{end}}", "No", tVal, true},
+
+	// Erroneous function calls (check args).
+	{".BinaryFuncTooFew", "{{call .BinaryFunc `1`}}", "", tVal, false},
+	{".BinaryFuncTooMany", "{{call .BinaryFunc `1` `2` `3`}}", "", tVal, false},
+	{".BinaryFuncBad0", "{{call .BinaryFunc 1 3}}", "", tVal, false},
+	{".BinaryFuncBad1", "{{call .BinaryFunc `1` 3}}", "", tVal, false},
+	{".VariadicFuncBad0", "{{call .VariadicFunc 3}}", "", tVal, false},
+	{".VariadicFuncIntBad0", "{{call .VariadicFuncInt `x`}}", "", tVal, false},
 
 	// Pipelines.
 	{"pipeline", "-{{.Method0 | .Method2 .U16}}-", "-Method2: 16 M0-", tVal, true},
-	{"pipeline func", "-{{.Func `llo` | .Func `he` }}-", "-<he+<llo>>-", tVal, true},
+	{"pipeline func", "-{{call .VariadicFunc `llo` | call .VariadicFunc `he` }}-", "-<he+<llo>>-", tVal, true},
 
 	// If.
 	{"if true", "{{if true}}TRUE{{end}}", "TRUE", tVal, true},
diff --git a/src/pkg/text/template/funcs.go b/src/pkg/text/template/funcs.go
index d6e4bf1a21..525179cb49 100644
--- a/src/pkg/text/template/funcs.go
+++ b/src/pkg/text/template/funcs.go
@@ -24,6 +24,7 @@ type FuncMap map[string]interface{}
 
 var builtins = FuncMap{
 	"and":      and,
+	"call":     call,
 	"html":     HTMLEscaper,
 	"index":    index,
 	"js":       JSEscaper,
@@ -151,6 +152,53 @@ func length(item interface{}) (int, error) {
 	return 0, fmt.Errorf("len of type %s", v.Type())
 }
 
+// Function invocation
+
+// call returns the result of evaluating the the first argument as a function.
+// The function must return 1 result, or 2 results, the second of which is an error.
+func call(fn interface{}, args ...interface{}) (interface{}, error) {
+	v := reflect.ValueOf(fn)
+	typ := v.Type()
+	if typ.Kind() != reflect.Func {
+		return nil, fmt.Errorf("non-function of type %s", typ)
+	}
+	if !goodFunc(typ) {
+		return nil, fmt.Errorf("function called with %d args; should be 1 or 2", typ.NumOut())
+	}
+	numIn := typ.NumIn()
+	var dddType reflect.Type
+	if typ.IsVariadic() {
+		if len(args) < numIn-1 {
+			return nil, fmt.Errorf("wrong number of args: got %d want at least %d", len(args), numIn-1)
+		}
+		dddType = typ.In(numIn - 1).Elem()
+	} else {
+		if len(args) != numIn {
+			return nil, fmt.Errorf("wrong number of args: got %d want %d", len(args), numIn)
+		}
+	}
+	argv := make([]reflect.Value, len(args))
+	for i, arg := range args {
+		value := reflect.ValueOf(arg)
+		// Compute the expected type. Clumsy because of variadics.
+		var argType reflect.Type
+		if !typ.IsVariadic() || i < numIn-1 {
+			argType = typ.In(i)
+		} else {
+			argType = dddType
+		}
+		if !value.Type().AssignableTo(argType) {
+			return nil, fmt.Errorf("arg %d has type %s; should be %s", i, value.Type(), argType)
+		}
+		argv[i] = reflect.ValueOf(arg)
+	}
+	result := v.Call(argv)
+	if len(result) == 2 {
+		return result[0].Interface(), result[1].Interface().(error)
+	}
+	return result[0].Interface(), nil
+}
+
 // Boolean logic.
 
 func truth(a interface{}) bool {