Attempts to fix issue #266

src/parse.cpp

@@ -2065,6 +2065,73 @@ expression* parser::maybe_wrap_erroneous_arrow_function(
   }
 }
 
+bool parser::has_potential_side_effects(expression* ast) {
+  switch (ast->kind()) {
+  case expression_kind::_class:
+  case expression_kind::_new:
+  case expression_kind::assignment:
+  case expression_kind::await:
+  case expression_kind::binary_operator:  // TODO(keyehzh): '===' and '!==' are
+                                          // side-effect-free
+  case expression_kind::call:
+  case expression_kind::compound_assignment:
+  case expression_kind::conditional_assignment:
+  case expression_kind::dot:
+  case expression_kind::import:
+  case expression_kind::index:
+  case expression_kind::rw_unary_prefix:
+  case expression_kind::rw_unary_suffix:
+  case expression_kind::spread:
+  case expression_kind::tagged_template_literal:
+  case expression_kind::unary_operator:
+  case expression_kind::yield_many:
+  case expression_kind::yield_none:
+  case expression_kind::yield_one:
+    return true;
+
+  case expression_kind::_invalid:
+  case expression_kind::function:
+  case expression_kind::literal:
+  case expression_kind::named_function:
+  case expression_kind::new_target:
+  case expression_kind::private_variable:
+  case expression_kind::super:
+  case expression_kind::variable:
+    return false;
+
+  case expression_kind::_typeof:
+    return this->has_potential_side_effects(ast->child(0));
+
+  case expression_kind::_template:
+  case expression_kind::array:
+  case expression_kind::arrow_function_with_expression:
+  case expression_kind::arrow_function_with_statements:
+  case expression_kind::trailing_comma:
+    for (int i = 0; i < ast->child_count(); i++) {
+      if (this->has_potential_side_effects(ast->child(i))) return true;
+    }
+    return false;
+
+  case expression_kind::conditional:
+    return this->has_potential_side_effects(ast->child_0()) ||
+           this->has_potential_side_effects(ast->child_1()) ||
+           this->has_potential_side_effects(ast->child_2());
+
+  case expression_kind::object: {
+    for (int i = 0; i < ast->object_entry_count(); i++) {
+      auto entry = ast->object_entry(i);
+      if (entry.property.has_value()) {
+        if (this->has_potential_side_effects(*entry.property) ||
+            this->has_potential_side_effects(entry.value))
+          return true;
+      }
+    }
+    return false;
+  }
+  }
+  return false;
+}
+
 void parser::consume_semicolon() {
   switch (this->peek().type) {
   case token_type::semicolon:

src/quick-lint-js/error.h

@@ -214,6 +214,13 @@
       .error(QLJS_TRANSLATABLE("'else' has no corresponding 'if'"),            \
              else_token))                                                      \
                                                                                \
+  QLJS_ERROR_TYPE(                                                             \
+      error_else_with_conditional_missing_if, "E202",                          \
+      { source_code_span else_token; },                                        \
+      .warning(QLJS_TRANSLATABLE("'else' with condition followed by block; "   \
+                                 "maybe 'else if' was intended"),              \
+               else_token))                                                    \
+                                                                               \
   QLJS_ERROR_TYPE(                                                             \
       error_escaped_character_disallowed_in_identifiers, "E012",               \
       { source_code_span escape_sequence; },                                   \

src/quick-lint-js/parse.h

@@ -658,6 +658,8 @@ class parser {
     return this->parse_expression(precedence{});
   }
 
+  bool has_potential_side_effects(expression *ast);
+
  private:
   enum class variable_context {
     lhs,
@@ -2515,8 +2517,41 @@ class parser {
     }
 
     if (this->peek().type == token_type::kw_else) {
+      source_code_span else_span = this->peek().span();
       this->skip();
-      parse_and_visit_body();
+
+      switch (this->peek().type) {
+      default:
+        parse_and_visit_body();
+        break;
+
+      case token_type::left_paren:
+        expression *ast = this->parse_expression(precedence{});
+        this->visit_expression(ast, v, variable_context::rhs);
+
+        bool is_invalidating_if = false;
+
+        switch (ast->kind()) {
+        default:
+          break;
+
+        case expression_kind::arrow_function_with_expression:
+        case expression_kind::arrow_function_with_statements:
+          is_invalidating_if = true;
+          break;
+        }
+
+        if (this->peek().type == token_type::left_curly) {
+          if (!is_invalidating_if && !this->has_potential_side_effects(ast)) {
+            this->error_reporter_->report(
+                error_else_with_conditional_missing_if{
+                    .else_token = else_span,
+                });
+          }
+          parse_and_visit_body();
+        }
+        break;
+      }
     }
   }