summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>2019-03-06 09:07:49 +0100
committerBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>2019-03-06 22:52:38 +0100
commit02eaddc2fbe92c26e67d9f82dd9aabecbbf2106c (patch)
tree517220ebb06a75e89d506183a621123d49ee20c6
parentbdf47e8da80f87b7689badf48a6b8672c048d7e4 (diff)
tpl/tplimpl: Fix template truth logic
Before this commit, due to a bug in Go's `text/template` package, this would print different output for typed nil interface values: ``` {{ if .AuthenticatedUser }}User is authenticated!{{ else }}{{ end }} {{ if not .AuthenticatedUser }}{{ else }}}User is authenticated!{{ end }} ``` This commit works around this by wrapping every `if` and `with` with a custom `getif` template func with truth logic that matches `not`, `and` and `or`. Those 3 template funcs from Go's stdlib are now pulled into Hugo's source tree and adjusted to support custom zero values, e.g. types that implement `IsZero`. This means that you can now do: ``` {{ with .Date }}{{ . }}{{ end }} ``` And it would work as expected. Fixes #5738
-rw-r--r--common/hreflect/helpers.go91
-rw-r--r--common/hreflect/helpers_test.go42
-rw-r--r--common/types/types.go6
-rw-r--r--go.sum1
-rw-r--r--tpl/compare/init.go21
-rw-r--r--tpl/compare/truth.go73
-rw-r--r--tpl/compare/truth_test.go60
-rw-r--r--tpl/tplimpl/template_ast_transformers.go56
-rw-r--r--tpl/tplimpl/template_ast_transformers_test.go90
9 files changed, 420 insertions, 20 deletions
diff --git a/common/hreflect/helpers.go b/common/hreflect/helpers.go
new file mode 100644
index 000000000..db7b208b5
--- /dev/null
+++ b/common/hreflect/helpers.go
@@ -0,0 +1,91 @@
+// Copyright 2019 The Hugo Authors. All rights reserved.
+// Some functions in this file (see comments) 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 hreflect contains reflect helpers.
+package hreflect
+
+import (
+ "reflect"
+
+ "github.com/gohugoio/hugo/common/types"
+)
+
+// IsTruthful returns whether in represents a truthful value.
+// See IsTruthfulValue
+func IsTruthful(in interface{}) bool {
+ switch v := in.(type) {
+ case reflect.Value:
+ return IsTruthfulValue(v)
+ default:
+ return IsTruthfulValue(reflect.ValueOf(in))
+ }
+
+}
+
+var zeroType = reflect.TypeOf((*types.Zeroer)(nil)).Elem()
+
+// IsTruthfulValue returns whether the given value has a meaningful truth value.
+// This is based on template.IsTrue in Go's stdlib, but also considers
+// IsZero and any interface value will be unwrapped before it's considered
+// for truthfulness.
+//
+// Based on:
+// https://github.com/golang/go/blob/178a2c42254166cffed1b25fb1d3c7a5727cada6/src/text/template/exec.go#L306
+func IsTruthfulValue(val reflect.Value) (truth bool) {
+ val = indirectInterface(val)
+
+ if !val.IsValid() {
+ // Something like var x interface{}, never set. It's a form of nil.
+ return
+ }
+
+ if val.Type().Implements(zeroType) {
+ return !val.Interface().(types.Zeroer).IsZero()
+ }
+
+ switch val.Kind() {
+ case reflect.Array, reflect.Map, reflect.Slice, reflect.String:
+ truth = val.Len() > 0
+ case reflect.Bool:
+ truth = val.Bool()
+ case reflect.Complex64, reflect.Complex128:
+ truth = val.Complex() != 0
+ case reflect.Chan, reflect.Func, reflect.Ptr, reflect.Interface:
+ truth = !val.IsNil()
+ case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+ truth = val.Int() != 0
+ case reflect.Float32, reflect.Float64:
+ truth = val.Float() != 0
+ case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
+ truth = val.Uint() != 0
+ case reflect.Struct:
+ truth = true // Struct values are always true.
+ default:
+ return
+ }
+
+ return
+}
+
+// Based on: https://github.com/golang/go/blob/178a2c42254166cffed1b25fb1d3c7a5727cada6/src/text/template/exec.go#L931
+func indirectInterface(v reflect.Value) reflect.Value {
+ if v.Kind() != reflect.Interface {
+ return v
+ }
+ if v.IsNil() {
+ return reflect.Value{}
+ }
+ return v.Elem()
+}
diff --git a/common/hreflect/helpers_test.go b/common/hreflect/helpers_test.go
new file mode 100644
index 000000000..3c9179394
--- /dev/null
+++ b/common/hreflect/helpers_test.go
@@ -0,0 +1,42 @@
+// 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 hreflect
+
+import (
+ "reflect"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestIsTruthful(t *testing.T) {
+ assert := require.New(t)
+
+ assert.True(IsTruthful(true))
+ assert.False(IsTruthful(false))
+ assert.True(IsTruthful(time.Now()))
+ assert.False(IsTruthful(time.Time{}))
+}
+
+func BenchmarkIsTruthFul(b *testing.B) {
+ v := reflect.ValueOf("Hugo")
+
+ b.ResetTimer()
+ for i := 0; i < b.N; i++ {
+ if !IsTruthfulValue(v) {
+ b.Fatal("not truthful")
+ }
+ }
+}
diff --git a/common/types/types.go b/common/types/types.go
index ca74391f8..95e72d99b 100644
--- a/common/types/types.go
+++ b/common/types/types.go
@@ -50,3 +50,9 @@ func NewKeyValuesStrings(key string, values ...string) KeyValues {
}
return KeyValues{Key: key, Values: iv}
}
+
+// Zeroer, as implemented by time.Time, will be used by the truth template
+// funcs in Hugo (if, with, not, and, or).
+type Zeroer interface {
+ IsZero() bool
+}
diff --git a/go.sum b/go.sum
index 79d7c1eb0..0227e3b9c 100644
--- a/go.sum
+++ b/go.sum
@@ -76,6 +76,7 @@ github.com/magefile/mage v1.4.0 h1:RI7B1CgnPAuu2O9lWszwya61RLmfL0KCdo+QyyI/Bhk=
github.com/magefile/mage v1.4.0/go.mod h1:IUDi13rsHje59lecXokTfGX0QIzO45uVPlXnJYsXepA=
github.com/magiconair/properties v1.8.0 h1:LLgXmsheXeRoUOBOjtwPQCWIYqM/LU1ayDtDePerRcY=
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
+github.com/markbates/inflect v0.0.0-20171215194931-a12c3aec81a6 h1:LZhVjIISSbj8qLf2qDPP0D8z0uvOWAW5C85ly5mJW6c=
github.com/markbates/inflect v0.0.0-20171215194931-a12c3aec81a6/go.mod h1:oTeZL2KHA7CUX6X+fovmK9OvIOFuqu0TwdQrZjLTh88=
github.com/matryer/try v0.0.0-20161228173917-9ac251b645a2/go.mod h1:0KeJpeMD6o+O4hW7qJOT7vyQPKrWmj26uf5wMc/IiIs=
github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
diff --git a/tpl/compare/init.go b/tpl/compare/init.go
index f766ef890..619293203 100644
--- a/tpl/compare/init.go
+++ b/tpl/compare/init.go
@@ -71,6 +71,27 @@ 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
new file mode 100644
index 000000000..85ee22121
--- /dev/null
+++ b/tpl/compare/truth.go
@@ -0,0 +1,73 @@
+// 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
new file mode 100644
index 000000000..04d897212
--- /dev/null
+++ b/tpl/compare/truth_test.go
@@ -0,0 +1,60 @@
+// 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"
+
+ "github.com/gohugoio/hugo/common/hreflect"
+ "github.com/stretchr/testify/require"
+)
+
+func TestTruth(t *testing.T) {
+ n := New()
+
+ 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) {
+ assert := require.New(t)
+ assert.True(n.Not(falsev))
+ assert.False(n.Not(truthv))
+ })
+
+ t.Run("getIf", func(t *testing.T) {
+ assert := require.New(t)
+ assertTruth(t, n.getIf(reflect.ValueOf(nil)), false)
+ s := reflect.ValueOf("Hugo")
+ assert.Equal(s, n.getIf(s))
+ })
+}
diff --git a/tpl/tplimpl/template_ast_transformers.go b/tpl/tplimpl/template_ast_transformers.go
index f32b189ff..e1cfb1aa4 100644
--- a/tpl/tplimpl/template_ast_transformers.go
+++ b/tpl/tplimpl/template_ast_transformers.go
@@ -85,31 +85,59 @@ func applyTemplateTransformers(templ *parse.Tree, lookupFn func(name string) *pa
c := newTemplateContext(lookupFn)
- c.paramsKeysToLower(templ.Root)
+ c.applyTransformations(templ.Root)
return nil
}
-// paramsKeysToLower is made purposely non-generic to make it not so tempting
-// to do more of these hard-to-maintain AST transformations.
-func (c *templateContext) paramsKeysToLower(n parse.Node) {
+// 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
+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 two things:
+// 1) Make all .Params.CamelCase and similar into lowercase.
+// 2) Wraps every with and if pipe in getif
+func (c *templateContext) applyTransformations(n parse.Node) {
switch x := n.(type) {
case *parse.ListNode:
if x != nil {
- c.paramsKeysToLowerForNodes(x.Nodes...)
+ c.applyTransformationsToNodes(x.Nodes...)
}
case *parse.ActionNode:
- c.paramsKeysToLowerForNodes(x.Pipe)
+ c.applyTransformationsToNodes(x.Pipe)
case *parse.IfNode:
- c.paramsKeysToLowerForNodes(x.Pipe, x.List, x.ElseList)
+ c.applyTransformationsToNodes(x.Pipe, x.List, x.ElseList)
+ c.wrapWithGetIf(x.Pipe)
case *parse.WithNode:
- c.paramsKeysToLowerForNodes(x.Pipe, x.List, x.ElseList)
+ c.applyTransformationsToNodes(x.Pipe, x.List, x.ElseList)
+ c.wrapWithGetIf(x.Pipe)
case *parse.RangeNode:
- c.paramsKeysToLowerForNodes(x.Pipe, x.List, x.ElseList)
+ c.applyTransformationsToNodes(x.Pipe, x.List, x.ElseList)
case *parse.TemplateNode:
subTempl := c.getIfNotVisited(x.Name)
if subTempl != nil {
- c.paramsKeysToLowerForNodes(subTempl.Root)
+ c.applyTransformationsToNodes(subTempl.Root)
}
case *parse.PipeNode:
if len(x.Decl) == 1 && len(x.Cmds) == 1 {
@@ -118,7 +146,7 @@ func (c *templateContext) paramsKeysToLower(n parse.Node) {
}
for _, cmd := range x.Cmds {
- c.paramsKeysToLower(cmd)
+ c.applyTransformations(cmd)
}
case *parse.CommandNode:
@@ -129,7 +157,7 @@ func (c *templateContext) paramsKeysToLower(n parse.Node) {
case *parse.VariableNode:
c.updateIdentsIfNeeded(an.Ident)
case *parse.PipeNode:
- c.paramsKeysToLower(an)
+ c.applyTransformations(an)
case *parse.ChainNode:
// site.Params...
if len(an.Field) > 1 && an.Field[0] == paramsIdentifier {
@@ -140,9 +168,9 @@ func (c *templateContext) paramsKeysToLower(n parse.Node) {
}
}
-func (c *templateContext) paramsKeysToLowerForNodes(nodes ...parse.Node) {
+func (c *templateContext) applyTransformationsToNodes(nodes ...parse.Node) {
for _, node := range nodes {
- c.paramsKeysToLower(node)
+ c.applyTransformations(node)
}
}
diff --git a/tpl/tplimpl/template_ast_transformers_test.go b/tpl/tplimpl/template_ast_transformers_test.go
index 45cf4399a..611f5d8ca 100644
--- a/tpl/tplimpl/template_ast_transformers_test.go
+++ b/tpl/tplimpl/template_ast_transformers_test.go
@@ -15,9 +15,14 @@ package tplimpl
import (
"bytes"
"fmt"
+ "html/template"
"testing"
+ "time"
- "html/template"
+ "github.com/gohugoio/hugo/tpl"
+
+ "github.com/gohugoio/hugo/deps"
+ "github.com/gohugoio/hugo/hugofs"
"github.com/spf13/cast"
@@ -26,6 +31,7 @@ import (
var (
testFuncs = map[string]interface{}{
+ "getif": func(v interface{}) interface{} { return v },
"ToTime": func(v interface{}) interface{} { return cast.ToTime(v) },
"First": func(v ...interface{}) interface{} { return v[0] },
"Echo": func(v interface{}) interface{} { return v },
@@ -183,7 +189,7 @@ func TestParamsKeysToLower(t *testing.T) {
require.Equal(t, -1, c.decl.indexOfReplacementStart([]string{}))
- c.paramsKeysToLower(templ.Tree.Root)
+ c.applyTransformations(templ.Tree.Root)
var b bytes.Buffer
@@ -265,7 +271,7 @@ func BenchmarkTemplateParamsKeysToLower(b *testing.B) {
for i := 0; i < b.N; i++ {
c := newTemplateContext(createParseTreeLookup(templates[i]))
- c.paramsKeysToLower(templ.Tree.Root)
+ c.applyTransformations(templ.Tree.Root)
}
}
@@ -304,7 +310,7 @@ Pretty First3: {{ $__amber_4.COLORS.PRETTY.FIRST}}
c := newTemplateContext(createParseTreeLookup(templ))
- c.paramsKeysToLower(templ.Tree.Root)
+ c.applyTransformations(templ.Tree.Root)
var b bytes.Buffer
@@ -348,7 +354,7 @@ P2: {{ .Params.LOWER }}
c := newTemplateContext(createParseTreeLookup(overlayTpl))
- c.paramsKeysToLower(overlayTpl.Tree.Root)
+ c.applyTransformations(overlayTpl.Tree.Root)
var b bytes.Buffer
@@ -377,6 +383,78 @@ func TestTransformRecursiveTemplate(t *testing.T) {
require.NoError(t, err)
c := newTemplateContext(createParseTreeLookup(templ))
- c.paramsKeysToLower(templ.Tree.Root)
+ c.applyTransformations(templ.Tree.Root)
+
+}
+
+type I interface {
+ Method0()
+}
+
+type T struct {
+ NonEmptyInterfaceTypedNil I
+}
+
+func (T) Method0() {
+}
+
+func TestInsertIsZeroFunc(t *testing.T) {
+ t.Parallel()
+
+ assert := require.New(t)
+
+ var (
+ ctx = map[string]interface{}{
+ "True": true,
+ "Now": time.Now(),
+ "TimeZero": time.Time{},
+ "T": &T{NonEmptyInterfaceTypedNil: (*T)(nil)},
+ }
+
+ templ = `
+{{ 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 }}
+
+
+{{ define "mytemplate" }}
+{{ if .TimeZero }}.TimeZero1: mytemplate: TRUE{{ else }}.TimeZero1: mytemplate: FALSE{{ end }}
+{{ end }}
+
+`
+ )
+
+ v := newTestConfig()
+ fs := hugofs.NewMem(v)
+
+ depsCfg := newDepsConfig(v)
+ depsCfg.Fs = fs
+ d, err := deps.New(depsCfg)
+ assert.NoError(err)
+
+ provider := DefaultTemplateProvider
+ provider.Update(d)
+
+ h := d.Tmpl.(handler)
+
+ assert.NoError(h.addTemplate("mytemplate.html", templ))
+
+ tt, _ := d.Tmpl.Lookup("mytemplate.html")
+ result, err := tt.(tpl.TemplateExecutor).ExecuteToString(ctx)
+ assert.NoError(err)
+
+ assert.Contains(result, ".True: TRUE")
+ assert.Contains(result, ".TimeZero1: FALSE")
+ assert.Contains(result, ".TimeZero2: FALSE")
+ assert.Contains(result, ".TimeZero3: TRUE")
+ assert.Contains(result, ".Now: TRUE")
+ assert.Contains(result, "TimeZero1 with: FALSE")
+ assert.Contains(result, ".TimeZero1: mytemplate: FALSE")
+ assert.Contains(result, ".NonEmptyInterfaceTypedNil: FALSE")
}