text/template/parse, html/template: copy Tree.text during html template clone
The root cause of the panic reported in https://code.google.com/p/go/issues/detail?id=5980
is that parse's Tree.Text wasn't being copied during the clone.
Fix this by adding and using a Copy method for parse.Tree.
Fixes #5980.
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/12420044
diff --git a/src/pkg/html/template/clone_test.go b/src/pkg/html/template/clone_test.go
index 2663cdd..e11bff2 100644
--- a/src/pkg/html/template/clone_test.go
+++ b/src/pkg/html/template/clone_test.go
@@ -6,6 +6,8 @@
import (
"bytes"
+ "errors"
+ "io/ioutil"
"testing"
"text/template/parse"
)
@@ -146,3 +148,41 @@
Must(t1.New("t1").Parse(`{{define "foo"}}foo{{end}}`))
t1.Clone()
}
+
+// Ensure that this guarantee from the docs is upheld:
+// "Further calls to Parse in the copy will add templates
+// to the copy but not to the original."
+func TestCloneThenParse(t *testing.T) {
+ t0 := Must(New("t0").Parse(`{{define "a"}}{{template "embedded"}}{{end}}`))
+ t1 := Must(t0.Clone())
+ Must(t1.Parse(`{{define "embedded"}}t1{{end}}`))
+ if len(t0.Templates())+1 != len(t1.Templates()) {
+ t.Error("adding a template to a clone added it to the original")
+ }
+ // double check that the embedded template isn't available in the original
+ err := t0.ExecuteTemplate(ioutil.Discard, "a", nil)
+ if err == nil {
+ t.Error("expected 'no such template' error")
+ }
+}
+
+// https://code.google.com/p/go/issues/detail?id=5980
+func TestFuncMapWorksAfterClone(t *testing.T) {
+ funcs := FuncMap{"customFunc": func() (string, error) {
+ return "", errors.New("issue5980")
+ }}
+
+ // get the expected error output (no clone)
+ uncloned := Must(New("").Funcs(funcs).Parse("{{customFunc}}"))
+ wantErr := uncloned.Execute(ioutil.Discard, nil)
+
+ // toClone must be the same as uncloned. It has to be recreated from scratch,
+ // since cloning cannot occur after execution.
+ toClone := Must(New("").Funcs(funcs).Parse("{{customFunc}}"))
+ cloned := Must(toClone.Clone())
+ gotErr := cloned.Execute(ioutil.Discard, nil)
+
+ if wantErr.Error() != gotErr.Error() {
+ t.Errorf("clone error message mismatch want %q got %q", wantErr, gotErr)
+ }
+}
diff --git a/src/pkg/html/template/template.go b/src/pkg/html/template/template.go
index 5862f01f..db7244e 100644
--- a/src/pkg/html/template/template.go
+++ b/src/pkg/html/template/template.go
@@ -190,12 +190,7 @@
if src == nil || src.escaped {
return nil, fmt.Errorf("html/template: cannot Clone %q after it has executed", t.Name())
}
- if x.Tree != nil {
- x.Tree = &parse.Tree{
- Name: x.Tree.Name,
- Root: x.Tree.Root.CopyList(),
- }
- }
+ x.Tree = x.Tree.Copy()
ret.set[name] = &Template{
false,
x,
diff --git a/src/pkg/text/template/parse/parse.go b/src/pkg/text/template/parse/parse.go
index be83e77..34112fb 100644
--- a/src/pkg/text/template/parse/parse.go
+++ b/src/pkg/text/template/parse/parse.go
@@ -30,6 +30,19 @@
vars []string // variables defined at the moment.
}
+// Copy returns a copy of the Tree. Any parsing state is discarded.
+func (t *Tree) Copy() *Tree {
+ if t == nil {
+ return nil
+ }
+ return &Tree{
+ Name: t.Name,
+ ParseName: t.ParseName,
+ Root: t.Root.CopyList(),
+ text: t.text,
+ }
+}
+
// Parse returns a map from template name to parse.Tree, created by parsing the
// templates described in the argument string. The top-level template will be
// given the specified name. If an error is encountered, parsing stops and an
diff --git a/src/pkg/text/template/parse/parse_test.go b/src/pkg/text/template/parse/parse_test.go
index 049e65c..ba1a18e 100644
--- a/src/pkg/text/template/parse/parse_test.go
+++ b/src/pkg/text/template/parse/parse_test.go
@@ -332,6 +332,22 @@
}
}
+func TestErrorContextWithTreeCopy(t *testing.T) {
+ tree, err := New("root").Parse("{{if true}}{{end}}", "", "", make(map[string]*Tree), nil)
+ if err != nil {
+ t.Fatalf("unexpected tree parse failure: %v", err)
+ }
+ treeCopy := tree.Copy()
+ wantLocation, wantContext := tree.ErrorContext(tree.Root.Nodes[0])
+ gotLocation, gotContext := treeCopy.ErrorContext(treeCopy.Root.Nodes[0])
+ if wantLocation != gotLocation {
+ t.Errorf("wrong error location want %q got %q", wantLocation, gotLocation)
+ }
+ if wantContext != gotContext {
+ t.Errorf("wrong error location want %q got %q", wantContext, gotContext)
+ }
+}
+
// All failures, and the result is a string that must appear in the error message.
var errorTests = []parseTest{
// Check line numbers are accurate.
