From df524203b2ebbe16356e594e9cd6ab845caa15dd Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Mon, 23 Mar 2020 19:08:32 +0100
Subject: [PATCH] LibJS: Consume semicolon at the end of a statement

A bunch of code was relying on this not happenind, in particular the
parsing of "for" statements. Reorganized things so they work again.
---
 Libraries/LibJS/AST.h      |  9 ++++-----
 Libraries/LibJS/Parser.cpp | 37 +++++++++++++++++++++++++++----------
 Libraries/LibJS/Parser.h   |  1 +
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/Libraries/LibJS/AST.h b/Libraries/LibJS/AST.h
index 3f5e58bec5..d52335be8e 100644
--- a/Libraries/LibJS/AST.h
+++ b/Libraries/LibJS/AST.h
@@ -53,6 +53,7 @@ public:
     virtual bool is_identifier() const { return false; }
     virtual bool is_member_expression() const { return false; }
     virtual bool is_scope_node() const { return false; }
+    virtual bool is_variable_declaration() const { return false; }
 
 protected:
     ASTNode() {}
@@ -61,8 +62,6 @@ private:
 };
 
 class Statement : public ASTNode {
-public:
-    virtual bool is_variable_declaration() const { return false; }
 };
 
 class ErrorStatement final : public Statement {
@@ -259,7 +258,7 @@ private:
 
 class ForStatement : public Statement {
 public:
-    ForStatement(RefPtr<Statement> init, RefPtr<Expression> test, RefPtr<Expression> update, NonnullRefPtr<ScopeNode> body)
+    ForStatement(RefPtr<ASTNode> init, RefPtr<Expression> test, RefPtr<Expression> update, NonnullRefPtr<ScopeNode> body)
         : m_init(move(init))
         , m_test(move(test))
         , m_update(move(update))
@@ -267,7 +266,7 @@ public:
     {
     }
 
-    const Statement* init() const { return m_init; }
+    const ASTNode* init() const { return m_init; }
     const Expression* test() const { return m_test; }
     const Expression* update() const { return m_update; }
     const ScopeNode& body() const { return *m_body; }
@@ -278,7 +277,7 @@ public:
 private:
     virtual const char* class_name() const override { return "ForStatement"; }
 
-    RefPtr<Statement> m_init;
+    RefPtr<ASTNode> m_init;
     RefPtr<Expression> m_test;
     RefPtr<Expression> m_update;
     NonnullRefPtr<ScopeNode> m_body;
diff --git a/Libraries/LibJS/Parser.cpp b/Libraries/LibJS/Parser.cpp
index a6532fb775..082f83f457 100644
--- a/Libraries/LibJS/Parser.cpp
+++ b/Libraries/LibJS/Parser.cpp
@@ -181,6 +181,7 @@ NonnullRefPtr<Program> Parser::parse_program()
 
 NonnullRefPtr<Statement> Parser::parse_statement()
 {
+    auto statement = [this]() -> NonnullRefPtr<Statement> {
     switch (m_current_token.type()) {
     case TokenType::Function:
         return parse_function_node<FunctionDeclaration>();
@@ -197,17 +198,16 @@ NonnullRefPtr<Statement> Parser::parse_statement()
     case TokenType::If:
         return parse_if_statement();
     default:
-        if (match_expression()) {
-            auto statement = adopt(*new ExpressionStatement(parse_expression(0)));
-            if (match(TokenType::Semicolon))
-                consume();
-            return statement;
-        }
+        if (match_expression())
+            return adopt(*new ExpressionStatement(parse_expression(0)));
         m_has_errors = true;
         expected("statement (missing switch case)");
         consume();
         return create_ast_node<ErrorStatement>();
-    }
+    } }();
+    if (match(TokenType::Semicolon))
+        consume();
+    return statement;
 }
 
 NonnullRefPtr<Expression> Parser::parse_primary_expression()
@@ -394,7 +394,7 @@ NonnullRefPtr<Expression> Parser::parse_secondary_expression(NonnullRefPtr<Expre
         return create_ast_node<MemberExpression>(move(lhs), parse_expression(min_precedence, associativity));
     case TokenType::BracketOpen: {
         consume(TokenType::BracketOpen);
-        auto expression = create_ast_node<MemberExpression>(move(lhs), parse_expression(min_precedence, associativity), true);
+        auto expression = create_ast_node<MemberExpression>(move(lhs), parse_expression(0), true);
         consume(TokenType::BracketClose);
         return expression;
     }
@@ -539,12 +539,17 @@ NonnullRefPtr<ForStatement> Parser::parse_for_statement()
 
     consume(TokenType::ParenOpen);
 
-    RefPtr<Statement> init;
+    RefPtr<ASTNode> init;
     switch (m_current_token.type()) {
     case TokenType::Semicolon:
         break;
     default:
-        init = parse_statement();
+        if (match_expression())
+            init = parse_expression(0);
+        else if (match_variable_declaration())
+            init = parse_variable_declaration();
+        else
+            ASSERT_NOT_REACHED();
         break;
     }
 
@@ -582,6 +587,18 @@ bool Parser::match(TokenType type) const
     return m_current_token.type() == type;
 }
 
+bool Parser::match_variable_declaration() const
+{
+    switch (m_current_token.type()) {
+    case TokenType::Var:
+    case TokenType::Let:
+    case TokenType::Const:
+        return true;
+    default:
+        return false;
+    }
+}
+
 bool Parser::match_expression() const
 {
     auto type = m_current_token.type();
diff --git a/Libraries/LibJS/Parser.h b/Libraries/LibJS/Parser.h
index e7b3833d47..4634a02b7f 100644
--- a/Libraries/LibJS/Parser.h
+++ b/Libraries/LibJS/Parser.h
@@ -70,6 +70,7 @@ private:
     bool match_unary_prefixed_expression() const;
     bool match_secondary_expression() const;
     bool match_statement() const;
+    bool match_variable_declaration() const;
     bool match(TokenType type) const;
     bool done() const;
     void expected(const char* what);