#265 Warn on redundant semicolon after else

docs/errors/E204.md

@@ -0,0 +1,30 @@
+# E204: semicolon after else may be causing unexpected behavior.
+
+A semicolon next to the `else` keyword may cause its body to be executed even when previous `if` statement condition is true.
+
+    if (true) {
+        console.log("true");
+    } else; {
+        console.log("else");
+    }
+
+Example output
+```
+> true
+> else
+```
+
+To avoid this behavior, remove the semicolon between the else keyword and the opening brace of the else block.
+
+    if (true) {
+        console.log("true");
+    } else {
+        console.log("else");
+    }
+
+Or if the else body is not required remove it completely.
+
+    if (true) {
+        console.log("true");
+    }
+    console.log("always");

src/quick-lint-js/error.h

@@ -209,6 +209,12 @@
           QLJS_TRANSLATABLE("commas are not allowed after spread parameter"),  \
           comma))                                                              \
                                                                                \
+  QLJS_ERROR_TYPE(                                                             \
+      error_else_has_empty_body, "E205", { source_code_span where; },          \
+      .warning(QLJS_TRANSLATABLE("else has empty body; consider removing the " \
+                                 "redundant 'else'"),                          \
+               where))                                                         \
+                                                                               \
   QLJS_ERROR_TYPE(                                                             \
       error_else_has_no_if, "E065", { source_code_span else_token; },          \
       .error(QLJS_TRANSLATABLE("'else' has no corresponding 'if'"),            \
@@ -1048,6 +1054,13 @@
           QLJS_TRANSLATABLE("for-of loop expression cannot have semicolons"),  \
           semicolon))                                                          \
                                                                                \
+  QLJS_ERROR_TYPE(                                                             \
+      error_unexpected_semicolon_after_else, "E204",                           \
+      { 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; },                                        \

src/quick-lint-js/parse.h

@@ -2516,7 +2516,20 @@ class parser {
 
     if (this->peek().type == token_type::kw_else) {
       this->skip();
+
+      const token token_after_else = this->peek();
       parse_and_visit_body();
+
+      if (token_after_else.type == token_type::semicolon &&
+          this->peek().type == token_type::left_curly) {
+        if (this->peek().has_leading_newline) {
+          this->error_reporter_->report(
+              error_else_has_empty_body{.where = this->peek().span()});
+        } else {
+          this->error_reporter_->report(error_unexpected_semicolon_after_else{
+              .semicolon = token_after_else.span()});
+        }
+      }
     }
   }
 

test/test-parse-statement.cpp

@@ -578,6 +578,44 @@ TEST(test_parse, else_without_if) {
   }
 }
 
+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"));  // (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";"))));
+  }
+}
+
+TEST(test_parse, else_has_empty_body) {
+  {
+    spy_visitor v;
+    padded_string code(u8"if (cond) { body; } else;\n{ }"_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"));  // (if)
+    EXPECT_THAT(
+        v.errors,
+        ElementsAre(ERROR_TYPE_FIELD(
+            error_else_has_empty_body, where,
+            offsets_matcher(&code, strlen(u8"if (cond) { body; } else {"),
+                            u8"}"))));
+  }
+}
+
 TEST(test_parse, block_statement) {
   {
     spy_visitor v = parse_and_visit_statement(u8"{ }"_sv);