Skip to content

Commit ddbea3e

Browse files
committed
[syntax-errors] Make duplicate parameter names a semantic error
Summary -- Moves `Parser::validate_parameters` to `SemanticSyntaxChecker::duplicate_parameter_name`. Test Plan -- Existing tests, with `## Errors` replaced with `## Semantic Syntax Errors` and an additional case for `lambda` expressions.
1 parent d6dcc37 commit ddbea3e

File tree

5 files changed

+70
-39
lines changed

5 files changed

+70
-39
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,8 @@ impl SemanticSyntaxContext for Checker<'_> {
552552
| SemanticSyntaxErrorKind::MultipleCaseAssignment(_)
553553
| SemanticSyntaxErrorKind::IrrefutableCasePattern(_)
554554
| SemanticSyntaxErrorKind::SingleStarredAssignment
555-
| SemanticSyntaxErrorKind::WriteToDebug(_) => {
555+
| SemanticSyntaxErrorKind::WriteToDebug(_)
556+
| SemanticSyntaxErrorKind::DuplicateParameter(_) => {
556557
if self.settings.preview.is_enabled() {
557558
self.semantic_errors.borrow_mut().push(error);
558559
}

crates/ruff_python_parser/src/parser/statement.rs

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
use compact_str::CompactString;
22
use std::fmt::Display;
33

4-
use rustc_hash::{FxBuildHasher, FxHashSet};
5-
64
use ruff_python_ast::name::Name;
75
use ruff_python_ast::{
86
self as ast, ExceptHandler, Expr, ExprContext, IpyEscapeKind, Operator, PythonVersion, Stmt,
@@ -3341,10 +3339,6 @@ impl<'src> Parser<'src> {
33413339

33423340
parameters.range = self.node_range(start);
33433341

3344-
// test_err params_duplicate_names
3345-
// def foo(a, a=10, *a, a, a: str, **a): ...
3346-
self.validate_parameters(&parameters);
3347-
33483342
parameters
33493343
}
33503344

@@ -3632,25 +3626,6 @@ impl<'src> Parser<'src> {
36323626
}
36333627
}
36343628

3635-
/// Validate that the given parameters doesn't have any duplicate names.
3636-
///
3637-
/// Report errors for all the duplicate names found.
3638-
fn validate_parameters(&mut self, parameters: &ast::Parameters) {
3639-
let mut all_arg_names =
3640-
FxHashSet::with_capacity_and_hasher(parameters.len(), FxBuildHasher);
3641-
3642-
for parameter in parameters {
3643-
let range = parameter.name().range();
3644-
let param_name = parameter.name().as_str();
3645-
if !all_arg_names.insert(param_name) {
3646-
self.add_error(
3647-
ParseErrorType::DuplicateParameter(param_name.to_string()),
3648-
range,
3649-
);
3650-
}
3651-
}
3652-
}
3653-
36543629
/// Classify the `match` soft keyword token.
36553630
///
36563631
/// # Panics

crates/ruff_python_parser/src/semantic_errors.rs

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use ruff_python_ast::{
1313
StmtImportFrom,
1414
};
1515
use ruff_text_size::{Ranged, TextRange};
16-
use rustc_hash::FxHashSet;
16+
use rustc_hash::{FxBuildHasher, FxHashSet};
1717

1818
#[derive(Debug)]
1919
pub struct SemanticSyntaxChecker {
@@ -66,8 +66,17 @@ impl SemanticSyntaxChecker {
6666
Self::irrefutable_match_case(match_stmt, ctx);
6767
Self::multiple_case_assignment(match_stmt, ctx);
6868
}
69-
Stmt::FunctionDef(ast::StmtFunctionDef { type_params, .. })
70-
| Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
69+
Stmt::FunctionDef(ast::StmtFunctionDef {
70+
type_params,
71+
parameters,
72+
..
73+
}) => {
74+
if let Some(type_params) = type_params {
75+
Self::duplicate_type_parameter_name(type_params, ctx);
76+
}
77+
Self::duplicate_parameter_name(parameters, ctx);
78+
}
79+
Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
7180
| Stmt::TypeAlias(ast::StmtTypeAlias { type_params, .. }) => {
7281
if let Some(type_params) = type_params {
7382
Self::duplicate_type_parameter_name(type_params, ctx);
@@ -230,6 +239,32 @@ impl SemanticSyntaxChecker {
230239
}
231240
}
232241

242+
fn duplicate_parameter_name<Ctx: SemanticSyntaxContext>(
243+
parameters: &ast::Parameters,
244+
ctx: &Ctx,
245+
) {
246+
if parameters.len() < 2 {
247+
return;
248+
}
249+
250+
let mut all_arg_names =
251+
FxHashSet::with_capacity_and_hasher(parameters.len(), FxBuildHasher);
252+
253+
for parameter in parameters {
254+
let range = parameter.name().range();
255+
let param_name = parameter.name().as_str();
256+
if !all_arg_names.insert(param_name) {
257+
// test_err params_duplicate_names
258+
// def foo(a, a=10, *a, a, a: str, **a): ...
259+
Self::add_error(
260+
ctx,
261+
SemanticSyntaxErrorKind::DuplicateParameter(param_name.to_string()),
262+
range,
263+
);
264+
}
265+
}
266+
}
267+
233268
fn multiple_case_assignment<Ctx: SemanticSyntaxContext>(stmt: &ast::StmtMatch, ctx: &Ctx) {
234269
for case in &stmt.cases {
235270
let mut visitor = MultipleCaseAssignmentVisitor {
@@ -368,6 +403,12 @@ impl SemanticSyntaxChecker {
368403
};
369404
}
370405
}
406+
Expr::Lambda(ast::ExprLambda {
407+
parameters: Some(parameters),
408+
..
409+
}) => {
410+
Self::duplicate_parameter_name(parameters, ctx);
411+
}
371412
_ => {}
372413
}
373414
}
@@ -462,6 +503,9 @@ impl Display for SemanticSyntaxError {
462503
write!(f, "cannot delete `__debug__` on Python {python_version} (syntax was removed in 3.9)")
463504
}
464505
},
506+
SemanticSyntaxErrorKind::DuplicateParameter(name) => {
507+
write!(f, r#"Duplicate parameter "{name}""#)
508+
}
465509
}
466510
}
467511
}
@@ -575,6 +619,16 @@ pub enum SemanticSyntaxErrorKind {
575619
///
576620
/// [BPO 45000]: https://github.com/python/cpython/issues/89163
577621
WriteToDebug(WriteToDebugKind),
622+
623+
/// Represents a duplicate parameter name in a function or lambda expression.
624+
///
625+
/// ## Examples
626+
///
627+
/// ```python
628+
/// def f(x, x): ...
629+
/// lambda x, x: ...
630+
/// ```
631+
DuplicateParameter(String),
578632
}
579633

