From deaa47c876e50e5018ac22ede69a79f35a611fa4 Mon Sep 17 00:00:00 2001 From: alandefreitas Date: Mon, 4 Sep 2023 18:48:06 -0300 Subject: [PATCH] feat: strict specs --- include/mrdox/Support/Handlebars.hpp | 12 ++ src/lib/Dom/Array.cpp | 23 ++-- src/lib/Dom/Object.cpp | 23 +--- src/lib/Dom/Value.cpp | 6 +- src/lib/Support/Handlebars.cpp | 124 +++++++++++++++---- src/test/lib/Support/Handlebars.cpp | 177 ++++++++++++++++++++++++++- 6 files changed, 301 insertions(+), 64 deletions(-) diff --git a/include/mrdox/Support/Handlebars.hpp b/include/mrdox/Support/Handlebars.hpp index 626bb326b..0e5d91e95 100644 --- a/include/mrdox/Support/Handlebars.hpp +++ b/include/mrdox/Support/Handlebars.hpp @@ -38,9 +38,20 @@ struct HandlebarsOptions bool noEscape = false; /** Templates will throw rather than ignore missing fields + + Run in strict mode. In this mode, templates will throw rather than + silently ignore missing fields. + */ bool strict = false; + /** Removes object existence checks when traversing paths + + This is a subset of strict mode that generates optimized + templates when the data inputs are known to be safe. + */ + bool assumeObjects = false; + /** Disable the auto-indent feature By default, an indented partial-call causes the output of the @@ -333,6 +344,7 @@ class MRDOX_DECL HandlebarsCallback std::vector blockParamIds_; std::function const* logger_; detail::RenderState* renderState_; + HandlebarsOptions const* opt_; friend class Handlebars; friend diff --git a/src/lib/Dom/Array.cpp b/src/lib/Dom/Array.cpp index 7b131cc38..5b02a5ff6 100644 --- a/src/lib/Dom/Array.cpp +++ b/src/lib/Dom/Array.cpp @@ -54,24 +54,17 @@ toString( Array const& arr) { if(arr.empty()) - return "[]"; - std::string s = "["; - { - auto const n = arr.size(); - auto insert = std::back_inserter(s); - for(std::size_t i = 0; i < n; ++i) + return ""; + std::string s; + auto const n = arr.size(); + if (n != 0) { + s += toString(arr.at(0)); + for(std::size_t i = 1; i < n; ++i) { - if(i != 0) - fmt::format_to(insert, - ", {}", - toString(arr.at(i))); - else - fmt::format_to(insert, - " {}", - toStringChild(arr.at(i))); + s += ','; + s += toString(arr.at(i)); } } - s += " ]"; return s; } diff --git a/src/lib/Dom/Object.cpp b/src/lib/Dom/Object.cpp index fbadd6d46..98ec3c587 100644 --- a/src/lib/Dom/Object.cpp +++ b/src/lib/Dom/Object.cpp @@ -89,26 +89,9 @@ exists(std::string_view key) const } std::string -toString( - Object const& obj) -{ - if(obj.empty()) - return "{}"; - std::string s = "{"; - { - auto insert = std::back_inserter(s); - for(auto const& kv : RangeFor(obj)) - { - if(! kv.first) - s.push_back(','); - fmt::format_to(insert, - " {} : {}", - kv.value.key, - toStringChild(kv.value.value)); - } - } - s += " }"; - return s; +toString(Object const&) +{ + return "[object Object]"; } //------------------------------------------------ diff --git a/src/lib/Dom/Value.cpp b/src/lib/Dom/Value.cpp index 312859434..8e3a8602a 100644 --- a/src/lib/Dom/Value.cpp +++ b/src/lib/Dom/Value.cpp @@ -471,11 +471,7 @@ toStringChild( return "[]"; } case Value::Kind::Object: - { - if(! value.obj_.empty()) - return "{...}"; - return "{}"; - } + return "[object Object]"; case Value::Kind::Function: return "[function]"; default: diff --git a/src/lib/Support/Handlebars.cpp b/src/lib/Support/Handlebars.cpp index eaad12610..1d6007c9f 100644 --- a/src/lib/Support/Handlebars.cpp +++ b/src/lib/Support/Handlebars.cpp @@ -606,7 +606,7 @@ find_position_in_text( if (res.line == 1) res.column = res.pos; else - res.column = res.pos - text.rfind('\n', res.pos); + res.column = res.pos - text.rfind('\n', res.pos) - 1; } return res; } @@ -648,7 +648,8 @@ std::pair lookupPropertyImpl( dom::Object const& context, std::string_view path, - detail::RenderState const& state) + detail::RenderState const& state, + HandlebarsOptions const& opt) { // Get first value from Object std::string_view segment = popFirstSegment(path); @@ -661,7 +662,21 @@ lookupPropertyImpl( } else if (!context.exists(literalSegment)) { - return {nullptr, false}; + if (opt.strict || (opt.assumeObjects && !path.empty())) + { + std::string msg = fmt::format( + "\"{}\" not defined in {}", literalSegment, toString(context)); + auto res = find_position_in_text(state.templateText0, literalSegment); + if (res) + { + throw HandlebarsError(msg, res.line, res.column, res.pos); + } + throw HandlebarsError(msg); + } + else + { + return {nullptr, false}; + } } else { @@ -679,9 +694,27 @@ lookupPropertyImpl( { auto obj = cur.getObject(); if (obj.exists(literalSegment)) + { cur = obj.find(literalSegment); + } else - return {nullptr, false}; + { + if (opt.strict) + { + std::string msg = fmt::format( + "\"{}\" not defined in {}", literalSegment, toString(cur)); + auto res = find_position_in_text(state.templateText0, literalSegment); + if (res) + { + throw HandlebarsError(msg, res.line, res.column, res.pos); + } + throw HandlebarsError(msg); + } + else + { + return {nullptr, false}; + } + } } // If current value is an Array, get the next value the stripped index else if (cur.isArray()) @@ -717,7 +750,9 @@ std::pair lookupPropertyImpl( dom::Value const& context, std::string_view path, - detail::RenderState const& state) { + detail::RenderState const& state, + HandlebarsOptions const& opt) +{ checkPath(path, state); // ============================================================== // "." / "this" @@ -728,12 +763,22 @@ lookupPropertyImpl( // Non-object key // ============================================================== if (context.kind() != dom::Kind::Object) { + if (opt.strict || opt.assumeObjects) + { + std::string msg = fmt::format("\"{}\" not defined in {}", path, context); + auto res = find_position_in_text(state.templateText0, path); + if (res) + { + throw HandlebarsError(msg, res.line, res.column, res.pos); + } + throw HandlebarsError(msg); + } return {nullptr, false}; } // ============================================================== // Object path // ============================================================== - return lookupPropertyImpl(context.getObject(), path, state); + return lookupPropertyImpl(context.getObject(), path, state, opt); } template S> @@ -741,19 +786,21 @@ std::pair lookupPropertyImpl( dom::Value const& data, S const& path, - detail::RenderState const& state) + detail::RenderState const& state, + HandlebarsOptions const& opt) { - return lookupPropertyImpl(data, std::string_view(path), state); + return lookupPropertyImpl(data, std::string_view(path), state, opt); } std::pair lookupPropertyImpl( dom::Value const& context, dom::Value const& path, - detail::RenderState const& state) + detail::RenderState const& state, + HandlebarsOptions const& opt) { if (path.isString()) - return lookupPropertyImpl(context, path.getString(), state); + return lookupPropertyImpl(context, path.getString(), state, opt); if (path.isInteger()) { if (context.isArray()) { auto& arr = context.getArray(); @@ -761,7 +808,7 @@ lookupPropertyImpl( return {nullptr, false}; return {arr.at(path.getInteger()), true}; } - return lookupPropertyImpl(context, std::to_string(path.getInteger()), state); + return lookupPropertyImpl(context, std::to_string(path.getInteger()), state, opt); } return {nullptr, false}; } @@ -772,7 +819,7 @@ lookupProperty( dom::Value const& context, dom::Value const& path) const { - return lookupPropertyImpl(context, path, *renderState_); + return lookupPropertyImpl(context, path, *renderState_, *opt_); } @@ -1679,12 +1726,15 @@ evalExpr( break; } } - auto [res, found] = lookupPropertyImpl(data, expression, state); + auto [res, found] = lookupPropertyImpl(data, expression, state, opt); return {res, found, false}; } // ============================================================== // Dotdot context path // ============================================================== + HandlebarsOptions noStrict = opt; + noStrict.strict = false; + noStrict.assumeObjects = false; if (expression.starts_with("..")) { // Get value from parent helper contexts std::size_t dotdots = 1; @@ -1704,7 +1754,7 @@ evalExpr( } dom::Value parentCtx = state.parentContext[state.parentContext.size() - dotdots]; - auto [res, found] = lookupPropertyImpl(parentCtx, expression, state); + auto [res, found] = lookupPropertyImpl(parentCtx, expression, state, noStrict); return {res, found, false}; } // ============================================================== @@ -1730,7 +1780,7 @@ evalExpr( bool defined; if (isPathedValue) { - std::tie(r, defined) = lookupPropertyImpl(context, expression, state); + std::tie(r, defined) = lookupPropertyImpl(context, expression, state, noStrict); if (defined) { return {r, defined, false}; } @@ -1739,7 +1789,7 @@ evalExpr( // ============================================================== // Block values // ============================================================== - std::tie(r, defined) = lookupPropertyImpl(state.blockValues, expression, state); + std::tie(r, defined) = lookupPropertyImpl(state.blockValues, expression, state, noStrict); if (defined) { return {r, defined, false, false, true}; @@ -1759,7 +1809,10 @@ evalExpr( // ============================================================== // Context values // ============================================================== - std::tie(r, defined) = lookupPropertyImpl(context, expression, state); + HandlebarsOptions strictOpt = opt; + strictOpt.strict = opt.strict && !opt.compat; + strictOpt.assumeObjects = opt.assumeObjects && !opt.compat; + std::tie(r, defined) = lookupPropertyImpl(context, expression, state, strictOpt); if (defined) { return {r, defined, false}; } @@ -1772,13 +1825,19 @@ evalExpr( auto parentContexts = std::ranges::views::reverse(state.parentContext); for (auto parentContext: parentContexts) { - std::tie(r, defined) = lookupPropertyImpl(parentContext, expression, state); + std::tie(r, defined) = lookupPropertyImpl(parentContext, expression, state, noStrict); if (defined) { return {r, defined, false}; } } } + + if (opt.strict) + { + std::string msg = fmt::format("\"{}\" not defined", expression); + throw HandlebarsError(msg); + } return {nullptr, false, false}; } @@ -2093,7 +2152,9 @@ renderExpression( cb.context_ = &context; cb.data_ = &state.data; cb.logger_ = &logger_; - setupArgs(tag.arguments, context, state, args, cb, opt); + HandlebarsOptions noStrict = opt; + noStrict.strict = false; + setupArgs(tag.arguments, context, state, args, cb, noStrict); auto [res, render] = fn(args, cb); if (render == HelperBehavior::RENDER_RESULT) { format_to(out, res, opt2); @@ -2125,7 +2186,9 @@ renderExpression( dom::Array args = dom::newArray(); HandlebarsCallback cb; cb.name_ = helper_expr; - setupArgs(tag.arguments, context, state, args, cb, opt); + HandlebarsOptions noStrict = opt; + noStrict.strict = false; + setupArgs(tag.arguments, context, state, args, cb, noStrict); auto v2 = resV.value.getFunction().call(args).value(); format_to(out, v2, opt2); } @@ -2135,6 +2198,11 @@ renderExpression( } return; } + else if (opt.strict) + { + std::string msg = fmt::format("\"{}\" not defined in {}", helper_expr, toString(context)); + throw HandlebarsError(msg); + } // ============================================================== // helperMissing hook @@ -2252,6 +2320,7 @@ setupArgs( } } cb.renderState_ = &state; + cb.opt_ = &opt; } void @@ -2604,7 +2673,20 @@ renderBlock( arguments = tag.helper; } } - setupArgs(arguments, context, state, args, cb, opt); + else if (opt.strict && !found) + { + std::string msg = fmt::format( + "\"{}\" not defined in {}", tag.helper, toString(context)); + auto res = find_position_in_text(state.templateText0, tag.helper); + if (res) + { + throw HandlebarsError(msg, res.line, res.column, res.pos); + } + throw HandlebarsError(msg); + } + HandlebarsOptions noStrict = opt; + noStrict.strict = opt.strict && emulateMustache; + setupArgs(arguments, context, state, args, cb, noStrict); // ============================================================== // Setup block parameters diff --git a/src/test/lib/Support/Handlebars.cpp b/src/test/lib/Support/Handlebars.cpp index e1c0f324d..80ea501e6 100644 --- a/src/test/lib/Support/Handlebars.cpp +++ b/src/test/lib/Support/Handlebars.cpp @@ -4975,12 +4975,177 @@ void strict() { // https://github.com/handlebars-lang/handlebars.js/blob/4.x/spec/strict.js + + Handlebars hbs; + + HandlebarsOptions opt; + opt.strict = true; + + // should error on missing property lookup + { + BOOST_TEST_THROW_WITH( + hbs.render("{{hello}}", dom::Object{}, opt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:2"); + } + + // should error on missing child + { + // { hello: { bar: 'foo' } } + dom::Object ctx; + dom::Object hello; + hello.set("bar", "foo"); + ctx.set("hello", hello); + BOOST_TEST(hbs.render("{{hello.bar}}", ctx, opt) == "foo"); + + // { hello: {} } + ctx.set("hello", dom::Object()); + BOOST_TEST_THROW_WITH( + hbs.render("{{hello.bar}}", ctx, opt), + HandlebarsError, "\"bar\" not defined in [object Object] - 1:8"); + } + + // should handle explicit undefined + { + // { hello: { bar: undefined } } + dom::Object ctx; + dom::Object hello; + hello.set("bar", nullptr); + ctx.set("hello", hello); + BOOST_TEST(hbs.render("{{hello.bar}}", ctx, opt) == ""); + } + + // should error on missing property lookup in known helpers mode + { + BOOST_TEST_THROW_WITH( + hbs.render("{{hello}}", dom::Object{}, opt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:2"); + } + + // should error on missing context + { + BOOST_TEST_THROW_WITH( + hbs.render("{{hello}}", dom::Object{}, opt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:2"); + } + + // should error on missing data lookup + { + std::string string = "{{@hello}}"; + BOOST_TEST_THROW_WITH( + hbs.render(string, dom::Object{}, opt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:3"); + dom::Object data; + data.set("hello", "foo"); + opt.data = data; + BOOST_TEST(hbs.render(string, dom::Object{}, opt) == "foo"); + opt.data = nullptr; + } + + // should not run helperMissing for helper calls + { + std::string string = "{{hello foo}}"; + dom::Object ctx; + ctx.set("foo", true); + BOOST_TEST_THROW_WITH( + hbs.render(string, ctx, opt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:2"); + + string = "{{#hello foo}}{{/hello}}"; + BOOST_TEST_THROW_WITH( + hbs.render(string, ctx, opt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:3"); + } + + // should throw on ambiguous blocks + { + BOOST_TEST_THROW_WITH( + hbs.render("{{#hello}}{{/hello}}", dom::Object{}, opt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:3"); + + BOOST_TEST_THROW_WITH( + hbs.render("{{^hello}}{{/hello}}", dom::Object{}, opt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:3"); + + dom::Object ctx; + ctx.set("hello", dom::Object()); + BOOST_TEST_THROW_WITH( + hbs.render("{{#hello.bar}}{{/hello.bar}}", ctx, opt), + HandlebarsError, "\"bar\" not defined in [object Object] - 1:9"); + } + + // should allow undefined parameters when passed to helpers + { + BOOST_TEST(hbs.render("{{#unless foo}}success{{/unless}}", dom::Object{}, opt) == "success"); + } + + // should allow undefined hash when passed to helpers + { + std::string string = "{{helper value=@foo}}"; + hbs.registerHelper("helper", [](dom::Array const& args, HandlebarsCallback const& options) { + BOOST_TEST(options.hash().exists("value")); + BOOST_TEST(options.hash().find("value").isNull()); + return "success"; + }); + BOOST_TEST(hbs.render(string, dom::Object{}, opt) == "success"); + } + + // should show error location on missing property lookup + { + std::string string = "\n\n\n {{hello}}"; + BOOST_TEST_THROW_WITH( + hbs.render(string, dom::Object{}, opt), + HandlebarsError, "\"hello\" not defined in [object Object] - 4:5"); + } } void -mustache_compat_spec() +assume_objects() { - // https://github.com/handlebars-lang/handlebars.js/blob/4.x/spec/spec.js + // https://github.com/handlebars-lang/handlebars.js/blob/4.x/spec/strict.js + Handlebars hbs; + + HandlebarsOptions assumeOpt; + assumeOpt.assumeObjects = true; + + // should ignore missing property + { + BOOST_TEST(hbs.render("{{hello}}", dom::Object{}, assumeOpt) == ""); + } + + // should ignore missing child + { + dom::Object ctx; + ctx.set("hello", dom::Object()); + BOOST_TEST(hbs.render("{{hello.bar}}", ctx, assumeOpt) == ""); + } + + // should error on missing object + { + BOOST_TEST_THROW_WITH( + hbs.render("{{hello.bar}}", dom::Object{}, assumeOpt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:2"); + } + + // should error on missing context + { + dom::Value ctx(nullptr); + BOOST_TEST_THROW_WITH( + hbs.render("{{hello}}", ctx, assumeOpt), + HandlebarsError, "\"hello\" not defined in null - 1:2"); + } + + // should error on missing data lookup + { + dom::Value ctx(nullptr); + BOOST_TEST_THROW_WITH( + hbs.render("{{@hello.bar}}", ctx, assumeOpt), + HandlebarsError, "\"hello\" not defined in [object Object] - 1:3"); + } + + // should execute blockHelperMissing + { + BOOST_TEST(hbs.render("{{^hello}}foo{{/hello}}", dom::Object{}, assumeOpt) == "foo"); + } } void @@ -4989,6 +5154,11 @@ utils() // https://github.com/handlebars-lang/handlebars.js/blob/4.x/spec/utils.js } +void +mustache_compat_spec() +{ + // https://github.com/handlebars-lang/handlebars.js/blob/4.x/spec/spec.js +} void run() @@ -5017,8 +5187,9 @@ run() helpers(); track_ids(); strict(); - mustache_compat_spec(); + assume_objects(); utils(); + mustache_compat_spec(); } };