From b9eab22a16b757da96555349c64ec5e61861a4a4 Mon Sep 17 00:00:00 2001 From: Pranjal Raihan Date: Sun, 22 Dec 2024 20:49:12 -0800 Subject: [PATCH] Implement with blocks Summary: This diff implements `{{#with}}` blocks that can "de-structure" an object. https://handlebarsjs.com/guide/block-helpers.html#the-with-helper Reviewed By: createdbysk Differential Revision: D67570283 fbshipit-source-id: a28acf9873cf5b77283617bd98b63f03190d0cd8 --- .../thrift/src/thrift/compiler/whisker/ast.h | 14 +++ .../src/thrift/compiler/whisker/parser.cc | 51 ++++++++++ .../src/thrift/compiler/whisker/print_ast.cc | 6 ++ .../src/thrift/compiler/whisker/render.cc | 31 ++++++ .../compiler/whisker/test/parser_test.cc | 58 ++++++++++++ .../compiler/whisker/test/render_test.cc | 94 +++++++++++++++++++ .../src/thrift/doc/contributions/whisker.md | 4 - 7 files changed, 254 insertions(+), 4 deletions(-) diff --git a/third-party/thrift/src/thrift/compiler/whisker/ast.h b/third-party/thrift/src/thrift/compiler/whisker/ast.h index 07fcac507e296..528cf37eeeaaf 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/ast.h +++ b/third-party/thrift/src/thrift/compiler/whisker/ast.h @@ -30,6 +30,7 @@ struct newline; struct comment; struct section_block; struct conditional_block; +struct with_block; struct partial_apply; struct interpolation; struct let_statement; @@ -44,6 +45,7 @@ using body = std::variant< interpolation, section_block, conditional_block, + with_block, let_statement, partial_apply>; using bodies = std::vector; @@ -259,6 +261,18 @@ struct conditional_block { std::optional else_clause; }; +/** + * A Whisker construct for "de-structuring" a map-like object. + * This matches Handlebars: + * https://handlebarsjs.com/guide/builtin-helpers.html#with + */ +struct with_block { + source_range loc; + + expression value; + bodies body_elements; +}; + /* * A valid Whisker path component for partial application. See whisker::lexer * for its definition. diff --git a/third-party/thrift/src/thrift/compiler/whisker/parser.cc b/third-party/thrift/src/thrift/compiler/whisker/parser.cc index 68b47623ffc2c..b4f78b406bd65 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/parser.cc +++ b/third-party/thrift/src/thrift/compiler/whisker/parser.cc @@ -755,6 +755,7 @@ class parser { ast::interpolation, ast::section_block, ast::conditional_block, + ast::with_block, ast::let_statement, ast::partial_apply>; // template → { interpolation | block | statement | partial-apply } @@ -784,6 +785,8 @@ class parser { templ = std::move(variable).consume_and_advance(&scan); } else if (parse_result conditional_block = parse_conditional_block(scan)) { templ = std::move(conditional_block).consume_and_advance(&scan); + } else if (parse_result with_block = parse_with_block(scan)) { + templ = std::move(with_block).consume_and_advance(&scan); } else if (parse_result let_statement = parse_let_statement(scan)) { templ = std::move(let_statement).consume_and_advance(&scan); } else if (parse_result section_block = parse_section_block(scan)) { @@ -1173,6 +1176,54 @@ class parser { scan}; } + // with-block → { with-block-open ~ body* ~ with-block-close } + // with-block-open → { "{{" ~ "#" ~ "with" ~ expression ~ "}}" } + // with-block-close → { "{{" ~ "/" ~ "with" ~ "}}" } + parse_result parse_with_block(parser_scan_window scan) { + assert(scan.empty()); + const auto scan_start = scan.start; + + if (!(try_consume_token(&scan, tok::open) && + try_consume_token(&scan, tok::pound) && + try_consume_token(&scan, tok::kw_with))) { + return no_parse_result(); + } + scan = scan.make_fresh(); + + parse_result value = parse_expression(scan); + if (!value.has_value()) { + report_expected(scan, fmt::format("expression to open with-block")); + } + ast::expression expr = std::move(value).consume_and_advance(&scan); + if (!try_consume_token(&scan, tok::close)) { + report_expected(scan, fmt::format("{} to open with-block", tok::close)); + } + scan = scan.make_fresh(); + + ast::bodies bodies = parse_bodies(scan).consume_and_advance(&scan); + + const auto expect_on_close = [&](tok kind) { + if (!try_consume_token(&scan, kind)) { + report_expected( + scan, + fmt::format("{} to close with-block '{}'", kind, expr.to_string())); + } + }; + + expect_on_close(tok::open); + expect_on_close(tok::slash); + expect_on_close(tok::kw_with); + expect_on_close(tok::close); + + return { + ast::with_block{ + scan.with_start(scan_start).range(), + std::move(expr), + std::move(bodies), + }, + scan}; + } + // partial-apply → { "{{" ~ ">" ~ partial-lookup ~ "}}" } parse_result parse_partial_apply( parser_scan_window scan) { diff --git a/third-party/thrift/src/thrift/compiler/whisker/print_ast.cc b/third-party/thrift/src/thrift/compiler/whisker/print_ast.cc index f8decb5748f83..18da8bf2353cc 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/print_ast.cc +++ b/third-party/thrift/src/thrift/compiler/whisker/print_ast.cc @@ -90,6 +90,12 @@ struct ast_visitor { visit(else_clause->body_elements, else_scope.open_node()); } } + void visit( + const ast::with_block& with_block, tree_printer::scope scope) const { + scope.println(" with-block {}", location(with_block.loc)); + visit(with_block.value, scope.open_property()); + visit(with_block.body_elements, scope.open_node()); + } void visit(const ast::partial_apply& partial_apply, tree_printer::scope scope) const { scope.println( diff --git a/third-party/thrift/src/thrift/compiler/whisker/render.cc b/third-party/thrift/src/thrift/compiler/whisker/render.cc index 022e78bcdd0bd..f57f51dce20ee 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/render.cc +++ b/third-party/thrift/src/thrift/compiler/whisker/render.cc @@ -564,6 +564,37 @@ class render_engine { } } + void visit(const ast::with_block& with_block) { + const ast::expression& expr = with_block.value; + object value = evaluate(expr); + value.visit( + [&](const map&) { + // maps can be de-structured. + }, + [&](const native_object::ptr& o) { + // map-like native objects can be de-structured. + if (o->as_map_like() == nullptr) { + diags_.error( + expr.loc.begin, + "Expression '{}' is a native_object which is not map-like. The encountered value is:\n{}", + expr.to_string(), + to_string(value)); + throw abort_rendering(); + } + }, + [&](auto&&) { + diags_.error( + expr.loc.begin, + "Expression '{}' does not evaluate to a map. The encountered value is:\n{}", + expr.to_string(), + to_string(value)); + throw abort_rendering(); + }); + eval_context_.push_scope(value); + visit(with_block.body_elements); + eval_context_.pop_scope(); + } + void visit(const ast::partial_apply& partial_apply) { std::vector path; path.reserve(partial_apply.path.parts.size()); diff --git a/third-party/thrift/src/thrift/compiler/whisker/test/parser_test.cc b/third-party/thrift/src/thrift/compiler/whisker/test/parser_test.cc index 838eab0847478..44102d65aef02 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/test/parser_test.cc +++ b/third-party/thrift/src/thrift/compiler/whisker/test/parser_test.cc @@ -704,6 +704,64 @@ TEST_F(ParserTest, let_statement_keyword) { 1))); } +TEST_F(ParserTest, with_block) { + auto ast = parse_ast( + "{{#with foo.bar}}\n" + "{{bar}}\n" + "{{/with}}\n"); + EXPECT_EQ( + to_string(*ast), + "root [path/to/test-1.whisker]\n" + "|- with-block \n" + "| `- expression 'foo.bar'\n" + "| |- interpolation 'bar'\n" + "| |- newline '\\n'\n"); +} + +TEST_F(ParserTest, with_block_no_expression) { + auto ast = parse_ast( + "{{#with}}\n" + "{{bar}}\n" + "{{/with}}\n"); + EXPECT_FALSE(ast.has_value()); + EXPECT_THAT( + diagnostics, + testing::ElementsAre(diagnostic( + diagnostic_level::error, + "expected expression to open with-block but found `}}`", + path_to_file(1), + 1))); +} + +TEST_F(ParserTest, with_block_multiple_expression) { + auto ast = parse_ast( + "{{#with foo.bar bar.baz}}\n" + "{{bar}}\n" + "{{/with}}\n"); + EXPECT_FALSE(ast.has_value()); + EXPECT_THAT( + diagnostics, + testing::ElementsAre(diagnostic( + diagnostic_level::error, + "expected `}}` to open with-block but found identifier", + path_to_file(1), + 1))); +} + +TEST_F(ParserTest, with_block_missing_close) { + auto ast = parse_ast( + "{{#with foo.bar}}\n" + "{{bar}}\n"); + EXPECT_FALSE(ast.has_value()); + EXPECT_THAT( + diagnostics, + testing::ElementsAre(diagnostic( + diagnostic_level::error, + "expected `{{` to close with-block 'foo.bar' but found EOF", + path_to_file(1), + 3))); +} + TEST_F(ParserTest, comment) { auto ast = parse_ast("Hello{{! #$^& random text }}world"); diff --git a/third-party/thrift/src/thrift/compiler/whisker/test/render_test.cc b/third-party/thrift/src/thrift/compiler/whisker/test/render_test.cc index dfe1cdc2c71a3..4f87738f781c0 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/test/render_test.cc +++ b/third-party/thrift/src/thrift/compiler/whisker/test/render_test.cc @@ -26,7 +26,35 @@ namespace w = whisker::make; namespace whisker { namespace { + class empty_native_object : public native_object {}; + +/** + * When looking up a property, always returns a whisker::string that is the + * property name repeated twice. + */ +class double_property_name + : public native_object, + public native_object::map_like, + public std::enable_shared_from_this { + public: + std::shared_ptr as_map_like() const override { + return shared_from_this(); + } + + const object* lookup_property(std::string_view id) const override { + if (auto cached = cached_.find(id); cached != cached_.end()) { + return &cached->second; + } + auto [result, inserted] = + cached_.insert({std::string(id), w::string(fmt::format("{0}{0}", id))}); + assert(inserted); + return &result->second; + } + + mutable std::map> cached_; +}; + } // namespace TEST_F(RenderTest, basic) { @@ -683,6 +711,72 @@ TEST_F(RenderTest, let_statement_rebinding_error) { 2))); } +TEST_F(RenderTest, with_block) { + auto result = render( + "{{#with news}}\n" + " {{#if has-update?}}\n" + " Stuff is {{foo}} happening!\n" + " {{/if has-update?}}\n" + "{{/with}}\n", + w::map( + {{"news", + w::map( + {{"has-update?", w::boolean(true)}, + {"foo", w::string("now")}})}})); + EXPECT_THAT(diagnostics(), testing::IsEmpty()); + EXPECT_EQ(*result, " Stuff is now happening!\n"); +} + +TEST_F(RenderTest, with_not_map) { + auto result = render( + "{{#with news}}\n" + "{{/with}}\n", + w::map({{"news", w::array({w::i64(0)})}})); + EXPECT_FALSE(result.has_value()); + EXPECT_THAT( + diagnostics(), + testing::ElementsAre(diagnostic( + diagnostic_level::error, + "Expression 'news' does not evaluate to a map. The encountered value is:\n" + "array (size=1)\n" + "`-[0]\n" + " |-i64(0)\n", + path_to_file, + 1))); +} + +TEST_F(RenderTest, with_map_like_native_object) { + auto result = render( + "{{#with doubler}}\n" + "{{foo}} {{bar}}\n" + "{{#with .}}\n" + "{{baz}}\n" + "{{/with}}\n" + "{{/with}}\n", + w::map({{"doubler", w::make_native_object()}})); + EXPECT_THAT(diagnostics(), testing::IsEmpty()); + EXPECT_EQ( + *result, + "foofoo barbar\n" + "bazbaz\n"); +} + +TEST_F(RenderTest, with_not_map_like_native_object) { + auto result = render( + "{{#with empty}}\n" + "{{/with}}\n", + w::map({{"empty", w::make_native_object()}})); + EXPECT_FALSE(result.has_value()); + EXPECT_THAT( + diagnostics(), + testing::ElementsAre(diagnostic( + diagnostic_level::error, + "Expression 'empty' is a native_object which is not map-like. The encountered value is:\n" + "\n", + path_to_file, + 1))); +} + TEST_F(RenderTest, printable_types_strict_failure) { { auto result = render( diff --git a/third-party/thrift/src/thrift/doc/contributions/whisker.md b/third-party/thrift/src/thrift/doc/contributions/whisker.md index 147706e65168d..dad1816bba0db 100644 --- a/third-party/thrift/src/thrift/doc/contributions/whisker.md +++ b/third-party/thrift/src/thrift/doc/contributions/whisker.md @@ -446,10 +446,6 @@ Whisker `{{#each}}` blocks are based on [EmberJS `{{#each}}`](https://guides.emb ### With Blocks -:::warning -`{{#with}}` blocks have not been implemented yet. -::: - Whisker supports a block type for de-structuring: `{{#with}}`. A typical de-structuring block might look like: ```handlebars