Warn if semicolon between else and opening brace and no newline.

quick-lint · May 16, 2021 · 82917f4 · 82917f4
1 parent 2ea8012
commit 82917f4
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 20 deletions.
diff --git a/docs/errors/E220.md b/docs/errors/E220.md
@@ -0,0 +1,13 @@
+# E220: Semicolon after else may cause unexpected execution of its body.
+
+A semicolon next to the `else` keyword can cause its body to be executed even when previous `if` statement condition is true.
+
+    if (true) {
+        console.log("true");
+    } else; {
+        console.log("& false");
+    }
+    > true
+    > & false
+
+To avoid this behavior, remove the semicolon between the else keyword and the opening brace of the else block.
diff --git a/src/quick-lint-js/error.h b/src/quick-lint-js/error.h
@@ -1028,6 +1028,13 @@
           QLJS_TRANSLATABLE("for-of loop expression cannot have semicolons"),  \
           semicolon))                                                          \
                                                                                \
+  QLJS_ERROR_TYPE(                                                             \
+      error_unexpected_semicolon_after_else, "E202",                           \
+      { source_code_span semicolon; },                                         \
+      .warning(QLJS_TRANSLATABLE("semicolon after else may be causing "        \
+                                 "unexpected behavior"),                       \
+             semicolon))                                                       \
+                                                                               \
   QLJS_ERROR_TYPE(                                                             \
       error_no_digits_in_binary_number, "E049",                                \
       { source_code_span characters; },                                        \
@@ -1139,12 +1146,6 @@
       { source_code_span continue_statement; },                                \
       .error(QLJS_TRANSLATABLE("continue can only be used inside of a loop"),  \
              continue_statement))                                              \
-                                                                               \
-  QLJS_ERROR_TYPE(                                                             \
-      error_redundant_semicolon_after_else, "E202",                            \
-      { source_code_span semicolon; },                                         \
-      .warning(QLJS_TRANSLATABLE("redundant semicolon after else"),            \
-              semicolon))                                                      \
   /* END */
 
 namespace quick_lint_js {

diff --git a/src/quick-lint-js/parse.h b/src/quick-lint-js/parse.h
@@ -2516,14 +2516,18 @@ class parser {
 
     if (this->peek().type == token_type::kw_else) {
       this->skip();
-      if (this->peek().type == token_type::semicolon) {
-        source_code_span semicolon = this->peek().span();
-        this->error_reporter_->report(error_redundant_semicolon_after_else{
-          .semicolon = source_code_span(
-            semicolon.begin(), semicolon.end()),
+
+      bool semicolon_after_else = this->peek().type == token_type::semicolon;
+      const char8 *token_after_else_begin = this->peek().begin;
+
+      parse_and_visit_body();
+      if (semicolon_after_else &&
+          this->peek().type == token_type::left_curly &&
+          !this->peek().has_leading_newline) {
+        this->error_reporter_->report(error_unexpected_semicolon_after_else{
+            .semicolon = source_code_span(token_after_else_begin, token_after_else_begin+1)
         });
       }
-      parse_and_visit_body();
     }
   }
 

diff --git a/test/test-parse-statement.cpp b/test/test-parse-statement.cpp
@@ -578,19 +578,20 @@ TEST(test_parse, else_without_if) {
   }
 }
 
-TEST(test_parse, else_redundant_semicolon) {
+TEST(test_parse, else_unexpected_semicolon) {
   {
     spy_visitor v;
     padded_string code(u8"if (cond) { body; } else; { body; }"_sv);
     parser p(&code, &v);
     EXPECT_TRUE(p.parse_and_visit_statement(v));
-    EXPECT_THAT(v.visits, ElementsAre("visit_variable_use",      // cond
-                                      "visit_enter_block_scope",  // (if)
-                                      "visit_variable_use",       // body
-                                      "visit_exit_block_scope")); // (else)
-    EXPECT_THAT(v.errors, ElementsAre(ERROR_TYPE_FIELD(
-                              error_redundant_semicolon_after_else, semicolon,
-                              offsets_matcher(&code, strlen(u8"if (cond) { body; } else"), u8";"))));
+    EXPECT_THAT(v.visits, ElementsAre("visit_variable_use",        // cond
+                                      "visit_enter_block_scope",   // (if)
+                                      "visit_variable_use",        // body
+                                      "visit_exit_block_scope"));  // (if)
+    EXPECT_THAT(v.errors,
+                ElementsAre(ERROR_TYPE_FIELD(
+                    error_unexpected_semicolon_after_else, semicolon,
+                    offsets_matcher(&code, strlen(u8"if (cond) { body; } else"), u8";"))));
   }
 }