Skip to content

Commit

Permalink
Implement {{#unless}}
Browse files Browse the repository at this point in the history 
Summary:
This is the counter-part to `{{#if}}` blocks from Handlebars:
* https://handlebarsjs.com/guide/builtin-helpers.html#unless

Until we get support for expressions this is handy alternative to `{{#if (not ...)}}`

Reviewed By: vitaut

Differential Revision: D62472450

fbshipit-source-id: 77675aebe2194fa76536280c0be096a4b68ce52b
  • Loading branch information
@praihan @facebook-github-bot
praihan authored and facebook-github-bot committed Sep 12, 2024
1 parent a3103a5 commit fbe05df
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 39 deletions.
20 changes: 14 additions & 6 deletions thrift/compiler/whisker/ast.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ struct text;
struct newline;
struct comment;
struct section_block;
struct if_block;
struct conditional_block;
struct partial_apply;
struct variable;

Expand All @@ -42,7 +42,7 @@ using body = std::variant<
comment,
variable,
section_block,
if_block,
conditional_block,
partial_apply>;
using bodies = std::vector<body>;

Expand Down Expand Up @@ -126,20 +126,28 @@ struct variable {
struct section_block {
source_range loc;
/**
* {{# ⇒ inverted == true
* {{^ ⇒ inverted == false
* {{# ⇒ inverted == false
* {{^ ⇒ inverted == true
*/
bool inverted;
variable_lookup variable;
bodies body_elements;
};

/**
* A Whisker construct for conditionals. This matches Handlebars:
* A Whisker construct for conditionals. A conditional_block represents one of
* two (very similar) block types: if-block and unless-block.
* This matches Handlebars:
* https://handlebarsjs.com/guide/builtin-helpers.html#if
* https://handlebarsjs.com/guide/builtin-helpers.html#unless
*/
struct if_block {
struct conditional_block {
source_range loc;
/**
* {{#if ⇒ unless == false
* {{#unless ⇒ unless == true
*/
bool unless;
variable_lookup variable;
bodies body_elements;

Expand Down
55 changes: 38 additions & 17 deletions thrift/compiler/whisker/parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ class parser {
using template_body = std::variant<
ast::variable,
ast::section_block,
ast::if_block,
ast::conditional_block,
ast::partial_apply>;
// template → { variable | section-block | partial-apply }
parse_result<template_body> parse_template(parser_scan_window scan) {
Expand All @@ -789,8 +789,8 @@ class parser {
std::optional<template_body> templ;
if (parse_result variable = parse_variable(scan)) {
templ = std::move(variable).consume_and_advance(&scan);
} else if (parse_result if_block = parse_if_block(scan)) {
templ = std::move(if_block).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 section_block = parse_section_block(scan)) {
templ = std::move(section_block).consume_and_advance(&scan);
} else if (parse_result partial_apply = parse_partial_apply(scan)) {
Expand Down Expand Up @@ -973,35 +973,52 @@ class parser {
scan};
}

// if-block → { if-block-open ~ body* ~ else-block? ~ if-block-close }
// if-block-open → { "{{" ~ "#" ~ "if" ~ variable-lookup ~ "}}" }
// conditional-block →
// { cond-block-open ~ body* ~ else-block? ~ cond-block-close }
// cond-block-open →
// { "{{" ~ "#" ~ ("if" | "unless") ~ variable-lookup ~ "}}" }
// else-block → { "{{" ~ "else" ~ "}}" ~ body* }
// if-block-close → { "{{" ~ "/" ~ "if" ~ "}}" }
parse_result<ast::if_block> parse_if_block(parser_scan_window scan) {
// cond-block-close → { "{{" ~ "/" ~ ("if" | "unless") ~ "}}" }
//
// NOTE: the "if" or "unless" must match between open and close
parse_result<ast::conditional_block> parse_conditional_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_if))) {
try_consume_token(&scan, tok::pound))) {
return no_parse_result();
}
bool inverted;
if (try_consume_token(&scan, tok::kw_if)) {
inverted = false;
} else if (try_consume_token(&scan, tok::kw_unless)) {
inverted = true;
} else {
return no_parse_result();
}
scan = scan.make_fresh();

const std::string_view block_name = inverted ? "unless" : "if";

parse_result lookup = parse_variable_lookup(scan);
if (!lookup.has_value()) {
report_expected(scan, "variable-lookup to open if-block");
report_expected(
scan, fmt::format("variable-lookup to open {}-block", block_name));
}
ast::variable_lookup open = std::move(lookup).consume_and_advance(&scan);
if (!try_consume_token(&scan, tok::close)) {
report_expected(scan, fmt::format("{} to open if-block", tok::close));
report_expected(
scan, fmt::format("{} to open {}-block", tok::close, block_name));
}
scan = scan.make_fresh();

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

auto else_ = std::invoke(
[this, scan]() mutable -> parse_result<ast::if_block::else_block> {
[this,
scan]() mutable -> parse_result<ast::conditional_block::else_block> {
const auto else_scan_start = scan.start;
if (parse_result e = parse_else_clause(scan)) {
std::ignore = std::move(e).consume_and_advance(&scan);
Expand All @@ -1011,13 +1028,13 @@ class parser {
scan = scan.make_fresh();
auto else_bodies = parse_bodies(scan).consume_and_advance(&scan);
return {
ast::if_block::else_block{
ast::conditional_block::else_block{
scan.with_start(else_scan_start).range(),
std::move(else_bodies)},
scan};
});
auto else_block =
std::invoke([&]() -> std::optional<ast::if_block::else_block> {
std::invoke([&]() -> std::optional<ast::conditional_block::else_block> {
if (else_.has_value()) {
return std::move(else_).consume_and_advance(&scan);
}
Expand All @@ -1029,17 +1046,21 @@ class parser {
report_expected(
scan,
fmt::format(
"{} to close if-block '{}'", kind, open.chain_string()));
"{} to close {}-block '{}'",
kind,
block_name,
open.chain_string()));
}
};
expect_on_close(tok::open);
expect_on_close(tok::slash);
expect_on_close(tok::kw_if);
expect_on_close(inverted ? tok::kw_unless : tok::kw_if);
expect_on_close(tok::close);

return {
ast::if_block{
ast::conditional_block{
scan.with_start(scan_start).range(),
inverted,
std::move(open),
std::move(bodies),
std::move(else_block),
Expand Down
15 changes: 10 additions & 5 deletions thrift/compiler/whisker/print_ast.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,17 @@ struct ast_visitor {
visit(body, scope.open_node());
}
}
void visit(const ast::if_block& if_block, tree_printer::scope scope) const {
scope.println(" if-block {}", location(if_block.loc));
visit(if_block.variable, scope.open_property());
visit(if_block.body_elements, scope.open_node());
void visit(
const ast::conditional_block& conditional_block,
tree_printer::scope scope) const {
scope.println(
" {}-block {}",
conditional_block.unless ? "unless" : "if",
location(conditional_block.loc));
visit(conditional_block.variable, scope.open_property());
visit(conditional_block.body_elements, scope.open_node());

if (auto else_clause = if_block.else_clause) {
if (auto else_clause = conditional_block.else_clause) {
auto else_scope = scope.open_property();
else_scope.println(" else-block {}", location(else_clause->loc));
visit(else_clause->body_elements, else_scope.open_node());
Expand Down
15 changes: 8 additions & 7 deletions thrift/compiler/whisker/render.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,11 @@ class render_engine {
});
}

void visit(const ast::if_block& if_block) {
const object& condition = lookup_variable(if_block.variable);
void visit(const ast::conditional_block& conditional_block) {
const object& condition = lookup_variable(conditional_block.variable);

const auto maybe_report_coercion = [&] {
maybe_report_boolean_coercion(if_block.variable, condition);
maybe_report_boolean_coercion(conditional_block.variable, condition);
};

const auto do_visit = [&](const object& scope,
Expand All @@ -489,10 +489,11 @@ class render_engine {
};

const auto do_conditional_visit = [&](bool condition) {
if (condition) {
do_visit(whisker::make::null, if_block.body_elements);
} else if (if_block.else_clause.has_value()) {
do_visit(whisker::make::null, if_block.else_clause->body_elements);
if (condition ^ conditional_block.unless) {
do_visit(whisker::make::null, conditional_block.body_elements);
} else if (conditional_block.else_clause.has_value()) {
do_visit(
whisker::make::null, conditional_block.else_clause->body_elements);
}
};

Expand Down
25 changes: 21 additions & 4 deletions thrift/compiler/whisker/test/parser_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,21 +311,21 @@ TEST_F(ParserTest, basic_if_else) {
"| |- newline <line:4:24, line:5:1> '\\n'\n");
}

TEST_F(ParserTest, if_block_repeated_else) {
TEST_F(ParserTest, unless_block_repeated_else) {
auto ast = parse_ast(
"{{#if news.has-update?}}\n"
"{{#unless news.has-update?}}\n"
" Stuff is {{foo}} happening!\n"
"{{else}}\n"
" Nothing is happening!\n"
"{{else}}\n"
" Nothing is happening!\n"
"{{/if}}");
"{{/unless}}");
EXPECT_FALSE(ast.has_value());
EXPECT_THAT(
diagnostics,
testing::ElementsAre(diagnostic(
diagnostic_level::error,
"expected `/` to close if-block 'news.has-update?' but found `else`",
"expected `/` to close unless-block 'news.has-update?' but found `else`",
path_to_file(1),
5)));
}
Expand Down Expand Up @@ -421,6 +421,23 @@ TEST_F(ParserTest, if_close_by_itself) {
1)));
}

TEST_F(ParserTest, conditional_block_mismatched_open_and_close) {
auto ast = parse_ast(
"{{#unless news.has-update?}}\n"
" Stuff is happening!\n"
"{{else}}\n"
" Nothing is happening!\n"
"{{/if}}");
EXPECT_FALSE(ast.has_value());
EXPECT_THAT(
diagnostics,
testing::ElementsAre(diagnostic(
diagnostic_level::error,
"expected `unless` to close unless-block 'news.has-update?' but found `if`",
path_to_file(1),
5)));
}

TEST_F(ParserTest, basic_partial_apply) {
auto ast = parse_ast("{{> path / to / file }}");
EXPECT_EQ(
Expand Down
44 changes: 44 additions & 0 deletions thrift/compiler/whisker/test/render_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,25 @@ TEST_F(RenderTest, if_block) {
}
}

TEST_F(RenderTest, unless_block) {
{
auto result = render(
"{{#unless news.has-update?}}\n"
"Stuff is {{foo}} happening!\n"
"{{/unless}}\n",
w::map(
{{"news", w::map({{"has-update?", w::boolean(false)}})},
{"foo", w::string("now")}}));
EXPECT_EQ(*result, "Stuff is now happening!\n");
}
{
auto result = render(
"{{#unless news.has-update?}}Stuff is {{foo}} happening!{{/unless}}",
w::map({{"news", w::map({{"has-update?", w::boolean(true)}})}}));
EXPECT_EQ(*result, "");
}
}

TEST_F(RenderTest, if_else_block) {
{
auto result = render(
Expand All @@ -497,6 +516,31 @@ TEST_F(RenderTest, if_else_block) {
}
}

TEST_F(RenderTest, unless_else_block) {
{
auto result = render(
"{{#unless news.has-update?}}\n"
"Nothing is happening!\n"
"{{else}}\n"
"Stuff is {{foo}} happening!\n"
"{{/unless}}\n",
w::map({{"news", w::map({{"has-update?", w::boolean(false)}})}}));
EXPECT_EQ(*result, "Nothing is happening!\n");
}
{
auto result = render(
"{{#unless news.has-update?}}\n"
"Nothing is happening!\n"
"{{else}}\n"
"Stuff is {{foo}} happening!\n"
"{{/unless}}\n",
w::map(
{{"news", w::map({{"has-update?", w::boolean(true)}})},
{"foo", w::string("now")}}));
EXPECT_EQ(*result, "Stuff is now happening!\n");
}
}

TEST_F(RenderTest, printable_types_strict_failure) {
{
auto result = render(
Expand Down

0 comments on commit fbe05df

Please sign in to comment.