From 87929ad5f1306cb889cbc85ddfdb9404d9b5c6ad Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 29 Apr 2024 11:36:15 +0100 Subject: [PATCH] Add convenience methods for iterating over all parameter nodes in a function (#11174) --- crates/ruff_linter/src/checkers/ast/mod.rs | 83 +----- .../flake8_annotations/rules/definition.rs | 18 +- .../rules/hardcoded_password_default.rs | 6 +- .../rules/ssl_with_bad_defaults.rs | 61 ++--- .../function_call_in_argument_default.rs | 6 +- .../rules/loop_variable_overrides_iterator.rs | 6 +- .../rules/mutable_argument_default.rs | 7 +- .../rules/unnecessary_map.rs | 15 +- .../rules/no_return_argument_annotation.rs | 48 +--- .../rules/redundant_numeric_union.rs | 22 +- .../rules/flake8_pyi/rules/simple_defaults.rs | 12 +- .../flake8_pytest_style/rules/fixture.rs | 35 +-- .../rules/unused_arguments.rs | 36 +-- .../rules/invalid_first_argument_name.rs | 8 +- .../pycodestyle/rules/lambda_assignment.rs | 4 +- .../src/rules/pydocstyle/rules/sections.rs | 18 +- .../rules/pylint/rules/too_many_arguments.rs | 5 +- .../rules/pylint/rules/too_many_positional.rs | 8 +- .../src/rules/ruff/rules/implicit_optional.rs | 6 +- crates/ruff_python_ast/src/helpers.rs | 45 +--- crates/ruff_python_ast/src/node.rs | 44 ++- crates/ruff_python_ast/src/nodes.rs | 253 ++++++++++++++++-- crates/ruff_python_ast/src/visitor.rs | 45 +--- .../src/other/parameters.rs | 6 +- .../src/parser/statement.rs | 33 +-- ...alid_syntax@params_duplicate_names.py.snap | 6 +- .../src/analyze/typing.rs | 5 +- 27 files changed, 396 insertions(+), 445 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 7388edf3deeff..566924b532fff 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -31,8 +31,8 @@ use std::path::Path; use itertools::Itertools; use log::debug; use ruff_python_ast::{ - self as ast, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext, FStringElement, - Keyword, MatchCase, Parameter, ParameterWithDefault, Parameters, Pattern, Stmt, Suite, UnaryOp, + self as ast, AnyParameterRef, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext, + FStringElement, Keyword, MatchCase, Parameter, Parameters, Pattern, Stmt, Suite, UnaryOp, }; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -604,15 +604,11 @@ impl<'a> Visitor<'a> for Checker<'a> { self.visit_type_params(type_params); } - for parameter_with_default in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) - { - if let Some(expr) = ¶meter_with_default.parameter.annotation { - if singledispatch { + for parameter in &**parameters { + if let Some(expr) = parameter.annotation() { + if singledispatch && !parameter.is_variadic() { self.visit_runtime_required_annotation(expr); + singledispatch = false; } else { match annotation { AnnotationContext::RuntimeRequired => { @@ -625,41 +621,10 @@ impl<'a> Visitor<'a> for Checker<'a> { self.visit_annotation(expr); } } - }; - } - if let Some(expr) = ¶meter_with_default.default { - self.visit_expr(expr); - } - singledispatch = false; - } - if let Some(arg) = ¶meters.vararg { - if let Some(expr) = &arg.annotation { - match annotation { - AnnotationContext::RuntimeRequired => { - self.visit_runtime_required_annotation(expr); - } - AnnotationContext::RuntimeEvaluated => { - self.visit_runtime_evaluated_annotation(expr); - } - AnnotationContext::TypingOnly => { - self.visit_annotation(expr); - } } } - } - if let Some(arg) = ¶meters.kwarg { - if let Some(expr) = &arg.annotation { - match annotation { - AnnotationContext::RuntimeRequired => { - self.visit_runtime_required_annotation(expr); - } - AnnotationContext::RuntimeEvaluated => { - self.visit_runtime_evaluated_annotation(expr); - } - AnnotationContext::TypingOnly => { - self.visit_annotation(expr); - } - } + if let Some(expr) = parameter.default() { + self.visit_expr(expr); } } for expr in returns { @@ -1043,19 +1008,11 @@ impl<'a> Visitor<'a> for Checker<'a> { ) => { // Visit the default arguments, but avoid the body, which will be deferred. if let Some(parameters) = parameters { - for ParameterWithDefault { - default, - parameter: _, - range: _, - } in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + for default in parameters + .iter_non_variadic_params() + .filter_map(|param| param.default.as_deref()) { - if let Some(expr) = &default { - self.visit_expr(expr); - } + self.visit_expr(default); } } @@ -1483,20 +1440,8 @@ impl<'a> Visitor<'a> for Checker<'a> { // Step 1: Binding. // Bind, but intentionally avoid walking default expressions, as we handle them // upstream. - for parameter_with_default in ¶meters.posonlyargs { - self.visit_parameter(¶meter_with_default.parameter); - } - for parameter_with_default in ¶meters.args { - self.visit_parameter(¶meter_with_default.parameter); - } - if let Some(arg) = ¶meters.vararg { - self.visit_parameter(arg); - } - for parameter_with_default in ¶meters.kwonlyargs { - self.visit_parameter(¶meter_with_default.parameter); - } - if let Some(arg) = ¶meters.kwarg { - self.visit_parameter(arg); + for parameter in parameters.iter().map(AnyParameterRef::as_parameter) { + self.visit_parameter(parameter); } // Step 4: Analysis diff --git a/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs b/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs index f847fd6954eed..25b0119a656af 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs +++ b/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs @@ -582,6 +582,7 @@ fn is_stub_function(function_def: &ast::StmtFunctionDef, checker: &Checker) -> b } /// Generate flake8-annotation checks for a given `Definition`. +/// ANN001, ANN401 pub(crate) fn definition( checker: &Checker, definition: &Definition, @@ -615,23 +616,14 @@ pub(crate) fn definition( let is_overridden = visibility::is_override(decorator_list, checker.semantic()); - // ANN001, ANN401 + // If this is a non-static method, skip `cls` or `self`. for ParameterWithDefault { parameter, default: _, range: _, - } in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) - .skip( - // If this is a non-static method, skip `cls` or `self`. - usize::from( - is_method && !visibility::is_staticmethod(decorator_list, checker.semantic()), - ), - ) - { + } in parameters.iter_non_variadic_params().skip(usize::from( + is_method && !visibility::is_staticmethod(decorator_list, checker.semantic()), + )) { // ANN401 for dynamically typed parameters if let Some(annotation) = ¶meter.annotation { has_any_typed_arg = true; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_password_default.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_password_default.rs index 06c087c09ef3b..969364a271a35 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_password_default.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_password_default.rs @@ -74,11 +74,7 @@ pub(crate) fn hardcoded_password_default(checker: &mut Checker, parameters: &Par parameter, default, range: _, - } in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + } in parameters.iter_non_variadic_params() { let Some(default) = default else { continue; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_bad_defaults.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_bad_defaults.rs index 67f4033e6022b..da60e3736d616 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_bad_defaults.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_bad_defaults.rs @@ -49,44 +49,35 @@ impl Violation for SslWithBadDefaults { /// S503 pub(crate) fn ssl_with_bad_defaults(checker: &mut Checker, function_def: &StmtFunctionDef) { - function_def + for default in function_def .parameters - .posonlyargs - .iter() - .chain( - function_def - .parameters - .args - .iter() - .chain(function_def.parameters.kwonlyargs.iter()), - ) - .for_each(|param| { - if let Some(default) = ¶m.default { - match default.as_ref() { - Expr::Name(ast::ExprName { id, range, .. }) => { - if is_insecure_protocol(id.as_str()) { - checker.diagnostics.push(Diagnostic::new( - SslWithBadDefaults { - protocol: id.to_string(), - }, - *range, - )); - } - } - Expr::Attribute(ast::ExprAttribute { attr, range, .. }) => { - if is_insecure_protocol(attr.as_str()) { - checker.diagnostics.push(Diagnostic::new( - SslWithBadDefaults { - protocol: attr.to_string(), - }, - *range, - )); - } - } - _ => {} + .iter_non_variadic_params() + .filter_map(|param| param.default.as_deref()) + { + match default { + Expr::Name(ast::ExprName { id, range, .. }) => { + if is_insecure_protocol(id.as_str()) { + checker.diagnostics.push(Diagnostic::new( + SslWithBadDefaults { + protocol: id.to_string(), + }, + *range, + )); } } - }); + Expr::Attribute(ast::ExprAttribute { attr, range, .. }) => { + if is_insecure_protocol(attr.as_str()) { + checker.diagnostics.push(Diagnostic::new( + SslWithBadDefaults { + protocol: attr.to_string(), + }, + *range, + )); + } + } + _ => {} + } + } } /// Returns `true` if the given protocol name is insecure. diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs index dd677dcd69c33..a6a30a8d20f65 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs @@ -139,11 +139,7 @@ pub(crate) fn function_call_in_argument_default(checker: &mut Checker, parameter default, parameter, range: _, - } in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + } in parameters.iter_non_variadic_params() { if let Some(expr) = &default { if !parameter.annotation.as_ref().is_some_and(|expr| { diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs index b43fccbb95813..24c2004453c73 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs @@ -105,11 +105,7 @@ impl<'a> Visitor<'a> for NameFinder<'a> { parameter, default: _, range: _, - } in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + } in parameters.iter_non_variadic_params() { self.names.remove(parameter.name.as_str()); } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs index e028eb02c2084..e15ca868abc32 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs @@ -89,12 +89,7 @@ pub(crate) fn mutable_argument_default(checker: &mut Checker, function_def: &ast parameter, default, range: _, - } in function_def - .parameters - .posonlyargs - .iter() - .chain(&function_def.parameters.args) - .chain(&function_def.parameters.kwonlyargs) + } in function_def.parameters.iter_non_variadic_params() { let Some(default) = default else { continue; diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_map.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_map.rs index dc36188799993..a1dd7fe08d143 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_map.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_map.rs @@ -107,10 +107,7 @@ pub(crate) fn unnecessary_map( if parameters.as_ref().is_some_and(|parameters| { late_binding(parameters, body) || parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + .iter_non_variadic_params() .any(|param| param.default.is_some()) || parameters.vararg.is_some() || parameters.kwarg.is_some() @@ -152,10 +149,7 @@ pub(crate) fn unnecessary_map( if parameters.as_ref().is_some_and(|parameters| { late_binding(parameters, body) || parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + .iter_non_variadic_params() .any(|param| param.default.is_some()) || parameters.vararg.is_some() || parameters.kwarg.is_some() @@ -207,10 +201,7 @@ pub(crate) fn unnecessary_map( if parameters.as_ref().is_some_and(|parameters| { late_binding(parameters, body) || parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + .iter_non_variadic_params() .any(|param| param.default.is_some()) || parameters.vararg.is_some() || parameters.kwarg.is_some() diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs index a09872435c008..8ebaab9edb49a 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs @@ -2,7 +2,7 @@ use std::fmt; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{Expr, Parameters}; +use ruff_python_ast::{AnyParameterRef, Parameters}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -58,43 +58,21 @@ pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: & // Ex) def func(arg: NoReturn): ... // Ex) def func(arg: NoReturn, /): ... // Ex) def func(*, arg: NoReturn): ... - for annotation in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) - .filter_map(|arg| arg.parameter.annotation.as_ref()) - { - check_no_return_argument_annotation(checker, annotation); - } - // Ex) def func(*args: NoReturn): ... - if let Some(arg) = ¶meters.vararg { - if let Some(annotation) = &arg.annotation { - check_no_return_argument_annotation(checker, annotation); - } - } - // Ex) def func(**kwargs: NoReturn): ... - if let Some(arg) = ¶meters.kwarg { - if let Some(annotation) = &arg.annotation { - check_no_return_argument_annotation(checker, annotation); - } - } -} - -fn check_no_return_argument_annotation(checker: &mut Checker, annotation: &Expr) { - if checker.semantic().match_typing_expr(annotation, "NoReturn") { - checker.diagnostics.push(Diagnostic::new( - NoReturnArgumentAnnotationInStub { - module: if checker.settings.target_version >= Py311 { - TypingModule::Typing - } else { - TypingModule::TypingExtensions + for annotation in parameters.iter().filter_map(AnyParameterRef::annotation) { + if checker.semantic().match_typing_expr(annotation, "NoReturn") { + checker.diagnostics.push(Diagnostic::new( + NoReturnArgumentAnnotationInStub { + module: if checker.settings.target_version >= Py311 { + TypingModule::Typing + } else { + TypingModule::TypingExtensions + }, }, - }, - annotation.range(), - )); + annotation.range(), + )); + } } } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs index b8426eae856d9..5e8651b5389cf 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs @@ -1,6 +1,6 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{Expr, Parameters}; +use ruff_python_ast::{AnyParameterRef, Expr, Parameters}; use ruff_python_semantic::analyze::typing::traverse_union; use ruff_text_size::Ranged; @@ -61,25 +61,7 @@ impl Violation for RedundantNumericUnion { /// PYI041 pub(crate) fn redundant_numeric_union(checker: &mut Checker, parameters: &Parameters) { - for annotation in parameters - .args - .iter() - .chain(parameters.posonlyargs.iter()) - .chain(parameters.kwonlyargs.iter()) - .filter_map(|arg| arg.parameter.annotation.as_ref()) - { - check_annotation(checker, annotation); - } - - // If annotations on `args` or `kwargs` are flagged by this rule, the annotations themselves - // are not accurate, but check them anyway. It's possible that flagging them will help the user - // realize they're incorrect. - for annotation in parameters - .vararg - .iter() - .chain(parameters.kwarg.iter()) - .filter_map(|arg| arg.annotation.as_ref()) - { + for annotation in parameters.iter().filter_map(AnyParameterRef::annotation) { check_annotation(checker, annotation); } } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/simple_defaults.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/simple_defaults.rs index 870e77e35a1ae..65bd78fad1fae 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/simple_defaults.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/simple_defaults.rs @@ -495,11 +495,7 @@ pub(crate) fn typed_argument_simple_defaults(checker: &mut Checker, parameters: parameter, default, range: _, - } in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + } in parameters.iter_non_variadic_params() { let Some(default) = default else { continue; @@ -530,11 +526,7 @@ pub(crate) fn argument_simple_defaults(checker: &mut Checker, parameters: &Param parameter, default, range: _, - } in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + } in parameters.iter_non_variadic_params() { let Some(default) = default else { continue; diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs index a65f59915f16e..2c7242ae3af84 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs @@ -8,7 +8,7 @@ use ruff_python_ast::name::UnqualifiedName; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; use ruff_python_ast::Decorator; -use ruff_python_ast::{self as ast, Expr, ParameterWithDefault, Parameters, Stmt}; +use ruff_python_ast::{self as ast, Expr, Parameters, Stmt}; use ruff_python_semantic::analyze::visibility::is_abstract; use ruff_python_semantic::SemanticModel; use ruff_text_size::Ranged; @@ -841,28 +841,17 @@ fn check_fixture_returns( /// PT019 fn check_test_function_args(checker: &mut Checker, parameters: &Parameters) { - parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) - .for_each( - |ParameterWithDefault { - parameter, - default: _, - range: _, - }| { - let name = ¶meter.name; - if name.starts_with('_') { - checker.diagnostics.push(Diagnostic::new( - PytestFixtureParamWithoutValue { - name: name.to_string(), - }, - parameter.range(), - )); - } - }, - ); + for parameter in parameters.iter_non_variadic_params() { + let name = ¶meter.parameter.name; + if name.starts_with('_') { + checker.diagnostics.push(Diagnostic::new( + PytestFixtureParamWithoutValue { + name: name.to_string(), + }, + parameter.range(), + )); + } + } } /// PT020 diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 0d962d389665a..37d7ebffebc06 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -1,5 +1,3 @@ -use std::iter; - use regex::Regex; use ruff_python_ast as ast; use ruff_python_ast::{Parameter, Parameters}; @@ -224,19 +222,20 @@ fn function( diagnostics: &mut Vec, ) { let args = parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + .iter_non_variadic_params() .map(|parameter_with_default| ¶meter_with_default.parameter) .chain( - iter::once::>(parameters.vararg.as_deref()) - .flatten() + parameters + .vararg + .as_deref() + .into_iter() .skip(usize::from(ignore_variadic_names)), ) .chain( - iter::once::>(parameters.kwarg.as_deref()) - .flatten() + parameters + .kwarg + .as_deref() + .into_iter() .skip(usize::from(ignore_variadic_names)), ); call( @@ -260,20 +259,21 @@ fn method( diagnostics: &mut Vec, ) { let args = parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + .iter_non_variadic_params() .skip(1) .map(|parameter_with_default| ¶meter_with_default.parameter) .chain( - iter::once::>(parameters.vararg.as_deref()) - .flatten() + parameters + .vararg + .as_deref() + .into_iter() .skip(usize::from(ignore_variadic_names)), ) .chain( - iter::once::>(parameters.kwarg.as_deref()) - .flatten() + parameters + .kwarg + .as_deref() + .into_iter() .skip(usize::from(ignore_variadic_names)), ); call( diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs index aa049a5e05176..26f91091b154f 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_first_argument_name.rs @@ -257,15 +257,9 @@ fn rename_parameter( ) -> Result> { // Don't fix if another parameter has the valid name. if parameters - .posonlyargs .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) .skip(1) - .map(|parameter_with_default| ¶meter_with_default.parameter) - .chain(parameters.vararg.as_deref()) - .chain(parameters.kwarg.as_deref()) - .any(|parameter| ¶meter.name == function_type.valid_first_argument_name()) + .any(|parameter| parameter.name() == function_type.valid_first_argument_name()) { return Ok(None); } diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/lambda_assignment.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/lambda_assignment.rs index e48ff57b3a56a..5c41c5c44a002 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/lambda_assignment.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/lambda_assignment.rs @@ -199,8 +199,8 @@ fn function( let parameters = parameters.cloned().unwrap_or_default(); if let Some(annotation) = annotation { if let Some((arg_types, return_type)) = extract_types(annotation, semantic) { - // A `lambda` expression can only have positional and positional-only - // arguments. The order is always positional-only first, then positional. + // A `lambda` expression can only have positional-only and positional-or-keyword + // arguments. The order is always positional-only first, then positional-or-keyword. let new_posonlyargs = parameters .posonlyargs .iter() diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs index ea02dc57309c7..b381fcca2bbe6 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs @@ -1755,23 +1755,19 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: & // Look for arguments that weren't included in the docstring. let mut missing_arg_names: FxHashSet = FxHashSet::default(); + + // If this is a non-static method, skip `cls` or `self`. for ParameterWithDefault { parameter, default: _, range: _, } in function .parameters - .posonlyargs - .iter() - .chain(&function.parameters.args) - .chain(&function.parameters.kwonlyargs) - .skip( - // If this is a non-static method, skip `cls` or `self`. - usize::from( - docstring.definition.is_method() - && !is_staticmethod(&function.decorator_list, checker.semantic()), - ), - ) + .iter_non_variadic_params() + .skip(usize::from( + docstring.definition.is_method() + && !is_staticmethod(&function.decorator_list, checker.semantic()), + )) { let arg_name = parameter.name.as_str(); if !arg_name.starts_with('_') && !docstrings_args.contains(arg_name) { diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs index ea20fc78b449e..53580749d7cd5 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs @@ -61,10 +61,7 @@ impl Violation for TooManyArguments { pub(crate) fn too_many_arguments(checker: &mut Checker, function_def: &ast::StmtFunctionDef) { let num_arguments = function_def .parameters - .args - .iter() - .chain(&function_def.parameters.kwonlyargs) - .chain(&function_def.parameters.posonlyargs) + .iter_non_variadic_params() .filter(|arg| { !checker .settings diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs index 0ec44c685512c..27d73583eabda 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_positional.rs @@ -62,14 +62,14 @@ pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::Stm // Count the number of positional arguments. let num_positional_args = function_def .parameters - .args + .posonlyargs .iter() - .chain(&function_def.parameters.posonlyargs) - .filter(|arg| { + .chain(&function_def.parameters.args) + .filter(|param| { !checker .settings .dummy_variable_rgx - .is_match(&arg.parameter.name) + .is_match(¶m.parameter.name) }) .count(); diff --git a/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs b/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs index c9e3b06e0df04..180409066f05a 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs @@ -167,11 +167,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, parameters: &Parameters) parameter, default, range: _, - } in parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .chain(¶meters.kwonlyargs) + } in parameters.iter_non_variadic_params() { let Some(default) = default else { continue }; if !default.is_none_literal_expr() { diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index eed56dec65ff3..3beaf4a270136 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -348,39 +348,18 @@ pub fn any_over_stmt(stmt: &Stmt, func: &dyn Fn(&Expr) -> bool) -> bool { returns, .. }) => { - parameters - .posonlyargs - .iter() - .chain(parameters.args.iter().chain(parameters.kwonlyargs.iter())) - .any(|parameter| { - parameter - .default - .as_ref() - .is_some_and(|expr| any_over_expr(expr, func)) - || parameter - .parameter - .annotation - .as_ref() - .is_some_and(|expr| any_over_expr(expr, func)) - }) - || parameters.vararg.as_ref().is_some_and(|parameter| { - parameter - .annotation - .as_ref() - .is_some_and(|expr| any_over_expr(expr, func)) - }) - || parameters.kwarg.as_ref().is_some_and(|parameter| { - parameter - .annotation - .as_ref() - .is_some_and(|expr| any_over_expr(expr, func)) - }) - || type_params.as_ref().is_some_and(|type_params| { - type_params - .iter() - .any(|type_param| any_over_type_param(type_param, func)) - }) - || body.iter().any(|stmt| any_over_stmt(stmt, func)) + parameters.iter().any(|param| { + param + .default() + .is_some_and(|default| any_over_expr(default, func)) + || param + .annotation() + .is_some_and(|annotation| any_over_expr(annotation, func)) + }) || type_params.as_ref().is_some_and(|type_params| { + type_params + .iter() + .any(|type_param| any_over_type_param(type_param, func)) + }) || body.iter().any(|stmt| any_over_stmt(stmt, func)) || decorator_list .iter() .any(|decorator| any_over_expr(&decorator.expression, func)) diff --git a/crates/ruff_python_ast/src/node.rs b/crates/ruff_python_ast/src/node.rs index 7d5024e420ff2..d3192e2750036 100644 --- a/crates/ruff_python_ast/src/node.rs +++ b/crates/ruff_python_ast/src/node.rs @@ -1,12 +1,13 @@ use crate::visitor::preorder::PreorderVisitor; use crate::{ - self as ast, Alias, ArgOrKeyword, Arguments, Comprehension, Decorator, ExceptHandler, Expr, - FStringElement, Keyword, MatchCase, Mod, Parameter, ParameterWithDefault, Parameters, Pattern, - PatternArguments, PatternKeyword, Stmt, StmtAnnAssign, StmtAssert, StmtAssign, StmtAugAssign, - StmtBreak, StmtClassDef, StmtContinue, StmtDelete, StmtExpr, StmtFor, StmtFunctionDef, - StmtGlobal, StmtIf, StmtImport, StmtImportFrom, StmtIpyEscapeCommand, StmtMatch, StmtNonlocal, - StmtPass, StmtRaise, StmtReturn, StmtTry, StmtTypeAlias, StmtWhile, StmtWith, TypeParam, - TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams, WithItem, + self as ast, Alias, AnyParameterRef, ArgOrKeyword, Arguments, Comprehension, Decorator, + ExceptHandler, Expr, FStringElement, Keyword, MatchCase, Mod, Parameter, ParameterWithDefault, + Parameters, Pattern, PatternArguments, PatternKeyword, Stmt, StmtAnnAssign, StmtAssert, + StmtAssign, StmtAugAssign, StmtBreak, StmtClassDef, StmtContinue, StmtDelete, StmtExpr, + StmtFor, StmtFunctionDef, StmtGlobal, StmtIf, StmtImport, StmtImportFrom, StmtIpyEscapeCommand, + StmtMatch, StmtNonlocal, StmtPass, StmtRaise, StmtReturn, StmtTry, StmtTypeAlias, StmtWhile, + StmtWith, TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams, + WithItem, }; use ruff_text_size::{Ranged, TextRange}; use std::ptr::NonNull; @@ -4221,28 +4222,13 @@ impl AstNode for Parameters { where V: PreorderVisitor<'a> + ?Sized, { - let ast::Parameters { - range: _, - posonlyargs, - args, - vararg, - kwonlyargs, - kwarg, - } = self; - for arg in posonlyargs.iter().chain(args) { - visitor.visit_parameter_with_default(arg); - } - - if let Some(arg) = vararg { - visitor.visit_parameter(arg); - } - - for arg in kwonlyargs { - visitor.visit_parameter_with_default(arg); - } - - if let Some(arg) = kwarg { - visitor.visit_parameter(arg); + for parameter in self { + match parameter { + AnyParameterRef::NonVariadic(parameter_with_default) => { + visitor.visit_parameter_with_default(parameter_with_default); + } + AnyParameterRef::Variadic(parameter) => visitor.visit_parameter(parameter), + } } } } diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index ff54724a2568d..3fd425684b0e4 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -2,6 +2,7 @@ use std::fmt; use std::fmt::Debug; +use std::iter::FusedIterator; use std::ops::Deref; use std::slice::{Iter, IterMut}; use std::sync::OnceLock; @@ -3175,6 +3176,63 @@ pub struct Decorator { pub expression: Expr, } +/// Enumeration of the two kinds of parameter +#[derive(Debug, PartialEq, Clone, Copy)] +pub enum AnyParameterRef<'a> { + /// Variadic parameters cannot have default values, + /// e.g. both `*args` and `**kwargs` in the following function: + /// + /// ```python + /// def foo(*args, **kwargs): pass + /// ``` + Variadic(&'a Parameter), + + /// Non-variadic parameters can have default values, + /// though they won't necessarily always have them: + /// + /// ```python + /// def bar(a=1, /, b=2, *, c=3): pass + /// ``` + NonVariadic(&'a ParameterWithDefault), +} + +impl<'a> AnyParameterRef<'a> { + pub const fn as_parameter(self) -> &'a Parameter { + match self { + Self::NonVariadic(param) => ¶m.parameter, + Self::Variadic(param) => param, + } + } + + pub const fn name(self) -> &'a Identifier { + &self.as_parameter().name + } + + pub const fn is_variadic(self) -> bool { + matches!(self, Self::Variadic(_)) + } + + pub fn annotation(self) -> Option<&'a Expr> { + self.as_parameter().annotation.as_deref() + } + + pub fn default(self) -> Option<&'a Expr> { + match self { + Self::NonVariadic(param) => param.default.as_deref(), + Self::Variadic(_) => None, + } + } +} + +impl Ranged for AnyParameterRef<'_> { + fn range(&self) -> TextRange { + match self { + Self::NonVariadic(param) => param.range, + Self::Variadic(param) => param.range, + } + } +} + /// An alternative type of AST `arguments`. This is ruff_python_parser-friendly and human-friendly definition of function arguments. /// This form also has advantage to implement pre-order traverse. /// @@ -3196,37 +3254,56 @@ pub struct Parameters { } impl Parameters { - /// Returns the [`ParameterWithDefault`] with the given name, or `None` if no such [`ParameterWithDefault`] exists. - pub fn find(&self, name: &str) -> Option<&ParameterWithDefault> { + /// Returns an iterator over all non-variadic parameters included in this [`Parameters`] node. + /// + /// The variadic parameters (`.vararg` and `.kwarg`) can never have default values; + /// non-variadic parameters sometimes will. + pub fn iter_non_variadic_params(&self) -> impl Iterator { self.posonlyargs .iter() .chain(&self.args) .chain(&self.kwonlyargs) + } + + /// Returns the [`ParameterWithDefault`] with the given name, or `None` if no such [`ParameterWithDefault`] exists. + pub fn find(&self, name: &str) -> Option<&ParameterWithDefault> { + self.iter_non_variadic_params() .find(|arg| arg.parameter.name.as_str() == name) } - /// Returns `true` if a parameter with the given name included in this [`Parameters`]. + /// Returns an iterator over all parameters included in this [`Parameters`] node. + pub fn iter(&self) -> ParametersIterator { + ParametersIterator::new(self) + } + + /// Returns the total number of parameters included in this [`Parameters`] node. + pub fn len(&self) -> usize { + let Parameters { + range: _, + posonlyargs, + args, + vararg, + kwonlyargs, + kwarg, + } = self; + // Safety: a Python function can have an arbitrary number of parameters, + // so theoretically this could be a number that wouldn't fit into a usize, + // which would lead to a panic. A Python function with that many parameters + // is extremely unlikely outside of generated code, however, and it's even + // more unlikely that we'd find a function with that many parameters in a + // source-code file <=4GB large (Ruff's maximum). + posonlyargs + .len() + .checked_add(args.len()) + .and_then(|length| length.checked_add(usize::from(vararg.is_some()))) + .and_then(|length| length.checked_add(kwonlyargs.len())) + .and_then(|length| length.checked_add(usize::from(kwarg.is_some()))) + .expect("Failed to fit the number of parameters into a usize") + } + + /// Returns `true` if a parameter with the given name is included in this [`Parameters`]. pub fn includes(&self, name: &str) -> bool { - if self - .posonlyargs - .iter() - .chain(&self.args) - .chain(&self.kwonlyargs) - .any(|arg| arg.parameter.name.as_str() == name) - { - return true; - } - if let Some(arg) = &self.vararg { - if arg.name.as_str() == name { - return true; - } - } - if let Some(arg) = &self.kwarg { - if arg.name.as_str() == name { - return true; - } - } - false + self.iter().any(|param| param.name() == name) } /// Returns `true` if the [`Parameters`] is empty. @@ -3239,6 +3316,136 @@ impl Parameters { } } +pub struct ParametersIterator<'a> { + posonlyargs: Iter<'a, ParameterWithDefault>, + args: Iter<'a, ParameterWithDefault>, + vararg: Option<&'a Parameter>, + kwonlyargs: Iter<'a, ParameterWithDefault>, + kwarg: Option<&'a Parameter>, +} + +impl<'a> ParametersIterator<'a> { + fn new(parameters: &'a Parameters) -> Self { + let Parameters { + range: _, + posonlyargs, + args, + vararg, + kwonlyargs, + kwarg, + } = parameters; + Self { + posonlyargs: posonlyargs.iter(), + args: args.iter(), + vararg: vararg.as_deref(), + kwonlyargs: kwonlyargs.iter(), + kwarg: kwarg.as_deref(), + } + } +} + +impl<'a> Iterator for ParametersIterator<'a> { + type Item = AnyParameterRef<'a>; + + fn next(&mut self) -> Option { + let ParametersIterator { + posonlyargs, + args, + vararg, + kwonlyargs, + kwarg, + } = self; + + if let Some(param) = posonlyargs.next() { + return Some(AnyParameterRef::NonVariadic(param)); + } + if let Some(param) = args.next() { + return Some(AnyParameterRef::NonVariadic(param)); + } + if let Some(param) = vararg.take() { + return Some(AnyParameterRef::Variadic(param)); + } + if let Some(param) = kwonlyargs.next() { + return Some(AnyParameterRef::NonVariadic(param)); + } + kwarg.take().map(AnyParameterRef::Variadic) + } + + fn size_hint(&self) -> (usize, Option) { + let ParametersIterator { + posonlyargs, + args, + vararg, + kwonlyargs, + kwarg, + } = self; + + let posonlyargs_len = posonlyargs.len(); + let args_len = args.len(); + let vararg_len = usize::from(vararg.is_some()); + let kwonlyargs_len = kwonlyargs.len(); + let kwarg_len = usize::from(kwarg.is_some()); + + let lower = posonlyargs_len + .saturating_add(args_len) + .saturating_add(vararg_len) + .saturating_add(kwonlyargs_len) + .saturating_add(kwarg_len); + + let upper = posonlyargs_len + .checked_add(args_len) + .and_then(|length| length.checked_add(vararg_len)) + .and_then(|length| length.checked_add(kwonlyargs_len)) + .and_then(|length| length.checked_add(kwarg_len)); + + (lower, upper) + } + + fn last(mut self) -> Option { + self.next_back() + } +} + +impl<'a> DoubleEndedIterator for ParametersIterator<'a> { + fn next_back(&mut self) -> Option { + let ParametersIterator { + posonlyargs, + args, + vararg, + kwonlyargs, + kwarg, + } = self; + + if let Some(param) = kwarg.take() { + return Some(AnyParameterRef::Variadic(param)); + } + if let Some(param) = kwonlyargs.next_back() { + return Some(AnyParameterRef::NonVariadic(param)); + } + if let Some(param) = vararg.take() { + return Some(AnyParameterRef::Variadic(param)); + } + if let Some(param) = args.next_back() { + return Some(AnyParameterRef::NonVariadic(param)); + } + posonlyargs.next_back().map(AnyParameterRef::NonVariadic) + } +} + +impl<'a> FusedIterator for ParametersIterator<'a> {} + +/// We rely on the same invariants outlined in the comment above `Parameters::len()` +/// in order to implement `ExactSizeIterator` here +impl<'a> ExactSizeIterator for ParametersIterator<'a> {} + +impl<'a> IntoIterator for &'a Parameters { + type IntoIter = ParametersIterator<'a>; + type Item = AnyParameterRef<'a>; + fn into_iter(self) -> Self::IntoIter { + self.iter() + } +} + /// An alternative type of AST `arg`. This is used for each function argument that might have a default value. /// Used by `Arguments` original type. /// diff --git a/crates/ruff_python_ast/src/visitor.rs b/crates/ruff_python_ast/src/visitor.rs index e243ff5d0ae6e..86471765cc963 100644 --- a/crates/ruff_python_ast/src/visitor.rs +++ b/crates/ruff_python_ast/src/visitor.rs @@ -4,11 +4,11 @@ pub mod preorder; pub mod transformer; use crate::{ - self as ast, Alias, Arguments, BoolOp, BytesLiteral, CmpOp, Comprehension, Decorator, - ElifElseClause, ExceptHandler, Expr, ExprContext, FString, FStringElement, FStringPart, - Keyword, MatchCase, Operator, Parameter, Parameters, Pattern, PatternArguments, PatternKeyword, - Stmt, StringLiteral, TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, - TypeParams, UnaryOp, WithItem, + self as ast, Alias, AnyParameterRef, Arguments, BoolOp, BytesLiteral, CmpOp, Comprehension, + Decorator, ElifElseClause, ExceptHandler, Expr, ExprContext, FString, FStringElement, + FStringPart, Keyword, MatchCase, Operator, Parameter, Parameters, Pattern, PatternArguments, + PatternKeyword, Stmt, StringLiteral, TypeParam, TypeParamParamSpec, TypeParamTypeVar, + TypeParamTypeVarTuple, TypeParams, UnaryOp, WithItem, }; /// A trait for AST visitors. Visits all nodes in the AST recursively in evaluation-order. @@ -607,36 +607,15 @@ pub fn walk_arguments<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, arguments: & pub fn walk_parameters<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, parameters: &'a Parameters) { // Defaults are evaluated before annotations. - for arg in ¶meters.posonlyargs { - if let Some(default) = &arg.default { - visitor.visit_expr(default); - } - } - for arg in ¶meters.args { - if let Some(default) = &arg.default { - visitor.visit_expr(default); - } - } - for arg in ¶meters.kwonlyargs { - if let Some(default) = &arg.default { - visitor.visit_expr(default); - } + for default in parameters + .iter_non_variadic_params() + .filter_map(|param| param.default.as_deref()) + { + visitor.visit_expr(default); } - for arg in ¶meters.posonlyargs { - visitor.visit_parameter(&arg.parameter); - } - for arg in ¶meters.args { - visitor.visit_parameter(&arg.parameter); - } - if let Some(arg) = ¶meters.vararg { - visitor.visit_parameter(arg); - } - for arg in ¶meters.kwonlyargs { - visitor.visit_parameter(&arg.parameter); - } - if let Some(arg) = ¶meters.kwarg { - visitor.visit_parameter(arg); + for parameter in parameters.iter().map(AnyParameterRef::as_parameter) { + visitor.visit_parameter(parameter); } } diff --git a/crates/ruff_python_formatter/src/other/parameters.rs b/crates/ruff_python_formatter/src/other/parameters.rs index da42179920cf7..6670344d6bde5 100644 --- a/crates/ruff_python_formatter/src/other/parameters.rs +++ b/crates/ruff_python_formatter/src/other/parameters.rs @@ -240,11 +240,7 @@ impl FormatNodeRule for FormatParameters { Ok(()) }); - let num_parameters = posonlyargs.len() - + args.len() - + usize::from(vararg.is_some()) - + kwonlyargs.len() - + usize::from(kwarg.is_some()); + let num_parameters = item.len(); if self.parentheses == ParametersParentheses::Never { write!(f, [group(&format_inner), dangling_comments(dangling)]) diff --git a/crates/ruff_python_parser/src/parser/statement.rs b/crates/ruff_python_parser/src/parser/statement.rs index 02c5af7f2286c..69d7ec8a57abd 100644 --- a/crates/ruff_python_parser/src/parser/statement.rs +++ b/crates/ruff_python_parser/src/parser/statement.rs @@ -3371,34 +3371,15 @@ impl<'src> Parser<'src> { /// /// 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.posonlyargs.len() - + parameters.args.len() - + usize::from(parameters.vararg.is_some()) - + parameters.kwonlyargs.len() - + usize::from(parameters.kwarg.is_some()), - BuildHasherDefault::default(), - ); + let mut all_arg_names = + FxHashSet::with_capacity_and_hasher(parameters.len(), BuildHasherDefault::default()); - let posonlyargs = parameters.posonlyargs.iter(); - let args = parameters.args.iter(); - let kwonlyargs = parameters.kwonlyargs.iter(); - - let vararg = parameters.vararg.as_deref(); - let kwarg = parameters.kwarg.as_deref(); - - for arg in posonlyargs - .chain(args) - .chain(kwonlyargs) - .map(|arg| &arg.parameter) - .chain(vararg) - .chain(kwarg) - { - let range = arg.name.range; - let arg_name = arg.name.as_str(); - if !all_arg_names.insert(arg_name) { + 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(arg_name.to_string()), + ParseErrorType::DuplicateParameter(param_name.to_string()), range, ); } 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 5d244c2b06f15..47f14abfa5744 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 @@ -141,19 +141,19 @@ Module( | 1 | def foo(a, a=10, *a, a, a: str, **a): ... - | ^ Syntax Error: Duplicate parameter "a" + | ^ Syntax Error: Duplicate parameter "a" | | 1 | def foo(a, a=10, *a, a, a: str, **a): ... - | ^ Syntax Error: Duplicate parameter "a" + | ^ Syntax Error: Duplicate parameter "a" | | 1 | def foo(a, a=10, *a, a, a: str, **a): ... - | ^ Syntax Error: Duplicate parameter "a" + | ^ Syntax Error: Duplicate parameter "a" | diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 4d239017ddfe7..88d0441e68667 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -736,10 +736,7 @@ fn find_parameter<'a>( binding: &Binding, ) -> Option<&'a ParameterWithDefault> { parameters - .args - .iter() - .chain(parameters.posonlyargs.iter()) - .chain(parameters.kwonlyargs.iter()) + .iter_non_variadic_params() .find(|arg| arg.parameter.name.range() == binding.range()) }