From 74fc34a5a611bdedc9b7a6976758fb3fb7143e1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Mon, 13 Aug 2018 20:50:07 +0200 Subject: [PATCH] tpl/tplimpl: Reimplement the ".Params tolower" template transformer All `.Params` are stored lowercase, but it should work to access them `.Page.camelCase` etc. There was, however, some holes in the logic with the old transformer. This commit fixes that by applying a blacklist instead of the old whitelist logic. `.Param` is a very distinct key. The original case will be kept in `.Data.Params.myParam`, but other than that it will be lowercased. This implementation is also faster: ```bash benchmark old ns/op new ns/op delta BenchmarkTemplateParamsKeysToLower-4 15237 9581 -37.12% benchmark old allocs new allocs delta BenchmarkTemplateParamsKeysToLower-4 92 40 -56.52% benchmark old bytes new bytes delta BenchmarkTemplateParamsKeysToLower-4 2656 1360 -48.80% ``` Fixes #5068 --- tpl/tplimpl/template_ast_transformers.go | 156 ++++++++---------- tpl/tplimpl/template_ast_transformers_test.go | 22 ++- 2 files changed, 92 insertions(+), 86 deletions(-) diff --git a/tpl/tplimpl/template_ast_transformers.go b/tpl/tplimpl/template_ast_transformers.go index cfdc35593a8..241ca3f773e 100644 --- a/tpl/tplimpl/template_ast_transformers.go +++ b/tpl/tplimpl/template_ast_transformers.go @@ -34,17 +34,6 @@ var reservedContainers = map[string]bool{ "Data": true, } -var paramsPaths = [][]string{ - {"Params"}, - {"Site", "Params"}, - - // Site and Page referenced from shortcodes - {"Page", "Site", "Params"}, - {"Page", "Params"}, - - {"Site", "Language", "Params"}, -} - type templateContext struct { decl decl visited map[string]bool @@ -165,6 +154,7 @@ func (c *templateContext) updateIdentsIfNeeded(idents []string) { for i := index; i < len(idents); i++ { idents[i] = strings.ToLower(idents[i]) } + } // indexOfReplacementStart will return the index of where to start doing replacement, @@ -177,17 +167,21 @@ func (d decl) indexOfReplacementStart(idents []string) int { return -1 } - first := idents[0] - firstIsVar := first[0] == '$' - - if l == 1 && !firstIsVar { - // This can not be a Params.x - return -1 + if l == 1 { + first := idents[0] + if first == paramsIdentifier || first[0] == '$' { + // This can not be a Params.x + return -1 + } } var lookFurther bool + var needsVarExpansion bool for _, ident := range idents { - if ident == "Params" || ident[0] == '$' { + if ident[0] == '$' { + lookFurther = true + needsVarExpansion = true + } else if ident == paramsIdentifier { lookFurther = true break } @@ -197,10 +191,64 @@ func (d decl) indexOfReplacementStart(idents []string) int { return -1 } + var resolvedIdents []string + + if !needsVarExpansion { + resolvedIdents = idents + } else { + var ok bool + resolvedIdents, ok = d.resolveVariables(idents) + if !ok { + return -1 + } + } + + var paramFound bool + for i, ident := range resolvedIdents { + if ident == paramsIdentifier { + if i > 0 { + container := resolvedIdents[i-1] + if reservedContainers[container] { + // .Data.Params.someKey + return -1 + } + } + paramFound = true + + break + } + } + + if !paramFound { + return -1 + } + + var paramSeen bool + idx := -1 + for i, ident := range idents { + if ident == "" || ident[0] == '$' { + continue + } + if ident == paramsIdentifier { + paramSeen = true + idx = -1 + } else { + if paramSeen { + return i + } + if idx == -1 { + idx = i + } + } + } + return idx + +} + +func (d decl) resolveVariables(idents []string) ([]string, bool) { var ( - resolvedIdents []string - replacements []string - replaced []string + replacements []string + replaced []string ) // An Ident can start out as one of @@ -215,7 +263,7 @@ func (d decl) indexOfReplacementStart(idents []string) int { if i > 20 { // bail out - return -1 + return nil, false } potentialVar := replacements[i] @@ -234,7 +282,7 @@ func (d decl) indexOfReplacementStart(idents []string) int { if !ok { // Temporary range vars. We do not care about those. - return -1 + return nil, false } replacement = strings.TrimPrefix(replacement, ".") @@ -251,66 +299,6 @@ func (d decl) indexOfReplacementStart(idents []string) int { } } - resolvedIdents = append(replaced, idents[1:]...) - - paramIdx := -1 - for i, ident := range resolvedIdents { - if ident == paramsIdentifier { - if i == len(resolvedIdents)-2 { - if i > 0 { - container := resolvedIdents[i-1] - if reservedContainers[container] { - break - } - } - paramIdx = i - } - break - } - } - - if paramIdx == -1 { - return -1 - } - - return len(idents) - 1 - -} - -func indexOfFirstRealIdentAfterWords(resolvedIdents, idents []string, words ...string) int { - if !sliceStartsWith(resolvedIdents, words...) { - return -1 - } - - for i, ident := range idents { - if ident == "" || ident[0] == '$' { - continue - } - found := true - for _, word := range words { - if ident == word { - found = false - break - } - } - if found { - return i - } - } - - return -1 -} - -func sliceStartsWith(slice []string, words ...string) bool { + return append(replaced, idents[1:]...), true - if len(slice) < len(words) { - return false - } - - for i, word := range words { - if word != slice[i] { - return false - } - } - return true } diff --git a/tpl/tplimpl/template_ast_transformers_test.go b/tpl/tplimpl/template_ast_transformers_test.go index 2b8d85f0963..8a9ea35757a 100644 --- a/tpl/tplimpl/template_ast_transformers_test.go +++ b/tpl/tplimpl/template_ast_transformers_test.go @@ -31,6 +31,7 @@ var ( "Slice": []int{1, 3}, "Params": map[string]interface{}{ "lower": "P1L", + "slice": []int{1, 3}, }, "CurrentSection": map[string]interface{}{ "Params": map[string]interface{}{ @@ -115,6 +116,8 @@ RANGE: {{ . }}: {{ $.Params.LOWER }} F1: {{ printf "themes/%s-theme" .Site.Params.LOWER }} F2: {{ Echo (printf "themes/%s-theme" $lower) }} F3: {{ Echo (printf "themes/%s-theme" .Site.Params.LOWER) }} + +PSLICE: {{ range .Params.SLICE }}PSLICE{{.}}|{{ end }} ` ) @@ -170,7 +173,9 @@ func TestParamsKeysToLower(t *testing.T) { require.Contains(t, result, "F2: themes/P2L-theme") require.Contains(t, result, "F3: themes/P2L-theme") - // Issue #5068 and similar + require.Contains(t, result, "PSLICE: PSLICE1|PSLICE3|") + + // Issue #5068 require.Contains(t, result, "PCurrentSection: pcurrentsection") } @@ -199,13 +204,16 @@ func BenchmarkTemplateParamsKeysToLower(b *testing.B) { } } -func TestParamsKeysToLowVars(t *testing.T) { +func TestParamsKeysToLowerVars(t *testing.T) { t.Parallel() var ( ctx = map[string]interface{}{ "Params": map[string]interface{}{ "colors": map[string]interface{}{ "blue": "Amber", + "pretty": map[string]interface{}{ + "first": "Indigo", + }, }, }, } @@ -214,8 +222,14 @@ func TestParamsKeysToLowVars(t *testing.T) { paramsTempl = ` {{$__amber_1 := .Params.Colors}} {{$__amber_2 := $__amber_1.Blue}} +{{$__amber_3 := $__amber_1.Pretty}} +{{$__amber_4 := .Params}} + Color: {{$__amber_2}} Blue: {{ $__amber_1.Blue}} +Pretty First1: {{ $__amber_3.First}} +Pretty First2: {{ $__amber_1.Pretty.First}} +Pretty First3: {{ $__amber_4.COLORS.PRETTY.FIRST}} ` ) @@ -234,6 +248,10 @@ Blue: {{ $__amber_1.Blue}} result := b.String() require.Contains(t, result, "Color: Amber") + require.Contains(t, result, "Blue: Amber") + require.Contains(t, result, "Pretty First1: Indigo") + require.Contains(t, result, "Pretty First2: Indigo") + require.Contains(t, result, "Pretty First3: Indigo") }