fix(es/parser): enforce UniqueFormalParameters

.changeset/unique-formal-parameters.md

@@ -0,0 +1,6 @@
+---
+swc_core: patch
+swc_ecma_parser: patch
+---
+
+fix(es/parser): enforce UniqueFormalParameters for methods, arrows, and non-simple forms

crates/swc_ecma_parser/src/error.rs

@@ -214,6 +214,10 @@ pub enum SyntaxError {
     DuplicatedRegExpFlags(char),
     UnknownRegExpFlags,
 
+    /// Duplicate binding in UniqueFormalParameters / non-simple
+    /// FormalParameters.
+    DuplicateFormalParameter(Atom),
+
     TS1003,
     TS1005,
     TS1009,
@@ -592,6 +596,10 @@ impl SyntaxError {
             }
             SyntaxError::UnknownRegExpFlags => "Unknown regular expression flags.".into(),
 
+            SyntaxError::DuplicateFormalParameter(name) => {
+                format!("Duplicate parameter name not allowed in this context: {name}").into()
+            }
+
             SyntaxError::TS1003 => "Expected an identifier".into(),
             SyntaxError::TS1005 => "Expected a semicolon".into(),
             SyntaxError::TS1009 => "Trailing comma is not allowed".into(),

crates/swc_ecma_parser/src/parser/class_and_fn.rs

@@ -348,6 +348,14 @@ impl<I: Tokens> Parser<I> {
                 params.is_simple_parameter_list(),
             )?;
 
+            // `function f(a, a) { "use strict"; }` is an early error even though
+            // the parameter list is simple and the outer context may be sloppy.
+            if let Some(body) = &body {
+                if params.is_simple_parameter_list() && has_use_strict(body).is_some() {
+                    p.ensure_unique_formal_params(params.iter().map(|param| &param.pat));
+                }
+            }
+
             if p.syntax().flow() && body.is_none() && !p.ctx().contains(Context::InDeclare) {
                 p.emit_err(p.input().cur_span(), SyntaxError::TS1005);
             }
@@ -1293,7 +1301,7 @@ impl<I: Tokens> Parser<I> {
                 }));
             } else {
                 return self.make_method(
-                    Self::parse_formal_params,
+                    Self::parse_unique_formal_params,
                     MakeMethodArgs {
                         start,
                         is_optional,
@@ -1448,7 +1456,7 @@ impl<I: Tokens> Parser<I> {
                 ),
                 Token::Set => self.make_method(
                     |p| {
-                        let params = p.parse_formal_params()?;
+                        let params = p.parse_unique_formal_params()?;
 
                         if p.syntax().flow() && params.iter().any(|p| !is_not_this(p)) {
                             p.emit_err(key_span, SyntaxError::TS1003);

crates/swc_ecma_parser/src/parser/expr.rs

@@ -2716,6 +2716,8 @@ impl<I: Tokens> Parser<I> {
         };
 
         let validate_arrow_params = |p: &mut Self, params: &[Pat], is_async: bool| {
+            p.ensure_unique_formal_params(params.iter());
+
             for param in params {
                 if is_async {
                     match param {

crates/swc_ecma_parser/src/parser/object.rs

@@ -422,7 +422,7 @@ impl<I: Tokens> Parser<I> {
                                     Vec::new(),
                                     start,
                                     |p| {
-                                        let params = p.parse_formal_params()?;
+                                        let params = p.parse_unique_formal_params()?;
 
                                         if params.iter().filter(|p| is_not_this(p)).count() != 1 {
                                             p.emit_err(key_span, SyntaxError::SetterParam);

crates/swc_ecma_parser/src/parser/pat.rs

@@ -1,9 +1,11 @@
 //! 13.3.3 Destructuring Binding Patterns
 
+use rustc_hash::FxHashSet;
+use swc_atoms::Atom;
 use swc_common::Spanned;
 
 use super::*;
-use crate::parser::{expr::AssignTargetOrSpread, Parser};
+use crate::parser::{expr::AssignTargetOrSpread, util::IsSimpleParameterList, Parser};
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) enum PatType {
@@ -77,6 +79,63 @@ impl<I: Tokens> Parser<I> {
         }
     }
 
+    /// Enforce UniqueFormalParameters: BoundNames must not contain duplicates.
+    ///
+    /// Used for methods, setters, arrows, constructors, strict-mode functions,
+    /// and any FormalParameters list that is not a simple parameter list.
+    pub(crate) fn ensure_unique_formal_params<'a, Iter>(&mut self, pats: Iter)
+    where
+        Iter: IntoIterator<Item = &'a Pat>,
+    {
+        let mut names = FxHashSet::default();
+        for pat in pats {
+            self.collect_unique_formal_bindings(pat, &mut names);
+        }
+    }
+
+    fn collect_unique_formal_bindings(&mut self, pat: &Pat, names: &mut FxHashSet<Atom>) {
+        match pat {
+            Pat::Ident(i) => {
+                if !names.insert(i.id.sym.clone()) {
+                    self.emit_err(
+                        i.id.span,
+                        SyntaxError::DuplicateFormalParameter(i.id.sym.clone()),
+                    );
+                }
+            }
+            Pat::Array(arr) => {
+                for elem in arr.elems.iter().flatten() {
+                    self.collect_unique_formal_bindings(elem, names);
+                }
+            }
+            Pat::Rest(r) => self.collect_unique_formal_bindings(&r.arg, names),
+            Pat::Object(obj) => {
+                for prop in &obj.props {
+                    match prop {
+                        ObjectPatProp::KeyValue(KeyValuePatProp { value, .. })
+                        | ObjectPatProp::Rest(RestPat { arg: value, .. }) => {
+                            self.collect_unique_formal_bindings(value, names);
+                        }
+                        ObjectPatProp::Assign(AssignPatProp { key, .. }) => {
+                            if !names.insert(key.sym.clone()) {
+                                self.emit_err(
+                                    key.span,
+                                    SyntaxError::DuplicateFormalParameter(key.sym.clone()),
+                                );
+                            }
+                        }
+                        #[cfg(swc_ast_unknown)]
+                        _ => unreachable!(),
+                    }
+                }
+            }
+            Pat::Assign(a) => self.collect_unique_formal_bindings(&a.left, names),
+            Pat::Invalid(_) | Pat::Expr(_) => {}
+            #[cfg(swc_ast_unknown)]
+            _ => unreachable!(),
+        }
+    }
+
     fn assign_pat_type_ann(&mut self, pat: &mut Pat, span: Span, type_ann: Box<TsType>) {
         let type_ann = Some(Box::new(TsTypeAnn { span, type_ann }));
 
@@ -767,6 +826,36 @@ impl<I: Tokens> Parser<I> {
             }
         }
 
+        // Class constructors always use UniqueFormalParameters (class bodies are
+        // strict mode code).
+        {
+            let mut names = FxHashSet::default();
+            for p in &params {
+                match p {
+                    ParamOrTsParamProp::Param(param) => {
+                        self.collect_unique_formal_bindings(&param.pat, &mut names);
+                    }
+                    ParamOrTsParamProp::TsParamProp(prop) => match &prop.param {
+                        TsParamPropParam::Ident(i) => {
+                            if !names.insert(i.id.sym.clone()) {
+                                self.emit_err(
+                                    i.id.span,
+                                    SyntaxError::DuplicateFormalParameter(i.id.sym.clone()),
+                                );
+                            }
+                        }
+                        TsParamPropParam::Assign(a) => {
+                            self.collect_unique_formal_bindings(&a.left, &mut names);
+                        }
+                        #[cfg(swc_ast_unknown)]
+                        _ => {}
+                    },
+                    #[cfg(swc_ast_unknown)]
+                    _ => {}
+                }
+            }
+        }
+
         Ok(params)
     }
 
@@ -867,12 +956,32 @@ impl<I: Tokens> Parser<I> {
             }
         }
 
+        // UniqueFormalParameters / FormalParameters early errors:
+        // duplicate BoundNames are always illegal in strict mode, and also
+        // illegal whenever the parameter list is not simple.
+        if self.ctx().contains(Context::Strict) || !params.is_simple_parameter_list() {
+            self.ensure_unique_formal_params(params.iter().map(|p| &p.pat));
+        }
+
+        if self.ctx().contains(Context::Strict) {
+            for param in params.iter() {
+                self.pat_is_valid_argument_in_strict(&param.pat)
+            }
+        }
+
         Ok(params)
     }
 
     pub(crate) fn parse_unique_formal_params(&mut self) -> PResult<Vec<Param>> {
-        // FIXME: This is wrong
-        self.parse_formal_params()
+        let params = self.parse_formal_params()?;
+        // Methods / object methods always require UniqueFormalParameters,
+        // including simple duplicate bindings that sloppy-mode functions allow.
+        // `parse_formal_params` already checks when !simple or Strict; cover the
+        // remaining simple+non-strict case used by class/object methods.
+        if params.is_simple_parameter_list() && !self.ctx().contains(Context::Strict) {
+            self.ensure_unique_formal_params(params.iter().map(|p| &p.pat));
+        }
+        Ok(params)
     }
 
     pub(super) fn parse_paren_items_as_params(

crates/swc_ecma_parser/tests/errors/unique-formal-params/arrow/input.js

@@ -0,0 +1,4 @@
+(a, a) => a;
+async (a, a) => a;
+({a}, a) => a;
+(a, a = 1) => a;

crates/swc_ecma_parser/tests/errors/unique-formal-params/arrow/input.js.swc-stderr

@@ -0,0 +1,26 @@
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/arrow/input.js:1:1]
+ 1 | (a, a) => a;
+   :     ^
+ 2 | async (a, a) => a;
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/arrow/input.js:2:1]
+ 1 | (a, a) => a;
+ 2 | async (a, a) => a;
+   :           ^
+ 3 | ({a}, a) => a;
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/arrow/input.js:3:1]
+ 2 | async (a, a) => a;
+ 3 | ({a}, a) => a;
+   :       ^
+ 4 | (a, a = 1) => a;
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/arrow/input.js:4:1]
+ 3 | ({a}, a) => a;
+ 4 | (a, a = 1) => a;
+   :     ^
+   `----

crates/swc_ecma_parser/tests/errors/unique-formal-params/async-method/input.js

@@ -0,0 +1 @@
+({ async m(a, a) { return a; } });

crates/swc_ecma_parser/tests/errors/unique-formal-params/async-method/input.js.swc-stderr

@@ -0,0 +1,5 @@
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/async-method/input.js:1:1]
+ 1 | ({ async m(a, a) { return a; } });
+   :               ^
+   `----

crates/swc_ecma_parser/tests/errors/unique-formal-params/class-constructor/input.js

@@ -0,0 +1,6 @@
+class C {
+  constructor(a, a) {}
+}
+class D {
+  constructor([a], a) {}
+}

crates/swc_ecma_parser/tests/errors/unique-formal-params/class-constructor/input.js.swc-stderr

@@ -0,0 +1,14 @@
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/class-constructor/input.js:2:1]
+ 1 | class C {
+ 2 |   constructor(a, a) {}
+   :                  ^
+ 3 | }
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/class-constructor/input.js:5:1]
+ 4 | class D {
+ 5 |   constructor([a], a) {}
+   :                    ^
+ 6 | }
+   `----

crates/swc_ecma_parser/tests/errors/unique-formal-params/class-method/input.js

@@ -0,0 +1,6 @@
+class C {
+  m(a, a) { return a; }
+  static m2(a, a) { return a; }
+  async m3(a, a) { return a; }
+  *m4(a, a) { return a; }
+}

crates/swc_ecma_parser/tests/errors/unique-formal-params/class-method/input.js.swc-stderr

@@ -0,0 +1,28 @@
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/class-method/input.js:2:1]
+ 1 | class C {
+ 2 |   m(a, a) { return a; }
+   :        ^
+ 3 |   static m2(a, a) { return a; }
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/class-method/input.js:3:1]
+ 2 |   m(a, a) { return a; }
+ 3 |   static m2(a, a) { return a; }
+   :                ^
+ 4 |   async m3(a, a) { return a; }
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/class-method/input.js:4:1]
+ 3 |   static m2(a, a) { return a; }
+ 4 |   async m3(a, a) { return a; }
+   :               ^
+ 5 |   *m4(a, a) { return a; }
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/class-method/input.js:5:1]
+ 4 |   async m3(a, a) { return a; }
+ 5 |   *m4(a, a) { return a; }
+   :          ^
+ 6 | }
+   `----

crates/swc_ecma_parser/tests/errors/unique-formal-params/generator-method/input.js

@@ -0,0 +1 @@
+({ *m(a, a) { return a; } });

crates/swc_ecma_parser/tests/errors/unique-formal-params/generator-method/input.js.swc-stderr

@@ -0,0 +1,5 @@
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/generator-method/input.js:1:1]
+ 1 | ({ *m(a, a) { return a; } });
+   :          ^
+   `----

crates/swc_ecma_parser/tests/errors/unique-formal-params/non-simple-fn/input.js

@@ -0,0 +1,4 @@
+function f([a], a) {}
+function g({a}, a) {}
+function h(a, a = 1) {}
+function i(a = 1, a) {}

crates/swc_ecma_parser/tests/errors/unique-formal-params/non-simple-fn/input.js.swc-stderr

@@ -0,0 +1,26 @@
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/non-simple-fn/input.js:1:1]
+ 1 | function f([a], a) {}
+   :                 ^
+ 2 | function g({a}, a) {}
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/non-simple-fn/input.js:2:1]
+ 1 | function f([a], a) {}
+ 2 | function g({a}, a) {}
+   :                 ^
+ 3 | function h(a, a = 1) {}
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/non-simple-fn/input.js:3:1]
+ 2 | function g({a}, a) {}
+ 3 | function h(a, a = 1) {}
+   :               ^
+ 4 | function i(a = 1, a) {}
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/non-simple-fn/input.js:4:1]
+ 3 | function h(a, a = 1) {}
+ 4 | function i(a = 1, a) {}
+   :                   ^
+   `----

crates/swc_ecma_parser/tests/errors/unique-formal-params/object-method/input.js

@@ -0,0 +1,4 @@
+({ m(a, a) { return a; } });
+({ m([a], a) { return a; } });
+({ m({a}, a) { return a; } });
+({ m(a, a = 1) { return a; } });

crates/swc_ecma_parser/tests/errors/unique-formal-params/object-method/input.js.swc-stderr

@@ -0,0 +1,26 @@
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/object-method/input.js:1:1]
+ 1 | ({ m(a, a) { return a; } });
+   :         ^
+ 2 | ({ m([a], a) { return a; } });
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/object-method/input.js:2:1]
+ 1 | ({ m(a, a) { return a; } });
+ 2 | ({ m([a], a) { return a; } });
+   :           ^
+ 3 | ({ m({a}, a) { return a; } });
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/object-method/input.js:3:1]
+ 2 | ({ m([a], a) { return a; } });
+ 3 | ({ m({a}, a) { return a; } });
+   :           ^
+ 4 | ({ m(a, a = 1) { return a; } });
+   `----
+  x Duplicate parameter name not allowed in this context: a
+   ,-[$DIR/tests/errors/unique-formal-params/object-method/input.js:4:1]
+ 3 | ({ m({a}, a) { return a; } });
+ 4 | ({ m(a, a = 1) { return a; } });
+   :         ^
+   `----