Add convenience methods for iterating over all parameter nodes in a function (#11174)

This commit is contained in:
Alex Waygood 2024-04-29 11:36:15 +01:00 committed by GitHub
parent 8a887daeb4
commit 87929ad5f1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
27 changed files with 399 additions and 448 deletions

View file

@ -31,8 +31,8 @@ use std::path::Path;
use itertools::Itertools; use itertools::Itertools;
use log::debug; use log::debug;
use ruff_python_ast::{ use ruff_python_ast::{
self as ast, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext, FStringElement, self as ast, AnyParameterRef, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext,
Keyword, MatchCase, Parameter, ParameterWithDefault, Parameters, Pattern, Stmt, Suite, UnaryOp, FStringElement, Keyword, MatchCase, Parameter, Parameters, Pattern, Stmt, Suite, UnaryOp,
}; };
use ruff_text_size::{Ranged, TextRange, TextSize}; use ruff_text_size::{Ranged, TextRange, TextSize};
@ -604,15 +604,11 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.visit_type_params(type_params); self.visit_type_params(type_params);
} }
for parameter_with_default in parameters for parameter in &**parameters {
.posonlyargs if let Some(expr) = parameter.annotation() {
.iter() if singledispatch && !parameter.is_variadic() {
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{
if let Some(expr) = &parameter_with_default.parameter.annotation {
if singledispatch {
self.visit_runtime_required_annotation(expr); self.visit_runtime_required_annotation(expr);
singledispatch = false;
} else { } else {
match annotation { match annotation {
AnnotationContext::RuntimeRequired => { AnnotationContext::RuntimeRequired => {
@ -625,42 +621,11 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.visit_annotation(expr); self.visit_annotation(expr);
} }
} }
}; }
} }
if let Some(expr) = &parameter_with_default.default { if let Some(expr) = parameter.default() {
self.visit_expr(expr); self.visit_expr(expr);
} }
singledispatch = false;
}
if let Some(arg) = &parameters.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) = &parameters.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);
}
}
}
} }
for expr in returns { for expr in returns {
match annotation { match annotation {
@ -1043,19 +1008,11 @@ impl<'a> Visitor<'a> for Checker<'a> {
) => { ) => {
// Visit the default arguments, but avoid the body, which will be deferred. // Visit the default arguments, but avoid the body, which will be deferred.
if let Some(parameters) = parameters { if let Some(parameters) = parameters {
for ParameterWithDefault { for default in parameters
default, .iter_non_variadic_params()
parameter: _, .filter_map(|param| param.default.as_deref())
range: _,
} in parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{ {
if let Some(expr) = &default { self.visit_expr(default);
self.visit_expr(expr);
}
} }
} }
@ -1483,20 +1440,8 @@ impl<'a> Visitor<'a> for Checker<'a> {
// Step 1: Binding. // Step 1: Binding.
// Bind, but intentionally avoid walking default expressions, as we handle them // Bind, but intentionally avoid walking default expressions, as we handle them
// upstream. // upstream.
for parameter_with_default in &parameters.posonlyargs { for parameter in parameters.iter().map(AnyParameterRef::as_parameter) {
self.visit_parameter(&parameter_with_default.parameter); self.visit_parameter(parameter);
}
for parameter_with_default in &parameters.args {
self.visit_parameter(&parameter_with_default.parameter);
}
if let Some(arg) = &parameters.vararg {
self.visit_parameter(arg);
}
for parameter_with_default in &parameters.kwonlyargs {
self.visit_parameter(&parameter_with_default.parameter);
}
if let Some(arg) = &parameters.kwarg {
self.visit_parameter(arg);
} }
// Step 4: Analysis // Step 4: Analysis

View file

@ -582,6 +582,7 @@ fn is_stub_function(function_def: &ast::StmtFunctionDef, checker: &Checker) -> b
} }
/// Generate flake8-annotation checks for a given `Definition`. /// Generate flake8-annotation checks for a given `Definition`.
/// ANN001, ANN401
pub(crate) fn definition( pub(crate) fn definition(
checker: &Checker, checker: &Checker,
definition: &Definition, definition: &Definition,
@ -615,23 +616,14 @@ pub(crate) fn definition(
let is_overridden = visibility::is_override(decorator_list, checker.semantic()); 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 { for ParameterWithDefault {
parameter, parameter,
default: _, default: _,
range: _, range: _,
} in parameters } in parameters.iter_non_variadic_params().skip(usize::from(
.posonlyargs is_method && !visibility::is_staticmethod(decorator_list, checker.semantic()),
.iter() )) {
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.skip(
// If this is a non-static method, skip `cls` or `self`.
usize::from(
is_method && !visibility::is_staticmethod(decorator_list, checker.semantic()),
),
)
{
// ANN401 for dynamically typed parameters // ANN401 for dynamically typed parameters
if let Some(annotation) = &parameter.annotation { if let Some(annotation) = &parameter.annotation {
has_any_typed_arg = true; has_any_typed_arg = true;

View file

@ -74,11 +74,7 @@ pub(crate) fn hardcoded_password_default(checker: &mut Checker, parameters: &Par
parameter, parameter,
default, default,
range: _, range: _,
} in parameters } in parameters.iter_non_variadic_params()
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{ {
let Some(default) = default else { let Some(default) = default else {
continue; continue;

View file

@ -49,44 +49,35 @@ impl Violation for SslWithBadDefaults {
/// S503 /// S503
pub(crate) fn ssl_with_bad_defaults(checker: &mut Checker, function_def: &StmtFunctionDef) { pub(crate) fn ssl_with_bad_defaults(checker: &mut Checker, function_def: &StmtFunctionDef) {
function_def for default in function_def
.parameters .parameters
.posonlyargs .iter_non_variadic_params()
.iter() .filter_map(|param| param.default.as_deref())
.chain( {
function_def match default {
.parameters Expr::Name(ast::ExprName { id, range, .. }) => {
.args if is_insecure_protocol(id.as_str()) {
.iter() checker.diagnostics.push(Diagnostic::new(
.chain(function_def.parameters.kwonlyargs.iter()), SslWithBadDefaults {
) protocol: id.to_string(),
.for_each(|param| { },
if let Some(default) = &param.default { *range,
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,
));
}
}
_ => {}
} }
} }
}); 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. /// Returns `true` if the given protocol name is insecure.

View file

@ -139,11 +139,7 @@ pub(crate) fn function_call_in_argument_default(checker: &mut Checker, parameter
default, default,
parameter, parameter,
range: _, range: _,
} in parameters } in parameters.iter_non_variadic_params()
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{ {
if let Some(expr) = &default { if let Some(expr) = &default {
if !parameter.annotation.as_ref().is_some_and(|expr| { if !parameter.annotation.as_ref().is_some_and(|expr| {

View file

@ -105,11 +105,7 @@ impl<'a> Visitor<'a> for NameFinder<'a> {
parameter, parameter,
default: _, default: _,
range: _, range: _,
} in parameters } in parameters.iter_non_variadic_params()
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{ {
self.names.remove(parameter.name.as_str()); self.names.remove(parameter.name.as_str());
} }

View file

@ -89,12 +89,7 @@ pub(crate) fn mutable_argument_default(checker: &mut Checker, function_def: &ast
parameter, parameter,
default, default,
range: _, range: _,
} in function_def } in function_def.parameters.iter_non_variadic_params()
.parameters
.posonlyargs
.iter()
.chain(&function_def.parameters.args)
.chain(&function_def.parameters.kwonlyargs)
{ {
let Some(default) = default else { let Some(default) = default else {
continue; continue;

View file

@ -107,10 +107,7 @@ pub(crate) fn unnecessary_map(
if parameters.as_ref().is_some_and(|parameters| { if parameters.as_ref().is_some_and(|parameters| {
late_binding(parameters, body) late_binding(parameters, body)
|| parameters || parameters
.posonlyargs .iter_non_variadic_params()
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.any(|param| param.default.is_some()) .any(|param| param.default.is_some())
|| parameters.vararg.is_some() || parameters.vararg.is_some()
|| parameters.kwarg.is_some() || parameters.kwarg.is_some()
@ -152,10 +149,7 @@ pub(crate) fn unnecessary_map(
if parameters.as_ref().is_some_and(|parameters| { if parameters.as_ref().is_some_and(|parameters| {
late_binding(parameters, body) late_binding(parameters, body)
|| parameters || parameters
.posonlyargs .iter_non_variadic_params()
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.any(|param| param.default.is_some()) .any(|param| param.default.is_some())
|| parameters.vararg.is_some() || parameters.vararg.is_some()
|| parameters.kwarg.is_some() || parameters.kwarg.is_some()
@ -207,10 +201,7 @@ pub(crate) fn unnecessary_map(
if parameters.as_ref().is_some_and(|parameters| { if parameters.as_ref().is_some_and(|parameters| {
late_binding(parameters, body) late_binding(parameters, body)
|| parameters || parameters
.posonlyargs .iter_non_variadic_params()
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.any(|param| param.default.is_some()) .any(|param| param.default.is_some())
|| parameters.vararg.is_some() || parameters.vararg.is_some()
|| parameters.kwarg.is_some() || parameters.kwarg.is_some()

View file

@ -2,7 +2,7 @@ use std::fmt;
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, 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 ruff_text_size::Ranged;
use crate::checkers::ast::Checker; 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, /): ... // Ex) def func(arg: NoReturn, /): ...
// Ex) def func(*, arg: NoReturn): ... // Ex) def func(*, arg: NoReturn): ...
for annotation in parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.filter_map(|arg| arg.parameter.annotation.as_ref())
{
check_no_return_argument_annotation(checker, annotation);
}
// Ex) def func(*args: NoReturn): ... // Ex) def func(*args: NoReturn): ...
if let Some(arg) = &parameters.vararg {
if let Some(annotation) = &arg.annotation {
check_no_return_argument_annotation(checker, annotation);
}
}
// Ex) def func(**kwargs: NoReturn): ... // Ex) def func(**kwargs: NoReturn): ...
if let Some(arg) = &parameters.kwarg { for annotation in parameters.iter().filter_map(AnyParameterRef::annotation) {
if let Some(annotation) = &arg.annotation { if checker.semantic().match_typing_expr(annotation, "NoReturn") {
check_no_return_argument_annotation(checker, annotation); checker.diagnostics.push(Diagnostic::new(
} NoReturnArgumentAnnotationInStub {
} module: if checker.settings.target_version >= Py311 {
} TypingModule::Typing
} else {
fn check_no_return_argument_annotation(checker: &mut Checker, annotation: &Expr) { TypingModule::TypingExtensions
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(), ));
)); }
} }
} }

View file

@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, 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_python_semantic::analyze::typing::traverse_union;
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
@ -61,25 +61,7 @@ impl Violation for RedundantNumericUnion {
/// PYI041 /// PYI041
pub(crate) fn redundant_numeric_union(checker: &mut Checker, parameters: &Parameters) { pub(crate) fn redundant_numeric_union(checker: &mut Checker, parameters: &Parameters) {
for annotation in parameters for annotation in parameters.iter().filter_map(AnyParameterRef::annotation) {
.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())
{
check_annotation(checker, annotation); check_annotation(checker, annotation);
} }
} }

View file

@ -495,11 +495,7 @@ pub(crate) fn typed_argument_simple_defaults(checker: &mut Checker, parameters:
parameter, parameter,
default, default,
range: _, range: _,
} in parameters } in parameters.iter_non_variadic_params()
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{ {
let Some(default) = default else { let Some(default) = default else {
continue; continue;
@ -530,11 +526,7 @@ pub(crate) fn argument_simple_defaults(checker: &mut Checker, parameters: &Param
parameter, parameter,
default, default,
range: _, range: _,
} in parameters } in parameters.iter_non_variadic_params()
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{ {
let Some(default) = default else { let Some(default) = default else {
continue; continue;

View file

@ -8,7 +8,7 @@ use ruff_python_ast::name::UnqualifiedName;
use ruff_python_ast::visitor; use ruff_python_ast::visitor;
use ruff_python_ast::visitor::Visitor; use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::Decorator; 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::analyze::visibility::is_abstract;
use ruff_python_semantic::SemanticModel; use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
@ -841,28 +841,17 @@ fn check_fixture_returns(
/// PT019 /// PT019
fn check_test_function_args(checker: &mut Checker, parameters: &Parameters) { fn check_test_function_args(checker: &mut Checker, parameters: &Parameters) {
parameters for parameter in parameters.iter_non_variadic_params() {
.posonlyargs let name = &parameter.parameter.name;
.iter() if name.starts_with('_') {
.chain(&parameters.args) checker.diagnostics.push(Diagnostic::new(
.chain(&parameters.kwonlyargs) PytestFixtureParamWithoutValue {
.for_each( name: name.to_string(),
|ParameterWithDefault { },
parameter, parameter.range(),
default: _, ));
range: _, }
}| { }
let name = &parameter.name;
if name.starts_with('_') {
checker.diagnostics.push(Diagnostic::new(
PytestFixtureParamWithoutValue {
name: name.to_string(),
},
parameter.range(),
));
}
},
);
} }
/// PT020 /// PT020

View file

@ -1,5 +1,3 @@
use std::iter;
use regex::Regex; use regex::Regex;
use ruff_python_ast as ast; use ruff_python_ast as ast;
use ruff_python_ast::{Parameter, Parameters}; use ruff_python_ast::{Parameter, Parameters};
@ -224,19 +222,20 @@ fn function(
diagnostics: &mut Vec<Diagnostic>, diagnostics: &mut Vec<Diagnostic>,
) { ) {
let args = parameters let args = parameters
.posonlyargs .iter_non_variadic_params()
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.map(|parameter_with_default| &parameter_with_default.parameter) .map(|parameter_with_default| &parameter_with_default.parameter)
.chain( .chain(
iter::once::<Option<&Parameter>>(parameters.vararg.as_deref()) parameters
.flatten() .vararg
.as_deref()
.into_iter()
.skip(usize::from(ignore_variadic_names)), .skip(usize::from(ignore_variadic_names)),
) )
.chain( .chain(
iter::once::<Option<&Parameter>>(parameters.kwarg.as_deref()) parameters
.flatten() .kwarg
.as_deref()
.into_iter()
.skip(usize::from(ignore_variadic_names)), .skip(usize::from(ignore_variadic_names)),
); );
call( call(
@ -260,20 +259,21 @@ fn method(
diagnostics: &mut Vec<Diagnostic>, diagnostics: &mut Vec<Diagnostic>,
) { ) {
let args = parameters let args = parameters
.posonlyargs .iter_non_variadic_params()
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.skip(1) .skip(1)
.map(|parameter_with_default| &parameter_with_default.parameter) .map(|parameter_with_default| &parameter_with_default.parameter)
.chain( .chain(
iter::once::<Option<&Parameter>>(parameters.vararg.as_deref()) parameters
.flatten() .vararg
.as_deref()
.into_iter()
.skip(usize::from(ignore_variadic_names)), .skip(usize::from(ignore_variadic_names)),
) )
.chain( .chain(
iter::once::<Option<&Parameter>>(parameters.kwarg.as_deref()) parameters
.flatten() .kwarg
.as_deref()
.into_iter()
.skip(usize::from(ignore_variadic_names)), .skip(usize::from(ignore_variadic_names)),
); );
call( call(

View file

@ -257,15 +257,9 @@ fn rename_parameter(
) -> Result<Option<Fix>> { ) -> Result<Option<Fix>> {
// Don't fix if another parameter has the valid name. // Don't fix if another parameter has the valid name.
if parameters if parameters
.posonlyargs
.iter() .iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.skip(1) .skip(1)
.map(|parameter_with_default| &parameter_with_default.parameter) .any(|parameter| parameter.name() == function_type.valid_first_argument_name())
.chain(parameters.vararg.as_deref())
.chain(parameters.kwarg.as_deref())
.any(|parameter| &parameter.name == function_type.valid_first_argument_name())
{ {
return Ok(None); return Ok(None);
} }

View file

@ -199,8 +199,8 @@ fn function(
let parameters = parameters.cloned().unwrap_or_default(); let parameters = parameters.cloned().unwrap_or_default();
if let Some(annotation) = annotation { if let Some(annotation) = annotation {
if let Some((arg_types, return_type)) = extract_types(annotation, semantic) { if let Some((arg_types, return_type)) = extract_types(annotation, semantic) {
// A `lambda` expression can only have positional and positional-only // A `lambda` expression can only have positional-only and positional-or-keyword
// arguments. The order is always positional-only first, then positional. // arguments. The order is always positional-only first, then positional-or-keyword.
let new_posonlyargs = parameters let new_posonlyargs = parameters
.posonlyargs .posonlyargs
.iter() .iter()

View file

@ -1755,23 +1755,19 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: &
// Look for arguments that weren't included in the docstring. // Look for arguments that weren't included in the docstring.
let mut missing_arg_names: FxHashSet<String> = FxHashSet::default(); let mut missing_arg_names: FxHashSet<String> = FxHashSet::default();
// If this is a non-static method, skip `cls` or `self`.
for ParameterWithDefault { for ParameterWithDefault {
parameter, parameter,
default: _, default: _,
range: _, range: _,
} in function } in function
.parameters .parameters
.posonlyargs .iter_non_variadic_params()
.iter() .skip(usize::from(
.chain(&function.parameters.args) docstring.definition.is_method()
.chain(&function.parameters.kwonlyargs) && !is_staticmethod(&function.decorator_list, checker.semantic()),
.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()),
),
)
{ {
let arg_name = parameter.name.as_str(); let arg_name = parameter.name.as_str();
if !arg_name.starts_with('_') && !docstrings_args.contains(arg_name) { if !arg_name.starts_with('_') && !docstrings_args.contains(arg_name) {

View file

@ -61,10 +61,7 @@ impl Violation for TooManyArguments {
pub(crate) fn too_many_arguments(checker: &mut Checker, function_def: &ast::StmtFunctionDef) { pub(crate) fn too_many_arguments(checker: &mut Checker, function_def: &ast::StmtFunctionDef) {
let num_arguments = function_def let num_arguments = function_def
.parameters .parameters
.args .iter_non_variadic_params()
.iter()
.chain(&function_def.parameters.kwonlyargs)
.chain(&function_def.parameters.posonlyargs)
.filter(|arg| { .filter(|arg| {
!checker !checker
.settings .settings

View file

@ -62,14 +62,14 @@ pub(crate) fn too_many_positional(checker: &mut Checker, function_def: &ast::Stm
// Count the number of positional arguments. // Count the number of positional arguments.
let num_positional_args = function_def let num_positional_args = function_def
.parameters .parameters
.args .posonlyargs
.iter() .iter()
.chain(&function_def.parameters.posonlyargs) .chain(&function_def.parameters.args)
.filter(|arg| { .filter(|param| {
!checker !checker
.settings .settings
.dummy_variable_rgx .dummy_variable_rgx
.is_match(&arg.parameter.name) .is_match(&param.parameter.name)
}) })
.count(); .count();

View file

@ -167,11 +167,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, parameters: &Parameters)
parameter, parameter,
default, default,
range: _, range: _,
} in parameters } in parameters.iter_non_variadic_params()
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{ {
let Some(default) = default else { continue }; let Some(default) = default else { continue };
if !default.is_none_literal_expr() { if !default.is_none_literal_expr() {

View file

@ -348,39 +348,18 @@ pub fn any_over_stmt(stmt: &Stmt, func: &dyn Fn(&Expr) -> bool) -> bool {
returns, returns,
.. ..
}) => { }) => {
parameters parameters.iter().any(|param| {
.posonlyargs param
.iter() .default()
.chain(parameters.args.iter().chain(parameters.kwonlyargs.iter())) .is_some_and(|default| any_over_expr(default, func))
.any(|parameter| { || param
parameter .annotation()
.default .is_some_and(|annotation| any_over_expr(annotation, func))
.as_ref() }) || type_params.as_ref().is_some_and(|type_params| {
.is_some_and(|expr| any_over_expr(expr, func)) type_params
|| parameter .iter()
.parameter .any(|type_param| any_over_type_param(type_param, func))
.annotation }) || body.iter().any(|stmt| any_over_stmt(stmt, func))
.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))
|| decorator_list || decorator_list
.iter() .iter()
.any(|decorator| any_over_expr(&decorator.expression, func)) .any(|decorator| any_over_expr(&decorator.expression, func))

View file

@ -1,12 +1,13 @@
use crate::visitor::preorder::PreorderVisitor; use crate::visitor::preorder::PreorderVisitor;
use crate::{ use crate::{
self as ast, Alias, ArgOrKeyword, Arguments, Comprehension, Decorator, ExceptHandler, Expr, self as ast, Alias, AnyParameterRef, ArgOrKeyword, Arguments, Comprehension, Decorator,
FStringElement, Keyword, MatchCase, Mod, Parameter, ParameterWithDefault, Parameters, Pattern, ExceptHandler, Expr, FStringElement, Keyword, MatchCase, Mod, Parameter, ParameterWithDefault,
PatternArguments, PatternKeyword, Stmt, StmtAnnAssign, StmtAssert, StmtAssign, StmtAugAssign, Parameters, Pattern, PatternArguments, PatternKeyword, Stmt, StmtAnnAssign, StmtAssert,
StmtBreak, StmtClassDef, StmtContinue, StmtDelete, StmtExpr, StmtFor, StmtFunctionDef, StmtAssign, StmtAugAssign, StmtBreak, StmtClassDef, StmtContinue, StmtDelete, StmtExpr,
StmtGlobal, StmtIf, StmtImport, StmtImportFrom, StmtIpyEscapeCommand, StmtMatch, StmtNonlocal, StmtFor, StmtFunctionDef, StmtGlobal, StmtIf, StmtImport, StmtImportFrom, StmtIpyEscapeCommand,
StmtPass, StmtRaise, StmtReturn, StmtTry, StmtTypeAlias, StmtWhile, StmtWith, TypeParam, StmtMatch, StmtNonlocal, StmtPass, StmtRaise, StmtReturn, StmtTry, StmtTypeAlias, StmtWhile,
TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams, WithItem, StmtWith, TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams,
WithItem,
}; };
use ruff_text_size::{Ranged, TextRange}; use ruff_text_size::{Ranged, TextRange};
use std::ptr::NonNull; use std::ptr::NonNull;
@ -4221,28 +4222,13 @@ impl AstNode for Parameters {
where where
V: PreorderVisitor<'a> + ?Sized, V: PreorderVisitor<'a> + ?Sized,
{ {
let ast::Parameters { for parameter in self {
range: _, match parameter {
posonlyargs, AnyParameterRef::NonVariadic(parameter_with_default) => {
args, visitor.visit_parameter_with_default(parameter_with_default);
vararg, }
kwonlyargs, AnyParameterRef::Variadic(parameter) => visitor.visit_parameter(parameter),
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);
} }
} }
} }

View file

@ -2,6 +2,7 @@
use std::fmt; use std::fmt;
use std::fmt::Debug; use std::fmt::Debug;
use std::iter::FusedIterator;
use std::ops::Deref; use std::ops::Deref;
use std::slice::{Iter, IterMut}; use std::slice::{Iter, IterMut};
use std::sync::OnceLock; use std::sync::OnceLock;
@ -3175,6 +3176,63 @@ pub struct Decorator {
pub expression: Expr, 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) => &param.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. /// 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. /// This form also has advantage to implement pre-order traverse.
/// ///
@ -3196,37 +3254,56 @@ pub struct Parameters {
} }
impl Parameters { impl Parameters {
/// Returns the [`ParameterWithDefault`] with the given name, or `None` if no such [`ParameterWithDefault`] exists. /// Returns an iterator over all non-variadic parameters included in this [`Parameters`] node.
pub fn find(&self, name: &str) -> Option<&ParameterWithDefault> { ///
/// 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<Item = &ParameterWithDefault> {
self.posonlyargs self.posonlyargs
.iter() .iter()
.chain(&self.args) .chain(&self.args)
.chain(&self.kwonlyargs) .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) .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 { pub fn includes(&self, name: &str) -> bool {
if self self.iter().any(|param| param.name() == name)
.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
} }
/// Returns `true` if the [`Parameters`] is empty. /// 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<Self::Item> {
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<usize>) {
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::Item> {
self.next_back()
}
}
impl<'a> DoubleEndedIterator for ParametersIterator<'a> {
fn next_back(&mut self) -> Option<Self::Item> {
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. /// An alternative type of AST `arg`. This is used for each function argument that might have a default value.
/// Used by `Arguments` original type. /// Used by `Arguments` original type.
/// ///

View file

@ -4,11 +4,11 @@ pub mod preorder;
pub mod transformer; pub mod transformer;
use crate::{ use crate::{
self as ast, Alias, Arguments, BoolOp, BytesLiteral, CmpOp, Comprehension, Decorator, self as ast, Alias, AnyParameterRef, Arguments, BoolOp, BytesLiteral, CmpOp, Comprehension,
ElifElseClause, ExceptHandler, Expr, ExprContext, FString, FStringElement, FStringPart, Decorator, ElifElseClause, ExceptHandler, Expr, ExprContext, FString, FStringElement,
Keyword, MatchCase, Operator, Parameter, Parameters, Pattern, PatternArguments, PatternKeyword, FStringPart, Keyword, MatchCase, Operator, Parameter, Parameters, Pattern, PatternArguments,
Stmt, StringLiteral, TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, PatternKeyword, Stmt, StringLiteral, TypeParam, TypeParamParamSpec, TypeParamTypeVar,
TypeParams, UnaryOp, WithItem, TypeParamTypeVarTuple, TypeParams, UnaryOp, WithItem,
}; };
/// A trait for AST visitors. Visits all nodes in the AST recursively in evaluation-order. /// 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) { pub fn walk_parameters<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, parameters: &'a Parameters) {
// Defaults are evaluated before annotations. // Defaults are evaluated before annotations.
for arg in &parameters.posonlyargs { for default in parameters
if let Some(default) = &arg.default { .iter_non_variadic_params()
visitor.visit_expr(default); .filter_map(|param| param.default.as_deref())
} {
} visitor.visit_expr(default);
for arg in &parameters.args {
if let Some(default) = &arg.default {
visitor.visit_expr(default);
}
}
for arg in &parameters.kwonlyargs {
if let Some(default) = &arg.default {
visitor.visit_expr(default);
}
} }
for arg in &parameters.posonlyargs { for parameter in parameters.iter().map(AnyParameterRef::as_parameter) {
visitor.visit_parameter(&arg.parameter); visitor.visit_parameter(parameter);
}
for arg in &parameters.args {
visitor.visit_parameter(&arg.parameter);
}
if let Some(arg) = &parameters.vararg {
visitor.visit_parameter(arg);
}
for arg in &parameters.kwonlyargs {
visitor.visit_parameter(&arg.parameter);
}
if let Some(arg) = &parameters.kwarg {
visitor.visit_parameter(arg);
} }
} }

View file

@ -240,11 +240,7 @@ impl FormatNodeRule<Parameters> for FormatParameters {
Ok(()) Ok(())
}); });
let num_parameters = posonlyargs.len() let num_parameters = item.len();
+ args.len()
+ usize::from(vararg.is_some())
+ kwonlyargs.len()
+ usize::from(kwarg.is_some());
if self.parentheses == ParametersParentheses::Never { if self.parentheses == ParametersParentheses::Never {
write!(f, [group(&format_inner), dangling_comments(dangling)]) write!(f, [group(&format_inner), dangling_comments(dangling)])

View file

@ -3371,34 +3371,15 @@ impl<'src> Parser<'src> {
/// ///
/// Report errors for all the duplicate names found. /// Report errors for all the duplicate names found.
fn validate_parameters(&mut self, parameters: &ast::Parameters) { fn validate_parameters(&mut self, parameters: &ast::Parameters) {
let mut all_arg_names = FxHashSet::with_capacity_and_hasher( let mut all_arg_names =
parameters.posonlyargs.len() FxHashSet::with_capacity_and_hasher(parameters.len(), BuildHasherDefault::default());
+ parameters.args.len()
+ usize::from(parameters.vararg.is_some())
+ parameters.kwonlyargs.len()
+ usize::from(parameters.kwarg.is_some()),
BuildHasherDefault::default(),
);
let posonlyargs = parameters.posonlyargs.iter(); for parameter in parameters {
let args = parameters.args.iter(); let range = parameter.name().range();
let kwonlyargs = parameters.kwonlyargs.iter(); let param_name = parameter.name().as_str();
if !all_arg_names.insert(param_name) {
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) {
self.add_error( self.add_error(
ParseErrorType::DuplicateParameter(arg_name.to_string()), ParseErrorType::DuplicateParameter(param_name.to_string()),
range, range,
); );
} }

View file

@ -139,6 +139,12 @@ Module(
| |
|
1 | def foo(a, a=10, *a, a, a: str, **a): ...
| ^ Syntax Error: Duplicate parameter "a"
|
| |
1 | def foo(a, a=10, *a, a, a: str, **a): ... 1 | def foo(a, a=10, *a, a, a: str, **a): ...
| ^ Syntax Error: Duplicate parameter "a" | ^ Syntax Error: Duplicate parameter "a"
@ -151,12 +157,6 @@ Module(
| |
|
1 | def foo(a, a=10, *a, a, a: str, **a): ...
| ^ Syntax Error: Duplicate parameter "a"
|
| |
1 | def foo(a, a=10, *a, a, a: str, **a): ... 1 | def foo(a, a=10, *a, a, a: str, **a): ...
| ^ Syntax Error: Duplicate parameter "a" | ^ Syntax Error: Duplicate parameter "a"

View file

@ -736,10 +736,7 @@ fn find_parameter<'a>(
binding: &Binding, binding: &Binding,
) -> Option<&'a ParameterWithDefault> { ) -> Option<&'a ParameterWithDefault> {
parameters parameters
.args .iter_non_variadic_params()
.iter()
.chain(parameters.posonlyargs.iter())
.chain(parameters.kwonlyargs.iter())
.find(|arg| arg.parameter.name.range() == binding.range()) .find(|arg| arg.parameter.name.range() == binding.range())
} }