580634
#[derive(Debug, Clone, PartialEq, Eq, Hash)]

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,16 @@ Module(
284284
```
285285
## Errors
286286

287+
|
288+
7 | lambda a, *a: 1
289+
8 |
290+
9 | lambda a, *, **a: 1
291+
| ^^^ Syntax Error: Expected one or more keyword parameter after '*' separator
292+
|
293+
294+
295+
## Semantic Syntax Errors
296+
287297
|
288298
1 | lambda a, a: 1
289299
| ^ Syntax Error: Duplicate parameter "a"
@@ -322,14 +332,6 @@ Module(
322332
|
323333

324334

325-
|
326-
7 | lambda a, *a: 1
327-
8 |
328-
9 | lambda a, *, **a: 1
329-
| ^^^ Syntax Error: Expected one or more keyword parameter after '*' separator
330-
|
331-
332-
333335
|
334336
7 | lambda a, *a: 1
335337
8 |

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
---
22
source: crates/ruff_python_parser/tests/fixtures.rs
33
input_file: crates/ruff_python_parser/resources/inline/err/params_duplicate_names.py
4-
snapshot_kind: text
54
---
65
## AST
76

@@ -132,7 +131,7 @@ Module(
132131
},
133132
)
134133
```
135-
## Errors
134+
## Semantic Syntax Errors
136135

137136
|
138137
1 | def foo(a, a=10, *a, a, a: str, **a): ...

0 commit comments

Comments
 (0)