From 9523ea0ac76d34a13ce45918e98535bd5dd2fffc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Wed, 18 Dec 2019 16:50:34 +0100 Subject: [PATCH] tpl: Get rid of the custom template truth logic Fixes #6615 --- hugolib/template_test.go | 28 +++++++ scripts/fork_go_templates/main.go | 1 + tpl/compare/init.go | 21 ------ tpl/compare/truth.go | 73 ------------------- tpl/compare/truth_test.go | 60 --------------- .../go_templates/texttemplate/exec.go | 2 +- .../texttemplate/hugo_template.go | 6 ++ tpl/tplimpl/template_ast_transformers.go | 35 +-------- tpl/tplimpl/template_ast_transformers_test.go | 71 ------------------ 9 files changed, 39 insertions(+), 258 deletions(-) delete mode 100644 tpl/compare/truth.go delete mode 100644 tpl/compare/truth_test.go diff --git a/hugolib/template_test.go b/hugolib/template_test.go index 4c41894cabd..80a703801db 100644 --- a/hugolib/template_test.go +++ b/hugolib/template_test.go @@ -337,6 +337,34 @@ Partial cached3: {{ partialCached "p1" "input3" $key2 }} `) } +// https://github.com/gohugoio/hugo/issues/6615 +func TestTemplateTruth(t *testing.T) { + b := newTestSitesBuilder(t) + b.WithTemplatesAdded("index.html", ` +{{ $p := index site.RegularPages 0 }} +{{ $zero := $p.ExpiryDate }} +{{ $notZero := time.Now }} + +if: Zero: {{ if $zero }}FAIL{{ else }}OK{{ end }} +if: Not Zero: {{ if $notZero }}OK{{ else }}Fail{{ end }} +not: Zero: {{ if not $zero }}OK{{ else }}FAIL{{ end }} +not: Not Zero: {{ if not $notZero }}FAIL{{ else }}OK{{ end }} + +with: Zero {{ with $zero }}FAIL{{ else }}OK{{ end }} + +`) + + b.Build(BuildCfg{}) + + b.AssertFileContent("public/index.html", ` +if: Zero: OK +if: Not Zero: OK +not: Zero: OK +not: Not Zero: OK +with: Zero OK +`) +} + func TestTemplateDependencies(t *testing.T) { b := newTestSitesBuilder(t).Running() diff --git a/scripts/fork_go_templates/main.go b/scripts/fork_go_templates/main.go index 127d7ce0374..d9d05679752 100644 --- a/scripts/fork_go_templates/main.go +++ b/scripts/fork_go_templates/main.go @@ -60,6 +60,7 @@ var ( "func (s *state) evalFunction", "func (s *state) evalFunctionOld", "func (s *state) evalField(", "func (s *state) evalFieldOld(", "func (s *state) evalCall(", "func (s *state) evalCallOld(", + "func isTrue(val reflect.Value) (truth, ok bool) {", "func isTrueOld(val reflect.Value) (truth, ok bool) {", ) htmlTemplateReplacers = strings.NewReplacer( diff --git a/tpl/compare/init.go b/tpl/compare/init.go index 2e536ff04ab..3b9dc68566b 100644 --- a/tpl/compare/init.go +++ b/tpl/compare/init.go @@ -71,27 +71,6 @@ func init() { [][2]string{}, ) - ns.AddMethodMapping(ctx.And, - []string{"and"}, - [][2]string{}, - ) - - ns.AddMethodMapping(ctx.Or, - []string{"or"}, - [][2]string{}, - ) - - // getif is used internally by Hugo. Do not document. - ns.AddMethodMapping(ctx.getIf, - []string{"getif"}, - [][2]string{}, - ) - - ns.AddMethodMapping(ctx.Not, - []string{"not"}, - [][2]string{}, - ) - ns.AddMethodMapping(ctx.Conditional, []string{"cond"}, [][2]string{ diff --git a/tpl/compare/truth.go b/tpl/compare/truth.go deleted file mode 100644 index 85ee22121e9..00000000000 --- a/tpl/compare/truth.go +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright 2019 The Hugo Authors. All rights reserved. -// The functions in this file is based on the Go source code, copyright -// The Go Authors and governed by a BSD-style license. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package compare provides template functions for comparing values. -package compare - -import ( - "reflect" - - "github.com/gohugoio/hugo/common/hreflect" -) - -// Boolean logic, based on: -// https://github.com/golang/go/blob/178a2c42254166cffed1b25fb1d3c7a5727cada6/src/text/template/funcs.go#L302 - -func truth(arg reflect.Value) bool { - return hreflect.IsTruthfulValue(arg) -} - -// getIf will return the given arg if it is considered truthful, else an empty string. -func (*Namespace) getIf(arg reflect.Value) reflect.Value { - if truth(arg) { - return arg - } - return reflect.ValueOf("") -} - -// And computes the Boolean AND of its arguments, returning -// the first false argument it encounters, or the last argument. -func (*Namespace) And(arg0 reflect.Value, args ...reflect.Value) reflect.Value { - if !truth(arg0) { - return arg0 - } - for i := range args { - arg0 = args[i] - if !truth(arg0) { - break - } - } - return arg0 -} - -// Or computes the Boolean OR of its arguments, returning -// the first true argument it encounters, or the last argument. -func (*Namespace) Or(arg0 reflect.Value, args ...reflect.Value) reflect.Value { - if truth(arg0) { - return arg0 - } - for i := range args { - arg0 = args[i] - if truth(arg0) { - break - } - } - return arg0 -} - -// Not returns the Boolean negation of its argument. -func (*Namespace) Not(arg reflect.Value) bool { - return !truth(arg) -} diff --git a/tpl/compare/truth_test.go b/tpl/compare/truth_test.go deleted file mode 100644 index 4c83e8b0aa6..00000000000 --- a/tpl/compare/truth_test.go +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright 2019 The Hugo Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package compare - -import ( - "reflect" - "testing" - "time" - - qt "github.com/frankban/quicktest" - "github.com/gohugoio/hugo/common/hreflect" -) - -func TestTruth(t *testing.T) { - n := New(false) - - truthv, falsev := reflect.ValueOf(time.Now()), reflect.ValueOf(false) - - assertTruth := func(t *testing.T, v reflect.Value, expected bool) { - if hreflect.IsTruthfulValue(v) != expected { - t.Fatal("truth mismatch") - } - } - - t.Run("And", func(t *testing.T) { - assertTruth(t, n.And(truthv, truthv), true) - assertTruth(t, n.And(truthv, falsev), false) - - }) - - t.Run("Or", func(t *testing.T) { - assertTruth(t, n.Or(truthv, truthv), true) - assertTruth(t, n.Or(falsev, truthv, falsev), true) - assertTruth(t, n.Or(falsev, falsev), false) - }) - - t.Run("Not", func(t *testing.T) { - c := qt.New(t) - c.Assert(n.Not(falsev), qt.Equals, true) - c.Assert(n.Not(truthv), qt.Equals, false) - }) - - t.Run("getIf", func(t *testing.T) { - c := qt.New(t) - assertTruth(t, n.getIf(reflect.ValueOf(nil)), false) - s := reflect.ValueOf("Hugo") - c.Assert(n.getIf(s), qt.Equals, s) - }) -} diff --git a/tpl/internal/go_templates/texttemplate/exec.go b/tpl/internal/go_templates/texttemplate/exec.go index 078bcf643d6..f400abd9cfe 100644 --- a/tpl/internal/go_templates/texttemplate/exec.go +++ b/tpl/internal/go_templates/texttemplate/exec.go @@ -307,7 +307,7 @@ func IsTrue(val interface{}) (truth, ok bool) { return isTrue(reflect.ValueOf(val)) } -func isTrue(val reflect.Value) (truth, ok bool) { +func isTrueOld(val reflect.Value) (truth, ok bool) { if !val.IsValid() { // Something like var x interface{}, never set. It's a form of nil. return false, true diff --git a/tpl/internal/go_templates/texttemplate/hugo_template.go b/tpl/internal/go_templates/texttemplate/hugo_template.go index a39f027fb30..d881064e52c 100644 --- a/tpl/internal/go_templates/texttemplate/hugo_template.go +++ b/tpl/internal/go_templates/texttemplate/hugo_template.go @@ -17,6 +17,8 @@ import ( "io" "reflect" + "github.com/gohugoio/hugo/common/hreflect" + "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate/parse" ) @@ -301,3 +303,7 @@ func (s *state) evalCall(dot, fun reflect.Value, node parse.Node, name string, a } return v } + +func isTrue(val reflect.Value) (truth, ok bool) { + return hreflect.IsTruthfulValue(val), true +} diff --git a/tpl/tplimpl/template_ast_transformers.go b/tpl/tplimpl/template_ast_transformers.go index 997126a327a..d3614981970 100644 --- a/tpl/tplimpl/template_ast_transformers.go +++ b/tpl/tplimpl/template_ast_transformers.go @@ -195,36 +195,9 @@ func (c *templateContext) wrapInPartialReturnWrapper(n *parse.ListNode) *parse.L } -// The truth logic in Go's template package is broken for certain values -// for the if and with keywords. This works around that problem by wrapping -// the node passed to if/with in a getif conditional. -// getif works slightly different than the Go built-in in that it also -// considers any IsZero methods on the values (as in time.Time). -// See https://github.com/gohugoio/hugo/issues/5738 -// TODO(bep) get rid of this. -func (c *templateContext) wrapWithGetIf(p *parse.PipeNode) { - if len(p.Cmds) == 0 { - return - } - - // getif will return an empty string if not evaluated as truthful, - // which is when we need the value in the with clause. - firstArg := parse.NewIdentifier("getif") - secondArg := p.CopyPipe() - newCmd := p.Cmds[0].Copy().(*parse.CommandNode) - - // secondArg is a PipeNode and will behave as it was wrapped in parens, e.g: - // {{ getif (len .Params | eq 2) }} - newCmd.Args = []parse.Node{firstArg, secondArg} - - p.Cmds = []*parse.CommandNode{newCmd} - -} - -// applyTransformations do 3 things: -// 1) Wraps every with and if pipe in getif -// 2) Parses partial return statement. -// 3) Tracks template (partial) dependencies and some other info. +// applyTransformations do 2 things: +// 1) Parses partial return statement. +// 2) Tracks template (partial) dependencies and some other info. func (c *templateContext) applyTransformations(n parse.Node) (bool, error) { switch x := n.(type) { case *parse.ListNode: @@ -235,10 +208,8 @@ func (c *templateContext) applyTransformations(n parse.Node) (bool, error) { c.applyTransformationsToNodes(x.Pipe) case *parse.IfNode: c.applyTransformationsToNodes(x.Pipe, x.List, x.ElseList) - c.wrapWithGetIf(x.Pipe) case *parse.WithNode: c.applyTransformationsToNodes(x.Pipe, x.List, x.ElseList) - c.wrapWithGetIf(x.Pipe) case *parse.RangeNode: c.applyTransformationsToNodes(x.Pipe, x.List, x.ElseList) case *parse.TemplateNode: diff --git a/tpl/tplimpl/template_ast_transformers_test.go b/tpl/tplimpl/template_ast_transformers_test.go index 1e2ff2124c1..5efa6a14d80 100644 --- a/tpl/tplimpl/template_ast_transformers_test.go +++ b/tpl/tplimpl/template_ast_transformers_test.go @@ -13,12 +13,9 @@ package tplimpl import ( - "strings" - "github.com/gohugoio/hugo/hugofs/files" "testing" - "time" template "github.com/gohugoio/hugo/tpl/internal/go_templates/htmltemplate" "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate/parse" @@ -74,74 +71,6 @@ type T struct { func (T) Method0() { } -func TestInsertIsZeroFunc(t *testing.T) { - t.Parallel() - - c := qt.New(t) - - var ( - ctx = map[string]interface{}{ - "True": true, - "Now": time.Now(), - "TimeZero": time.Time{}, - "T": &T{NonEmptyInterfaceTypedNil: (*T)(nil)}, - } - - templ1 = ` -{{ if .True }}.True: TRUE{{ else }}.True: FALSE{{ end }} -{{ if .TimeZero }}.TimeZero1: TRUE{{ else }}.TimeZero1: FALSE{{ end }} -{{ if (.TimeZero) }}.TimeZero2: TRUE{{ else }}.TimeZero2: FALSE{{ end }} -{{ if not .TimeZero }}.TimeZero3: TRUE{{ else }}.TimeZero3: FALSE{{ end }} -{{ if .Now }}.Now: TRUE{{ else }}.Now: FALSE{{ end }} -{{ with .TimeZero }}.TimeZero1 with: {{ . }}{{ else }}.TimeZero1 with: FALSE{{ end }} -{{ template "mytemplate" . }} -{{ if .T.NonEmptyInterfaceTypedNil }}.NonEmptyInterfaceTypedNil: TRUE{{ else }}.NonEmptyInterfaceTypedNil: FALSE{{ end }} -{{ template "other-file-template" . }} -{{ define "mytemplate" }} -{{ if .TimeZero }}.TimeZero1: mytemplate: TRUE{{ else }}.TimeZero1: mytemplate: FALSE{{ end }} -{{ end }} -` - - // https://github.com/gohugoio/hugo/issues/5865 - templ2 = `{{ define "other-file-template" }} -{{ if .TimeZero }}.TimeZero1: other-file-template: TRUE{{ else }}.TimeZero1: other-file-template: FALSE{{ end }} -{{ end }} -` - ) - - d := newD(c) - h := d.Tmpl.(*templateHandler) - - // HTML templates - c.Assert(h.AddTemplate("mytemplate.html", templ1), qt.IsNil) - c.Assert(h.AddTemplate("othertemplate.html", templ2), qt.IsNil) - - // Text templates - c.Assert(h.AddTemplate("_text/mytexttemplate.txt", templ1), qt.IsNil) - c.Assert(h.AddTemplate("_text/myothertexttemplate.txt", templ2), qt.IsNil) - - c.Assert(h.markReady(), qt.IsNil) - - for _, name := range []string{"mytemplate.html", "mytexttemplate.txt"} { - var sb strings.Builder - tt, _ := d.Tmpl.Lookup(name) - err := h.Execute(tt, &sb, ctx) - c.Assert(err, qt.IsNil) - result := sb.String() - - c.Assert(result, qt.Contains, ".True: TRUE") - c.Assert(result, qt.Contains, ".TimeZero1: FALSE") - c.Assert(result, qt.Contains, ".TimeZero2: FALSE") - c.Assert(result, qt.Contains, ".TimeZero3: TRUE") - c.Assert(result, qt.Contains, ".Now: TRUE") - c.Assert(result, qt.Contains, "TimeZero1 with: FALSE") - c.Assert(result, qt.Contains, ".TimeZero1: mytemplate: FALSE") - c.Assert(result, qt.Contains, ".TimeZero1: other-file-template: FALSE") - c.Assert(result, qt.Contains, ".NonEmptyInterfaceTypedNil: FALSE") - } - -} - func TestCollectInfo(t *testing.T) { configStr := `{ "version": 42 }`