shithub: hugo

Download patch

ref: 02eaddc2fbe92c26e67d9f82dd9aabecbbf2106c
parent: bdf47e8da80f87b7689badf48a6b8672c048d7e4
author: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
date: Wed Mar 6 04:07:49 EST 2019 

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


--- /dev/null
+++ b/common/hreflect/helpers.go
@@ -1,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()
+}
--- /dev/null
+++ b/common/hreflect/helpers_test.go
@@ -1,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")
+		}
+	}
+}
--- a/common/types/types.go
+++ b/common/types/types.go
@@ -50,3 +50,9 @@
 	}
 	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
+}
--- a/go.sum
+++ b/go.sum
@@ -76,6 +76,7 @@
 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=
--- a/tpl/compare/init.go
+++ b/tpl/compare/init.go
@@ -71,6 +71,27 @@
 			[][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{
--- /dev/null
+++ b/tpl/compare/truth.go
@@ -1,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)
+}
--- /dev/null
+++ b/tpl/compare/truth_test.go
@@ -1,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))
+	})
+}
--- a/tpl/tplimpl/template_ast_transformers.go
+++ b/tpl/tplimpl/template_ast_transformers.go
@@ -85,31 +85,59 @@
 
 	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 @@
 		}
 
 		for _, cmd := range x.Cmds {
-			c.paramsKeysToLower(cmd)
+			c.applyTransformations(cmd)
 		}
 
 	case *parse.CommandNode:
@@ -129,7 +157,7 @@
 			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) paramsKeysToLowerForNodes(nodes ...parse.Node) {
+func (c *templateContext) applyTransformationsToNodes(nodes ...parse.Node) {
 	for _, node := range nodes {
-		c.paramsKeysToLower(node)
+		c.applyTransformations(node)
 	}
 }
 
--- a/tpl/tplimpl/template_ast_transformers_test.go
+++ b/tpl/tplimpl/template_ast_transformers_test.go
@@ -15,10 +15,15 @@
 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"
 
 	"github.com/stretchr/testify/require"
@@ -26,6 +31,7 @@
 
 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 @@
 
 	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 @@
 
 	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 @@
 
 	c := newTemplateContext(createParseTreeLookup(templ))
 
-	c.paramsKeysToLower(templ.Tree.Root)
+	c.applyTransformations(templ.Tree.Root)
 
 	var b bytes.Buffer
 
@@ -348,7 +354,7 @@
 
 	c := newTemplateContext(createParseTreeLookup(overlayTpl))
 
-	c.paramsKeysToLower(overlayTpl.Tree.Root)
+	c.applyTransformations(overlayTpl.Tree.Root)
 
 	var b bytes.Buffer
 
@@ -377,6 +383,78 @@
 	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")
 
 }
-- 
⑨