perf(semantic): check duplicate parameters in Binder of FormalParameters (#1840)

crates/oxc_linter/src/rules/unicorn/no_array_for_each.rs

@@ -118,8 +118,8 @@ fn test() {
         r"array.forEach(({foo}, index = foo) => {})",
         r"array.forEach((element, {bar = element}) => {})",
         r"array.forEach(({foo}, {bar = foo}) => {})",
-        r"foo.forEach(function(element, element) {})",
-        r"foo.forEach(function element(element, element) {})",
+        r"foo.forEach(function(element, element1) {})",
+        r"foo.forEach(function element(element, element1) {})",
         r"this._listeners.forEach((listener: () => void) => listener());",
         r"return foo.forEach(element => {bar(element)});",
     ];

crates/oxc_linter/src/snapshots/no_array_for_each.snap

@@ -74,14 +74,14 @@ expression: no_array_for_each
 
   ⚠ eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
    ╭─[no_array_for_each.tsx:1:1]
- 1 │ foo.forEach(function(element, element) {})
+ 1 │ foo.forEach(function(element, element1) {})
    ·     ───────
    ╰────
   help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.
 
   ⚠ eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
    ╭─[no_array_for_each.tsx:1:1]
- 1 │ foo.forEach(function element(element, element) {})
+ 1 │ foo.forEach(function element(element, element1) {})
    ·     ───────
    ╰────
   help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.

crates/oxc_semantic/src/binder.rs

@@ -2,7 +2,10 @@
 
 #[allow(clippy::wildcard_imports)]
 use oxc_ast::ast::*;
-use oxc_ast::{syntax_directed_operations::BoundNames, AstKind};
+use oxc_ast::{
+    syntax_directed_operations::{BoundNames, IsSimpleParameterList},
+    AstKind,
+};
 use oxc_span::{Atom, SourceType};
 
 use crate::{scope::ScopeFlags, symbol::SymbolFlags, SemanticBuilder, VariableInfo};
@@ -182,9 +185,29 @@ impl<'a> Binder for Function<'a> {
 }
 
 impl<'a> Binder for FormalParameters<'a> {
+    // Binds the formal parameters of a function or method.
     fn bind(&self, builder: &mut SemanticBuilder) {
         let includes = SymbolFlags::FunctionScopedVariable;
-        let excludes = SymbolFlags::FunctionScopedVariableExcludes;
+
+        let is_not_allowed_duplicate_parameters = matches!(
+                self.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.
+            !self.is_simple_parameter_list();
+
+        let excludes = if is_not_allowed_duplicate_parameters {
+            SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes
+        } else {
+            SymbolFlags::FunctionScopedVariableExcludes
+        };
         let is_signature = self.kind == FormalParameterKind::Signature;
         self.bound_names(&mut |ident| {
             if !is_signature {

crates/oxc_semantic/src/checker/javascript.rs

@@ -1,7 +1,7 @@
 #[allow(clippy::wildcard_imports)]
 use oxc_ast::{
     ast::*,
-    syntax_directed_operations::{BoundNames, IsSimpleParameterList, PropName},
+    syntax_directed_operations::{IsSimpleParameterList, PropName},
     AstKind,
 };
 use oxc_diagnostics::{
@@ -15,7 +15,6 @@ use oxc_syntax::{
     NumberBase,
 };
 use phf::{phf_set, Set};
-use rustc_hash::FxHashMap;
 
 use crate::{builder::SemanticBuilder, diagnostics::Redeclaration, scope::ScopeFlags, AstNode};
 
@@ -157,15 +156,6 @@ fn check_module_record(ctx: &SemanticBuilder<'_>) {
     }
 }
 
-fn check_duplicate_bound_names<T: BoundNames>(bound_names: &T, ctx: &SemanticBuilder<'_>) {
-    let mut idents: FxHashMap<Atom, Span> = FxHashMap::default();
-    bound_names.bound_names(&mut |ident| {
-        if let Some(old_span) = idents.insert(ident.name.clone(), ident.span) {
-            ctx.error(Redeclaration(ident.name.clone(), old_span, ident.span));
-        }
-    });
-}
-
 #[derive(Debug, Error, Diagnostic)]
 #[error("Cannot use await in class static initialization block")]
 #[diagnostic()]
@@ -936,20 +926,6 @@ fn check_formal_parameters<'a>(
             ctx.error(ARestParameterCannotHaveAnInitializer(pat.span));
         }
     }
-
-    if params.is_empty() {
-        return;
-    }
-
-    // Note: all other cases forbid duplicate parameter names.
-    if params.kind == FormalParameterKind::FormalParameter
-        && !ctx.strict_mode()
-        && params.is_simple_parameter_list()
-    {
-        return;
-    }
-
-    check_duplicate_bound_names(params, ctx);
 }
 
 fn check_array_pattern(pattern: &ArrayPattern, ctx: &SemanticBuilder<'_>) {

crates/oxc_semantic/src/checker/typescript.rs

@@ -1,12 +1,14 @@
+use oxc_ast::syntax_directed_operations::BoundNames;
 #[allow(clippy::wildcard_imports)]
 use oxc_ast::{ast::*, AstKind};
 use oxc_diagnostics::{
     miette::{self, Diagnostic},
     thiserror::Error,
 };
-use oxc_span::{GetSpan, Span};
+use oxc_span::{Atom, GetSpan, Span};
+use rustc_hash::FxHashMap;
 
-use crate::{builder::SemanticBuilder, AstNode};
+use crate::{builder::SemanticBuilder, diagnostics::Redeclaration, AstNode};
 
 pub struct EarlyErrorTypeScript;
 
@@ -18,11 +20,27 @@ impl EarlyErrorTypeScript {
         #[allow(clippy::single_match)]
         match kind {
             AstKind::SimpleAssignmentTarget(target) => check_simple_assignment_target(target, ctx),
+            AstKind::FormalParameters(params) => check_formal_parameters(params, ctx),
             _ => {}
         }
     }
 }
 
+fn check_formal_parameters(params: &FormalParameters, ctx: &SemanticBuilder<'_>) {
+    if !params.is_empty() && params.kind == FormalParameterKind::Signature {
+        check_duplicate_bound_names(params, ctx);
+    }
+}
+
+fn check_duplicate_bound_names<T: BoundNames>(bound_names: &T, ctx: &SemanticBuilder<'_>) {
+    let mut idents: FxHashMap<Atom, Span> = FxHashMap::default();
+    bound_names.bound_names(&mut |ident| {
+        if let Some(old_span) = idents.insert(ident.name.clone(), ident.span) {
+            ctx.error(Redeclaration(ident.name.clone(), old_span, ident.span));
+        }
+    });
+}
+
 fn check_simple_assignment_target<'a>(
     target: &SimpleAssignmentTarget<'a>,
     ctx: &SemanticBuilder<'a>,