Skip to content

Commit

Permalink
Add else-if clauses
Browse files Browse the repository at this point in the history
Summary: Avoids the need to nest conditional blocks when writing more than two conditions that are mutually exhaustive.

Reviewed By: yoney

Differential Revision: D67605161

fbshipit-source-id: 88f0048c67623e2417a3b45bfd468d5623ee6551
  • Loading branch information
iahs authored and facebook-github-bot committed Dec 26, 2024
1 parent e4d8c5e commit 5fb2241
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 7 deletions.
8 changes: 8 additions & 0 deletions thrift/compiler/whisker/ast.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_block> else_if_clauses;

// The {{#else}} clause, if present.
struct else_block {
source_range loc;
Expand Down
69 changes: 65 additions & 4 deletions thrift/compiler/whisker/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<ast::conditional_block::else_if_block> 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" ~ "}}" }
Expand All @@ -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<ast::body> if body was found but consisted
Expand All @@ -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(
Expand Down Expand Up @@ -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 ~ "}}" }
Expand Down Expand Up @@ -1255,6 +1310,11 @@ class parser {

ast::bodies bodies = parse_bodies(scan).consume_and_advance(&scan);

std::vector<ast::conditional_block::else_if_block> 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<ast::conditional_block::else_block> {
const auto else_scan_start = scan.start;
Expand Down Expand Up @@ -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};
Expand Down
6 changes: 6 additions & 0 deletions thrift/compiler/whisker/print_ast.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
13 changes: 13 additions & 0 deletions thrift/compiler/whisker/render.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
44 changes: 44 additions & 0 deletions thrift/compiler/whisker/test/parser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 <line:1:1, line:7:25>\n"
"| `- expression <line:1:7, col:23> 'news.has-update?'\n"
"| |- text <line:2:1, col:26> ' New stuff is happening!'\n"
"| |- newline <line:2:26, line:3:1> '\\n'\n"
"| `- else-if-block <line:3:1, line:5:1>\n"
"| |- text <line:4:1, col:32> ' Important stuff is happening!'\n"
"| |- newline <line:4:32, line:5:1> '\\n'\n"
"| `- else-block <line:5:1, line:7:1>\n"
"| |- text <line:6:1, col:24> ' Nothing is happening!'\n"
"| |- newline <line:6:24, line:7:1> '\\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"
Expand Down
33 changes: 33 additions & 0 deletions thrift/compiler/whisker/test/render_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ class double_property_name
mutable std::map<std::string, object, std::less<>> cached_;
};

const auto T = w::boolean(true);
const auto F = w::boolean(false);

} // namespace

TEST_F(RenderTest, basic) {
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 6 additions & 3 deletions thrift/doc/contributions/whisker.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -325,8 +327,9 @@ I don't know who you are.
<Grammar>

```
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 ~ "}}" }
```
Expand Down

0 comments on commit 5fb2241

Please sign in to comment.