diff --git a/thrift/compiler/whisker/ast.h b/thrift/compiler/whisker/ast.h index af2a5e1598d..ce1b116b321 100644 --- a/thrift/compiler/whisker/ast.h +++ b/thrift/compiler/whisker/ast.h @@ -343,6 +343,14 @@ struct conditional_block { expression condition; bodies body_elements; + // Any {{#else if}} clauses, if present. + struct else_if_block { + source_range loc; + expression condition; + bodies body_elements; + }; + std::vector else_if_clauses; + // The {{#else}} clause, if present. struct else_block { source_range loc; diff --git a/thrift/compiler/whisker/parser.cc b/thrift/compiler/whisker/parser.cc index ecbccb7a4ee..be647f42e2d 100644 --- a/thrift/compiler/whisker/parser.cc +++ b/thrift/compiler/whisker/parser.cc @@ -592,6 +592,48 @@ class parser { } } + // Parses the "{{#else if}}" clause which is a separator between two + // ast::bodies. + // + // else-if-block → + // { "{{" ~ "#" ~ "else" ~ " " ~ "if" ~ expression ~ "}}" ~ body* } + parse_result parse_else_if_clause( + 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))) { + return no_parse_result(); + } + if (!try_consume_token(&scan, tok::kw_else)) { + return no_parse_result(); + } + if (!try_consume_token(&scan, tok::kw_if)) { + return no_parse_result(); + } + scan = scan.make_fresh(); + + parse_result condition = parse_expression(scan); + if (!condition.has_value()) { + report_expected(scan, fmt::format("expression in else-if clause")); + } + ast::expression cond = std::move(condition).consume_and_advance(&scan); + if (!try_consume_token(&scan, tok::close)) { + report_expected(scan, fmt::format("{} in else-if clause", tok::close)); + } + scan = scan.make_fresh(); + + ast::bodies bodies = parse_bodies(scan).consume_and_advance(&scan); + + return parse_result{ + ast::conditional_block::else_if_block{ + scan.with_start(scan_start).range(), + std::move(cond), + std::move(bodies)}, + scan}; + } + // Parses the "{{#else}}" clause which is a separator between two ast::bodies. // // else-clause → { "{{" ~ "#" ~ "else" ~ "}}" } @@ -605,6 +647,16 @@ class parser { return {{}, scan}; } + // Parses the beginning of an "{{#else [if]" clause which is a separator + // between two ast::bodies. + // + // { "{{" ~ "#" ~ "else" } + bool peek_else_clause(parser_scan_window scan) { + return try_consume_token(&scan, tok::open) && + try_consume_token(&scan, tok::pound) && + try_consume_token(&scan, tok::kw_else); + } + // Returns an empty parse result if no body was found. // // Returns an empty optional if body was found but consisted @@ -624,9 +676,9 @@ class parser { body = std::move(maybe_text).consume_and_advance(&scan); } else if (parse_result maybe_newline = parse_newline(scan)) { body = std::move(maybe_newline).consume_and_advance(&scan); - } else if (parse_result else_clause = parse_else_clause(scan)) { - // The "{{#else}}" clause marks the end of the current block (and the - // beginning of the next one). + } else if (peek_else_clause(scan)) { + // The "{{#else [if]}}" clause marks the end of the current block (and + // the beginning of the next one). break; } else if (parse_result templ = parse_template(scan)) { detail::variant_match( @@ -1221,9 +1273,12 @@ class parser { } // conditional-block → - // { cond-block-open ~ body* ~ else-block? ~ cond-block-close } + // { cond-block-open ~ body* ~ else-if-block* ~ else-block? ~ + // cond-block-close } // cond-block-open → // { "{{" ~ "#" ~ "if" ~ expression ~ "}}" } + // else-if-block → + // { "{{" ~ "#" ~ "else" ~ " " ~ "if" ~ expression ~ "}}" ~ body* } // else-block → { "{{" ~ "#" ~ "else" ~ "}}" ~ body* } // cond-block-close → // { "{{" ~ "/" ~ "if" ~ expression ~ "}}" } @@ -1255,6 +1310,11 @@ class parser { ast::bodies bodies = parse_bodies(scan).consume_and_advance(&scan); + std::vector else_if_blocks; + while (parse_result else_if = parse_else_if_clause(scan)) { + else_if_blocks.push_back(std::move(else_if).consume_and_advance(&scan)); + } + auto else_block = std::invoke([&]() -> std::optional { const auto else_scan_start = scan.start; @@ -1306,6 +1366,7 @@ class parser { scan.with_start(scan_start).range(), std::move(open), std::move(bodies), + std::move(else_if_blocks), std::move(else_block), }, scan}; diff --git a/thrift/compiler/whisker/print_ast.cc b/thrift/compiler/whisker/print_ast.cc index 9f57d03c5c0..1b70fb09720 100644 --- a/thrift/compiler/whisker/print_ast.cc +++ b/thrift/compiler/whisker/print_ast.cc @@ -81,6 +81,12 @@ struct ast_visitor { visit(conditional_block.condition, scope.open_property()); visit(conditional_block.body_elements, scope.open_node()); + for (const auto& else_if_clause : conditional_block.else_if_clauses) { + auto else_if_scope = scope.open_property(); + else_if_scope.println(" else-if-block {}", location(else_if_clause.loc)); + visit(else_if_clause.body_elements, else_if_scope.open_node()); + } + if (auto else_clause = conditional_block.else_clause) { auto else_scope = scope.open_property(); else_scope.println(" else-block {}", location(else_clause->loc)); diff --git a/thrift/compiler/whisker/render.cc b/thrift/compiler/whisker/render.cc index 266b4f29792..111da02b26b 100644 --- a/thrift/compiler/whisker/render.cc +++ b/thrift/compiler/whisker/render.cc @@ -584,9 +584,22 @@ class render_engine { eval_context_.pop_scope(); }; + // Returns whether the else clause should be evaluated. + auto visit_else_if = [&](const ast::conditional_block& b) { + for (const auto& clause : b.else_if_clauses) { + if (evaluate_as_bool(clause.condition)) { + do_visit(clause.body_elements); + return true; + } + } + return false; + }; + const bool condition = evaluate_as_bool(conditional_block.condition); if (condition) { do_visit(conditional_block.body_elements); + } else if (visit_else_if(conditional_block)) { + // An else if clause was rendered. } else if (conditional_block.else_clause.has_value()) { do_visit(conditional_block.else_clause->body_elements); } diff --git a/thrift/compiler/whisker/test/parser_test.cc b/thrift/compiler/whisker/test/parser_test.cc index 73f818732c2..b9fa664ac87 100644 --- a/thrift/compiler/whisker/test/parser_test.cc +++ b/thrift/compiler/whisker/test/parser_test.cc @@ -472,6 +472,50 @@ TEST_F(ParserTest, conditional_block_mismatched_lookup) { 5))); } +TEST_F(ParserTest, conditional_block_else_if) { + auto ast = parse_ast( + "{{#if news.has-update?}}\n" + " New stuff is happening!\n" + "{{#else if news.is-important?}}\n" + " Important stuff is happening!\n" + "{{#else}}\n" + " Nothing is happening!\n" + "{{/if news.has-update?}}"); + EXPECT_THAT(diagnostics, testing::IsEmpty()); + EXPECT_EQ( + to_string(*ast), + "root [path/to/test-1.whisker]\n" + "|- if-block \n" + "| `- expression 'news.has-update?'\n" + "| |- text ' New stuff is happening!'\n" + "| |- newline '\\n'\n" + "| `- else-if-block \n" + "| |- text ' Important stuff is happening!'\n" + "| |- newline '\\n'\n" + "| `- else-block \n" + "| |- text ' Nothing is happening!'\n" + "| |- newline '\\n'\n"); +} + +TEST_F(ParserTest, conditional_block_else_if_after_else) { + auto ast = parse_ast( + "{{#if news.has-update?}}\n" + " New stuff is happening!\n" + "{{#else}}\n" + " Nothing is happening!\n" + "{{#else if news.is-important?}}\n" + " Important stuff is happening!\n" + "{{/if news.has-update?}}"); + EXPECT_FALSE(ast.has_value()); + EXPECT_THAT( + diagnostics, + testing::ElementsAre(diagnostic( + diagnostic_level::error, + "expected `/` to close if-block 'news.has-update?' but found `#`", + path_to_file(1), + 5))); +} + TEST_F(ParserTest, conditional_block_with_not) { auto ast = parse_ast( "{{#if (not news.has-update?)}}\n" diff --git a/thrift/compiler/whisker/test/render_test.cc b/thrift/compiler/whisker/test/render_test.cc index 3a7e3cc5b6c..6ba9e2bfc3a 100644 --- a/thrift/compiler/whisker/test/render_test.cc +++ b/thrift/compiler/whisker/test/render_test.cc @@ -55,6 +55,9 @@ class double_property_name mutable std::map> cached_; }; +const auto T = w::boolean(true); +const auto F = w::boolean(false); + } // namespace TEST_F(RenderTest, basic) { @@ -576,6 +579,36 @@ TEST_F(RenderTest, if_else_block) { } } +TEST_F(RenderTest, else_if_blocks) { + const std::string template_text = + "{{#if a}}\n" + "a\n" + "{{#else if b}}\n" + "b\n" + "{{#else if c}}\n" + "c\n" + "{{#else}}\n" + "d\n" + "{{/if a}}\n"; + + { + auto result = render(template_text, w::map({{"a", T}})); + EXPECT_EQ(*result, "a\n"); + } + { + auto result = render(template_text, w::map({{"a", F}, {"b", T}})); + EXPECT_EQ(*result, "b\n"); + } + { + auto result = render(template_text, w::map({{"a", F}, {"b", F}, {"c", T}})); + EXPECT_EQ(*result, "c\n"); + } + { + auto result = render(template_text, w::map({{"a", F}, {"b", F}, {"c", F}})); + EXPECT_EQ(*result, "d\n"); + } +} + TEST_F(RenderTest, unless_else_block) { { auto result = render( diff --git a/thrift/doc/contributions/whisker.md b/thrift/doc/contributions/whisker.md index 9263ff3ce75..eb70c317192 100644 --- a/thrift/doc/contributions/whisker.md +++ b/thrift/doc/contributions/whisker.md @@ -278,14 +278,16 @@ Whisker supports a conditionally rendering block type: `{{#if}}`. A typical cond ```handlebars {{#if person.hasName}} Greetings, {{person.name}}! +{{#else if person.hasId}} +Beep boop, {{person.id}}! {{#else}} I don't know who you are. {{/if person.hasName}} ``` -`{{#if}}` blocks can **optionally** include one `{{#else}}` statement. When omitted, the behavior matches an `{{#else}}` with an empty body. +`{{#if}}` blocks **may** include any number of `{{# else if ...}}` statements followed by one optional `{{#else}}` statement. The body of the first condition to obtain will be rendered, otherwise the body following the `{{#else}}` statement if provided, otherwise an empty block. -In this example, `person.hasName` is the *condition*. The condition **must** be an `expression` that evaluates to a `boolean`. If its value is `true`, then the body before the `{{#else}}` is rendered. Otherwise, the body after the `{{#else}}` is rendered. +In this example, `person.hasName` is the *condition*. The condition **must** be an `expression` that evaluates to a `boolean`. The closing tag must exactly replicate the `expression` of the matching opening tag. This serves to improve readability of complex nested conditions. @@ -325,8 +327,9 @@ I don't know who you are. ``` -if-block → { if-block-open ~ body* ~ else-block? ~ if-block-close } +if-block → { if-block-open ~ body* ~ else-if-block* ~ else-block? ~ if-block-close } if-block-open → { "{{" ~ "#" ~ "if" ~ expression ~ "}}" } +else-if-block → { "{{" ~ "#" ~ "else" ~ "if" ~ expression ~ "}}" ~ body* } else-block → { "{{" ~ "#" ~ "else" ~ "}}" ~ body* } if-block-close → { "{{" ~ "/" ~ "if" ~ expression ~ "}}" } ```