text/template: incorrect range syntax is accepted by the parser #28437

bep · 2018-10-27T12:06:58Z

Note: This isn't me suggesting that you should fix this. I have had some people reporting broken Hugo sites because of this, and I was "I don't see how that can have worked" until the third person arrived with the same issue.

The program below works on Go 1.10, fails in Go 1.11:

go/bep/temp  master ✗                                                                                                                                                                                                                  9m ⚑
▶ go version && go run main.go
go version go1.11.1 darwin/amd64
2018/10/27 14:02:36 template: :2:13: executing "" at <.abc>: undefined variable: $v
exit status 1

go/bep/temp  master ✗                                                                                                                                                                                                                10m ⚑  ⍉
▶ go1.10.4 version && go1.10.4 run main.go
go version go1.10.4 darwin/amd64
abc

package main

import (
	"bytes"
	"fmt"
	"log"
	"strings"
	"text/template"
)

func main() {
	data := map[string]interface{}{
		"abc": []string{"a", "b", "c"},
	}
	tpl := `
{{ range $v, .abc }}{{ $v }}{{ end }}
`

	var buf bytes.Buffer
	tmpl, err := template.New("").Parse(tpl)
	if err != nil {
		log.Fatal(err)
	}

	if err := tmpl.Execute(&buf, data); err != nil {
		log.Fatal(err)
	}

	result := strings.TrimSpace(buf.String())

	fmt.Println(result)

}

https://play.golang.org/p/5xrifxQFHQd

mvdan · 2018-10-27T12:15:10Z

Minified repro: https://play.golang.org/p/266xq0CRTIY

Without digging into this, it seems like broken syntax to me. I presume that's why you don't necessarily think that this is a bug in 1.11.

Do you have another reason to call this a regression, besides it having worked on 1.10? For example, did it work on many previous Go versions, or does the syntax make sense following any previous text/template docs?

bep · 2018-10-27T12:19:04Z

Without digging into this, it seems like broken syntax to me. I presume that's why you don't necessarily think that this is a bug in 1.11.

Yes, but it depends on the definition of "a bug". It has broken some apps/sites that worked before Go 1.11, but the error message is loud and clear, which is good.

mvdan · 2018-10-27T20:27:31Z

Yes - agreed that this isn't a clear case. Given that the template doesn't make sense, appears to be undocumented behavior, and errors out in an obvious way, I'm leaning towards leaving things the way they are.

It would be a different scenario if templates were breaking in silent or hard to debug ways, or if the template made some sense syntactically.

If you think we should do something, do you have anything in mind besides reverting to 1.10's behavior? If we did that, we'd likely have to document this syntax and add regression tests for it, which I presume is not something we want to do.

/cc @robpike for a second opinion.

bep · 2018-10-27T20:42:37Z

If you think we should do something, do you have anything in mind besides reverting to 1.10's behavior?

I think you should just leave it as is. This issue can serve as documentation. Some people have found an "accidental feature" that is for most people (and according to documentation) obvious wrong syntax. I created this issue out of curiousness more than anything.

mvdan · 2018-10-28T12:30:46Z

I think you should just leave it as is. This issue can serve as documentation.

Agreed. I'm curious as to exactly what CL fixed this weird behavior, and I'd like to add a regression test, so I'll leave this issue open until I have a look at that.

mvdan · 2018-10-28T16:12:24Z

This actually is a bug. text/template/parse should be rejecting that syntax, but isn't. The only reason why 1.11 is erroring when executing the template is because the new assignment logic gets confused by the lack of :=, and tries to assign to an existing variable $v, which has not been declared.

This is a subtle syntax parser bug that has existed for a while. Both 1.10 and 1.11 parse the template with no errors, which is incorrect. Will send a fix shortly, which should mean that the error happens sooner and has a clearer message.

gopherbot · 2018-10-28T16:32:17Z

Change https://golang.org/cl/145282 mentions this issue: text/template/parse: error on bad range variables

mvdan · 2018-10-28T16:33:33Z

@bep please take a look at the CL above, and feel free to give it a try. With my minified program using template.Must, now I get:

 $ go run f.go
panic: template: :1: range can only initialize variables

goroutine 1 [running]:
text/template.Must(...)
        /home/mvdan/tip/src/text/template/helper.go:23
main.main()
        /home/mvdan/f.go:12 +0x27a
exit status 2

I don't know if you're on Gerrit, but reviews are very much welcome :)

mvdan added the NeedsInvestigation label Oct 27, 2018

mvdan added this to the Go1.12 milestone Oct 27, 2018

mvdan added NeedsDecision and removed NeedsInvestigation labels Oct 27, 2018

mvdan self-assigned this Oct 28, 2018

mvdan changed the title ~~text/template: Regression from 1.10 to 1.11 in range variable assignment~~ text/template: weird range syntax broken from 1.10 to 1.11 Oct 28, 2018

mvdan added NeedsFix and removed NeedsDecision labels Oct 28, 2018

mvdan changed the title ~~text/template: weird range syntax broken from 1.10 to 1.11~~ text/template: incorrect range syntax is accepted by the parser Oct 28, 2018

gopherbot closed this as completed in 1399b52 Oct 28, 2018

golang locked and limited conversation to collaborators Oct 28, 2019

gopherbot added the FrozenDueToAge label Oct 28, 2019

rsc unassigned mvdan Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

text/template: incorrect range syntax is accepted by the parser #28437

text/template: incorrect range syntax is accepted by the parser #28437

bep commented Oct 27, 2018

mvdan commented Oct 27, 2018 •

edited

Loading

bep commented Oct 27, 2018

mvdan commented Oct 27, 2018

bep commented Oct 27, 2018

mvdan commented Oct 28, 2018

mvdan commented Oct 28, 2018 •

edited

Loading

gopherbot commented Oct 28, 2018

mvdan commented Oct 28, 2018

text/template: incorrect range syntax is accepted by the parser #28437

text/template: incorrect range syntax is accepted by the parser #28437

Comments

bep commented Oct 27, 2018

mvdan commented Oct 27, 2018 • edited Loading

bep commented Oct 27, 2018

mvdan commented Oct 27, 2018

bep commented Oct 27, 2018

mvdan commented Oct 28, 2018

mvdan commented Oct 28, 2018 • edited Loading

gopherbot commented Oct 28, 2018

mvdan commented Oct 28, 2018

mvdan commented Oct 27, 2018 •

edited

Loading

mvdan commented Oct 28, 2018 •

edited

Loading