LibJS: Use a synthetic constructor if class with parent doesn't have one

We already did this but it called the @@iterator method of
%Array.prototype% visible to the user for example by overriding that
method. This should not be visible so we use a special version of
SuperCall now.

Author: davidot

Userland/Libraries/LibJS/AST.cpp

@@ -463,7 +463,22 @@ Completion SuperCall::execute(Interpreter& interpreter, GlobalObject& global_obj
 
     // 4. Let argList be ? ArgumentListEvaluation of Arguments.
     MarkedVector<Value> arg_list(vm.heap());
-    TRY(argument_list_evaluation(interpreter, global_object, m_arguments, arg_list));
+    if (m_is_synthetic == IsPartOfSyntheticConstructor::Yes) {
+        // NOTE: This is the case where we have a fake constructor(...args) { super(...args); } which
+        //       shouldn't call @@iterator of %Array.prototype%.
+        VERIFY(m_arguments.size() == 1);
+        VERIFY(m_arguments[0].is_spread);
+        auto const& argument = m_arguments[0];
+        auto value = MUST(argument.value->execute(interpreter, global_object)).release_value();
+        VERIFY(value.is_object() && is<Array>(value.as_object()));
+
+        auto& array_value = static_cast<Array const&>(value.as_object());
+        auto length = MUST(length_of_array_like(global_object, array_value));
+        for (size_t i = 0; i < length; ++i)
+            arg_list.append(array_value.get_without_side_effects(PropertyKey { i }));
+    } else {
+        TRY(argument_list_evaluation(interpreter, global_object, m_arguments, arg_list));
+    }
 
     // 5. If IsConstructor(func) is false, throw a TypeError exception.
     if (!func || !Value(func).is_constructor())
@@ -1827,7 +1842,7 @@ ThrowCompletionOr<ECMAScriptFunctionObject*> ClassExpression::class_definition_e
         vm.running_execution_context().private_environment = outer_private_environment;
     };
 
-    // FIXME: Step 14.a is done in the parser. But maybe it shouldn't?
+    // FIXME: Step 14.a is done in the parser. By using a synthetic super(...args) which does not call @@iterator of %Array.prototype%
     auto class_constructor_value = TRY(m_constructor->execute(interpreter, global_object)).release_value();
 
     update_function_name(class_constructor_value, class_name);

Userland/Libraries/LibJS/AST.h

