diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 3b779b3c5a..82406f3107 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/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 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(value.as_object())); + + auto& array_value = static_cast(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 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); diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h index 8586ba0cf6..1c6c7886ef 100644 --- a/Userland/Libraries/LibJS/AST.h +++ b/Userland/Libraries/LibJS/AST.h @@ -1492,17 +1492,34 @@ public: 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 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 const m_arguments; + IsPartOfSyntheticConstructor const m_is_synthetic; }; enum class AssignmentOp { diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp index 7dcc2bd71f..a30d5e5cd1 100644 --- a/Userland/Libraries/LibJS/Parser.cpp +++ b/Userland/Libraries/LibJS/Parser.cpp @@ -1313,9 +1313,16 @@ NonnullRefPtr 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( { m_state.current_token.filename(), rule_start.position(), position() }, - Vector { CallExpression::Argument { create_ast_node({ m_state.current_token.filename(), rule_start.position(), position() }, "args"), true } }); + SuperCall::IsPartOfSyntheticConstructor::Yes, + CallExpression::Argument { create_ast_node({ 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. @@ -1323,7 +1330,7 @@ NonnullRefPtr Parser::parse_class_expression(bool expect_class_ constructor = create_ast_node( { 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( diff --git a/Userland/Libraries/LibJS/Tests/classes/class-inheritance.js b/Userland/Libraries/LibJS/Tests/classes/class-inheritance.js index 3ec85ec3ee..1ef24303f0 100644 --- a/Userland/Libraries/LibJS/Tests/classes/class-inheritance.js +++ b/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); +});