text/template: detect unexported fields better

Moves the error detection back into execution, where it used to be,
and improves the error message.
Rolls back most of 6009048, which broke lower-case keys in maps.
If it weren't for maps we could detect this at compile time rather than
execution time.

Fixes #3542.

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

diff --git a/src/pkg/text/template/exec.go b/src/pkg/text/template/exec.go
index feb434a3be97f115a5047ae33156dda6dbbe2cc3..b8d23d43f97b59bb8e63c26af31bc4ed5609a5ca 100644 (file)
--- a/src/pkg/text/template/exec.go
+++ b/src/pkg/text/template/exec.go
@@ -12,6 +12,8 @@ import (
        "sort"
        "strings"
        "text/template/parse"
+       "unicode"
+       "unicode/utf8"
 )
 
 // state represents the state of an execution. It's not part of the
@@ -414,9 +416,13 @@ func (s *state) evalField(dot reflect.Value, fieldName string, args []parse.Node
                return s.evalCall(dot, method, fieldName, args, final)
        }
        hasArgs := len(args) > 1 || final.IsValid()
-       // It's not a method; is it a field of a struct?
+       // It's not a method; must be a field of a struct or an element of a map. The receiver must not be nil.
        receiver, isNil := indirect(receiver)
-       if receiver.Kind() == reflect.Struct {
+       if isNil {
+               s.errorf("nil pointer evaluating %s.%s", typ, fieldName)
+       }
+       switch receiver.Kind() {
+       case reflect.Struct:
                tField, ok := receiver.Type().FieldByName(fieldName)
                if ok {
                        field := receiver.FieldByIndex(tField.Index)
@@ -428,9 +434,11 @@ func (s *state) evalField(dot reflect.Value, fieldName string, args []parse.Node
                                return field
                        }
                }
-       }
-       // If it's a map, attempt to use the field name as a key.
-       if receiver.Kind() == reflect.Map {
+               if !isExported(fieldName) {
+                       s.errorf("%s is not an exported field of struct type %s", fieldName, typ)
+               }
+       case reflect.Map:
+               // If it's a map, attempt to use the field name as a key.
                nameVal := reflect.ValueOf(fieldName)
                if nameVal.Type().AssignableTo(receiver.Type().Key()) {
                        if hasArgs {
@@ -439,13 +447,16 @@ func (s *state) evalField(dot reflect.Value, fieldName string, args []parse.Node
                        return receiver.MapIndex(nameVal)
                }
        }
-       if isNil {
-               s.errorf("nil pointer evaluating %s.%s", typ, fieldName)
-       }
        s.errorf("can't evaluate field %s in type %s", fieldName, typ)
        panic("not reached")
 }
 
+// isExported reports whether the field name (which starts with a period) can be accessed.
+func isExported(fieldName string) bool {
+       r, _ := utf8.DecodeRuneInString(fieldName[1:]) // drop the period
+       return unicode.IsUpper(r)
+}
+
 var (
        errorType       = reflect.TypeOf((*error)(nil)).Elem()
        fmtStringerType = reflect.TypeOf((*fmt.Stringer)(nil)).Elem()

diff --git a/src/pkg/text/template/exec_test.go b/src/pkg/text/template/exec_test.go
index 37d25f470cd5890c043a834995eca7f4f5ab5f0b..8f6c67bbaa6b6092ca7d1804b7faf374a444b7a4 100644 (file)
--- a/src/pkg/text/template/exec_test.go
+++ b/src/pkg/text/template/exec_test.go
@@ -65,6 +65,8 @@ type T struct {
        VariadicFuncInt func(int, ...string) string
        // Template to test evaluation of templates.
        Tmpl *Template
+       // Unexported field; cannot be accessed by template.
+       unexported int
 }
 
 type U struct {
@@ -232,6 +234,7 @@ var execTests = []execTest{
        // Fields of structs.
        {".X", "-{{.X}}-", "-x-", tVal, true},
        {".U.V", "-{{.U.V}}-", "-v-", tVal, true},
+       {".unexported", "{{.unexported}}", "", tVal, false},
 
        // Fields on maps.
        {"map .one", "{{.MSI.one}}", "1", tVal, true},
@@ -473,6 +476,8 @@ var execTests = []execTest{
        // Pipelined arg was not being type-checked.
        {"bug8a", "{{3|oneArg}}", "", tVal, false},
        {"bug8b", "{{4|dddArg 3}}", "", tVal, false},
+       // A bug was introduced that broke map lookups for lower-case names.
+       {"bug9", "{{.cause}}", "neglect", map[string]string{"cause": "neglect"}, true},
 }
 
 func zeroArgs() string {

diff --git a/src/pkg/text/template/parse/parse.go b/src/pkg/text/template/parse/parse.go
index cbb1686a7f5b359dc8ddbe07a21745e4a5cf6c72..7970b5fcc649ac154822cb2484cb2d05d5819b01 100644 (file)
--- a/src/pkg/text/template/parse/parse.go
+++ b/src/pkg/text/template/parse/parse.go
@@ -14,7 +14,6 @@ import (
        "runtime"
        "strconv"
        "unicode"
-       "unicode/utf8"
 )
 
 // Tree is the representation of a single parsed template.
@@ -474,9 +473,6 @@ Loop:
                case itemVariable:
                        cmd.append(t.useVar(token.val))
                case itemField:
-                       if !isExported(token.val) {
-                               t.errorf("field %q not exported; cannot be evaluated", token.val)
-                       }
                        cmd.append(newField(token.val))
                case itemBool:
                        cmd.append(newBool(token.val == "true"))
@@ -502,12 +498,6 @@ Loop:
        return cmd
 }
 
-// isExported reports whether the field name (which starts with a period) can be accessed.
-func isExported(fieldName string) bool {
-       r, _ := utf8.DecodeRuneInString(fieldName[1:]) // drop the period
-       return unicode.IsUpper(r)
-}
-
 // hasFunction reports if a function name exists in the Tree's maps.
 func (t *Tree) hasFunction(name string) bool {
        for _, funcMap := range t.funcs {

diff --git a/src/pkg/text/template/parse/parse_test.go b/src/pkg/text/template/parse/parse_test.go
index fb98613fe15ce4ca92247a87807f33dc68fb3faf..b2e788238d3acb200c3416e8c4fe21ce35b86385 100644 (file)
--- a/src/pkg/text/template/parse/parse_test.go
+++ b/src/pkg/text/template/parse/parse_test.go
@@ -230,7 +230,6 @@ var parseTests = []parseTest{
        {"invalid punctuation", "{{printf 3, 4}}", hasError, ""},
        {"multidecl outside range", "{{with $v, $u := 3}}{{end}}", hasError, ""},
        {"too many decls in range", "{{range $u, $v, $w := 3}}{{end}}", hasError, ""},
-       {"unexported field", "{{.local}}", hasError, ""},
        // Equals (and other chars) do not assignments make (yet).
        {"bug0a", "{{$x := 0}}{{$x}}", noError, "{{$x := 0}}{{$x}}"},
        {"bug0b", "{{$x = 1}}{{$x}}", hasError, ""},