From 644b4f420133d3945eca02d2734b7819e02f3c47 Mon Sep 17 00:00:00 2001
From: 0xtechnobabble <0xtechnobabble@protonmail.com>
Date: Sat, 14 Mar 2020 13:56:49 +0200
Subject: [PATCH] LibJS: Evaluate for statements in their own scope if
 necessary

We now evaluate for loops in their own scope if their init statement is
a lexical declaration.

Evaluating for loops in their own scope allow us to obtain expected
behaviour, which means for example, that the block-scoped variables
declared in a  for statement will be limited to the scope of the for
loop's body and  statement and not to that of the current scope (i.e the
one where the for statement was made)
---
 Libraries/LibJS/AST.cpp         | 10 ++++++++++
 Libraries/LibJS/AST.h           |  4 ++++
 Libraries/LibJS/Interpreter.cpp |  4 ++--
 Libraries/LibJS/Interpreter.h   |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp
index abfccf1f143..2b6ede9ba0c 100644
--- a/Libraries/LibJS/AST.cpp
+++ b/Libraries/LibJS/AST.cpp
@@ -95,6 +95,13 @@ Value WhileStatement::execute(Interpreter& interpreter) const
 
 Value ForStatement::execute(Interpreter& interpreter) const
 {
+    OwnPtr<BlockStatement> wrapper;
+
+    if (m_init->is_variable_declaration() && static_cast<const VariableDeclaration*>(m_init.ptr())->declaration_type() != DeclarationType::Var) {
+        wrapper = make<BlockStatement>();
+        interpreter.enter_scope(*wrapper, {}, ScopeType::Block);
+    }
+
     Value last_value = js_undefined();
 
     if (m_init)
@@ -114,6 +121,9 @@ Value ForStatement::execute(Interpreter& interpreter) const
         }
     }
 
+    if (wrapper)
+        interpreter.exit_scope(*wrapper);
+
     return last_value;
 }
 
diff --git a/Libraries/LibJS/AST.h b/Libraries/LibJS/AST.h
index 2d2bceba312..f2ec48a393d 100644
--- a/Libraries/LibJS/AST.h
+++ b/Libraries/LibJS/AST.h
@@ -50,6 +50,8 @@ private:
 };
 
 class Statement : public ASTNode {
+public:
+    virtual bool is_variable_declaration() const { return false; }
 };
 
 class ErrorStatement final : public Statement {
@@ -480,7 +482,9 @@ public:
     {
     }
 
+    virtual bool is_variable_declaration() const override { return true; }
     const Identifier& name() const { return *m_name; }
+    DeclarationType declaration_type() const { return m_declaration_type; }
 
     virtual Value execute(Interpreter&) const override;
     virtual void dump(int indent) const override;
diff --git a/Libraries/LibJS/Interpreter.cpp b/Libraries/LibJS/Interpreter.cpp
index 99049a36c4d..dfbaa35ebb8 100644
--- a/Libraries/LibJS/Interpreter.cpp
+++ b/Libraries/LibJS/Interpreter.cpp
@@ -68,8 +68,8 @@ void Interpreter::enter_scope(const ScopeNode& scope_node, Vector<Argument> argu
 
 void Interpreter::exit_scope(const ScopeNode& scope_node)
 {
-    ASSERT(&m_scope_stack.last().scope_node == &scope_node);
-    m_scope_stack.take_last();
+    while (&m_scope_stack.last().scope_node != &scope_node)
+        m_scope_stack.take_last();
 }
 
 void Interpreter::do_return()
diff --git a/Libraries/LibJS/Interpreter.h b/Libraries/LibJS/Interpreter.h
index 9164e286b3d..61b584c068a 100644
--- a/Libraries/LibJS/Interpreter.h
+++ b/Libraries/LibJS/Interpreter.h
@@ -76,10 +76,10 @@ public:
 
     void collect_roots(Badge<Heap>, HashTable<Cell*>&);
 
-private:
     void enter_scope(const ScopeNode&, Vector<Argument>, ScopeType);
     void exit_scope(const ScopeNode&);
 
+private:
     Heap m_heap;
 
     Vector<ScopeFrame> m_scope_stack;