From 29ae32aa84d2cf1fad6bf4c9c1608714cf82533c Mon Sep 17 00:00:00 2001 From: Rob Figueiredo Date: Wed, 18 Nov 2020 16:58:54 -0500 Subject: [PATCH] Support for {@param ...}, aka "header params" Upstream Closure Templates has removed support for params specified in soydoc and now requires them specified using "header params". This change adds support for parsing header params and treating them similarly to soydoc params, having an unknown type. For backwards compatibility, the header params are added to `Template.SoyDoc.Params`, so calling code should not require updates. The result is that this library will not report errors that the Java library would report, but that level of compatibility is sufficient for our purposes, and a lot of development would be required to support the param type system. --- ast/node.go | 43 ++++++++++++++++ parse/lexer.go | 59 ++++++++++++++++++++++ parse/lexer_test.go | 76 +++++++++++++++++++++++++++- parse/parse.go | 24 +++++++++ parse/parse_test.go | 27 ++++++++++ parsepasses/datarefcheck.go | 11 +++-- parsepasses/datarefcheck_test.go | 85 +++++++++++++++++++++++++++++--- soyhtml/exec.go | 2 + soyhtml/exec_test.go | 31 ++++++++++++ template/registry.go | 23 +++++++++ template/template.go | 6 +-- 11 files changed, 373 insertions(+), 14 deletions(-) diff --git a/ast/node.go b/ast/node.go index 879e729..1f01000 100644 --- a/ast/node.go +++ b/ast/node.go @@ -121,6 +121,49 @@ func (n *TemplateNode) Children() []Node { return []Node{n.Body} } +// TypeNode holds a type definition for a template parameter. +// +// Presently this is just a string value which is not validated or processed. +// Backwards-incompatible changes in the future may elaborate this data model to +// add functionality. +type TypeNode struct { + Pos + Expr string +} + +func (n TypeNode) String() string { + return n.Expr +} + +// HeaderParamNode holds a parameter declaration. +// +// HeaderParams MUST appear at the beginning of a TemplateNode's Body. +type HeaderParamNode struct { + Pos + Optional bool + Name string + Type TypeNode // empty if inferred from the default value + Default Node // nil if no default was specified +} + +func (n *HeaderParamNode) String() string { + var expr string + if !n.Optional { + expr = "{@param " + } else { + expr = "{@param? " + } + expr += n.Name + ":" + if typ := n.Type.String(); typ != "" { + expr += " " + typ + " " + } + if n.Default != nil { + expr += "= " + n.Default.String() + } + expr += "}\n" + return expr +} + type SoyDocNode struct { Pos Params []*SoyDocParamNode diff --git a/parse/lexer.go b/parse/lexer.go index 4b9bf94..2756aa5 100644 --- a/parse/lexer.go +++ b/parse/lexer.go @@ -101,6 +101,11 @@ const ( itemSoyDocEnd // */ itemComment // line comments (//) or block comments (/*) + // Headers + itemHeaderParam // {@param NAME: TYPE = DEFAULT} + itemHeaderOptionalParam // {@param? NAME: TYPE = DEFAULT} + itemHeaderParamType // e.g. ? any int string map list [age: int, name: string] + // Commands itemCommand // used only to delimit the commands itemAlias // {alias ...} @@ -612,6 +617,8 @@ func lexInsideTag(l *lexer) stateFn { return lexIdent case r == ',': l.emit(itemComma) + case r == '@': + return lexHeaderParam default: return l.errorf("unrecognized character in action: %#U", r) } @@ -839,6 +846,49 @@ func lexIdent(l *lexer) stateFn { return lexInsideTag } +// lexHeaderParam lexes a "header param" such as {@param ...}. +// '@' has just been read. +func lexHeaderParam(l *lexer) stateFn { + if !strings.HasPrefix(l.input[l.pos:], "param") { + return l.errorf("expected {@param ...}") + } + l.pos += ast.Pos(len("param")) + + if l.next() == '?' { + l.emit(itemHeaderOptionalParam) + } else { + l.backup() + l.emit(itemHeaderParam) + } + skipSpace(l) + + // Consume the (simple) identifier. + for isAlphaNumeric(l.next()) {} + l.backup() + l.emit(itemIdent) + skipSpace(l) + + // Consume the ':' + if l.next() != ':' { + return l.errorf("expected {@param name: ...}") + } + l.emit(itemColon) + skipSpace(l) + + // Consume until the equals or end of the tag. + var lastNonSpace = l.pos + for ch := l.next(); ch != '=' && ch != '}' ; ch = l.next() { + if !isSpace(ch) { + lastNonSpace = l.pos + } + } + l.pos = lastNonSpace + l.emit(itemHeaderParamType) + skipSpace(l) + + return lexInsideTag +} + // lexCss scans the body of the {css} command into an itemText. // This is required because css classes are unquoted and may have hyphens (and // thus are not recognized as idents). @@ -1033,3 +1083,12 @@ func allSpaceWithNewline(str string) bool { } return seenNewline } + +func skipSpace(l *lexer) { + ch := l.next() + for isSpaceEOL(ch) { + ch = l.next() + } + l.backup() + l.ignore() +} diff --git a/parse/lexer_test.go b/parse/lexer_test.go index b48eaf0..67a0412 100644 --- a/parse/lexer_test.go +++ b/parse/lexer_test.go @@ -1,6 +1,8 @@ package parse -import "testing" +import ( + "testing" +) type lexTest struct { name string @@ -540,6 +542,78 @@ var lexTests = []lexTest{ tEOF, }}, + {"@param", ` +{@param NAME: ?} // A required param. +{@param NAME: any} // A required param of type any. +{@param NAME:= 'default'} // A default param with an inferred type. +{@param NAME: int = 10} // A default param with an explicit type. +{@param? NAME: [age: int, name: string]} // An optional param. +{@param? NAME: map} // A map param. +{@param? NAME: list} // A list param. +`, []item{ + tLeft, + {itemHeaderParam, 0, "@param"}, + {itemIdent, 0, "NAME"}, + {itemColon, 0, ":"}, + {itemHeaderParamType, 0, "?"}, + tRight, + {itemComment, 0, "// A required param.\n"}, + + tLeft, + {itemHeaderParam, 0, "@param"}, + {itemIdent, 0, "NAME"}, + {itemColon, 0, ":"}, + {itemHeaderParamType, 0, "any"}, + tRight, + {itemComment, 0, "// A required param of type any.\n"}, + + tLeft, + {itemHeaderParam, 0, "@param"}, + {itemIdent, 0, "NAME"}, + {itemColon, 0, ":"}, + {itemHeaderParamType, 0, ""}, + {itemEquals, 0, "="}, + {itemString, 0, "'default'"}, + tRight, + {itemComment, 0, "// A default param with an inferred type.\n"}, + + tLeft, + {itemHeaderParam, 0, "@param"}, + {itemIdent, 0, "NAME"}, + {itemColon, 0, ":"}, + {itemHeaderParamType, 0, "int"}, + {itemEquals, 0, "="}, + {itemInteger, 0, "10"}, + tRight, + {itemComment, 0, "// A default param with an explicit type.\n"}, + + tLeft, + {itemHeaderOptionalParam, 0, "@param?"}, + {itemIdent, 0, "NAME"}, + {itemColon, 0, ":"}, + {itemHeaderParamType, 0, "[age: int, name: string]"}, + tRight, + {itemComment, 0, "// An optional param.\n"}, + + tLeft, + {itemHeaderOptionalParam, 0, "@param?"}, + {itemIdent, 0, "NAME"}, + {itemColon, 0, ":"}, + {itemHeaderParamType, 0, "map"}, + tRight, + {itemComment, 0, "// A map param.\n"}, + + tLeft, + {itemHeaderOptionalParam, 0, "@param?"}, + {itemIdent, 0, "NAME"}, + {itemColon, 0, ":"}, + {itemHeaderParamType, 0, "list"}, + tRight, + {itemComment, 0, "// A list param.\n"}, + + tEOF, + }}, + {"let", `{let $ident: 1+1/}{let $ident}content{/let}`, []item{ tLeft, {itemLet, 0, "let"}, diff --git a/parse/parse.go b/parse/parse.go index 1dae391..9498623 100644 --- a/parse/parse.go +++ b/parse/parse.go @@ -133,6 +133,8 @@ func (t *tree) beginTag() ast.Node { return t.parseNamespace(token) case itemTemplate: return t.parseTemplate(token) + case itemHeaderParam, itemHeaderOptionalParam: + return t.parseHeaderParam(token) case itemIf: t.notmsg(token) return t.parseIf(token) @@ -759,6 +761,28 @@ func (t *tree) parseTemplate(token item) ast.Node { return tmpl } +func (t *tree) parseHeaderParam(token item) ast.Node { + const ctx = "@param tag" + var opt = token.typ == itemHeaderOptionalParam + var name = t.expect(itemIdent, ctx) + t.expect(itemColon, ctx) + var typ = t.expect(itemHeaderParamType, ctx) + var defval ast.Node + if tok := t.next(); tok.typ == itemEquals { + defval = t.parseExpr(0) + } else { + t.backup() + } + t.expect(itemRightDelim, ctx) + return &ast.HeaderParamNode{ + Pos: token.pos, + Optional: opt, + Name: name.val, + Type: ast.TypeNode{Pos: typ.pos, Expr: typ.val}, + Default: defval, + } +} + // Expressions ---------- // Expr returns the parsed representation of the given Soy expression. diff --git a/parse/parse_test.go b/parse/parse_test.go index 0e98144..c6640a4 100644 --- a/parse/parse_test.go +++ b/parse/parse_test.go @@ -97,6 +97,27 @@ var parseTests = []parseTest{ {0, "name", false}, }})}, + {"header param", `{template .main} +{@param NAME:?} +{@param NAME:any} // A required param of type any. +{@param NAME:='default'} +{@param NAME:int=10} +{@param? NAME:[age: int, name: string]} +{@param? NAME:map} +{@param? NAME:list} +Hello world. +{/template} +`, tFile(&ast.TemplateNode{Name: ".main", Body: &ast.ListNode{Nodes: []ast.Node{ + &ast.HeaderParamNode{0, false, "NAME", ast.TypeNode{0, "?"}, nil}, + &ast.HeaderParamNode{0, false, "NAME", ast.TypeNode{0, "any"}, nil}, + &ast.HeaderParamNode{0, false, "NAME", ast.TypeNode{0, ""}, &ast.StringNode{0, "'default'", "default"}}, + &ast.HeaderParamNode{0, false, "NAME", ast.TypeNode{0, "int"}, &ast.IntNode{0, 10}}, + &ast.HeaderParamNode{0, true, "NAME", ast.TypeNode{0, "[age: int, name: string]"}, nil}, + &ast.HeaderParamNode{0, true, "NAME", ast.TypeNode{0, "map"}, nil}, + &ast.HeaderParamNode{0, true, "NAME", ast.TypeNode{0, "list"}, nil}, + &ast.RawTextNode{0, []byte("Hello world.")}, + }}})}, + {"rawtext (linejoin)", "\n a \n\tb\r\n c \n\n", tFile(newText(0, "a b c"))}, {"rawtext+html", "\n a
\n\tb\r\n\n c\n\n
", tFile(newText(0, "a
b c
"))}, {"rawtext+comment", "a
// comment \n\tb\t// comment2\r\n c\n\n", tFile( @@ -654,6 +675,12 @@ func eqTree(t *testing.T, expected, actual ast.Node) bool { case *ast.SwitchCaseNode: return eqTree(t, expected.(*ast.SwitchCaseNode).Body, actual.(*ast.SwitchCaseNode).Body) && eqNodes(t, expected.(*ast.SwitchCaseNode).Values, actual.(*ast.SwitchCaseNode).Values) + + case *ast.HeaderParamNode: + return eqbool(t, "@param optional?", expected.(*ast.HeaderParamNode).Optional, actual.(*ast.HeaderParamNode).Optional) && + eqstr(t, "@param name", expected.(*ast.HeaderParamNode).Name, actual.(*ast.HeaderParamNode).Name) && + eqstr(t, "@param type", expected.(*ast.HeaderParamNode).Type.Expr, actual.(*ast.HeaderParamNode).Type.Expr) && + eqTree(t, expected.(*ast.HeaderParamNode).Default, actual.(*ast.HeaderParamNode).Default) } panic(fmt.Sprintf("type not implemented: %T", actual)) } diff --git a/parsepasses/datarefcheck.go b/parsepasses/datarefcheck.go index 2639521..af42ddd 100644 --- a/parsepasses/datarefcheck.go +++ b/parsepasses/datarefcheck.go @@ -16,6 +16,7 @@ import ( // 5. {call}'d templates actually exist in the registry. // 6. any variable created by {let} is used somewhere // 7. {let} variable names are valid. ('ij' is not allowed.) +// 8. Only one parameter declaration mechanism (soydoc vs headers) is used. func CheckDataRefs(reg template.Registry) (err error) { var currentTemplate string defer func() { @@ -26,8 +27,8 @@ func CheckDataRefs(reg template.Registry) (err error) { for _, t := range reg.Templates { currentTemplate = t.Node.Name - tc := newTemplateChecker(reg, t.Doc.Params) - tc.checkTemplate(t.Node.Body) + tc := newTemplateChecker(reg, t) + tc.checkTemplate(t.Node) // check that all params appear in the usedKeys for _, param := range tc.params { @@ -47,9 +48,9 @@ type templateChecker struct { usedKeys []string } -func newTemplateChecker(reg template.Registry, params []*ast.SoyDocParamNode) *templateChecker { +func newTemplateChecker(reg template.Registry, tpl template.Template) *templateChecker { var paramNames []string - for _, param := range params { + for _, param := range tpl.Doc.Params { paramNames = append(paramNames, param.Name) } return &templateChecker{reg, paramNames, nil, nil, nil} @@ -69,6 +70,8 @@ func (tc *templateChecker) checkTemplate(node ast.Node) { tc.forVars = append(tc.forVars, node.Var) case *ast.DataRefNode: tc.visitKey(node.Key) + case *ast.HeaderParamNode: + panic(fmt.Errorf("unexpected {@param ...} tag found")) } if parent, ok := node.(ast.ParentNode); ok { tc.recurse(parent) diff --git a/parsepasses/datarefcheck_test.go b/parsepasses/datarefcheck_test.go index 558758c..41b6c87 100644 --- a/parsepasses/datarefcheck_test.go +++ b/parsepasses/datarefcheck_test.go @@ -38,6 +38,12 @@ Hello world /** @param paramName */ {template .paramOnly} Hello {$paramName} +{/template}`, true}, + + {` +{template .paramOnly} +{@param paramName: ?} +Hello {$paramName} {/template}`, true}, {` @@ -52,6 +58,18 @@ Hello {$param1} {$param2} {$let1} {$let2} {else} Goodbye {$param1} {$param2} {$let1} {/if} +{/template}`, true}, + + {` +{template .everything} +{@param param1: ?} +{@param? param2: ?} +{let $let1: 'hello'/} +{if true}{let $let2}let body{/let} +Hello {$param1} {$param2} {$let1} {$let2} +{else} +Goodbye {$param1} {$param2} {$let1} +{/if} {/template}`, true}, {` @@ -108,6 +126,12 @@ func TestAllParamsAreUsed(t *testing.T) { Hello {not true ? 'a' : 'b' + $used}. {/template}`, true}, + {` +{template .ParamUsedInExpr} +{@param used: ?} + Hello {not true ? 'a' : 'b' + $used}. +{/template}`, true}, + {` /** @param param */ {template .UsedInCallData} @@ -165,6 +189,21 @@ func TestAllParamsAreUsed(t *testing.T) { {$used} {/template}`, false}, + {` +/** + * @param used + * @param notused + */ +{template .CallPassesAllDataButNotDeclaredByCallee} + Hello {call .Other data="all"/}. +{/template} + +{template .Other} +{@param used: ?} +{@param? other: ?} + {$used} +{/template}`, false}, + {` /** @param var */ {template .ParamShadowedByLet} @@ -192,6 +231,21 @@ func TestAllCallParamsAreDeclaredByCallee(t *testing.T) { {template .Other} {$param1} {$param2} {/template} +`, true}, + + {` +{template .ParamsPresent} + {call .Other} + {param param1: 0/} + {param param2}hello{/param} + {/call} +{/template} + +{template .Other} +{@param param1: ?} +{@param? param2: ?} + {$param1} {$param2} +{/template} `, true}, {` @@ -369,6 +423,18 @@ func TestIJVarsAllowed(t *testing.T) { }) } +func TestTwoTypesOfParamsDisallowed(t *testing.T) { + runSimpleCheckerTests(t, []simpleCheckerTest{ + {` +/** @param var */ +{template .CalledTemplateDoesNotExist} +{@param var: ?} +Hello {$var} +{/template} +`, false}, + }) +} + func runSimpleCheckerTests(t *testing.T, tests []simpleCheckerTest) { var result []checkerTest for _, simpleTest := range tests { @@ -390,22 +456,29 @@ func runCheckerTests(t *testing.T, tests []checkerTest) { for _, body := range test.body { tree, err = parse.SoyFile("", body) if err != nil { - t.Error(err) - continue + break } - if err := reg.Add(tree); err != nil { + if err = reg.Add(tree); err != nil { + break + } + } + if err != nil { + if test.success { t.Error(err) - continue } + continue } err = CheckDataRefs(reg) if test.success && err != nil { t.Error(err) } else if !test.success && err == nil { - t.Errorf("%s: expected to fail validation, but no error was raised.", - reg.Templates[0].Node.Name) + var name = "(empty)" + if len(reg.Templates) > 0 { + name = reg.Templates[0].Node.Name + } + t.Errorf("%s: expected to fail validation, but no error was raised.", name) } } } diff --git a/soyhtml/exec.go b/soyhtml/exec.go index f7d6dc7..eab0ce8 100644 --- a/soyhtml/exec.go +++ b/soyhtml/exec.go @@ -86,6 +86,8 @@ func (s *state) walk(node ast.Node) { s.autoescape = node.Autoescape } s.walk(node.Body) + case *ast.HeaderParamNode: + // TODO: Validate param types. case *ast.ListNode: for _, node := range node.Nodes { s.walk(node) diff --git a/soyhtml/exec_test.go b/soyhtml/exec_test.go index f84bccc..1814eea 100644 --- a/soyhtml/exec_test.go +++ b/soyhtml/exec_test.go @@ -49,6 +49,16 @@ func TestBasicExec(t *testing.T) { /** @param name */ {template .sayHello} Hello {$name}! + {/template}`, + "Hello Rob!", + d{"name": "Rob"}, true}, + {"hello world w/ header param", "test.sayHello", + `{namespace test} + + {template .sayHello} + {@param name: ?} + {@param? optName: ?} + Hello {$name}! {/template}`, "Hello Rob!", d{"name": "Rob"}, true}, @@ -1110,6 +1120,27 @@ func TestMessages(t *testing.T) { }) } +func TestHeaderParams(t *testing.T) { + runExecTests(t, []execTest{ + { + name: "param works", + templateName: "test.main", + input: `{namespace test} +{template .main} +{@param a: ?} +{@param? opt: string} +Hello {$a} +{if $opt} and {$opt}{/if} +{/template} +`, + output: "Hello world", + data: d{"a": "world"}, + ok: true, + }, + }) +} + + // testing cross namespace stuff requires multiple file bodies type nsExecTest struct { name string diff --git a/template/registry.go b/template/registry.go index 55ef69f..f0ce497 100644 --- a/template/registry.go +++ b/template/registry.go @@ -58,6 +58,29 @@ func (r *Registry) Add(soyfile *ast.SoyFileNode) error { if !ok { sdn = &ast.SoyDocNode{tn.Pos, nil} } + hasSoyDocParams := len(sdn.Params) > 0 + + // Extract leading Header Params from the template body. + // Add them to Soy.Params for backwards compatibility. + var headerParams []*ast.HeaderParamNode + for _, n := range tn.Body.Nodes { + if param, ok := n.(*ast.HeaderParamNode); ok { + + headerParams = append(headerParams, param) + sdn.Params = append(sdn.Params, &ast.SoyDocParamNode{ + Pos: param.Pos, + Name: param.Name, + Optional: param.Optional, + }) + } else { + break + } + } + if len(headerParams) > 0 && hasSoyDocParams { + return fmt.Errorf("template may not have both soydoc and header params specified") + } + tn.Body.Nodes = tn.Body.Nodes[len(headerParams):] + r.Templates = append(r.Templates, Template{sdn, tn, ns}) r.sourceByTemplateName[tn.Name] = soyfile.Text r.fileByTemplateName[tn.Name] = soyfile.Name diff --git a/template/template.go b/template/template.go index 95bfd21..d79490c 100644 --- a/template/template.go +++ b/template/template.go @@ -5,7 +5,7 @@ import "github.com/robfig/soy/ast" // Template is a Soy template's parse tree, including the relevant context // (preceding soydoc and namespace). type Template struct { - Doc *ast.SoyDocNode // this template's SoyDoc - Node *ast.TemplateNode // this template's node - Namespace *ast.NamespaceNode // this template's namespace + Doc *ast.SoyDocNode // this template's SoyDoc, w/ header params added to Doc.Params + Node *ast.TemplateNode // this template's node + Namespace *ast.NamespaceNode // this template's namespace }