LibJS: Function.length respects default and rest parameters

"[Function.length is] the number of formal parameters. This number
excludes the rest parameter and only includes parameters before
the first one with a default value." - MDN

Libraries/LibJS/AST.cpp

@@ -70,14 +70,14 @@ Value ScopeNode::execute(Interpreter& interpreter) const
 
 Value FunctionDeclaration::execute(Interpreter& interpreter) const
 {
-    auto* function = ScriptFunction::create(interpreter.global_object(), name(), body(), parameters(), interpreter.current_environment());
+    auto* function = ScriptFunction::create(interpreter.global_object(), name(), body(), parameters(), function_length(), interpreter.current_environment());
     interpreter.set_variable(name(), function);
     return js_undefined();
 }
 
 Value FunctionExpression::execute(Interpreter& interpreter) const
 {
-    return ScriptFunction::create(interpreter.global_object(), name(), body(), parameters(), interpreter.current_environment());
+    return ScriptFunction::create(interpreter.global_object(), name(), body(), parameters(), function_length(), interpreter.current_environment());
 }
 
 Value ExpressionStatement::execute(Interpreter& interpreter) const

Libraries/LibJS/AST.h

@@ -166,23 +166,26 @@ public:
     const Vector<Parameter>& parameters() const { return m_parameters; };
 
 protected:
-    FunctionNode(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, NonnullRefPtrVector<VariableDeclaration> variables)
+    FunctionNode(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables)
         : m_name(name)
         , m_body(move(body))
         , m_parameters(move(parameters))
         , m_variables(move(variables))
+        , m_function_length(function_length)
     {
     }
 
     void dump(int indent, const char* class_name) const;
 
     const NonnullRefPtrVector<VariableDeclaration>& variables() const { return m_variables; }
+    i32 function_length() const { return m_function_length; }
 
 private:
     FlyString m_name;
     NonnullRefPtr<Statement> m_body;
     const Vector<Parameter> m_parameters;
     NonnullRefPtrVector<VariableDeclaration> m_variables;
+    const i32 m_function_length;
 };
 
 class FunctionDeclaration final
@@ -191,8 +194,8 @@ class FunctionDeclaration final
 public:
     static bool must_have_name() { return true; }
 
-    FunctionDeclaration(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, NonnullRefPtrVector<VariableDeclaration> variables)
+    FunctionDeclaration(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables)
-        : FunctionNode(name, move(body), move(parameters), move(variables))
+        : FunctionNode(name, move(body), move(parameters), function_length, move(variables))
     {
     }
 
@@ -208,8 +211,8 @@ class FunctionExpression final : public Expression
 public:
     static bool must_have_name() { return false; }
 
-    FunctionExpression(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, NonnullRefPtrVector<VariableDeclaration> variables)
+    FunctionExpression(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables)
-        : FunctionNode(name, move(body), move(parameters), move(variables))
+        : FunctionNode(name, move(body), move(parameters), function_length, move(variables))
     {
     }
 

Libraries/LibJS/Parser.cpp

@@ -285,6 +285,7 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
     Vector<FunctionNode::Parameter> parameters;
     bool parse_failed = false;
     bool has_rest_parameter = false;
