Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

text/template: incompatible change using parenthesized expression #35974

Closed
ianlancetaylor opened this issue Dec 4, 2019 · 4 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

Sample program:

package main

import (
	"log"
	"os"
	"text/template"
)

const source = "{{(len .FileName) gt 0}}\n"

func main() {
	t := template.Must(template.New("x").Parse(source))
	s := struct{ FileName string }{"filename"}
	if err := t.Execute(os.Stdout, &s); err != nil {
		log.Fatal(err)
	}
	s = struct{ FileName string }{""}
	if err := t.Execute(os.Stdout, &s); err != nil {
		log.Fatal(err)
	}
}

Using Go 1.2 through Go 1.13 this prints

8
0

Using tip this fails:

2019/12/04 13:45:36 template: x:1:2: executing "x" at <(len .FileName) gt 0>: can't give argument to non-function len .FileName

This is due to https://golang.org/cl/206124 "text/template: add error check for parenthesized first argument in pipeline", which was written to fix #31810.

Unless this example is somehow incorrect, I think that we need to roll back that CL and try again for Go 1.15.

CC @robpike

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Dec 4, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.14 milestone Dec 4, 2019
@bcmills
Copy link
Contributor

bcmills commented Dec 4, 2019

I think that example is incorrect. It is evaluating (len .FileName) and discarding, not evaluating, the rest of the pipeline: the gt function is prefix, not infix.

Compare https://play.golang.org/p/SgiFOedV7UE.

@neild
Copy link
Contributor

neild commented Dec 4, 2019

Yeah, this is subtle because cases like this one worked by accident: len .Filename (the portion of the pipeline which was used) is equivalent to gt (len .Filename) 0 (what the user intended to write).

@ianlancetaylor
Copy link
Contributor Author

OK, thanks. I should really try to learn the template language some day. So the change is good in that it detected a bug.

@bcmills
Copy link
Contributor

bcmills commented Dec 5, 2019

Note that the corrected version of the example (https://play.golang.org/p/DizRlDZsi8C) produces boolean results instead of numeric ones:

true
false

@golang golang locked and limited conversation to collaborators Dec 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

4 participants