LibJS: Allow the choice of a scope of declaration for a variable (#1408)

Previously, we were assuming all declared variables were bound to a
block scope, now, with the addition of declaration types, we can bind
a variable to a block scope using `let`, or a function scope (the scope
of the inner-most enclosing function of a `var` declaration) using
`var`.

Libraries/LibJS/AST.cpp

@@ -56,7 +56,7 @@ Value CallExpression::execute(Interpreter& interpreter) const
     auto* callee_object = callee.as_object();
     ASSERT(callee_object->is_function());
     auto& function = static_cast<Function&>(*callee_object);
-    return interpreter.run(function.body());
+    return interpreter.run(function.body(), ScopeType::Function);
 }
 
 Value ReturnStatement::execute(Interpreter& interpreter) const
@@ -379,17 +379,30 @@ void AssignmentExpression::dump(int indent) const
 
 Value VariableDeclaration::execute(Interpreter& interpreter) const
 {
-    interpreter.declare_variable(name().string());
+    interpreter.declare_variable(name().string(), m_declaration_type);
     if (m_initializer) {
         auto initalizer_result = m_initializer->execute(interpreter);
         interpreter.set_variable(name().string(), initalizer_result);
     }
+
     return js_undefined();
 }
 
 void VariableDeclaration::dump(int indent) const
 {
+    const char* op_string = nullptr;
+    switch (m_declaration_type) {
+    case DeclarationType::Let:
+        op_string = "Let";
+        break;
+    case DeclarationType::Var:
+        op_string = "Var";
+        break;
+    }
+
     ASTNode::dump(indent);
+    print_indent(indent + 1);
+    printf("%s\n", op_string);
     m_name->dump(indent + 1);
     if (m_initializer)
         m_initializer->dump(indent + 1);

Libraries/LibJS/AST.h

@@ -332,10 +332,16 @@ private:
     NonnullOwnPtr<Expression> m_rhs;
 };
 
+enum class DeclarationType {
+    Var,
+    Let,
+};
+
 class VariableDeclaration : public ASTNode {
 public:
-    VariableDeclaration(NonnullOwnPtr<Identifier> name, OwnPtr<Expression> initializer)
+    VariableDeclaration(NonnullOwnPtr<Identifier> name, OwnPtr<Expression> initializer, DeclarationType declaration_type)
-        : m_name(move(name))
+        : m_declaration_type(declaration_type)
+        , m_name(move(name))
         , m_initializer(move(initializer))
     {
     }
@@ -348,6 +354,7 @@ public:
 private:
     virtual const char* class_name() const override { return "VariableDeclaration"; }
 
+    DeclarationType m_declaration_type;
     NonnullOwnPtr<Identifier> m_name;
     OwnPtr<Expression> m_initializer;
 };

Libraries/LibJS/Forward.h

@@ -37,5 +37,6 @@ class Object;
 class PrimitiveString;
 class ScopeNode;
 class Value;
+enum class DeclarationType;
 
 }

Libraries/LibJS/Interpreter.cpp

@@ -42,9 +42,9 @@ Interpreter::~Interpreter()
 {
 }
 
-Value Interpreter::run(const ScopeNode& scope_node)
+Value Interpreter::run(const ScopeNode& scope_node, ScopeType scope_type)
 {
-    enter_scope(scope_node);
+    enter_scope(scope_node, scope_type);
 
     Value last_value = js_undefined();
     for (auto& node : scope_node.children()) {
@@ -55,9 +55,9 @@ Value Interpreter::run(const ScopeNode& scope_node)
     return last_value;
 }
 
-void Interpreter::enter_scope(const ScopeNode& scope_node)
+void Interpreter::enter_scope(const ScopeNode& scope_node, ScopeType scope_type)
 {
-    m_scope_stack.append({ scope_node, {} });
+    m_scope_stack.append({ scope_type, scope_node, {} });
 }
 
 void Interpreter::exit_scope(const ScopeNode& scope_node)
@@ -71,9 +71,24 @@ void Interpreter::do_return()
     dbg() << "FIXME: Implement Interpreter::do_return()";
 }
 
