text/template: towards better errors

Give the right name for errors, and add a test to check we're
getting the errors we expect.
Also fix an ordering bug (calling add after stopParse) that
caused a nil indirection rather than a helpful error.
Fixes #3280.

R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/6520043

diff --git a/src/pkg/text/template/parse/parse.go b/src/pkg/text/template/parse/parse.go
index 6177e32e732f246ef87fd53edfb94f90ba4f050b..c52e41d166dc1f70e978a8e1acf1c2199194f210 100644 (file)
--- a/src/pkg/text/template/parse/parse.go
+++ b/src/pkg/text/template/parse/parse.go
@@ -18,8 +18,9 @@ import (
 
 // Tree is the representation of a single parsed template.
 type Tree struct {
-       Name string    // name of the template represented by the tree.
-       Root *ListNode // top-level root of the tree.
+       Name      string    // name of the template represented by the tree.
+       ParseName string    // name of the top-level template during parsing, for error messages.
+       Root      *ListNode // top-level root of the tree.
        // Parsing only; cleared after parse.
        funcs     []map[string]interface{}
        lex       *lexer
@@ -114,7 +115,7 @@ func New(name string, funcs ...map[string]interface{}) *Tree {
 // errorf formats the error and terminates processing.
 func (t *Tree) errorf(format string, args ...interface{}) {
        t.Root = nil
-       format = fmt.Sprintf("template: %s:%d: %s", t.Name, t.lex.lineNumber(), format)
+       format = fmt.Sprintf("template: %s:%d: %s", t.ParseName, t.lex.lineNumber(), format)
        panic(fmt.Errorf(format, args...))
 }
 
@@ -203,6 +204,7 @@ func (t *Tree) atEOF() bool {
 // the treeSet map.
 func (t *Tree) Parse(s, leftDelim, rightDelim string, treeSet map[string]*Tree, funcs ...map[string]interface{}) (tree *Tree, err error) {
        defer t.recover(&err)
+       t.ParseName = t.Name
        t.startParse(funcs, lex(t.Name, s, leftDelim, rightDelim))
        t.parse(treeSet)
        t.add(treeSet)
@@ -257,6 +259,7 @@ func (t *Tree) parse(treeSet map[string]*Tree) (next Node) {
                        delim := t.next()
                        if t.nextNonSpace().typ == itemDefine {
                                newT := New("definition") // name will be updated once we know it.
+                               newT.ParseName = t.ParseName
                                newT.startParse(t.funcs, t.lex)
                                newT.parseDefinition(treeSet)
                                continue
@@ -289,8 +292,8 @@ func (t *Tree) parseDefinition(treeSet map[string]*Tree) {
        if end.Type() != nodeEnd {
                t.errorf("unexpected %s in %s", end, context)
        }
-       t.stopParse()
        t.add(treeSet)
+       t.stopParse()
 }
 
 // itemList:

diff --git a/src/pkg/text/template/parse/parse_test.go b/src/pkg/text/template/parse/parse_test.go
index 3838250ef293402dc280277ec3c4d51c21371a06..4be4ca077d4583b1dde1c32fd4cc8e2c07c558fd 100644 (file)
--- a/src/pkg/text/template/parse/parse_test.go
+++ b/src/pkg/text/template/parse/parse_test.go
@@ -7,10 +7,11 @@ package parse
 import (
        "flag"
        "fmt"
+       "strings"
        "testing"
 )
 
-var debug = flag.Bool("debug", false, "show the errors produced by the tests")
+var debug = flag.Bool("debug", false, "show the errors produced by the main tests")
 
 type numberTest struct {
        text      string
@@ -321,3 +322,73 @@ func TestIsEmpty(t *testing.T) {
                }
        }
 }
+
+// All failures, and the result is a string that must appear in the error message.
+var errorTests = []parseTest{
+       // Check line numbers are accurate.
+       {"unclosed1",
+               "line1\n{{",
+               hasError, `unclosed1:2: unexpected unclosed action in command`},
+       {"unclosed2",
+               "line1\n{{define `x`}}line2\n{{",
+               hasError, `unclosed2:3: unexpected unclosed action in command`},
+       // Specific errors.
+       {"function",
+               "{{foo}}",
+               hasError, `function "foo" not defined`},
+       {"comment",
+               "{{/*}}",
+               hasError, `unclosed comment`},
+       {"lparen",
+               "{{.X (1 2 3}}",
+               hasError, `unclosed left paren`},
+       {"rparen",
+               "{{.X 1 2 3)}}",
+               hasError, `unexpected ")"`},
+       {"space",
+               "{{`x`3}}",
+               hasError, `missing space?`},
+       {"idchar",
+               "{{a#}}",
+               hasError, `'#'`},
+       {"charconst",
+               "{{'a}}",
+               hasError, `unterminated character constant`},
+       {"stringconst",
+               `{{"a}}`,
+               hasError, `unterminated quoted string`},
+       {"rawstringconst",
+               "{{`a}}",
+               hasError, `unterminated raw quoted string`},
+       {"number",
+               "{{0xi}}",
+               hasError, `number syntax`},
+       {"multidefine",
+               "{{define `a`}}a{{end}}{{define `a`}}b{{end}}",
+               hasError, `multiple definition of template`},
+       {"eof",
+               "{{range .X}}",
+               hasError, `unexpected EOF`},
+       {"variable",
+               "{{$a.b := 23}}",
+               hasError, `illegal variable in declaration`},
+       {"multidecl",
+               "{{$a,$b,$c := 23}}",
+               hasError, `too many declarations`},
+       {"undefvar",
+               "{{$a}}",
+               hasError, `undefined variable`},
+}
+
+func TestErrors(t *testing.T) {
+       for _, test := range errorTests {
+               _, err := New(test.name).Parse(test.input, "", "", make(map[string]*Tree))
+               if err == nil {
+                       t.Errorf("%q: expected error", test.name)
+                       continue
+               }
+               if !strings.Contains(err.Error(), test.result) {
+                       t.Errorf("%q: error %q does not contain %q", test.name, err, test.result)
+               }
+       }
+}