+    i32 function_length = -1;
     while (true) {
         if (match(TokenType::Comma)) {
             if (has_rest_parameter) {
@@ -297,6 +298,7 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
             RefPtr<Expression> default_value;
             if (expect_parens && match(TokenType::Equals)) {
                 consume(TokenType::Equals);
+                function_length = parameters.size();
                 default_value = parse_expression(0);
             }
             parameters.append({ parameter_name, default_value });
@@ -307,6 +309,7 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
                 break;
             }
             has_rest_parameter = true;
+            function_length = parameters.size();
             auto parameter_name = consume(TokenType::Identifier).value();
             parameters.append({ parameter_name, nullptr, true });
         } else if (match(TokenType::ParenClose)) {
@@ -337,6 +340,9 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
     if (parse_failed)
         return nullptr;
 
+    if (function_length == -1) 
+        function_length = parameters.size();
+
     auto function_body_result = [this]() -> RefPtr<BlockStatement> {
         if (match(TokenType::CurlyOpen)) {
             // Parse a function body with statements
@@ -361,7 +367,7 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
     if (!function_body_result.is_null()) {
         state_rollback_guard.disarm();
         auto body = function_body_result.release_nonnull();
-        return create_ast_node<FunctionExpression>("", move(body), move(parameters), m_parser_state.m_var_scopes.take_last());
+        return create_ast_node<FunctionExpression>("", move(body), move(parameters), function_length, m_parser_state.m_var_scopes.take_last());
     }
 
     return nullptr;
@@ -863,10 +869,12 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(bool needs_function_
     }
     consume(TokenType::ParenOpen);
     Vector<FunctionNode::Parameter> parameters;
+    i32 function_length = -1;
     while (match(TokenType::Identifier) || match(TokenType::TripleDot)) {
         if (match(TokenType::TripleDot)) {
             consume();
             auto parameter_name = consume(TokenType::Identifier).value();
+            function_length = parameters.size();
             parameters.append({ parameter_name, nullptr, true });
             break;
         }
@@ -874,6 +882,7 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(bool needs_function_
         RefPtr<Expression> default_value;
         if (match(TokenType::Equals)) {
             consume(TokenType::Equals);
+            function_length = parameters.size();
             default_value = parse_expression(0);
         }
         parameters.append({ parameter_name, default_value });
@@ -882,9 +891,13 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(bool needs_function_
         consume(TokenType::Comma);
     }
     consume(TokenType::ParenClose);
+
+    if (function_length == -1)
+        function_length = parameters.size();
+        
     auto body = parse_block_statement();
     body->add_variables(m_parser_state.m_var_scopes.last());
-    return create_ast_node<FunctionNodeType>(name, move(body), move(parameters), NonnullRefPtrVector<VariableDeclaration>());
+    return create_ast_node<FunctionNodeType>(name, move(body), move(parameters), function_length, NonnullRefPtrVector<VariableDeclaration>());
 }
 
 NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration()

Libraries/LibJS/Runtime/ScriptFunction.cpp

@@ -47,17 +47,18 @@ static ScriptFunction* script_function_from(Interpreter& interpreter)
     return static_cast<ScriptFunction*>(this_object);
 }
 
-ScriptFunction* ScriptFunction::create(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, LexicalEnvironment* parent_environment)
+ScriptFunction* ScriptFunction::create(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment)
 {
-    return global_object.heap().allocate<ScriptFunction>(name, body, move(parameters), parent_environment, *global_object.function_prototype());
+    return global_object.heap().allocate<ScriptFunction>(name, body, move(parameters), m_function_length, parent_environment, *global_object.function_prototype());
 }
 
-ScriptFunction::ScriptFunction(const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, LexicalEnvironment* parent_environment, Object& prototype)
+ScriptFunction::ScriptFunction(const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype)
     : Function(prototype)
     , m_name(name)
     , m_body(body)
     , m_parameters(move(parameters))
     , m_parent_environment(parent_environment)
+    , m_function_length(m_function_length)
 {
     put("prototype", Object::create_empty(interpreter(), interpreter().global_object()), 0);
     put_native_property("length", length_getter, nullptr, Attribute::Configurable);
@@ -130,7 +131,7 @@ Value ScriptFunction::length_getter(Interpreter& interpreter)
     auto* function = script_function_from(interpreter);
     if (!function)
         return {};
-    return Value(static_cast<i32>(function->parameters().size()));
+    return Value(static_cast<i32>(function->m_function_length));
 }
 
 Value ScriptFunction::name_getter(Interpreter& interpreter)

Libraries/LibJS/Runtime/ScriptFunction.h

@@ -33,9 +33,9 @@ namespace JS {
 
 class ScriptFunction final : public Function {
 public:
-    static ScriptFunction* create(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, LexicalEnvironment* parent_environment);
+    static ScriptFunction* create(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment);
 
-    ScriptFunction(const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, LexicalEnvironment* parent_environment, Object& prototype);
+    ScriptFunction(const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype);
     virtual ~ScriptFunction();
 
     const Statement& body() const { return m_body; }
@@ -60,6 +60,7 @@ private:
     NonnullRefPtr<Statement> m_body;
     const Vector<FunctionNode::Parameter> m_parameters;
     LexicalEnvironment* m_parent_environment { nullptr };
+    i32 m_function_length;
 };
 
 }

Libraries/LibJS/Tests/function-length.js

@@ -11,6 +11,16 @@ try {
     assert((bar.length = 5) === 5);
     assert(bar.length === 3);
 
+    function baz(a, b = 1, c) {}
+    assert(baz.length === 1);
+    assert((baz.length = 5) === 5);
+    assert(baz.length === 1);
+
+    function qux(a, b, ...c) {}
+    assert(qux.length === 2);
+    assert((qux.length = 5) === 5);
+    assert(qux.length === 2);
+
     console.log("PASS");
 } catch (e) {
     console.log("FAIL: " + e);