Report error when 'if' is missing ( )

quick-lint · Jan 22, 2021 · e0525e4 · e0525e4
1 parent 79e91fb
commit e0525e4
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 4 deletions.
diff --git a/src/quick-lint-js/error-formatter.h b/src/quick-lint-js/error-formatter.h
@@ -105,6 +105,8 @@ class error_formatter {
 
   static source_code_span to_span(identifier ident) { return ident.span(); }
 
+  static string8_view to_string_view(string8_view s) { return s; }
+
   static string8_view to_string_view(const source_code_span &span) {
     return span.string_view();
   }

diff --git a/src/quick-lint-js/error.h b/src/quick-lint-js/error.h
@@ -134,6 +134,23 @@
                  "expected hexadecimal digits in Unicode escape sequence"),    \
              escape_sequence))                                                 \
                                                                                \
+  QLJS_ERROR_TYPE(                                                             \
+      error_expected_parentheses_around_if_condition,                          \
+      { source_code_span condition; },                                         \
+      .error(QLJS_TRANSLATABLE(                                                \
+                 "if statement needs parentheses around condition"),           \
+             condition))                                                       \
+                                                                               \
+  QLJS_ERROR_TYPE(                                                             \
+      error_expected_parenthesis_around_if_condition,                          \
+      {                                                                        \
+        source_code_span where;                                                \
+        char8 token;                                                           \
+      },                                                                       \
+      .error(                                                                  \
+          QLJS_TRANSLATABLE("if statement is missing '{1}' around condition"), \
+          where, string8_view(&token, 1)))                                     \
+                                                                               \
   QLJS_ERROR_TYPE(                                                             \
       error_invalid_binding_in_let_statement, { source_code_span where; },     \
       .error(QLJS_TRANSLATABLE("invalid binding in let statement"), where))    \

diff --git a/src/quick-lint-js/parse.h b/src/quick-lint-js/parse.h
@@ -1262,13 +1262,41 @@ class parser {
     QLJS_ASSERT(this->peek().type == token_type::kw_if);
     this->skip();
 
-    QLJS_PARSER_UNIMPLEMENTED_IF_NOT_TOKEN(token_type::left_paren);
-    this->skip();
+    bool have_condition_left_paren =
+        this->peek().type == token_type::left_paren;
+    if (have_condition_left_paren) {
+      this->skip();
+    }
+    const char8 *condition_begin = this->peek().begin;
 
     this->parse_and_visit_expression(v);
 
-    QLJS_PARSER_UNIMPLEMENTED_IF_NOT_TOKEN(token_type::right_paren);
-    this->skip();
+    const char8 *condition_end =
+        this->peek()
+            .begin;  // TODO(#139): Use the end of the previous token instead.
+    bool have_condition_right_paren =
+        this->peek().type == token_type::right_paren;
+    if (have_condition_right_paren) {
+      this->skip();
+    }
+
+    if (!have_condition_left_paren && !have_condition_right_paren) {
+      this->error_reporter_->report(
+          error_expected_parentheses_around_if_condition{
+              .condition = source_code_span(condition_begin, condition_end)});
+    } else if (!have_condition_right_paren) {
+      this->error_reporter_->report(
+          error_expected_parenthesis_around_if_condition{
+              .where = source_code_span(condition_end, condition_end),
+              .token = ')',
+          });
+    } else if (!have_condition_left_paren) {
+      this->error_reporter_->report(
+          error_expected_parenthesis_around_if_condition{
+              .where = source_code_span(condition_begin, condition_begin),
+              .token = '(',
+          });
+    }
 
     this->parse_and_visit_statement(v);
 

diff --git a/test/test-parse.cpp b/test/test-parse.cpp
@@ -2325,6 +2325,62 @@ TEST(test_parse, if_with_else) {
   }
 }
 
+TEST(test_parse, if_without_parens) {
+  {
+    spy_visitor v;
+    padded_string code(u8"if cond { body; }"_sv);
+    parser p(&code, &v);
+    p.parse_and_visit_statement(v);
+    EXPECT_THAT(v.visits, ElementsAre("visit_variable_use",       // cond
+                                      "visit_enter_block_scope",  //
+                                      "visit_variable_use",       // body
+                                      "visit_exit_block_scope"));
+    EXPECT_THAT(v.errors,
+                ElementsAre(ERROR_TYPE_FIELD(
+                    error_expected_parentheses_around_if_condition, condition,
+                    offsets_matcher(&code, strlen(u8"if "),
+                                    // TODO(#139): End the error at the 'd'.
+                                    strlen(u8"if cond ")))));
+  }
+
+  {
+    spy_visitor v;
+    padded_string code(u8"if (cond { body; }"_sv);
+    parser p(&code, &v);
+    p.parse_and_visit_statement(v);
+    EXPECT_THAT(v.visits, ElementsAre("visit_variable_use",       // cond
+                                      "visit_enter_block_scope",  //
+                                      "visit_variable_use",       // body
+                                      "visit_exit_block_scope"));
+    EXPECT_THAT(v.errors,
+                ElementsAre(ERROR_TYPE_2_FIELDS(
+                    error_expected_parenthesis_around_if_condition,  //
+                    where,
+                    // TODO(#139): Place the error immediately after the 'd'.
+                    offsets_matcher(&code, strlen(u8"if (cond "),
+                                    strlen(u8"if (cond ")),  //
+                    token, u8')')));
+  }
+
+  {
+    spy_visitor v;
+    padded_string code(u8"if cond) { body; }"_sv);
+    parser p(&code, &v);
+    p.parse_and_visit_statement(v);
+    EXPECT_THAT(v.visits, ElementsAre("visit_variable_use",       // cond
+                                      "visit_enter_block_scope",  //
+                                      "visit_variable_use",       // body
+                                      "visit_exit_block_scope"));
+    EXPECT_THAT(v.errors,
+                ElementsAre(ERROR_TYPE_2_FIELDS(
+                    error_expected_parenthesis_around_if_condition,  //
+                    where,
+                    offsets_matcher(&code, strlen(u8"if "),
+                                    strlen(u8"if ")),  //
+                    token, u8'(')));
+  }
+}
+
 TEST(test_parse, do_while) {
   {
     spy_visitor v = parse_and_visit_statement(u8"do { a; } while (b)"_sv);

diff --git a/test/test-text-error-reporter.cpp b/test/test-text-error-reporter.cpp
@@ -137,6 +137,19 @@ TEST_F(test_text_error_reporter, big_int_literal_contains_exponent) {
             "FILE:1:1: error: BigInt literal contains exponent\n");
 }
 
+TEST_F(test_text_error_reporter, expected_parenthesis_around_if_condition) {
+  padded_string input(u8"if cond) {}"_sv);
+  source_code_span parenthesis_span(&input[4 - 1], &input[4 - 1]);
+
+  this->make_reporter(&input).report(
+      error_expected_parenthesis_around_if_condition{
+          .where = parenthesis_span,
+          .token = '(',
+      });
+  EXPECT_EQ(this->get_output(),
+            "FILE:1:4: error: if statement is missing '(' around condition\n");
+}
+
 TEST_F(test_text_error_reporter, invalid_binding_in_let_statement) {
   padded_string input(u8"let 2 = 3;"_sv);
   source_code_span two_span(&input[5 - 1], &input[5 + 1 - 1]);