exp/template: remove reflect from the API

It means keeping two sets of maps, but things look cleaner from
the outside.

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

diff --git a/src/pkg/exp/template/funcs.go b/src/pkg/exp/template/funcs.go
index 0fbc0e4c6dfa512c9b573723ef131efe6247fea3..579c70099cbb54e3968bfbb3c37f1bec517ef938 100644 (file)
--- a/src/pkg/exp/template/funcs.go
+++ b/src/pkg/exp/template/funcs.go
@@ -22,22 +22,31 @@ import (
 // during execution, execution terminates and Execute returns an error.
 type FuncMap map[string]interface{}
 
-var builtins = map[string]reflect.Value{
-       "and":     reflect.ValueOf(and),
-       "html":    reflect.ValueOf(HTMLEscaper),
-       "index":   reflect.ValueOf(index),
-       "js":      reflect.ValueOf(JSEscaper),
-       "not":     reflect.ValueOf(not),
-       "or":      reflect.ValueOf(or),
-       "print":   reflect.ValueOf(fmt.Sprint),
-       "printf":  reflect.ValueOf(fmt.Sprintf),
-       "println": reflect.ValueOf(fmt.Sprintln),
-       "url":     reflect.ValueOf(URLEscaper),
+var builtins = FuncMap{
+       "and":     and,
+       "html":    HTMLEscaper,
+       "index":   index,
+       "js":      JSEscaper,
+       "not":     not,
+       "or":      or,
+       "print":   fmt.Sprint,
+       "printf":  fmt.Sprintf,
+       "println": fmt.Sprintln,
+       "url":     URLEscaper,
 }
 
-// addFuncs adds to values the functions in funcs, converting them to reflect.Values.
-func addFuncs(values map[string]reflect.Value, funcMap FuncMap) {
-       for name, fn := range funcMap {
+var builtinFuncs = createValueFuncs(builtins)
+
+// createValueFuncs turns a FuncMap into a map[string]reflect.Value
+func createValueFuncs(funcMap FuncMap) map[string]reflect.Value {
+       m := make(map[string]reflect.Value)
+       addValueFuncs(m, funcMap)
+       return m
+}
+
+// addValueFuncs adds to values the functions in funcs, converting them to reflect.Values.
+func addValueFuncs(out map[string]reflect.Value, in FuncMap) {
+       for name, fn := range in {
                v := reflect.ValueOf(fn)
                if v.Kind() != reflect.Func {
                        panic("value for " + name + " not a function")
@@ -45,7 +54,15 @@ func addFuncs(values map[string]reflect.Value, funcMap FuncMap) {
                if !goodFunc(v.Type()) {
                        panic(fmt.Errorf("can't handle multiple results from method/function %q", name))
                }
-               values[name] = v
+               out[name] = v
+       }
+}
+
+// addFuncs adds to values the functions in funcs. It does no checking of the input -
+// call addValueFuncs first.
+func addFuncs(out, in FuncMap) {
+       for name, fn := range in {
+               out[name] = fn
        }
 }
 
@@ -64,16 +81,16 @@ func goodFunc(typ reflect.Type) bool {
 // findFunction looks for a function in the template, set, and global map.
 func findFunction(name string, tmpl *Template, set *Set) (reflect.Value, bool) {
        if tmpl != nil {
-               if fn := tmpl.funcs[name]; fn.IsValid() {
+               if fn := tmpl.execFuncs[name]; fn.IsValid() {
                        return fn, true
                }
        }
        if set != nil {
-               if fn := set.funcs[name]; fn.IsValid() {
+               if fn := set.execFuncs[name]; fn.IsValid() {
                        return fn, true
                }
        }
-       if fn := builtins[name]; fn.IsValid() {
+       if fn := builtinFuncs[name]; fn.IsValid() {
                return fn, true
        }
        return reflect.Value{}, false

diff --git a/src/pkg/exp/template/parse.go b/src/pkg/exp/template/parse.go
index 9cc48c48f40c0e6dc7dc76acc1c920afc6ad309a..6db00c1c117ea0a1383dc7fc3aec56acc88b726f 100644 (file)
--- a/src/pkg/exp/template/parse.go
+++ b/src/pkg/exp/template/parse.go
@@ -14,8 +14,12 @@ import (
 type Template struct {
        name string
        *parse.Tree
-       funcs map[string]reflect.Value
-       set   *Set // can be nil.
+       // We use two maps, one for parsing and one for execution.
+       // This separation makes the API cleaner since it doesn't
+       // expose reflection to the client.
+       parseFuncs FuncMap
+       execFuncs  map[string]reflect.Value
+       set        *Set // can be nil.
 }
 
 // Name returns the name of the template.
@@ -28,8 +32,9 @@ func (t *Template) Name() string {
 // New allocates a new template with the given name.
 func New(name string) *Template {
        return &Template{
-               name:  name,
-               funcs: make(map[string]reflect.Value),
+               name:       name,
+               parseFuncs: make(FuncMap),
+               execFuncs:  make(map[string]reflect.Value),
        }
 }
 
@@ -38,14 +43,15 @@ func New(name string) *Template {
 // return type.
 // The return value is the template, so calls can be chained.
 func (t *Template) Funcs(funcMap FuncMap) *Template {
-       addFuncs(t.funcs, funcMap)
+       addValueFuncs(t.execFuncs, funcMap)
+       addFuncs(t.parseFuncs, funcMap)
        return t
 }
 
 // Parse parses the template definition string to construct an internal
 // representation of the template for execution.
 func (t *Template) Parse(s string) (tmpl *Template, err os.Error) {
-       t.Tree, err = parse.New(t.name).Parse(s, t.funcs, builtins)
+       t.Tree, err = parse.New(t.name).Parse(s, t.parseFuncs, builtins)
        if err != nil {
                return nil, err
        }
@@ -57,11 +63,11 @@ func (t *Template) Parse(s string) (tmpl *Template, err os.Error) {
 // to the set.
 // Function bindings are checked against those in the set.
 func (t *Template) ParseInSet(s string, set *Set) (tmpl *Template, err os.Error) {
-       var setFuncs map[string]reflect.Value
+       var setFuncs FuncMap
        if set != nil {
-               setFuncs = set.funcs
+               setFuncs = set.parseFuncs
        }
-       t.Tree, err = parse.New(t.name).Parse(s, t.funcs, setFuncs, builtins)
+       t.Tree, err = parse.New(t.name).Parse(s, t.parseFuncs, setFuncs, builtins)
        if err != nil {
                return nil, err
        }

diff --git a/src/pkg/exp/template/parse/parse.go b/src/pkg/exp/template/parse/parse.go
index 2ee08da74b9be3b0d307b7783d4ab98073f2afef..f8f9023e54fb6939731fa0fd57dbc5ca1cf6b28c 100644 (file)
--- a/src/pkg/exp/template/parse/parse.go
+++ b/src/pkg/exp/template/parse/parse.go
@@ -9,7 +9,6 @@ package parse
 import (
        "fmt"
        "os"
-       "reflect"
        "runtime"
        "strconv"
        "unicode"
@@ -20,7 +19,7 @@ type Tree struct {
        Name string    // Name is the name of the template.
        Root *ListNode // Root is the top-level root of the parse tree.
        // Parsing only; cleared after parse.
-       funcs     []map[string]reflect.Value
+       funcs     []map[string]interface{}
        lex       *lexer
        token     [2]item // two-token lookahead for parser.
        peekCount int
@@ -61,7 +60,7 @@ func (t *Tree) peek() item {
 // Parsing.
 
 // New allocates a new template with the given name.
-func New(name string, funcs ...map[string]reflect.Value) *Tree {
+func New(name string, funcs ...map[string]interface{}) *Tree {
        return &Tree{
                Name:  name,
                funcs: funcs,
@@ -110,7 +109,7 @@ func (t *Tree) recover(errp *os.Error) {
 }
 
 // startParse starts the template parsing from the lexer.
-func (t *Tree) startParse(funcs []map[string]reflect.Value, lex *lexer) {
+func (t *Tree) startParse(funcs []map[string]interface{}, lex *lexer) {
        t.Root = nil
        t.lex = lex
        t.vars = []string{"$"}
@@ -147,7 +146,7 @@ func (t *Tree) atEOF() bool {
 
 // Parse parses the template definition string to construct an internal
 // representation of the template for execution.
-func (t *Tree) Parse(s string, funcs ...map[string]reflect.Value) (tree *Tree, err os.Error) {
+func (t *Tree) Parse(s string, funcs ...map[string]interface{}) (tree *Tree, err os.Error) {
        defer t.recover(&err)
        t.startParse(funcs, lex(t.Name, s))
        t.parse(true)
@@ -371,7 +370,7 @@ Loop:
                case itemError:
                        t.errorf("%s", token.val)
                case itemIdentifier:
-                       if _, ok := t.findFunction(token.val); !ok {
+                       if !t.hasFunction(token.val) {
                                t.errorf("function %q not defined", token.val)
                        }
                        cmd.append(newIdentifier(token.val))
@@ -405,17 +404,17 @@ Loop:
        return cmd
 }
 
-// findFunction looks for a function in the Tree's maps.
-func (t *Tree) findFunction(name string) (reflect.Value, bool) {
+// hasFunction reports if a function name exists in the Tree's maps.
+func (t *Tree) hasFunction(name string) bool {
        for _, funcMap := range t.funcs {
                if funcMap == nil {
                        continue
                }
-               if fn := funcMap[name]; fn.IsValid() {
-                       return fn, true
+               if funcMap[name] != nil {
+                       return true
                }
        }
-       return reflect.Value{}, false
+       return false
 }
 
 // popVars trims the variable list to the specified length

diff --git a/src/pkg/exp/template/parse/parse_test.go b/src/pkg/exp/template/parse/parse_test.go
index f57dab8b23b2fbb788fba502de4774a5bc4a5550..1928c319dec6df6dcdb1a722c09ad84b0315a883 100644 (file)
--- a/src/pkg/exp/template/parse/parse_test.go
+++ b/src/pkg/exp/template/parse/parse_test.go
@@ -7,7 +7,6 @@ package parse
 import (
        "flag"
        "fmt"
-       "reflect"
        "testing"
 )
 
@@ -231,8 +230,8 @@ var parseTests = []parseTest{
        {"too many decls in range", "{{range $u, $v, $w := 3}}{{end}}", hasError, ""},
 }
 
-var builtins = map[string]reflect.Value{
-       "printf": reflect.ValueOf(fmt.Sprintf),
+var builtins = map[string]interface{}{
+       "printf": fmt.Sprintf,
 }
 
 func TestParse(t *testing.T) {

diff --git a/src/pkg/exp/template/parse/set.go b/src/pkg/exp/template/parse/set.go
index 91173d5c122ca2b0f76d507a7720470cf92ebe03..4820da925b2b1d22c770f1bb048f1e164b421938 100644 (file)
--- a/src/pkg/exp/template/parse/set.go
+++ b/src/pkg/exp/template/parse/set.go
@@ -7,14 +7,13 @@ package parse
 import (
        "fmt"
        "os"
-       "reflect"
        "strconv"
 )
 
 // Set returns a slice of Trees created by parsing the template set
 // definition in the argument string. If an error is encountered,
 // parsing stops and an empty slice is returned with the error.
-func Set(text string, funcs ...map[string]reflect.Value) (tree map[string]*Tree, err os.Error) {
+func Set(text string, funcs ...map[string]interface{}) (tree map[string]*Tree, err os.Error) {
        tree = make(map[string]*Tree)
        defer (*Tree)(nil).recover(&err)
        lex := lex("set", text)

diff --git a/src/pkg/exp/template/set.go b/src/pkg/exp/template/set.go
index f6f2a2c2765566d556fa8690b156ef90844d4ced..7f2813c048609fdf0b1b62973261874bdcd26a39 100644 (file)
--- a/src/pkg/exp/template/set.go
+++ b/src/pkg/exp/template/set.go
@@ -16,14 +16,16 @@ import (
 // The zero value represents an empty set.
 // A template may be a member of multiple sets.
 type Set struct {
-       tmpl  map[string]*Template
-       funcs map[string]reflect.Value
+       tmpl       map[string]*Template
+       parseFuncs FuncMap
+       execFuncs  map[string]reflect.Value
 }
 
 func (s *Set) init() {
        if s.tmpl == nil {
                s.tmpl = make(map[string]*Template)
-               s.funcs = make(map[string]reflect.Value)
+               s.parseFuncs = make(FuncMap)
+               s.execFuncs = make(map[string]reflect.Value)
        }
 }
 
@@ -33,7 +35,8 @@ func (s *Set) init() {
 // The return value is the set, so calls can be chained.
 func (s *Set) Funcs(funcMap FuncMap) *Set {
        s.init()
-       addFuncs(s.funcs, funcMap)
+       addValueFuncs(s.execFuncs, funcMap)
+       addFuncs(s.parseFuncs, funcMap)
        return s
 }
 
@@ -71,8 +74,8 @@ func (s *Set) Template(name string) *Template {
 }
 
 // FuncMap returns the set's function map.
-func (s *Set) FuncMap() map[string]reflect.Value {
-       return s.funcs
+func (s *Set) FuncMap() FuncMap {
+       return s.parseFuncs
 }
 
 // Execute applies the named template to the specified data object, writing
@@ -90,7 +93,7 @@ func (s *Set) Execute(wr io.Writer, name string, data interface{}) os.Error {
 // to the set.  If a template is redefined, the element in the set is
 // overwritten with the new definition.
 func (s *Set) Parse(text string) (*Set, os.Error) {
-       trees, err := parse.Set(text, s.funcs, builtins)
+       trees, err := parse.Set(text, s.parseFuncs, builtins)
        if err != nil {
                return nil, err
        }