Back out "Revert D67605161: [whisker] Add else-if clauses"

Summary:
Original commit changeset: eac9033f917e

Original Phabricator Diff: D67605161

Forward fix landed before revert

Reviewed By: thedavekwon

Differential Revision: D67658901

fbshipit-source-id: 8f2b54f522d466d1fedad23de67ab37778a4bc30

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_block> else_if_clauses;
+
   // The {{#else}} clause, if present.
   struct else_block {
     source_range loc;

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<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, "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, "`}}` in else-if clause");
+    }
+    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<ast::body> 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<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;
@@ -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};

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));

thrift/compiler/whisker/render.cc

@@ -593,9 +593,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);
     }

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 <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"

thrift/compiler/whisker/test/render_test.cc

@@ -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) {
@@ -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(

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