LibJS: Allow member expressions in binding patterns

Also allows literal string and numbers as property names in object
binding patterns.

Userland/Libraries/LibJS/AST.cpp

@@ -1331,6 +1331,8 @@ void BindingPattern::dump(int indent) const
             entry.alias.get<NonnullRefPtr<Identifier>>()->dump(indent + 3);
         } else if (entry.alias.has<NonnullRefPtr<BindingPattern>>()) {
             entry.alias.get<NonnullRefPtr<BindingPattern>>()->dump(indent + 3);
+        } else if (entry.alias.has<NonnullRefPtr<MemberExpression>>()) {
+            entry.alias.get<NonnullRefPtr<MemberExpression>>()->dump(indent + 3);
         } else {
             print_indent(indent + 3);
             outln("<empty>");

Userland/Libraries/LibJS/AST.h

@@ -27,6 +27,7 @@ namespace JS {
 class VariableDeclaration;
 class FunctionDeclaration;
 class Identifier;
+class MemberExpression;
 
 enum class FunctionKind {
     Generator,
@@ -319,7 +320,7 @@ struct BindingPattern : RefCounted<BindingPattern> {
     struct BindingEntry {
         // If this entry represents a BindingElement, then name will be Empty
         Variant<NonnullRefPtr<Identifier>, NonnullRefPtr<Expression>, Empty> name {};
-        Variant<NonnullRefPtr<Identifier>, NonnullRefPtr<BindingPattern>, Empty> alias {};
+        Variant<NonnullRefPtr<Identifier>, NonnullRefPtr<BindingPattern>, NonnullRefPtr<MemberExpression>, Empty> alias {};
         RefPtr<Expression> initializer {};
         bool is_rest { false };
 

Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp

@@ -778,6 +778,9 @@ static void generate_array_binding_pattern_bytecode(Bytecode::Generator& generat
                 auto target_reg = generator.allocate_register();
                 generator.emit<Bytecode::Op::Store>(target_reg);
                 generate_binding_pattern_bytecode(generator, pattern, target_reg);
+            },
+            [&](NonnullRefPtr<MemberExpression> const&) {
+                TODO();
             });
     };
 

Userland/Libraries/LibJS/Parser.cpp

@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2020, Stephan Unverwerth <s.unverwerth@serenityos.org>
  * Copyright (c) 2020-2021, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2021, David Tuin <david.tuin@gmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -1545,6 +1546,36 @@ NonnullRefPtr<Expression> Parser::parse_secondary_expression(NonnullRefPtr<Expre
     }
 }
 
+RefPtr<BindingPattern> Parser::synthesize_binding_pattern(Expression const& expression)
+{
+    VERIFY(is<ArrayExpression>(expression) || is<ObjectExpression>(expression));
+    // Clear any syntax error that has occurred in the range that 'expression' spans.
+    m_state.errors.remove_all_matching([range = expression.source_range()](auto const& error) {
+        return error.position.has_value() && range.contains(*error.position);
+    });
+    // Make a parser and parse the source for this expression as a binding pattern.
+    auto source = m_state.lexer.source().substring_view(expression.source_range().start.offset - 2, expression.source_range().end.offset - expression.source_range().start.offset);
+    Lexer lexer { source, m_state.lexer.filename(), expression.source_range().start.line, expression.source_range().start.column };
+    Parser parser { lexer };
+
+    parser.m_state.strict_mode = m_state.strict_mode;
+    parser.m_state.allow_super_property_lookup = m_state.allow_super_property_lookup;
+    parser.m_state.allow_super_constructor_call = m_state.allow_super_constructor_call;
+    parser.m_state.in_function_context = m_state.in_function_context;
+    parser.m_state.in_formal_parameter_context = m_state.in_formal_parameter_context;
+    parser.m_state.in_generator_function_context = m_state.in_generator_function_context;
+    parser.m_state.in_arrow_function_context = m_state.in_arrow_function_context;
+    parser.m_state.in_break_context = m_state.in_break_context;
+    parser.m_state.in_continue_context = m_state.in_continue_context;
+    parser.m_state.string_legacy_octal_escape_sequence_in_scope = m_state.string_legacy_octal_escape_sequence_in_scope;
+    parser.m_state.in_class_field_initializer = m_state.in_class_field_initializer;
+
+    auto result = parser.parse_binding_pattern(AllowDuplicates::Yes, AllowMemberExpressions::Yes);
+    if (parser.has_errors())
+        m_state.errors.extend(parser.errors());
+    return result;
+}
+
 NonnullRefPtr<AssignmentExpression> Parser::parse_assignment_expression(AssignmentOp assignment_op, NonnullRefPtr<Expression> lhs, int min_precedence, Associativity associativity)
 {
     auto rule_start = push_start();
@@ -1567,33 +1598,6 @@ NonnullRefPtr<AssignmentExpression> Parser::parse_assignment_expression(Assignme
     consume();
 
     if (assignment_op == AssignmentOp::Assignment) {
-        auto synthesize_binding_pattern = [this](Expression const& expression) -> RefPtr<BindingPattern> {
-            // Clear any syntax error that has occurred in the range that 'expression' spans.
-            m_state.errors.remove_all_matching([range = expression.source_range()](auto const& error) {
-                return error.position.has_value() && range.contains(*error.position);
-            });
-            // Make a parser and parse the source for this expression as a binding pattern.
-            auto source = m_state.lexer.source().substring_view(expression.source_range().start.offset - 2, expression.source_range().end.offset - expression.source_range().start.offset);
-            Lexer lexer { source, m_state.lexer.filename(), expression.source_range().start.line, expression.source_range().start.column };
-            Parser parser { lexer };
-
-            parser.m_state.strict_mode = m_state.strict_mode;
-            parser.m_state.allow_super_property_lookup = m_state.allow_super_property_lookup;
-            parser.m_state.allow_super_constructor_call = m_state.allow_super_constructor_call;
-            parser.m_state.in_function_context = m_state.in_function_context;
-            parser.m_state.in_formal_parameter_context = m_state.in_formal_parameter_context;
-            parser.m_state.in_generator_function_context = m_state.in_generator_function_context;
-            parser.m_state.in_arrow_function_context = m_state.in_arrow_function_context;
-            parser.m_state.in_break_context = m_state.in_break_context;
-            parser.m_state.in_continue_context = m_state.in_continue_context;
-            parser.m_state.string_legacy_octal_escape_sequence_in_scope = m_state.string_legacy_octal_escape_sequence_in_scope;
-            parser.m_state.in_class_field_initializer = m_state.in_class_field_initializer;
-
-            auto result = parser.parse_binding_pattern();
-            if (parser.has_errors())
-                m_state.errors.extend(parser.errors());
-            return result;
-        };
         if (is<ArrayExpression>(*lhs) || is<ObjectExpression>(*lhs)) {
             auto binding_pattern = synthesize_binding_pattern(*lhs);
             if (binding_pattern) {
@@ -1919,7 +1923,7 @@ Vector<FunctionNode::Parameter> Parser::parse_formal_parameters(int& function_le
     Vector<FunctionNode::Parameter> parameters;
 
     auto consume_identifier_or_binding_pattern = [&]() -> Variant<FlyString, NonnullRefPtr<BindingPattern>> {
-        if (auto pattern = parse_binding_pattern(true))
+        if (auto pattern = parse_binding_pattern(AllowDuplicates::No, AllowMemberExpressions::Yes))
             return pattern.release_nonnull();
 
         auto token = consume_identifier();
@@ -2001,7 +2005,7 @@ Vector<FunctionNode::Parameter> Parser::parse_formal_parameters(int& function_le
 
 static constexpr AK::Array<StringView, 36> s_reserved_words = { "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "import", "in", "instanceof", "new", "null", "return", "super", "switch", "this", "throw", "true", "try", "typeof", "var", "void", "while", "with" };
 
-RefPtr<BindingPattern> Parser::parse_binding_pattern(bool strict_checks)
+RefPtr<BindingPattern> Parser::parse_binding_pattern(Parser::AllowDuplicates allow_duplicates, Parser::AllowMemberExpressions allow_member_expressions)
 {
     auto rule_start = push_start();
 
@@ -2040,17 +2044,36 @@ RefPtr<BindingPattern> Parser::parse_binding_pattern(bool strict_checks)
         RefPtr<Expression> initializer = {};
 
         if (is_object) {
-            if (match_identifier_name()) {
+            bool needs_alias = false;
-                name = create_ast_node<Identifier>(
+            if (allow_member_expressions == AllowMemberExpressions::Yes && is_rest) {
-                    { m_state.current_token.filename(), rule_start.position(), position() },
+                auto expression_position = position();
-                    consume().value());
+                auto expression = parse_expression(2, Associativity::Right, { TokenType::Equals });
+                if (is<MemberExpression>(*expression))
+                    alias = static_ptr_cast<MemberExpression>(expression);
+                else if (is<Identifier>(*expression))
+                    name = static_ptr_cast<Identifier>(expression);
+                else
+                    syntax_error("Invalid destructuring assignment target", expression_position);
+            } else if (match_identifier_name() || match(TokenType::StringLiteral) || match(TokenType::NumericLiteral)) {
+                if (match(TokenType::StringLiteral) || match(TokenType::NumericLiteral))
+                    needs_alias = true;
+
+                if (match(TokenType::StringLiteral)) {
+                    auto token = consume(TokenType::StringLiteral);
+                    auto string_literal = parse_string_literal(token);
+
+                    name = create_ast_node<Identifier>(
+                        { m_state.current_token.filename(), rule_start.position(), position() },
+                        string_literal->value());
+                } else {
+                    name = create_ast_node<Identifier>(
+                        { m_state.current_token.filename(), rule_start.position(), position() },
+                        consume().value());
+                }
             } else if (match(TokenType::BracketOpen)) {
                 consume();
                 auto expression = parse_expression(0);
 
-                if (strict_checks && m_state.in_generator_function_context && is<YieldExpression>(*expression))
-                    syntax_error("Yield expression not allowed in formal parameter within generator context");
-
                 name = move(expression);
                 consume(TokenType::BracketClose);
             } else {
@@ -2060,45 +2083,69 @@ RefPtr<BindingPattern> Parser::parse_binding_pattern(bool strict_checks)
 
             if (!is_rest && match(TokenType::Colon)) {
                 consume();
-                if (match(TokenType::CurlyOpen) || match(TokenType::BracketOpen)) {
+                if (allow_member_expressions == AllowMemberExpressions::Yes) {
-                    auto binding_pattern = parse_binding_pattern(strict_checks);
+                    auto expression_position = position();
+                    auto expression = parse_expression(2, Associativity::Right, { TokenType::Equals });
+                    if (is<ArrayExpression>(*expression) || is<ObjectExpression>(*expression)) {
+                        if (auto synthesized_binding_pattern = synthesize_binding_pattern(*expression))
+                            alias = synthesized_binding_pattern.release_nonnull();
+                        else
+                            syntax_error("Invalid destructuring assignment target", expression_position);
+                    } else if (is<MemberExpression>(*expression)) {
+                        alias = static_ptr_cast<MemberExpression>(expression);
+                    } else if (is<Identifier>(*expression)) {
+                        alias = static_ptr_cast<Identifier>(expression);
+                    } else {
+                        syntax_error("Invalid destructuring assignment target", expression_position);
+                    }
+                } else if (match(TokenType::CurlyOpen) || match(TokenType::BracketOpen)) {
+                    auto binding_pattern = parse_binding_pattern(allow_duplicates, allow_member_expressions);
                     if (!binding_pattern)
                         return {};
                     alias = binding_pattern.release_nonnull();
                 } else if (match_identifier_name()) {
                     alias = create_ast_node<Identifier>(
                         { m_state.current_token.filename(), rule_start.position(), position() },
-                        consume_identifier().value());
+                        consume().value());
-                    if (strict_checks && match(TokenType::BracketOpen))
+
-                        syntax_error("Illegal property in declaration context");
                 } else {
                     expected("identifier or binding pattern");
                     return {};
                 }
+            } else if (needs_alias) {
+                expected("alias for string or numeric literal name");
+                return {};
             }
         } else {
-            if (match_identifier_name()) {
+            if (allow_member_expressions == AllowMemberExpressions::Yes) {
-                // BindingElement must always have an Empty name field
+                auto expression_position = position();
-                auto identifier_name = consume_identifier().value();
+                auto expression = parse_expression(2, Associativity::Right, { TokenType::Equals });
-                alias = create_ast_node<Identifier>(
-                    { m_state.current_token.filename(), rule_start.position(), position() },
-                    identifier_name);
 
-                if (strict_checks) {
+                if (is<ArrayExpression>(*expression) || is<ObjectExpression>(*expression)) {
-                    for (auto& entry : entries) {
+                    if (auto synthesized_binding_pattern = synthesize_binding_pattern(*expression))
-                        if (entry.alias.has<NonnullRefPtr<Identifier>>()) {
+                        alias = synthesized_binding_pattern.release_nonnull();
-                            if (entry.alias.get<NonnullRefPtr<Identifier>>()->string() == identifier_name)
+                    else
-                                syntax_error("Duplicate parameter names in bindings");
+                        syntax_error("Invalid destructuring assignment target", expression_position);
-                        }
+                } else if (is<MemberExpression>(*expression)) {
-                    }
+                    alias = static_ptr_cast<MemberExpression>(expression);
+                } else if (is<Identifier>(*expression)) {
+                    alias = static_ptr_cast<Identifier>(expression);
+                } else {
+                    syntax_error("Invalid destructuring assignment target", expression_position);
                 }
             } else if (match(TokenType::BracketOpen) || match(TokenType::CurlyOpen)) {
-                auto pattern = parse_binding_pattern(strict_checks);
+                auto pattern = parse_binding_pattern(allow_duplicates, allow_member_expressions);
                 if (!pattern) {
                     expected("binding pattern");
                     return {};
                 }
                 alias = pattern.release_nonnull();
+            } else if (match_identifier_name()) {
+                // BindingElement must always have an Empty name field
+                auto identifier_name = consume_identifier().value();
+                alias = create_ast_node<Identifier>(
+                    { m_state.current_token.filename(), rule_start.position(), position() },
+                    identifier_name);
             } else {
                 expected("identifier or binding pattern");
                 return {};
@@ -2118,8 +2165,6 @@ RefPtr<BindingPattern> Parser::parse_binding_pattern(bool strict_checks)
                 expected("initialization expression");
                 return {};
             }
-            if (strict_checks && is<YieldExpression>(*initializer))
-                syntax_error("Yield expression is not allow in formal parameter");
         }
 
         entries.append(BindingPattern::BindingEntry { move(name), move(alias), move(initializer), is_rest });
@@ -2145,7 +2190,7 @@ RefPtr<BindingPattern> Parser::parse_binding_pattern(bool strict_checks)
 
     Vector<StringView> bound_names;
     pattern->for_each_bound_name([&](auto& name) {
-        if (strict_checks) {
+        if (allow_duplicates == AllowDuplicates::No) {
             if (bound_names.contains_slow(name))
                 syntax_error("Duplicate parameter names in bindings");
             bound_names.append(name);
@@ -2231,25 +2276,13 @@ NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration(bool for_l
                     check_declarations(declaration);
                 }
             }
-        } else if (auto pattern = parse_binding_pattern(declaration_kind != DeclarationKind::Var)) {
+        } else if (auto pattern = parse_binding_pattern(declaration_kind != DeclarationKind::Var ? AllowDuplicates::No : AllowDuplicates::Yes, AllowMemberExpressions::No)) {
             target = pattern.release_nonnull();
 
             if ((declaration_kind == DeclarationKind::Let || declaration_kind == DeclarationKind::Const)) {
-                target.get<NonnullRefPtr<BindingPattern>>()->for_each_bound_name([this, &declarations](auto& name) {
+                target.get<NonnullRefPtr<BindingPattern>>()->for_each_bound_name([this](auto& name) {
                     if (name == "let"sv)
                         syntax_error("Lexical binding may not be called 'let'");
-
-                    // FIXME: Again we do not check everything here see for example the parameter check above.
-                    //        However a more sustainable solution should be used since these checks are now spread over multiple sites.
-
-                    for (auto& declaration : declarations) {
-                        declaration.target().visit(
-                            [&](NonnullRefPtr<Identifier> const& identifier) {
-                                if (identifier->string() == name)
-                                    syntax_error(String::formatted("Identifier '{}' has already been declared", name));
-                            },
-                            [&](auto const&) {});
-                    }
                 });
             }
         } else if (!m_state.in_generator_function_context && match(TokenType::Yield)) {
@@ -2612,7 +2645,7 @@ NonnullRefPtr<CatchClause> Parser::parse_catch_clause()
         if (match_identifier_name() && (!match(TokenType::Yield) || !m_state.in_generator_function_context))
             parameter = consume().value();
         else
-            pattern_parameter = parse_binding_pattern(true);
+            pattern_parameter = parse_binding_pattern(AllowDuplicates::No, AllowMemberExpressions::No);
         consume(TokenType::ParenClose);
     }
 

Userland/Libraries/LibJS/Parser.h

@@ -42,7 +42,18 @@ public:
     template<typename FunctionNodeType>
     NonnullRefPtr<FunctionNodeType> parse_function_node(u8 parse_options = FunctionNodeParseOptions::CheckForFunctionAndName);
     Vector<FunctionNode::Parameter> parse_formal_parameters(int& function_length, u8 parse_options = 0);
-    RefPtr<BindingPattern> parse_binding_pattern(bool strict_checks = false);
+
+    enum class AllowDuplicates {
+        Yes,
+        No
+    };
+
+    enum class AllowMemberExpressions {
+        Yes,
+        No
+    };
+
+    RefPtr<BindingPattern> parse_binding_pattern(AllowDuplicates is_var_declaration = AllowDuplicates::No, AllowMemberExpressions allow_member_expressions = AllowMemberExpressions::No);
 
     struct PrimaryExpressionParseResult {
         NonnullRefPtr<Expression> result;
@@ -179,6 +190,8 @@ private:
     void discard_saved_state();
     Position position() const;
 
+    RefPtr<BindingPattern> synthesize_binding_pattern(Expression const& expression);
+
     Token next_token();
 
     void check_identifier_name_for_assignment_validity(StringView, bool force_strict = false);

Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -197,6 +197,20 @@ void VM::assign(const NonnullRefPtr<BindingPattern>& target, Value value, Global
 
             auto& entry = binding.entries[i];
 
+            Optional<Reference> assignment_target;
+            entry.alias.visit(
+                [&](Empty) {},
+                [&](NonnullRefPtr<Identifier> const&) {
+                    // FIXME: We need to get the reference but bindings are broken so this doesn't work yet.
+                },
+                [&](NonnullRefPtr<BindingPattern> const&) {},
+                [&](NonnullRefPtr<MemberExpression> const& member_expression) {
+                    assignment_target = member_expression->to_reference(interpreter(), global_object);
+                });
+
+            if (exception())
+                return;
+
             if (entry.is_rest) {
                 VERIFY(i == binding.entries.size() - 1);
 
@@ -254,8 +268,15 @@ void VM::assign(const NonnullRefPtr<BindingPattern>& target, Value value, Global
                 },
                 [&](NonnullRefPtr<BindingPattern> const& pattern) {
                     assign(pattern, value, global_object, first_assignment, specific_scope);
+                },
+                [&](NonnullRefPtr<MemberExpression> const&) {
+                    VERIFY(assignment_target.has_value());
+                    assignment_target->put_value(global_object, value);
                 });
 
+            if (exception())
+                return;
+
             if (entry.is_rest)
                 break;
         }
@@ -269,10 +290,21 @@ void VM::assign(const NonnullRefPtr<BindingPattern>& target, Value value, Global
             VERIFY(!property.is_elision());
 
             PropertyName assignment_name;
+            Optional<Reference> assignment_target;
+
             JS::Value value_to_assign;
             if (property.is_rest) {
-                VERIFY(property.name.has<NonnullRefPtr<Identifier>>());
+                if (auto identifier_ptr = property.name.get_pointer<NonnullRefPtr<Identifier>>())
-                assignment_name = property.name.get<NonnullRefPtr<Identifier>>()->string();
+                    assignment_name = (*identifier_ptr)->string();
+                property.alias.visit(
+                    [&](Empty) {},
+                    [&](NonnullRefPtr<Identifier> const&) {
+                        // FIXME: We need to get the reference but bindings are broken so this doesn't work yet.
+                    },
+                    [&](NonnullRefPtr<BindingPattern> const&) {},
+                    [&](NonnullRefPtr<MemberExpression> const& member_expression) {
+                        assignment_target = member_expression->to_reference(interpreter(), global_object);
+                    });
 
                 auto* rest_object = Object::create(global_object, global_object.object_prototype());
                 for (auto& object_property : object->shape().property_table()) {
@@ -298,6 +330,15 @@ void VM::assign(const NonnullRefPtr<BindingPattern>& target, Value value, Global
                             return;
                         assignment_name = result.to_property_key(global_object);
                     });
+                property.alias.visit(
+                    [&](Empty) {},
+                    [&](NonnullRefPtr<Identifier> const&) {
+                        // FIXME: We need to get the reference but bindings are broken so this doesn't work yet.
+                    },
+                    [&](NonnullRefPtr<BindingPattern> const&) {},
+                    [&](NonnullRefPtr<MemberExpression> const& member_expression) {
+                        assignment_target = member_expression->to_reference(interpreter(), global_object);
+                    });
 
                 if (exception())
                     break;
@@ -327,8 +368,15 @@ void VM::assign(const NonnullRefPtr<BindingPattern>& target, Value value, Global
                 [&](NonnullRefPtr<BindingPattern> const& pattern) {
                     VERIFY(!property.is_rest);
                     assign(pattern, value_to_assign, global_object, first_assignment, specific_scope);
+                },
+                [&](NonnullRefPtr<MemberExpression> const&) {
+                    VERIFY(assignment_target.has_value());
+                    assignment_target->put_value(global_object, value_to_assign);
                 });
 
+            if (exception())
+                return;
+
             if (property.is_rest)
                 break;
         }