text/template/parse, html/template: copy Tree.text during html templa…

…te 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

src/pkg/html/template/clone_test.go

@@ -6,6 +6,8 @@ package template
 
 import (
 	"bytes"
+	"errors"
+	"io/ioutil"
 	"testing"
 	"text/template/parse"
 )
@@ -146,3 +148,41 @@ func TestCloneCrash(t *testing.T) {
 	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)
+	}
+}

src/pkg/html/template/template.go

@@ -190,12 +190,7 @@ func (t *Template) Clone() (*Template, error) {
 		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,

src/pkg/text/template/parse/parse.go

@@ -30,6 +30,19 @@ type Tree struct {
 	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

src/pkg/text/template/parse/parse_test.go

@@ -332,6 +332,22 @@ func TestIsEmpty(t *testing.T) {
 	}
 }
 
+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.