LibJS: Don't hoist functions under certain circumstances

When a lexical declaration with the same name as a function exists,
the function is not hoisted (annex B).

Userland/Libraries/LibJS/AST.cpp

@@ -2377,9 +2377,9 @@ void ScopeNode::add_functions(NonnullRefPtrVector<FunctionDeclaration> functions
     m_functions.extend(move(functions));
 }
 
-void ScopeNode::add_hoisted_functions(NonnullRefPtrVector<FunctionDeclaration> hoisted_functions)
+void ScopeNode::add_hoisted_function(NonnullRefPtr<FunctionDeclaration> hoisted_function)
 {
-    m_hoisted_functions.extend(move(hoisted_functions));
+    m_hoisted_functions.append(hoisted_function);
 }
 
 }

Userland/Libraries/LibJS/AST.h

@@ -145,7 +145,7 @@ public:
 
     void add_variables(NonnullRefPtrVector<VariableDeclaration>);
     void add_functions(NonnullRefPtrVector<FunctionDeclaration>);
-    void add_hoisted_functions(NonnullRefPtrVector<FunctionDeclaration>);
+    void add_hoisted_function(NonnullRefPtr<FunctionDeclaration>);
     NonnullRefPtrVector<VariableDeclaration> const& variables() const { return m_variables; }
     NonnullRefPtrVector<FunctionDeclaration> const& functions() const { return m_functions; }
     NonnullRefPtrVector<FunctionDeclaration> const& hoisted_functions() const { return m_hoisted_functions; }

Userland/Libraries/LibJS/Parser.cpp

@@ -53,6 +53,9 @@ public:
             m_parser.m_state.let_scopes.take_last();
 
         auto& popped = m_parser.m_state.current_scope;
+        // Manual clear required to resolve circular references
+        popped->hoisted_function_declarations.clear();
+
         m_parser.m_state.current_scope = popped->parent;
     }
 
@@ -63,8 +66,26 @@ public:
         if (m_mask & Let)
             scope_node->add_variables(m_parser.m_state.let_scopes.last());
 
-        scope_node->add_functions(m_parser.m_state.current_scope->function_declarations);
-        scope_node->add_hoisted_functions(m_parser.m_state.current_scope->hoisted_function_declarations);
+        auto& scope = m_parser.m_state.current_scope;
+        scope_node->add_functions(scope->function_declarations);
+
+        for (auto& hoistable_function : scope->hoisted_function_declarations) {
+            if (is_hoistable(hoistable_function)) {
+                scope_node->add_hoisted_function(hoistable_function.declaration);
+            }
+        }
+    }
+
+    static bool is_hoistable(Parser::Scope::HoistableDeclaration& declaration)
+    {
+        auto& name = declaration.declaration->name();
+        // See if we find any conflicting lexical declaration on the way up
+        for (RefPtr<Parser::Scope> scope = declaration.scope; !scope.is_null(); scope = scope->parent) {
+            if (scope->lexical_declarations.contains(name)) {
+                return false;
+            }
+        }
+        return true;
     }
 
     Parser& m_parser;
@@ -304,7 +325,8 @@ NonnullRefPtr<Declaration> Parser::parse_declaration()
     case TokenType::Function: {
         auto declaration = parse_function_node<FunctionDeclaration>();
         m_state.current_scope->function_declarations.append(declaration);
-        m_state.current_scope->get_current_function_scope()->hoisted_function_declarations.append(declaration);
+        auto hoisting_target = m_state.current_scope->get_current_function_scope();
+        hoisting_target->hoisted_function_declarations.append({ declaration, *m_state.current_scope });
         return declaration;
     }
     case TokenType::Let:
@@ -1760,10 +1782,23 @@ NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration(bool for_l
         consume_or_insert_semicolon();
 
     auto declaration = create_ast_node<VariableDeclaration>({ m_state.current_token.filename(), rule_start.position(), position() }, declaration_kind, move(declarations));
-    if (declaration_kind == DeclarationKind::Var)
+    if (declaration_kind == DeclarationKind::Var) {
         m_state.var_scopes.last().append(declaration);
-    else
+    } else {
         m_state.let_scopes.last().append(declaration);
+
+        for (auto& declarator : declaration->declarations()) {
+            declarator.target().visit(
+                [&](const NonnullRefPtr<Identifier>& id) {
+                    m_state.current_scope->lexical_declarations.set(id->string());
+                },
+                [&](const NonnullRefPtr<BindingPattern>& binding) {
+                    binding->for_each_bound_name([&](const auto& name) {
+                        m_state.current_scope->lexical_declarations.set(name);
+                    });
+                });
+        }
+    }
     return declaration;
 }
 

Userland/Libraries/LibJS/Parser.h

@@ -201,12 +201,18 @@ private:
             Function,
             Block,
         };
+        struct HoistableDeclaration {
+            NonnullRefPtr<FunctionDeclaration> declaration;
+            NonnullRefPtr<Scope> scope; // where it is actually declared
+        };
 
         Type type;
         RefPtr<Scope> parent;
 
         NonnullRefPtrVector<FunctionDeclaration> function_declarations;
-        NonnullRefPtrVector<FunctionDeclaration> hoisted_function_declarations;
+        Vector<HoistableDeclaration> hoisted_function_declarations;
+
+        HashTable<FlyString> lexical_declarations;
 
         explicit Scope(Type, RefPtr<Scope>);
         RefPtr<Scope> get_current_function_scope();