diff --git a/third-party/thrift/src/thrift/compiler/whisker/ast.h b/third-party/thrift/src/thrift/compiler/whisker/ast.h index fb193f54b75094..830b04188bf002 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/ast.h +++ b/third-party/thrift/src/thrift/compiler/whisker/ast.h @@ -152,19 +152,13 @@ struct section_block { }; /** - * A Whisker construct for conditionals. A conditional_block represents one of - * two (very similar) block types: if-block and unless-block. + * A Whisker construct for conditionals, i.e. the if-block. * This matches Handlebars: * https://handlebarsjs.com/guide/builtin-helpers.html#if - * https://handlebarsjs.com/guide/builtin-helpers.html#unless */ struct conditional_block { source_range loc; - /** - * {{#if ⇒ unless == false - * {{#unless ⇒ unless == true - */ - bool unless; + expression condition; bodies body_elements; diff --git a/third-party/thrift/src/thrift/compiler/whisker/parser.cc b/third-party/thrift/src/thrift/compiler/whisker/parser.cc index 5b99b41ec52e3b..7a3a0d1fb4bc79 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/parser.cc +++ b/third-party/thrift/src/thrift/compiler/whisker/parser.cc @@ -1016,13 +1016,12 @@ class parser { // conditional-block → // { cond-block-open ~ body* ~ else-block? ~ cond-block-close } // cond-block-open → - // { "{{" ~ "#" ~ ("if" | "unless") ~ expression ~ "}}" } + // { "{{" ~ "#" ~ "if" ~ expression ~ "}}" } // else-block → { "{{" ~ "#" ~ "else" ~ "}}" ~ body* } // cond-block-close → - // { "{{" ~ "/" ~ ("if" | "unless") ~ expression ~ "}}" } + // { "{{" ~ "/" ~ "if" ~ expression ~ "}}" } // - // NOTE: the "if" or "unless" and the expression must match between - // open and close + // NOTE: the expression must match between open and close parse_result parse_conditional_block( parser_scan_window scan) { assert(scan.empty()); @@ -1032,27 +1031,18 @@ class parser { 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 { + if (!try_consume_token(&scan, tok::kw_if)) { return no_parse_result(); } scan = scan.make_fresh(); - const std::string_view block_name = inverted ? "unless" : "if"; - parse_result condition = parse_expression(scan); if (!condition.has_value()) { - report_expected( - scan, fmt::format("expression to open {}-block", block_name)); + report_expected(scan, fmt::format("expression to open if-block")); } ast::expression open = std::move(condition).consume_and_advance(&scan); if (!try_consume_token(&scan, tok::close)) { - report_expected( - scan, fmt::format("{} to open {}-block", tok::close, block_name)); + report_expected(scan, fmt::format("{} to open if-block", tok::close)); } scan = scan.make_fresh(); @@ -1080,25 +1070,18 @@ class parser { if (!try_consume_token(&scan, kind)) { report_expected( scan, - fmt::format( - "{} to close {}-block '{}'", - kind, - block_name, - open.to_string())); + fmt::format("{} to close if-block '{}'", kind, open.to_string())); } }; expect_on_close(tok::open); expect_on_close(tok::slash); - expect_on_close(inverted ? tok::kw_unless : tok::kw_if); + expect_on_close(tok::kw_if); condition = parse_expression(scan.make_fresh()); if (!condition.has_value()) { report_expected( scan, - fmt::format( - "expression to close {}-block '{}'", - block_name, - open.to_string())); + fmt::format("expression to close if-block '{}'", open.to_string())); } ast::expression close = {std::move(condition).consume_and_advance(&scan)}; if (close.to_string() != open.to_string()) { @@ -1114,7 +1097,6 @@ class parser { return { ast::conditional_block{ scan.with_start(scan_start).range(), - inverted, std::move(open), std::move(bodies), std::move(else_block), 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 33bfda69ee456b..9789f33ee50043 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/print_ast.cc +++ b/third-party/thrift/src/thrift/compiler/whisker/print_ast.cc @@ -80,10 +80,7 @@ struct ast_visitor { 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)); + scope.println(" if-block {}", location(conditional_block.loc)); visit(conditional_block.condition, scope.open_property()); visit(conditional_block.body_elements, scope.open_node()); diff --git a/third-party/thrift/src/thrift/compiler/whisker/render.cc b/third-party/thrift/src/thrift/compiler/whisker/render.cc index 41ef178aa8ca9e..9ad043077782df 100644 --- a/third-party/thrift/src/thrift/compiler/whisker/render.cc +++ b/third-party/thrift/src/thrift/compiler/whisker/render.cc @@ -524,7 +524,7 @@ class render_engine { }; const bool condition = evaluate_as_bool(conditional_block.condition); - if (condition ^ conditional_block.unless) { + if (condition) { do_visit(conditional_block.body_elements); } else if (conditional_block.else_clause.has_value()) { do_visit(conditional_block.else_clause->body_elements); 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 ff9f72d7c96b5c..67096957caac20 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 @@ -313,19 +313,19 @@ TEST_F(ParserTest, basic_if_else) { TEST_F(ParserTest, unless_block_repeated_else) { auto ast = parse_ast( - "{{#unless news.has-update?}}\n" + "{{#if (not news.has-update?)}}\n" " Stuff is {{foo}} happening!\n" "{{#else}}\n" " Nothing is happening!\n" "{{#else}}\n" " Nothing is happening!\n" - "{{/unless news.has-update?}}"); + "{{/if (not news.has-update?)}}"); EXPECT_FALSE(ast.has_value()); EXPECT_THAT( diagnostics, testing::ElementsAre(diagnostic( diagnostic_level::error, - "expected `/` to close unless-block 'news.has-update?' but found `#`", + "expected `/` to close if-block '(not news.has-update?)' but found `#`", path_to_file(1), 5))); } @@ -423,17 +423,17 @@ TEST_F(ParserTest, if_close_by_itself) { TEST_F(ParserTest, conditional_block_mismatched_open_and_close) { auto ast = parse_ast( - "{{#unless news.has-update?}}\n" + "{{#if (not news.has-update?)}}\n" " Stuff 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 `unless` to close unless-block 'news.has-update?' but found `if`", + "expected `if` to close if-block '(not news.has-update?)' but found `unless`", path_to_file(1), 5))); } 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 c70d6c38fed13e..8bd47296d91f85 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 @@ -507,9 +507,9 @@ TEST_F(RenderTest, if_block) { TEST_F(RenderTest, unless_block) { { auto result = render( - "{{#unless news.has-update?}}\n" + "{{#if (not news.has-update?)}}\n" "Stuff is {{foo}} happening!\n" - "{{/unless news.has-update?}}\n", + "{{/if (not news.has-update?)}}\n", w::map( {{"news", w::map({{"has-update?", w::boolean(false)}})}, {"foo", w::string("now")}})); @@ -517,7 +517,7 @@ TEST_F(RenderTest, unless_block) { } { auto result = render( - "{{#unless news.has-update?}}Stuff is {{foo}} happening!{{/unless news.has-update?}}", + "{{#if (not news.has-update?)}}Stuff is {{foo}} happening!{{/if (not news.has-update?)}}", w::map({{"news", w::map({{"has-update?", w::boolean(true)}})}})); EXPECT_EQ(*result, ""); } @@ -551,21 +551,21 @@ TEST_F(RenderTest, if_else_block) { TEST_F(RenderTest, unless_else_block) { { auto result = render( - "{{#unless news.has-update?}}\n" + "{{#if (not news.has-update?)}}\n" "Nothing is happening!\n" "{{#else}}\n" "Stuff is {{foo}} happening!\n" - "{{/unless news.has-update?}}\n", + "{{/if (not news.has-update?)}}\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" + "{{#if (not news.has-update?)}}\n" "Nothing is happening!\n" "{{#else}}\n" "Stuff is {{foo}} happening!\n" - "{{/unless news.has-update?}}\n", + "{{/if (not news.has-update?)}}\n", w::map( {{"news", w::map({{"has-update?", w::boolean(true)}})}, {"foo", w::string("now")}})); diff --git a/third-party/thrift/src/thrift/doc/contributions/whisker.md b/third-party/thrift/src/thrift/doc/contributions/whisker.md index c7386e0f57c385..bbd48b228e66ed 100644 --- a/third-party/thrift/src/thrift/doc/contributions/whisker.md +++ b/third-party/thrift/src/thrift/doc/contributions/whisker.md @@ -284,26 +284,6 @@ I don't know who you are. -:::note -`{{#unless}}` blocks are functionally equivalent to `{{#if}}` with a negated condition. However, they are **deprecated** as their behavior can be replicated using `{{#if}}` with the `not` function. -::: - -Example of unless being redundant}> - -```handlebars title=example.whisker -{{! These two blocks have identical behavior }} - -{{#unless failed?}} -Nice! -{{/unless failed?}} - -{{#if (not failed?)}} -Nice! -{{/if (not failed?)}} -``` - - - ```