diff --git a/crates/oxc_semantic/src/binder.rs b/crates/oxc_semantic/src/binder.rs index aaf5bd88e9..30bc5947a0 100644 --- a/crates/oxc_semantic/src/binder.rs +++ b/crates/oxc_semantic/src/binder.rs @@ -4,7 +4,7 @@ use smallvec::SmallVec; use oxc_allocator::{GetAddress, UnstableAddress}; use oxc_ast::{AstKind, ast::*}; -use oxc_ecmascript::{BoundNames, IsSimpleParameterList}; +use oxc_ecmascript::BoundNames; use oxc_span::GetSpan; use oxc_str::Ident; use oxc_syntax::{node::NodeId, scope::ScopeFlags, scope::ScopeId, symbol::SymbolFlags}; @@ -250,30 +250,8 @@ impl<'a> Binder<'a> for BindingRestElement<'a> { impl<'a> Binder<'a> for FormalParameter<'a> { // Binds the FormalParameter of a function or method. fn bind(&self, builder: &mut SemanticBuilder<'a>) { - let parent_kind = builder.ancestry().parent_kind(); - let AstKind::FormalParameters(parameters) = parent_kind else { unreachable!() }; - let includes = SymbolFlags::FunctionScopedVariable; - - let is_not_allowed_duplicate_parameters = matches!( - parameters.kind, - // ArrowFormalParameters: UniqueFormalParameters - FormalParameterKind::ArrowFormalParameters | - // UniqueFormalParameters : FormalParameters - // * It is a Syntax Error if BoundNames of FormalParameters contains any duplicate elements. - FormalParameterKind::UniqueFormalParameters - ) || - // Multiple occurrences of the same BindingIdentifier in a FormalParameterList is only allowed for functions which have simple parameter lists and which are not defined in strict mode code. - builder.strict_mode() || - // FormalParameters : FormalParameterList - // * It is a Syntax Error if IsSimpleParameterList of FormalParameterList is false and BoundNames of FormalParameterList contains any duplicate elements. - !parameters.is_simple_parameter_list(); - - let excludes = if is_not_allowed_duplicate_parameters { - SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes - } else { - SymbolFlags::FunctionScopedVariableExcludes - }; + let excludes = builder.formal_parameter_excludes; self.bound_names(&mut |ident| { let symbol_id = builder.declare_symbol(ident.span, ident.name, includes, excludes); @@ -285,30 +263,11 @@ impl<'a> Binder<'a> for FormalParameter<'a> { impl<'a> Binder<'a> for FormalParameterRest<'a> { // Binds the FormalParameter of a function or method. fn bind(&self, builder: &mut SemanticBuilder<'a>) { - let parent_kind = builder.ancestry().parent_kind(); - let AstKind::FormalParameters(parameters) = parent_kind else { unreachable!() }; - let includes = SymbolFlags::FunctionScopedVariable; - - let is_not_allowed_duplicate_parameters = matches!( - parameters.kind, - // ArrowFormalParameters: UniqueFormalParameters - FormalParameterKind::ArrowFormalParameters | - // UniqueFormalParameters : FormalParameters - // * It is a Syntax Error if BoundNames of FormalParameters contains any duplicate elements. - FormalParameterKind::UniqueFormalParameters - ) || - // Multiple occurrences of the same BindingIdentifier in a FormalParameterList is only allowed for functions which have simple parameter lists and which are not defined in strict mode code. - builder.strict_mode() || - // FormalParameters : FormalParameterList - // * It is a Syntax Error if IsSimpleParameterList of FormalParameterList is false and BoundNames of FormalParameterList contains any duplicate elements. - !parameters.is_simple_parameter_list(); - - let excludes = if is_not_allowed_duplicate_parameters { - SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes - } else { - SymbolFlags::FunctionScopedVariableExcludes - }; + // A rest parameter makes the parameter list non-simple in every context, + // so duplicate bindings are always forbidden. + let excludes = + SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes; self.rest.argument.bound_names(&mut |ident| { let symbol_id = builder.declare_symbol(ident.span, ident.name, includes, excludes); diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 2ffd241792..ce812a12d5 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -17,6 +17,7 @@ use oxc_cfg::{ IterationInstructionKind, ReturnInstructionKind, }; use oxc_diagnostics::{Diagnostics, OxcDiagnostic}; +use oxc_ecmascript::IsSimpleParameterList; use oxc_span::{SourceType, Span}; use oxc_str::Ident; use oxc_syntax::{ @@ -81,6 +82,8 @@ pub struct SemanticBuilder<'a> { pub(crate) current_scope_id: ScopeId, pub(crate) module_instance_state_cache: FxHashMap, current_reference_flags: ReferenceFlags, + /// Duplicate-binding exclusions for the parameter list currently being visited. + pub(crate) formal_parameter_excludes: SymbolFlags, /// Nesting depth of TypeScript ambient contexts. ambient_depth: u32, /// Symbols that have been hoisted out of a scope (e.g. `var` declarations hoisted to @@ -149,6 +152,7 @@ impl<'a> SemanticBuilder<'a> { source_type: SourceType::default(), errors: RefCell::new(Diagnostics::new()), current_reference_flags: ReferenceFlags::empty(), + formal_parameter_excludes: SymbolFlags::FunctionScopedVariableExcludes, ambient_depth: 0, current_scope_id, module_instance_state_cache: FxHashMap::default(), @@ -2283,6 +2287,25 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { self.leave_node(kind); } + fn visit_formal_parameters(&mut self, it: &FormalParameters<'a>) { + // Checking whether a list is simple scans its parameters. Do this once per + // list, rather than once per parameter when binding each name. + let mut excludes = SymbolFlags::FunctionScopedVariableExcludes; + if matches!( + it.kind, + FormalParameterKind::ArrowFormalParameters + | FormalParameterKind::UniqueFormalParameters + ) || self.strict_mode() + || !it.is_simple_parameter_list() + { + excludes |= SymbolFlags::FunctionScopedVariable; + } + let previous = mem::replace(&mut self.formal_parameter_excludes, excludes); + oxc_ast_visit::walk::walk_formal_parameters(self, it); + // Default values and type annotations can contain nested parameter lists. + self.formal_parameter_excludes = previous; + } + fn visit_update_expression(&mut self, it: &UpdateExpression<'a>) { let kind = AstKind::UpdateExpression(self.alloc(it)); self.enter_node(kind); diff --git a/tasks/coverage/misc/fail/duplicate-parameter-after-nested-default.cjs b/tasks/coverage/misc/fail/duplicate-parameter-after-nested-default.cjs new file mode 100644 index 0000000000..81b89f32f0 --- /dev/null +++ b/tasks/coverage/misc/fail/duplicate-parameter-after-nested-default.cjs @@ -0,0 +1 @@ +function outer(value = function (inner, inner) {}, value) {} diff --git a/tasks/coverage/snapshots/lexer_misc.snap b/tasks/coverage/snapshots/lexer_misc.snap index 86e4284302..2ce05e0c87 100644 --- a/tasks/coverage/snapshots/lexer_misc.snap +++ b/tasks/coverage/snapshots/lexer_misc.snap @@ -1,3 +1,3 @@ lexer_misc Summary: -AST Parsed : 398/398 (100.00%) -Positive Passed: 398/398 (100.00%) +AST Parsed : 399/399 (100.00%) +Positive Passed: 399/399 (100.00%) diff --git a/tasks/coverage/snapshots/parser_misc.snap b/tasks/coverage/snapshots/parser_misc.snap index 56d89f5b5e..23fce65842 100644 --- a/tasks/coverage/snapshots/parser_misc.snap +++ b/tasks/coverage/snapshots/parser_misc.snap @@ -1,7 +1,7 @@ parser_misc Summary: AST Parsed : 131/131 (100.00%) Positive Passed: 131/131 (100.00%) -Negative Passed: 269/269 (100.00%) +Negative Passed: 270/270 (100.00%) × TS(1275): 'accessor' modifier can only appear on a property declaration. ╭─[misc/fail/accessor-modifier-abstract-method.ts:1:29] @@ -508,6 +508,14 @@ Negative Passed: 269/269 (100.00%) If you're in the middle of a rebase, the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased. If you have nested conflicts, resolve the outermost conflict first. + × Identifier `value` has already been declared + ╭─[misc/fail/duplicate-parameter-after-nested-default.cjs:1:16] + 1 │ function outer(value = function (inner, inner) {}, value) {} + · ──┬── ──┬── + · │ ╰── It can not be redeclared here + · ╰── `value` has already been declared here + ╰──── + × Identifier `#x` has already been declared ╭─[misc/fail/duplicate-private-declarations.js:1:24] 1 │ class DuplicateField { #x; x; #x; }