[syntax-errors] Make duplicate parameter names a semantic error (#17131)

Status
--

This is a pretty minor change, but it was breaking a red-knot mdtest
until #17463 landed. Now this should close #11934 as the last syntax
error being tracked there!

Summary
--

Moves `Parser::validate_parameters` to
`SemanticSyntaxChecker::duplicate_parameter_name`.

Test Plan
--

Existing tests, with `## Errors` replaced with `## Semantic Syntax
Errors`.

crates/ruff_linter/src/checkers/ast/mod.rs

@@ -615,7 +615,8 @@ impl SemanticSyntaxContext for Checker<'_> {
             | SemanticSyntaxErrorKind::DuplicateMatchKey(_)
             | SemanticSyntaxErrorKind::DuplicateMatchClassAttribute(_)
             | SemanticSyntaxErrorKind::InvalidStarExpression
-            | SemanticSyntaxErrorKind::AsyncComprehensionOutsideAsyncFunction(_) => {
+            | SemanticSyntaxErrorKind::AsyncComprehensionOutsideAsyncFunction(_)
+            | SemanticSyntaxErrorKind::DuplicateParameter(_) => {
                 if self.settings.preview.is_enabled() {
                     self.semantic_errors.borrow_mut().push(error);
                 }

crates/ruff_python_parser/src/error.rs

@@ -126,8 +126,6 @@ pub enum ParseErrorType {
     /// A default value was found for a `*` or `**` parameter.
     VarParameterWithDefault,
 
-    /// A duplicate parameter was found in a function definition or lambda expression.
-    DuplicateParameter(String),
     /// A keyword argument was repeated.
     DuplicateKeywordArgumentError(String),
 
@@ -285,9 +283,6 @@ impl std::fmt::Display for ParseErrorType {
                 f.write_str("Invalid augmented assignment target")
             }
             ParseErrorType::InvalidDeleteTarget => f.write_str("Invalid delete target"),
-            ParseErrorType::DuplicateParameter(arg_name) => {
-                write!(f, "Duplicate parameter {arg_name:?}")
-            }
             ParseErrorType::DuplicateKeywordArgumentError(arg_name) => {
                 write!(f, "Duplicate keyword argument {arg_name:?}")
             }

crates/ruff_python_parser/src/parser/statement.rs

@@ -1,8 +1,6 @@
 use compact_str::CompactString;
 use std::fmt::{Display, Write};
 
-use rustc_hash::{FxBuildHasher, FxHashSet};
-
 use ruff_python_ast::name::Name;
 use ruff_python_ast::{
     self as ast, ExceptHandler, Expr, ExprContext, IpyEscapeKind, Operator, PythonVersion, Stmt,
@@ -3339,10 +3337,6 @@ impl<'src> Parser<'src> {
 
         parameters.range = self.node_range(start);
 
-        // test_err params_duplicate_names
-        // def foo(a, a=10, *a, a, a: str, **a): ...
-        self.validate_parameters(&parameters);
-
         parameters
     }
 
@@ -3630,25 +3624,6 @@ impl<'src> Parser<'src> {
         }
     }
 
-    /// Validate that the given parameters doesn't have any duplicate names.
-    ///
-    /// Report errors for all the duplicate names found.
-    fn validate_parameters(&mut self, parameters: &ast::Parameters) {
-        let mut all_arg_names =
-            FxHashSet::with_capacity_and_hasher(parameters.len(), FxBuildHasher);
-
-        for parameter in parameters {
-            let range = parameter.name().range();
-            let param_name = parameter.name().as_str();
-            if !all_arg_names.insert(param_name) {
-                self.add_error(
-                    ParseErrorType::DuplicateParameter(param_name.to_string()),
-                    range,
-                );
-            }
-        }
-    }
-
     /// Classify the `match` soft keyword token.
     ///
     /// # Panics

crates/ruff_python_parser/src/semantic_errors.rs

@@ -13,7 +13,7 @@ use ruff_python_ast::{
     StmtImportFrom,
 };
 use ruff_text_size::{Ranged, TextRange, TextSize};
-use rustc_hash::FxHashSet;
+use rustc_hash::{FxBuildHasher, FxHashSet};
 
 #[derive(Debug, Default)]
 pub struct SemanticSyntaxChecker {
@@ -74,8 +74,17 @@ impl SemanticSyntaxChecker {
                     visitor.visit_pattern(&case.pattern);
                 }
             }
-            Stmt::FunctionDef(ast::StmtFunctionDef { type_params, .. })
+            Stmt::FunctionDef(ast::StmtFunctionDef {
-            | Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
+                type_params,
+                parameters,
+                ..
+            }) => {
+                if let Some(type_params) = type_params {
+                    Self::duplicate_type_parameter_name(type_params, ctx);
+                }
+                Self::duplicate_parameter_name(parameters, ctx);
+            }
+            Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
             | Stmt::TypeAlias(ast::StmtTypeAlias { type_params, .. }) => {
                 if let Some(type_params) = type_params {
                     Self::duplicate_type_parameter_name(type_params, ctx);
@@ -453,6 +462,32 @@ impl SemanticSyntaxChecker {
         }
     }
 
+    fn duplicate_parameter_name<Ctx: SemanticSyntaxContext>(
+        parameters: &ast::Parameters,
+        ctx: &Ctx,
+    ) {
+        if parameters.len() < 2 {
+            return;
+        }
+
+        let mut all_arg_names =
+            FxHashSet::with_capacity_and_hasher(parameters.len(), FxBuildHasher);
+
+        for parameter in parameters {
+            let range = parameter.name().range();
+            let param_name = parameter.name().as_str();
+            if !all_arg_names.insert(param_name) {
+                // test_err params_duplicate_names
+                // def foo(a, a=10, *a, a, a: str, **a): ...
+                Self::add_error(
+                    ctx,
+                    SemanticSyntaxErrorKind::DuplicateParameter(param_name.to_string()),
+                    range,
+                );
+            }
+        }
+    }
+
     fn irrefutable_match_case<Ctx: SemanticSyntaxContext>(stmt: &ast::StmtMatch, ctx: &Ctx) {
         // test_ok irrefutable_case_pattern_at_end
         // match x:
@@ -646,6 +681,12 @@ impl SemanticSyntaxChecker {
                 Self::yield_outside_function(ctx, expr, YieldOutsideFunctionKind::Await);
                 Self::await_outside_async_function(ctx, expr, AwaitOutsideAsyncFunctionKind::Await);
             }
+            Expr::Lambda(ast::ExprLambda {
+                parameters: Some(parameters),
+                ..
+            }) => {
+                Self::duplicate_parameter_name(parameters, ctx);
+            }
             _ => {}
         }
     }
@@ -889,6 +930,9 @@ impl Display for SemanticSyntaxError {
             SemanticSyntaxErrorKind::AwaitOutsideAsyncFunction(kind) => {
                 write!(f, "{kind} outside of an asynchronous function")
             }
+            SemanticSyntaxErrorKind::DuplicateParameter(name) => {
+                write!(f, r#"Duplicate parameter "{name}""#)
+            }
         }
     }
 }
@@ -1200,6 +1244,16 @@ pub enum SemanticSyntaxErrorKind {
     ///     async with x: ...      # error
     /// ```
     AwaitOutsideAsyncFunction(AwaitOutsideAsyncFunctionKind),
+
+    /// Represents a duplicate parameter name in a function or lambda expression.
+    ///
+    /// ## Examples
+    ///
+    /// ```python
+    /// def f(x, x): ...
+    /// lambda x, x: ...
+    /// ```
+    DuplicateParameter(String),
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]

crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__lambda_duplicate_parameters.py.snap

@@ -284,44 +284,6 @@ Module(
 ```
 ## Errors
 
-  |
-1 | lambda a, a: 1
-  |           ^ Syntax Error: Duplicate parameter "a"
-2 |
-3 | lambda a, *, a: 1
-  |
-
-
-  |
-1 | lambda a, a: 1
-2 |
-3 | lambda a, *, a: 1
-  |              ^ Syntax Error: Duplicate parameter "a"
-4 |
-5 | lambda a, a=20: 1
-  |
-
-
-  |
-3 | lambda a, *, a: 1
-4 |
-5 | lambda a, a=20: 1
-  |           ^ Syntax Error: Duplicate parameter "a"
-6 |
-7 | lambda a, *a: 1
-  |
-
-
-  |
-5 | lambda a, a=20: 1
-6 |
-7 | lambda a, *a: 1
-  |            ^ Syntax Error: Duplicate parameter "a"
-8 |
-9 | lambda a, *, **a: 1
-  |
-
-
   |
 7 | lambda a, *a: 1
 8 |
@@ -330,6 +292,46 @@ Module(
   |
 
 
+## Semantic Syntax Errors
+
+  |
+1 | lambda a, a: 1
+  |           ^ Syntax Error: Duplicate parameter "a"
+2 |
+3 | lambda a, *, a: 1
+  |
+
+
+  |
+1 | lambda a, a: 1
+2 |
+3 | lambda a, *, a: 1
+  |              ^ Syntax Error: Duplicate parameter "a"
+4 |
+5 | lambda a, a=20: 1
+  |
+
+
+  |
+3 | lambda a, *, a: 1
+4 |
+5 | lambda a, a=20: 1
+  |           ^ Syntax Error: Duplicate parameter "a"
+6 |
+7 | lambda a, *a: 1
+  |
+
+
+  |
+5 | lambda a, a=20: 1
+6 |
+7 | lambda a, *a: 1
+  |            ^ Syntax Error: Duplicate parameter "a"
+8 |
+9 | lambda a, *, **a: 1
+  |
+
+
   |
 7 | lambda a, *a: 1
 8 |

crates/ruff_python_parser/tests/snapshots/invalid_syntax@params_duplicate_names.py.snap

@@ -1,7 +1,6 @@
 ---
 source: crates/ruff_python_parser/tests/fixtures.rs
 input_file: crates/ruff_python_parser/resources/inline/err/params_duplicate_names.py
-snapshot_kind: text
 ---
 ## AST
 
@@ -132,7 +131,7 @@ Module(
     },
 )
 ```
-## Errors
+## Semantic Syntax Errors
 
   |
 1 | def foo(a, a=10, *a, a, a: str, **a): ...