-void Interpreter::declare_variable(String name)
+void Interpreter::declare_variable(String name, DeclarationType declaration_type)
 {
+    switch (declaration_type) {
+    case DeclarationType::Var:
+        for (ssize_t i = m_scope_stack.size() - 1; i >= 0; --i) {
+            auto& scope = m_scope_stack.at(i);
+            if (scope.type == ScopeType::Function) {
+                scope.variables.set(move(name), js_undefined());
+                return;
+            }
+        }
+
+        global_object().put(move(name), js_undefined());
+        break;
+    case DeclarationType::Let:
         m_scope_stack.last().variables.set(move(name), js_undefined());
+        break;
+    }
 }
 
 void Interpreter::set_variable(String name, Value value)

Libraries/LibJS/Interpreter.h

@@ -33,7 +33,13 @@
 
 namespace JS {
 
+enum class ScopeType {
+    Function,
+    Block,
+};
+
 struct ScopeFrame {
+    ScopeType type;
     const ScopeNode& scope_node;
     HashMap<String, Value> variables;
 };
@@ -43,7 +49,7 @@ public:
     Interpreter();
     ~Interpreter();
 
-    Value run(const ScopeNode&);
+    Value run(const ScopeNode&, ScopeType = ScopeType::Block);
 
     Object& global_object() { return *m_global_object; }
     const Object& global_object() const { return *m_global_object; }
@@ -54,12 +60,12 @@ public:
 
     Value get_variable(const String& name);
     void set_variable(String name, Value);
-    void declare_variable(String name);
+    void declare_variable(String name, DeclarationType);
 
     void collect_roots(Badge<Heap>, HashTable<Cell*>&);
 
 private:
-    void enter_scope(const ScopeNode&);
+    void enter_scope(const ScopeNode&, ScopeType);
     void exit_scope(const ScopeNode&);
 
     Heap m_heap;

Userland/js.cpp

@@ -92,10 +92,12 @@ void build_program(JS::Program& program)
     auto block = make<JS::BlockStatement>();
     block->append<JS::VariableDeclaration>(
         make<JS::Identifier>("a"),
-        make<JS::Literal>(JS::Value(5)));
+        make<JS::Literal>(JS::Value(5)),
+        JS::DeclarationType::Var);
     block->append<JS::VariableDeclaration>(
         make<JS::Identifier>("b"),
-        make<JS::Literal>(JS::Value(7)));
+        make<JS::Literal>(JS::Value(7)),
+        JS::DeclarationType::Var);
 
     block->append<JS::ReturnStatement>(
         make<JS::BinaryExpression>(
@@ -120,13 +122,38 @@ void build_program(JS::Program& program)
     auto block = make<JS::BlockStatement>();
     block->append<JS::VariableDeclaration>(
         make<JS::Identifier>("x"),
-        make<JS::ObjectExpression>());
+        make<JS::ObjectExpression>(),
+        JS::DeclarationType::Var);
     block->append<JS::CallExpression>("$gc");
 
     program.append<JS::FunctionDeclaration>("foo", move(block));
     program.append<JS::CallExpression>("foo");
 }
 #elif PROGRAM == 4
+void build_program(JS::Program& program)
+{
+    // function foo() {
+    //   function bar() {
+    //      var y = 6;
+    //   }
+    //
+    //   bar()
+    //   return y;
+    // }
+    // foo(); //I should return `undefined` because y is bound to the inner-most enclosing function, i.e the nested one (bar()), therefore, it's undefined in the scope of foo()
+
+    auto block_bar = make<JS::BlockStatement>();
+    block_bar->append<JS::VariableDeclaration>(make<JS::Identifier>("y"), make<JS::Literal>(JS::Value(6)), JS::DeclarationType::Var);
+
+    auto block_foo = make<JS::BlockStatement>();
+    block_foo->append<JS::FunctionDeclaration>("bar", move(block_bar));
+    block_foo->append<JS::CallExpression>("bar");
+    block_foo->append<JS::ReturnStatement>(make<JS::Identifier>("y"));
+
+    program.append<JS::FunctionDeclaration>("foo", move(block_foo));
+    program.append<JS::CallExpression>("foo");
+}
+#elif PROGRAM == 5
 void build_program(JS::Program& program, JS::Heap& heap)
 {
     // "hello friends".length