LibJS: Use Identifier to represent FunctionParameter name

Using identifier instead of string allows to store supplemental
information about whether it can be represented as local variable.

Userland/Libraries/LibJS/AST.cpp

@@ -2537,11 +2537,14 @@ void FunctionNode::dump(int indent, DeprecatedString const& class_name) const
 
         for (auto& parameter : m_parameters) {
             parameter.binding.visit(
-                [&](DeprecatedFlyString const& name) {
-                    print_indent(indent + 2);
-                    if (parameter.is_rest)
+                [&](Identifier const& identifier) {
+                    if (parameter.is_rest) {
+                        print_indent(indent + 2);
                         out("...");
-                    outln("{}", name);
+                        identifier.dump(0);
+                    } else {
+                        identifier.dump(indent + 2);
+                    }
                 },
                 [&](BindingPattern const& pattern) {
                     pattern.dump(indent + 2);
@@ -4479,6 +4482,16 @@ ThrowCompletionOr<void> ScopeNode::for_each_lexically_declared_name(ThrowComplet
     return {};
 }
 
+ThrowCompletionOr<void> ScopeNode::for_each_lexically_declared_identifier(ThrowCompletionOrVoidCallback<Identifier const&>&& callback) const
+{
+    for (auto const& declaration : m_lexical_declarations) {
+        TRY(declaration->for_each_bound_identifier([&](auto const& identifier) {
+            return callback(identifier);
+        }));
+    }
+    return {};
+}
+
 ThrowCompletionOr<void> ScopeNode::for_each_var_declared_name(ThrowCompletionOrVoidCallback<DeprecatedFlyString const&>&& callback) const
 {
     for (auto& declaration : m_var_declarations) {
@@ -4770,7 +4783,9 @@ ThrowCompletionOr<void> Program::global_declaration_instantiation(VM& vm, Global
     // 1. Let lexNames be the LexicallyDeclaredNames of script.
     // 2. Let varNames be the VarDeclaredNames of script.
     // 3. For each element name of lexNames, do
-    TRY(for_each_lexically_declared_name([&](DeprecatedFlyString const& name) -> ThrowCompletionOr<void> {
+    TRY(for_each_lexically_declared_identifier([&](Identifier const& identifier) -> ThrowCompletionOr<void> {
+        auto const& name = identifier.string();
+
         // a. If env.HasVarDeclaration(name) is true, throw a SyntaxError exception.
         if (global_environment.has_var_declaration(name))
             return vm.throw_completion<SyntaxError>(ErrorType::TopLevelVariableAlreadyDeclared, name);

Userland/Libraries/LibJS/AST.h

@@ -308,6 +308,7 @@ public:
 
     ThrowCompletionOr<void> for_each_lexically_scoped_declaration(ThrowCompletionOrVoidCallback<Declaration const&>&& callback) const;
     ThrowCompletionOr<void> for_each_lexically_declared_name(ThrowCompletionOrVoidCallback<DeprecatedFlyString const&>&& callback) const;
+    ThrowCompletionOr<void> for_each_lexically_declared_identifier(ThrowCompletionOrVoidCallback<Identifier const&>&& callback) const;
 
     ThrowCompletionOr<void> for_each_var_declared_name(ThrowCompletionOrVoidCallback<DeprecatedFlyString const&>&& callback) const;
     ThrowCompletionOr<void> for_each_var_declared_identifier(ThrowCompletionOrVoidCallback<Identifier const&>&& callback) const;
@@ -665,12 +666,6 @@ struct BindingPattern : RefCounted<BindingPattern> {
     Kind kind { Kind::Object };
 };
 
-struct FunctionParameter {
-    Variant<DeprecatedFlyString, NonnullRefPtr<BindingPattern const>> binding;
-    RefPtr<Expression const> default_value;
-    bool is_rest { false };
-};
-
 class Identifier final : public Expression {
 public:
     explicit Identifier(SourceRange source_range, DeprecatedFlyString string)
@@ -703,6 +698,12 @@ private:
     Optional<size_t> m_local_variable_index;
 };
 
+struct FunctionParameter {
+    Variant<NonnullRefPtr<Identifier const>, NonnullRefPtr<BindingPattern const>> binding;
+    RefPtr<Expression const> default_value;
+    bool is_rest { false };
+};
+
 class FunctionNode {
 public:
     StringView name() const { return m_name ? m_name->string().view() : ""sv; }

Userland/Libraries/LibJS/Parser.cpp

@@ -79,8 +79,8 @@ public:
         scope_pusher.m_function_parameters = parameters;
         for (auto& parameter : parameters) {
             parameter.binding.visit(
-                [&](DeprecatedFlyString const& name) {
-                    scope_pusher.m_forbidden_lexical_names.set(name);
+                [&](Identifier const& identifier) {
+                    scope_pusher.m_forbidden_lexical_names.set(identifier.string());
                 },
                 [&](NonnullRefPtr<BindingPattern const> const& binding_pattern) {
                     // NOTE: Nothing in the callback throws an exception.
@@ -864,7 +864,7 @@ static bool is_strict_reserved_word(StringView str)
 static bool is_simple_parameter_list(Vector<FunctionParameter> const& parameters)
 {
     return all_of(parameters, [](FunctionParameter const& parameter) {
-        return !parameter.is_rest && parameter.default_value.is_null() && parameter.binding.has<DeprecatedFlyString>();
+        return !parameter.is_rest && parameter.default_value.is_null() && parameter.binding.has<NonnullRefPtr<Identifier const>>();
     });
 }
 
@@ -939,7 +939,8 @@ RefPtr<FunctionExpression const> Parser::try_parse_arrow_function_expression(boo
             syntax_error("BindingIdentifier may not be 'arguments' or 'eval' in strict mode");
         if (is_async && token.value() == "await"sv)
             syntax_error("'await' is a reserved identifier in async functions");
-        parameters.append({ DeprecatedFlyString { token.value() }, {} });
+        auto identifier = create_ast_node<Identifier const>({ m_source_code, rule_start.position(), position() }, token.DeprecatedFlyString_value());
+        parameters.append({ identifier, {} });
     }
     // If there's a newline between the closing paren and arrow it's not a valid arrow function,
     // ASI should kick in instead (it'll then fail with "Unexpected token Arrow")
@@ -1003,8 +1004,8 @@ RefPtr<FunctionExpression const> Parser::try_parse_arrow_function_expression(boo
     if (body->in_strict_mode()) {
         for (auto& parameter : parameters) {
             parameter.binding.visit(
-                [&](DeprecatedFlyString const& name) {
-                    check_identifier_name_for_assignment_validity(name, true);
+                [&](Identifier const& identifier) {
+                    check_identifier_name_for_assignment_validity(identifier.string(), true);
                 },
                 [&](auto const&) {});
         }
@@ -1495,7 +1496,7 @@ NonnullRefPtr<ClassExpression const> Parser::parse_class_expression(bool expect_
             //          this function does not.
             //          So we use a custom version of SuperCall which doesn't use the @@iterator
             //          method on %Array.prototype% visibly.
-            DeprecatedFlyString argument_name = "args";
+            auto argument_name = create_ast_node<Identifier const>({ m_source_code, rule_start.position(), position() }, "args");
             auto super_call = create_ast_node<SuperCall>(
                 { m_source_code, rule_start.position(), position() },
                 SuperCall::IsPartOfSyntheticConstructor::Yes,
@@ -2675,7 +2676,9 @@ NonnullRefPtr<FunctionBody const> Parser::parse_function_body(Vector<FunctionPar
         Vector<StringView> parameter_names;
         for (auto& parameter : parameters) {
             parameter.binding.visit(
-                [&](DeprecatedFlyString const& parameter_name) {
+                [&](Identifier const& identifier) {
+                    auto const& parameter_name = identifier.string();
+
                     check_identifier_name_for_assignment_validity(parameter_name, function_body->in_strict_mode());
                     if (function_kind == FunctionKind::Generator && parameter_name == "yield"sv)
                         syntax_error("Parameter name 'yield' not allowed in this context");
@@ -2842,7 +2845,7 @@ Vector<FunctionParameter> Parser::parse_formal_parameters(int& function_length,
 
     Vector<FunctionParameter> parameters;
 
-    auto consume_identifier_or_binding_pattern = [&]() -> Variant<DeprecatedFlyString, NonnullRefPtr<BindingPattern const>> {
+    auto consume_identifier_or_binding_pattern = [&]() -> Variant<NonnullRefPtr<Identifier const>, NonnullRefPtr<BindingPattern const>> {
         if (auto pattern = parse_binding_pattern(AllowDuplicates::No, AllowMemberExpressions::No))
             return pattern.release_nonnull();
 
@@ -2853,8 +2856,8 @@ Vector<FunctionParameter> Parser::parse_formal_parameters(int& function_length,
 
         for (auto& parameter : parameters) {
             bool has_same_name = parameter.binding.visit(
-                [&](DeprecatedFlyString const& name) {
-                    return name == parameter_name;
+                [&](Identifier const& identifier) {
+                    return identifier.string() == parameter_name;
                 },
                 [&](NonnullRefPtr<BindingPattern const> const& bindings) {
                     bool found_duplicate = false;
@@ -2882,7 +2885,7 @@ Vector<FunctionParameter> Parser::parse_formal_parameters(int& function_length,
                 syntax_error(message, Position { token.line_number(), token.line_column() });
             break;
         }
-        return DeprecatedFlyString { token.value() };
+        return create_ast_node<Identifier const>({ m_source_code, rule_start.position(), position() }, token.DeprecatedFlyString_value());
     };
 
     while (match(TokenType::CurlyOpen) || match(TokenType::BracketOpen) || match_identifier() || match(TokenType::TripleDot)) {

Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp

@@ -1128,7 +1128,7 @@ Object* create_mapped_arguments_object(VM& vm, FunctionObject& function, Vector<
     VERIFY(formals.size() <= NumericLimits<i32>::max());
     for (i32 index = static_cast<i32>(formals.size()) - 1; index >= 0; --index) {
         // a. Let name be parameterNames[index].
-        auto const& name = formals[index].binding.get<DeprecatedFlyString>();
+        auto const& name = formals[index].binding.get<NonnullRefPtr<Identifier const>>()->string();
 
         // b. If name is not an element of mappedNames, then
         if (mapped_names.contains(name))

Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp

@@ -94,7 +94,7 @@ ECMAScriptFunctionObject::ECMAScriptFunctionObject(DeprecatedFlyString name, Dep
             return false;
         if (parameter.default_value)
             return false;
-        if (!parameter.binding.template has<DeprecatedFlyString>())
+        if (!parameter.binding.template has<NonnullRefPtr<Identifier const>>())
             return false;
         return true;
     });
@@ -353,8 +353,8 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia
             has_parameter_expressions = true;
 
         parameter.binding.visit(
-            [&](DeprecatedFlyString const& name) {
-                if (parameter_names.set(name) != AK::HashSetResult::InsertedNewEntry)
+            [&](Identifier const& identifier) {
+                if (parameter_names.set(identifier.string()) != AK::HashSetResult::InsertedNewEntry)
                     has_duplicates = true;
             },
             [&](NonnullRefPtr<BindingPattern const> const& pattern) {
@@ -362,8 +362,8 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia
                     has_parameter_expressions = true;
 
                 // NOTE: Nothing in the callback throws an exception.
-                MUST(pattern->for_each_bound_name([&](auto& name) {
-                    if (parameter_names.set(name) != AK::HashSetResult::InsertedNewEntry)
+                MUST(pattern->for_each_bound_identifier([&](auto& identifier) {
+                    if (parameter_names.set(identifier.string()) != AK::HashSetResult::InsertedNewEntry)
                         has_duplicates = true;
                 }));
             });
@@ -478,8 +478,8 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia
 
                 Environment* used_environment = has_duplicates ? nullptr : environment;
 
-                if constexpr (IsSame<DeprecatedFlyString const&, decltype(param)>) {
-                    Reference reference = TRY(vm.resolve_binding(param, used_environment));
+                if constexpr (IsSame<NonnullRefPtr<Identifier const> const&, decltype(param)>) {
+                    Reference reference = TRY(vm.resolve_binding(param->string(), used_environment));
                     // Here the difference from hasDuplicates is important
                     if (has_duplicates)
                         return reference.put_value(vm, argument_value);