@@ -1492,17 +1492,34 @@ class NewExpression final : public CallExpression {
 
 class SuperCall final : public Expression {
 public:
+    // This is here to be able to make a constructor like
+    // constructor(...args) { super(...args); } which does not use @@iterator of %Array.prototype%.
+    enum class IsPartOfSyntheticConstructor {
+        No,
+        Yes,
+    };
+
     SuperCall(SourceRange source_range, Vector<CallExpression::Argument> arguments)
         : Expression(source_range)
         , m_arguments(move(arguments))
+        , m_is_synthetic(IsPartOfSyntheticConstructor::No)
+    {
+    }
+
+    SuperCall(SourceRange source_range, IsPartOfSyntheticConstructor is_part_of_synthetic_constructor, CallExpression::Argument constructor_argument)
+        : Expression(source_range)
+        , m_arguments({ move(constructor_argument) })
+        , m_is_synthetic(IsPartOfSyntheticConstructor::Yes)
     {
+        VERIFY(is_part_of_synthetic_constructor == IsPartOfSyntheticConstructor::Yes);
     }
 
     virtual Completion execute(Interpreter&, GlobalObject&) const override;
     virtual void dump(int indent) const override;
 
 private:
     Vector<CallExpression::Argument> const m_arguments;
+    IsPartOfSyntheticConstructor const m_is_synthetic;
 };
 
 enum class AssignmentOp {

Userland/Libraries/LibJS/Parser.cpp

@@ -1313,17 +1313,24 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
         if (!super_class.is_null()) {
             // Set constructor to the result of parsing the source text
             // constructor(... args){ super (...args);}
+            // However: The most notable distinction is that while the aforementioned ECMAScript
+            //          source text observably calls the @@iterator method on %Array.prototype%,
+            //          this function does not.
+            //          So we use a custom version of SuperCall which doesn't use the @@iterator
+            //          method on %Array.prototype% visibly.
+            FlyString argument_name = "args";
             auto super_call = create_ast_node<SuperCall>(
                 { m_state.current_token.filename(), rule_start.position(), position() },
-                Vector { CallExpression::Argument { create_ast_node<Identifier>({ m_state.current_token.filename(), rule_start.position(), position() }, "args"), true } });
+                SuperCall::IsPartOfSyntheticConstructor::Yes,
+                CallExpression::Argument { create_ast_node<Identifier>({ m_state.current_token.filename(), rule_start.position(), position() }, "args"), true });
             // NOTE: While the JS approximation above doesn't do `return super(...args)`, the
             // abstract closure is expected to capture and return the result, so we do need a
             // return statement here to create the correct completion.
             constructor_body->append(create_ast_node<ReturnStatement>({ m_state.current_token.filename(), rule_start.position(), position() }, move(super_call)));
 
             constructor = create_ast_node<FunctionExpression>(
                 { m_state.current_token.filename(), rule_start.position(), position() }, class_name, "",
-                move(constructor_body), Vector { FunctionNode::Parameter { FlyString { "args" }, nullptr, true } }, 0, FunctionKind::Normal,
+                move(constructor_body), Vector { FunctionNode::Parameter { move(argument_name), nullptr, true } }, 0, FunctionKind::Normal,
                 /* is_strict_mode */ true, /* might_need_arguments_object */ false, /* contains_direct_call_to_eval */ false);
         } else {
             constructor = create_ast_node<FunctionExpression>(

Userland/Libraries/LibJS/Tests/classes/class-inheritance.js

@@ -449,3 +449,60 @@ test("super outside of derived class fails to parse", () => {
         new J();
     }).toThrowWithMessage(SyntaxError, "'super' keyword unexpected here");
 });
+
+test("When no constructor on deriving class @@iterator of %Array.prototype% is not visibly called", () => {
+    const oldIterator = Array.prototype[Symbol.iterator];
+    var calls = 0;
+    Array.prototype[Symbol.iterator] = function () {
+        ++calls;
+        expect().fail("Called @@iterator");
+    };
+
+    class Base {
+        constructor(value1, value2) {
+            this.value1 = value1;
+            this.value2 = value2;
+        }
+    }
+
+    class Derived extends Base {}
+
+    const noArgumentsDerived = new Derived();
+    expect(noArgumentsDerived.value1).toBeUndefined();
+    expect(noArgumentsDerived.value2).toBeUndefined();
+    expect(noArgumentsDerived).toBeInstanceOf(Base);
+    expect(noArgumentsDerived).toBeInstanceOf(Derived);
+
+    const singleArgumentDerived = new Derived(1);
+    expect(singleArgumentDerived.value1).toBe(1);
+    expect(singleArgumentDerived.value2).toBeUndefined();
+
+    const singleArrayArgumentDerived = new Derived([1, 2]);
+    expect(singleArrayArgumentDerived.value1).toEqual([1, 2]);
+    expect(singleArrayArgumentDerived.value2).toBeUndefined();
+
+    const doubleArgumentDerived = new Derived(1, 2);
+    expect(doubleArgumentDerived.value1).toBe(1);
+    expect(doubleArgumentDerived.value2).toBe(2);
+
+    expect(calls).toBe(0);
+
+    class Derived2 extends Base {
+        constructor(...args) {
+            super(...args);
+        }
+    }
+
+    expect(() => {
+        new Derived2();
+    }).toThrowWithMessage(ExpectationError, "Called @@iterator");
+
+    expect(calls).toBe(1);
+
+    Array.prototype[Symbol.iterator] = oldIterator;
+
+    // Now Derived2 is fine again.
+    expect(new Derived2()).toBeInstanceOf(Derived2);
+
+    expect(calls).toBe(1);
+});