diff --git a/Libraries/LibJS/Parser.cpp b/Libraries/LibJS/Parser.cpp index 764f6b5833..3a38761ff4 100644 --- a/Libraries/LibJS/Parser.cpp +++ b/Libraries/LibJS/Parser.cpp @@ -359,10 +359,11 @@ RefPtr Parser::try_parse_arrow_function_expression(bool expe // We have parens around the function parameters and can re-use the same parsing // logic used for regular functions: multiple parameters, default values, rest // parameter, maybe a trailing comma. If we have a new syntax error afterwards we - // know parsing failed and rollback the parser state. + // check if it's about a wrong token (something like duplicate parameter name must + // not abort), know parsing failed and rollback the parser state. auto previous_syntax_errors = m_parser_state.m_errors.size(); - parameters = parse_function_parameters(function_length); - if (m_parser_state.m_errors.size() > previous_syntax_errors) + parameters = parse_function_parameters(function_length, FunctionNodeParseOptions::IsArrowFunction); + if (m_parser_state.m_errors.size() > previous_syntax_errors && m_parser_state.m_errors[previous_syntax_errors].message.starts_with("Unexpected token")) return nullptr; if (!match(TokenType::ParenClose)) return nullptr; @@ -1314,7 +1315,34 @@ NonnullRefPtr Parser::parse_function_node(u8 parse_options) Vector Parser::parse_function_parameters(int& function_length, u8 parse_options) { + bool has_default_parameter = false; + bool has_rest_parameter = false; + Vector parameters; + + auto consume_and_validate_identifier = [&]() -> Token { + auto token = consume(TokenType::Identifier); + auto parameter_name = token.value(); + + for (auto& parameter : parameters) { + if (parameter_name != parameter.name) + continue; + String message; + if (parse_options & FunctionNodeParseOptions::IsArrowFunction) + message = String::formatted("Duplicate parameter '{}' not allowed in arrow function", parameter_name); + else if (m_parser_state.m_strict_mode) + message = String::formatted("Duplicate parameter '{}' not allowed in strict mode", parameter_name); + else if (has_default_parameter || match(TokenType::Equals)) + message = String::formatted("Duplicate parameter '{}' not allowed in function with default parameter", parameter_name); + else if (has_rest_parameter) + message = String::formatted("Duplicate parameter '{}' not allowed in function with rest parameter", parameter_name); + if (!message.is_empty()) + syntax_error(message, token.line_number(), token.line_column()); + break; + } + return token; + }; + while (match(TokenType::Identifier) || match(TokenType::TripleDot)) { if (parse_options & FunctionNodeParseOptions::IsGetterFunction) syntax_error("Getter function must have no arguments"); @@ -1322,15 +1350,17 @@ Vector Parser::parse_function_parameters(int& function_ syntax_error("Setter function must have one argument"); if (match(TokenType::TripleDot)) { consume(); - auto parameter_name = consume(TokenType::Identifier).value(); + has_rest_parameter = true; + auto parameter_name = consume_and_validate_identifier().value(); function_length = parameters.size(); parameters.append({ parameter_name, nullptr, true }); break; } - auto parameter_name = consume(TokenType::Identifier).value(); + auto parameter_name = consume_and_validate_identifier().value(); RefPtr default_value; if (match(TokenType::Equals)) { - consume(TokenType::Equals); + consume(); + has_default_parameter = true; function_length = parameters.size(); default_value = parse_expression(2); } diff --git a/Libraries/LibJS/Parser.h b/Libraries/LibJS/Parser.h index 565f3ff704..4be755ca8d 100644 --- a/Libraries/LibJS/Parser.h +++ b/Libraries/LibJS/Parser.h @@ -47,6 +47,7 @@ struct FunctionNodeParseOptions { AllowSuperConstructorCall = 1 << 2, IsGetterFunction = 1 << 3, IsSetterFunction = 1 << 4, + IsArrowFunction = 1 << 5, }; }; diff --git a/Libraries/LibJS/Tests/builtins/Array/Array.prototype.forEach.js b/Libraries/LibJS/Tests/builtins/Array/Array.prototype.forEach.js index bc4b5b9336..8baa2d5635 100644 --- a/Libraries/LibJS/Tests/builtins/Array/Array.prototype.forEach.js +++ b/Libraries/LibJS/Tests/builtins/Array/Array.prototype.forEach.js @@ -51,7 +51,7 @@ describe("normal behavior", () => { test("callback receives array", () => { var callbackCalled = 0; var a = [1, 2, 3]; - a.forEach((_, _, array) => { + a.forEach((_, __, array) => { callbackCalled++; expect(a).toEqual(array); a.push("test"); diff --git a/Libraries/LibJS/Tests/functions/function-duplicate-parameters.js b/Libraries/LibJS/Tests/functions/function-duplicate-parameters.js new file mode 100644 index 0000000000..8a44337ba3 --- /dev/null +++ b/Libraries/LibJS/Tests/functions/function-duplicate-parameters.js @@ -0,0 +1,45 @@ +test("function with duplicate parameter names", () => { + function foo(bar, _, bar) { + return bar; + } + expect(foo(1, 2, 3)).toBe(3); +}); + +test("syntax errors", () => { + // Regular function in strict mode + expect(` + "use strict"; + function foo(bar, bar) {} + `).not.toEval(); + + // Arrow function in strict mode + expect(` + "use strict"; + const foo = (bar, bar) => {}; + `).not.toEval(); + + // Arrow function in non-strict mode + expect(` + const foo = (bar, bar) => {}; + `).not.toEval(); + + // Regular function with rest parameter + expect(` + function foo(bar, ...bar) {} + `).not.toEval(); + + // Arrow function with rest parameter + expect(` + const foo = (bar, ...bar) => {}; + `).not.toEval(); + + // Regular function with default parameter + expect(` + function foo(bar, bar = 1) {} + `).not.toEval(); + + // Arrow function with default parameter + expect(` + const foo = (bar, bar = 1) => {}; + `).not.toEval(); +});