diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 5ae94f21b3..87169d7a91 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/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); } diff --git a/crates/ruff_python_parser/src/error.rs b/crates/ruff_python_parser/src/error.rs index 3673ded43a..bc170ba838 100644 --- a/crates/ruff_python_parser/src/error.rs +++ b/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:?}") } diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 86d6fde4c1..2361b252e6 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/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(¶meters); - 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 diff --git a/crates/ruff_python_parser/src/semantic_errors.rs b/crates/ruff_python_parser/src/semantic_errors.rs index 39b85e5392..fd41ea2187 100644 --- a/crates/ruff_python_parser/src/semantic_errors.rs +++ b/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::ClassDef(ast::StmtClassDef { type_params, .. }) + Stmt::FunctionDef(ast::StmtFunctionDef { + 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( + 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(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)] diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__lambda_duplicate_parameters.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__lambda_duplicate_parameters.py.snap index 04b08939f3..6ea4a192ed 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__lambda_duplicate_parameters.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@expressions__lambda_duplicate_parameters.py.snap @@ -284,6 +284,16 @@ Module( ``` ## Errors + | +7 | lambda a, *a: 1 +8 | +9 | lambda a, *, **a: 1 + | ^^^ Syntax Error: Expected one or more keyword parameter after '*' separator + | + + +## Semantic Syntax Errors + | 1 | lambda a, a: 1 | ^ Syntax Error: Duplicate parameter "a" @@ -322,14 +332,6 @@ Module( | - | -7 | lambda a, *a: 1 -8 | -9 | lambda a, *, **a: 1 - | ^^^ Syntax Error: Expected one or more keyword parameter after '*' separator - | - - | 7 | lambda a, *a: 1 8 | diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@params_duplicate_names.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@params_duplicate_names.py.snap index f7bfc72044..50e5e76378 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@params_duplicate_names.py.snap +++ b/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): ...