From d9a1034db05ff6627323339d9395e80e78e8f47a Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 1 Feb 2025 17:16:32 +0000 Subject: [PATCH] Add convenience helper methods for AST nodes representing function parameters (#15871) --- .../src/semantic_index/builder.rs | 2 +- .../src/types/infer.rs | 4 +-- .../src/types/signatures.rs | 10 +++---- .../src/rules/airflow/rules/removal_in_3.rs | 6 ++--- .../rules/fastapi_non_annotated_dependency.rs | 5 ++-- .../rules/fastapi_unused_path_parameter.rs | 20 +++++--------- .../flake8_annotations/rules/definition.rs | 20 ++++++-------- .../rules/hardcoded_password_default.rs | 13 +++------- ...olean_default_value_positional_argument.rs | 18 ++++--------- .../boolean_type_hint_positional_argument.rs | 15 ++++------- .../function_call_in_argument_default.rs | 15 ++++------- .../rules/loop_variable_overrides_iterator.rs | 11 +++----- .../rules/mutable_argument_default.rs | 19 +++++--------- .../rules/builtin_argument_shadowing.rs | 2 +- .../builtin_lambda_argument_shadowing.rs | 4 +-- .../flake8_pyi/rules/any_eq_ne_annotation.rs | 2 +- .../rules/custom_type_var_return_type.rs | 10 +++---- .../flake8_pyi/rules/exit_annotations.rs | 21 +++++++-------- .../rules/pre_pep570_positional_argument.rs | 4 +-- .../rules/flake8_pyi/rules/simple_defaults.rs | 26 +++++-------------- .../flake8_pytest_style/rules/fixture.rs | 2 +- .../rules/test_functions.rs | 20 ++++---------- .../rules/unused_arguments.rs | 4 +-- .../src/rules/pydocstyle/rules/sections.rs | 9 ++----- .../src/rules/pylint/rules/no_self_use.rs | 10 ++----- .../pylint/rules/self_or_cls_assignment.rs | 9 +++---- .../rules/pylint/rules/too_many_arguments.rs | 7 +---- .../rules/too_many_positional_arguments.rs | 7 +---- .../rules/super_call_with_parameters.rs | 12 +++------ .../pyupgrade/rules/use_pep646_unpack.rs | 6 ++--- .../refurb/rules/reimplemented_operator.rs | 6 ++--- .../src/rules/ruff/rules/implicit_optional.rs | 18 +++++-------- .../src/rules/ruff/rules/post_init_default.rs | 20 +++++++------- crates/ruff_python_ast/src/nodes.rs | 24 +++++++++++++++++ .../src/comments/placement.rs | 2 +- .../src/analyze/typing.rs | 5 ++-- 36 files changed, 150 insertions(+), 238 deletions(-) diff --git a/crates/red_knot_python_semantic/src/semantic_index/builder.rs b/crates/red_knot_python_semantic/src/semantic_index/builder.rs index 8252a5f7a4..183beebbf9 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/builder.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/builder.rs @@ -605,7 +605,7 @@ impl<'db> SemanticIndexBuilder<'db> { } fn declare_parameter(&mut self, parameter: &'db ast::ParameterWithDefault) { - let symbol = self.add_symbol(parameter.parameter.name.id().clone()); + let symbol = self.add_symbol(parameter.name().id().clone()); let definition = self.add_definition(symbol, parameter); diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index d3ee2cf777..40c9b02d40 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1294,7 +1294,7 @@ impl<'db> TypeInferenceBuilder<'db> { parameter: &ast::Parameter, definition: Definition<'db>, ) { - if let Some(annotation) = parameter.annotation.as_ref() { + if let Some(annotation) = parameter.annotation() { let _annotated_ty = self.file_expression_type(annotation); // TODO `tuple[annotated_ty, ...]` let ty = KnownClass::Tuple.to_instance(self.db()); @@ -1323,7 +1323,7 @@ impl<'db> TypeInferenceBuilder<'db> { parameter: &ast::Parameter, definition: Definition<'db>, ) { - if let Some(annotation) = parameter.annotation.as_ref() { + if let Some(annotation) = parameter.annotation() { let _annotated_ty = self.file_expression_type(annotation); // TODO `dict[str, annotated_ty]` let ty = KnownClass::Dict.to_instance(self.db()); diff --git a/crates/red_knot_python_semantic/src/types/signatures.rs b/crates/red_knot_python_semantic/src/types/signatures.rs index 3d350300a8..6174730bec 100644 --- a/crates/red_knot_python_semantic/src/types/signatures.rs +++ b/crates/red_knot_python_semantic/src/types/signatures.rs @@ -93,10 +93,9 @@ impl<'db> Parameters<'db> { kwarg, range: _, } = parameters; - let default_ty = |parameter_with_default: &ast::ParameterWithDefault| { - parameter_with_default - .default - .as_deref() + let default_ty = |param: &ast::ParameterWithDefault| { + param + .default() .map(|default| definition_expression_type(db, definition, default)) }; let positional_only = posonlyargs.iter().map(|arg| { @@ -243,8 +242,7 @@ impl<'db> Parameter<'db> { Self { name: Some(parameter.name.id.clone()), annotated_ty: parameter - .annotation - .as_deref() + .annotation() .map(|annotation| definition_expression_type(db, definition, annotation)), kind, } diff --git a/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs b/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs index 3dd06cb0a7..904c2f8240 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/removal_in_3.rs @@ -164,14 +164,14 @@ fn check_function_parameters(checker: &mut Checker, function_def: &StmtFunctionD } for param in function_def.parameters.iter_non_variadic_params() { - let param_name = param.parameter.name.as_str(); - if REMOVED_CONTEXT_KEYS.contains(¶m_name) { + let param_name = param.name(); + if REMOVED_CONTEXT_KEYS.contains(¶m_name.as_str()) { checker.diagnostics.push(Diagnostic::new( Airflow3Removal { deprecated: param_name.to_string(), replacement: Replacement::None, }, - param.parameter.name.range(), + param_name.range(), )); } } diff --git a/crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs b/crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs index 65ea5d6aa7..206106a3f0 100644 --- a/crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs +++ b/crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs @@ -107,8 +107,7 @@ pub(crate) fn fastapi_non_annotated_dependency( .iter() .chain(&function_def.parameters.kwonlyargs) { - let (Some(annotation), Some(default)) = - (¶meter.parameter.annotation, ¶meter.default) + let (Some(annotation), Some(default)) = (parameter.annotation(), parameter.default()) else { seen_default |= parameter.default.is_some(); continue; @@ -120,7 +119,7 @@ pub(crate) fn fastapi_non_annotated_dependency( annotation, default, kind: dependency, - name: ¶meter.parameter.name, + name: parameter.name(), range: parameter.range, }; seen_default = create_diagnostic( diff --git a/crates/ruff_linter/src/rules/fastapi/rules/fastapi_unused_path_parameter.rs b/crates/ruff_linter/src/rules/fastapi/rules/fastapi_unused_path_parameter.rs index d1ea93756c..0940ad3e6e 100644 --- a/crates/ruff_linter/src/rules/fastapi/rules/fastapi_unused_path_parameter.rs +++ b/crates/ruff_linter/src/rules/fastapi/rules/fastapi_unused_path_parameter.rs @@ -183,7 +183,7 @@ pub(crate) fn fastapi_unused_path_parameter( .parameters .posonlyargs .iter() - .any(|arg| arg.parameter.name.as_str() == path_param); + .any(|param| param.name() == path_param); let mut diagnostic = Diagnostic::new( FastApiUnusedPathParameter { @@ -258,25 +258,17 @@ impl<'a> Dependency<'a> { /// ): ... /// ``` fn from_parameter( - parameter_with_default: &'a ParameterWithDefault, + parameter: &'a ParameterWithDefault, semantic: &SemanticModel<'a>, ) -> Option { - let ParameterWithDefault { - parameter, default, .. - } = parameter_with_default; - - if let Some(dependency) = default - .as_deref() + if let Some(dependency) = parameter + .default() .and_then(|default| Self::from_default(default, semantic)) { return Some(dependency); } - let Expr::Subscript(ExprSubscript { value, slice, .. }) = - ¶meter.annotation.as_deref()? - else { - return None; - }; + let ExprSubscript { value, slice, .. } = parameter.annotation()?.as_subscript_expr()?; if !semantic.match_typing_expr(value, "Annotated") { return None; @@ -327,7 +319,7 @@ impl<'a> Dependency<'a> { }; let parameter_names = non_posonly_non_variadic_parameters(function_def) - .map(|ParameterWithDefault { parameter, .. }| &*parameter.name) + .map(|param| param.name().as_str()) .collect(); Some(Self::Function(parameter_names)) 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 b1ff9f4f14..8a3667771e 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs +++ b/crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs @@ -3,7 +3,7 @@ use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::helpers::ReturnStatementVisitor; use ruff_python_ast::identifier::Identifier; use ruff_python_ast::visitor::Visitor; -use ruff_python_ast::{self as ast, Expr, ParameterWithDefault, Stmt}; +use ruff_python_ast::{self as ast, Expr, Stmt}; use ruff_python_semantic::analyze::visibility; use ruff_python_semantic::Definition; use ruff_python_stdlib::typing::simple_magic_return_type; @@ -613,21 +613,17 @@ pub(crate) fn definition( let is_overridden = visibility::is_override(decorator_list, checker.semantic()); // If this is a non-static method, skip `cls` or `self`. - for ParameterWithDefault { - parameter, - default: _, - range: _, - } in parameters.iter_non_variadic_params().skip(usize::from( + for parameter 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 { + if let Some(annotation) = parameter.annotation() { has_any_typed_arg = true; if checker.enabled(Rule::AnyType) && !is_overridden { check_dynamically_typed( checker, annotation, - || parameter.name.to_string(), + || parameter.name().to_string(), &mut diagnostics, ); } @@ -636,14 +632,14 @@ pub(crate) fn definition( && checker .settings .dummy_variable_rgx - .is_match(¶meter.name)) + .is_match(parameter.name())) { if checker.enabled(Rule::MissingTypeFunctionArgument) { diagnostics.push(Diagnostic::new( MissingTypeFunctionArgument { - name: parameter.name.to_string(), + name: parameter.name().to_string(), }, - parameter.range(), + parameter.parameter.range(), )); } } @@ -915,7 +911,7 @@ pub(crate) fn definition( .posonlyargs .first() .or_else(|| parameters.args.first()) - .is_some_and(|first_param| first_param.parameter.annotation.is_some())) + .is_some_and(|first_param| first_param.annotation().is_some())) { diagnostics } else { 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 925002e066..1f7c25d2e2 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 @@ -1,4 +1,4 @@ -use ruff_python_ast::{Expr, Parameter, ParameterWithDefault, Parameters}; +use ruff_python_ast::{Expr, Parameter, Parameters}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; @@ -70,16 +70,11 @@ fn check_password_kwarg(parameter: &Parameter, default: &Expr) -> Option Visitor<'a> for NameFinder<'a> { visitor::walk_expr(self, body); if let Some(parameters) = parameters { - for ParameterWithDefault { - parameter, - default: _, - range: _, - } in parameters.iter_non_variadic_params() - { - self.names.remove(parameter.name.as_str()); + for parameter 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 d43fddbe61..219a444c4f 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 @@ -2,7 +2,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::helpers::is_docstring_stmt; use ruff_python_ast::name::QualifiedName; -use ruff_python_ast::{self as ast, Expr, Parameter, ParameterWithDefault}; +use ruff_python_ast::{self as ast, Expr, Parameter}; use ruff_python_codegen::{Generator, Stylist}; use ruff_python_index::Indexer; use ruff_python_semantic::analyze::function_type::is_stub; @@ -91,13 +91,8 @@ pub(crate) fn mutable_argument_default(checker: &mut Checker, function_def: &ast return; } - for ParameterWithDefault { - parameter, - default, - range: _, - } in function_def.parameters.iter_non_variadic_params() - { - let Some(default) = default else { + for parameter in function_def.parameters.iter_non_variadic_params() { + let Some(default) = parameter.default() else { continue; }; @@ -110,7 +105,7 @@ pub(crate) fn mutable_argument_default(checker: &mut Checker, function_def: &ast .collect(); if is_mutable_expr(default, checker.semantic()) - && !parameter.annotation.as_ref().is_some_and(|expr| { + && !parameter.annotation().is_some_and(|expr| { is_immutable_annotation(expr, checker.semantic(), extend_immutable_calls.as_slice()) }) { @@ -119,7 +114,7 @@ pub(crate) fn mutable_argument_default(checker: &mut Checker, function_def: &ast // If the function body is on the same line as the function def, do not fix if let Some(fix) = move_initialization( function_def, - parameter, + ¶meter.parameter, default, checker.semantic(), checker.locator(), @@ -165,12 +160,12 @@ fn move_initialization( // Add an `if`, to set the argument to its original value if still `None`. let mut content = String::new(); - content.push_str(&format!("if {} is None:", parameter.name.as_str())); + content.push_str(&format!("if {} is None:", parameter.name())); content.push_str(stylist.line_ending().as_str()); content.push_str(stylist.indentation()); content.push_str(&format!( "{} = {}", - parameter.name.as_str(), + parameter.name(), generator.expr(default) )); content.push_str(stylist.line_ending().as_str()); diff --git a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs index a406a179ae..d546e14e80 100644 --- a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs +++ b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs @@ -65,7 +65,7 @@ impl Violation for BuiltinArgumentShadowing { /// A002 pub(crate) fn builtin_argument_shadowing(checker: &mut Checker, parameter: &Parameter) { if shadows_builtin( - parameter.name.as_str(), + parameter.name(), checker.source_type, &checker.settings.flake8_builtins.builtins_ignorelist, checker.settings.target_version, diff --git a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_lambda_argument_shadowing.rs b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_lambda_argument_shadowing.rs index 049bc738ec..2a2c3cc2ee 100644 --- a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_lambda_argument_shadowing.rs +++ b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_lambda_argument_shadowing.rs @@ -39,9 +39,9 @@ pub(crate) fn builtin_lambda_argument_shadowing(checker: &mut Checker, lambda: & return; }; for param in parameters.iter_non_variadic_params() { - let name = ¶m.parameter.name; + let name = param.name(); if shadows_builtin( - name.as_ref(), + name, checker.source_type, &checker.settings.flake8_builtins.builtins_ignorelist, checker.settings.target_version, diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs index 83df7ce453..f129fa6e38 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/any_eq_ne_annotation.rs @@ -68,7 +68,7 @@ pub(crate) fn any_eq_ne_annotation(checker: &mut Checker, name: &str, parameters return; } - let Some(annotation) = ¶meters.args[1].parameter.annotation else { + let Some(annotation) = ¶meters.args[1].annotation() else { return; }; diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs index 0be37a8bca..82fb98497b 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs @@ -95,7 +95,7 @@ pub(crate) fn custom_type_var_return_type( .iter() .chain(¶meters.args) .next() - .and_then(|parameter_with_default| parameter_with_default.parameter.annotation.as_ref()) + .and_then(|parameter_with_default| parameter_with_default.annotation()) else { return; }; @@ -341,12 +341,8 @@ fn remove_first_parameter_annotation(parameters: &Parameters) -> Edit { // The first parameter is guaranteed to be `self`/`cls`, // as verified by `uses_custom_var()`. let mut non_variadic_positional = parameters.posonlyargs.iter().chain(¶meters.args); - let first = &non_variadic_positional.next().unwrap().parameter; - - let name_end = first.name.range.end(); - let annotation_end = first.range.end(); - - Edit::deletion(name_end, annotation_end) + let first = &non_variadic_positional.next().unwrap(); + Edit::deletion(first.name().end(), first.end()) } fn replace_return_annotation_with_self(self_symbol_binding: String, returns: &Expr) -> Edit { diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs index dc4b4ce768..8959cae007 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/exit_annotations.rs @@ -199,8 +199,7 @@ pub(crate) fn bad_exit_annotation(checker: &mut Checker, function: &StmtFunction fn check_short_args_list(checker: &mut Checker, parameters: &Parameters, func_kind: FuncKind) { if let Some(varargs) = ¶meters.vararg { if let Some(annotation) = varargs - .annotation - .as_ref() + .annotation() .filter(|ann| !is_object_or_unused(ann, checker.semantic())) { let mut diagnostic = Diagnostic::new( @@ -238,7 +237,7 @@ fn check_short_args_list(checker: &mut Checker, parameters: &Parameters, func_ki /// (that is not decorated with `@typing.overload`) are annotated correctly. fn check_positional_args_for_non_overloaded_method( checker: &mut Checker, - non_self_positional_args: &[&ParameterWithDefault], + non_self_positional_params: &[&ParameterWithDefault], kind: FuncKind, ) { // For each argument, define the predicate against which to check the annotation. @@ -252,8 +251,10 @@ fn check_positional_args_for_non_overloaded_method( (ErrorKind::ThirdArgBadAnnotation, is_traceback_type), ]; - for (arg, (error_info, predicate)) in non_self_positional_args.iter().take(3).zip(validations) { - let Some(annotation) = arg.parameter.annotation.as_ref() else { + for (param, (error_info, predicate)) in + non_self_positional_params.iter().take(3).zip(validations) + { + let Some(annotation) = param.annotation() else { continue; }; @@ -293,13 +294,9 @@ fn check_positional_args_for_overloaded_method( predicate: impl FnOnce(&Expr) -> bool, semantic: &SemanticModel, ) -> bool { - parameter - .parameter - .annotation - .as_ref() - .map_or(true, |annotation| { - predicate(annotation) || is_object_or_unused(annotation, semantic) - }) + parameter.annotation().map_or(true, |annotation| { + predicate(annotation) || is_object_or_unused(annotation, semantic) + }) } let semantic = checker.semantic(); diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/pre_pep570_positional_argument.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/pre_pep570_positional_argument.rs index f76a8c2ae9..6eae403344 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/pre_pep570_positional_argument.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/pre_pep570_positional_argument.rs @@ -100,7 +100,7 @@ pub(crate) fn pep_484_positional_parameter( /// Returns `true` if the [`ParameterWithDefault`] is an old-style positional-only parameter (i.e., /// its name starts with `__` and does not end with `__`). -fn is_old_style_positional_only(arg: &ParameterWithDefault) -> bool { - let arg_name = &arg.parameter.name; +fn is_old_style_positional_only(param: &ParameterWithDefault) -> bool { + let arg_name = param.name(); arg_name.starts_with("__") && !arg_name.ends_with("__") } 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 1ae0b94fa6..8c7dfff302 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 @@ -1,9 +1,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::name::QualifiedName; -use ruff_python_ast::{ - self as ast, Expr, Operator, ParameterWithDefault, Parameters, Stmt, UnaryOp, -}; +use ruff_python_ast::{self as ast, Expr, Operator, Parameters, Stmt, UnaryOp}; use ruff_python_semantic::{analyze::class::is_enumeration, ScopeKind, SemanticModel}; use ruff_text_size::Ranged; @@ -489,16 +487,11 @@ fn is_annotatable_type_alias(value: &Expr, semantic: &SemanticModel) -> bool { /// PYI011 pub(crate) fn typed_argument_simple_defaults(checker: &mut Checker, parameters: &Parameters) { - for ParameterWithDefault { - parameter, - default, - range: _, - } in parameters.iter_non_variadic_params() - { - let Some(default) = default else { + for parameter in parameters.iter_non_variadic_params() { + let Some(default) = parameter.default() else { continue; }; - if parameter.annotation.is_some() { + if parameter.annotation().is_some() { if !is_valid_default_value_with_annotation( default, true, @@ -520,16 +513,11 @@ pub(crate) fn typed_argument_simple_defaults(checker: &mut Checker, parameters: /// PYI014 pub(crate) fn argument_simple_defaults(checker: &mut Checker, parameters: &Parameters) { - for ParameterWithDefault { - parameter, - default, - range: _, - } in parameters.iter_non_variadic_params() - { - let Some(default) = default else { + for parameter in parameters.iter_non_variadic_params() { + let Some(default) = parameter.default() else { continue; }; - if parameter.annotation.is_none() { + if parameter.annotation().is_none() { if !is_valid_default_value_with_annotation( default, true, 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 5959fb7973..e900b280bc 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 @@ -809,7 +809,7 @@ fn check_fixture_returns(checker: &mut Checker, name: &str, body: &[Stmt], retur /// PT019 fn check_test_function_args(checker: &mut Checker, parameters: &Parameters) { for parameter in parameters.iter_non_variadic_params() { - let name = ¶meter.parameter.name; + let name = parameter.name(); if name.starts_with('_') { checker.diagnostics.push(Diagnostic::new( PytestFixtureParamWithoutValue { diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs index 6276db0c07..ace23d9634 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/test_functions.rs @@ -2,7 +2,7 @@ use crate::checkers::ast::Checker; use crate::rules::flake8_pytest_style::rules::helpers::is_likely_pytest_test; use ruff_diagnostics::{Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::{ParameterWithDefault, StmtFunctionDef}; +use ruff_python_ast::StmtFunctionDef; use ruff_text_size::Ranged; /// ## What it does @@ -58,25 +58,15 @@ pub(crate) fn parameter_with_default_argument( return; } - let parameters = function_def.parameters.as_ref(); - - for ParameterWithDefault { - parameter, - default, - range: pwd_range, - } in parameters.iter_non_variadic_params() - { - let Some(default) = default else { + for parameter in function_def.parameters.iter_non_variadic_params() { + let Some(default) = parameter.default() else { continue; }; - - let parameter_name = parameter.name.to_string(); + let parameter_name = parameter.name().to_string(); let kind = PytestParameterWithDefaultArgument { parameter_name }; let diagnostic = Diagnostic::new(kind, default.range()); - - let edit = Edit::deletion(parameter.end(), pwd_range.end()); + let edit = Edit::deletion(parameter.parameter.end(), parameter.end()); let fix = Fix::display_only_edit(edit); - checker.diagnostics.push(diagnostic.with_fix(fix)); } } 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 02ff9efbab..5aacd3985d 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 @@ -330,11 +330,11 @@ fn call<'a>( ) { diagnostics.extend(parameters.filter_map(|arg| { let binding = scope - .get(arg.name.as_str()) + .get(arg.name()) .map(|binding_id| semantic.binding(binding_id))?; if binding.kind.is_argument() && binding.is_unused() - && !dummy_variable_rgx.is_match(arg.name.as_str()) + && !dummy_variable_rgx.is_match(arg.name()) { Some(Diagnostic::new( argumentable.check_for(arg.name.to_string()), diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs index e0ad726471..cb2bb95331 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs @@ -8,7 +8,6 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::docstrings::{clean_space, leading_space}; use ruff_python_ast::identifier::Identifier; -use ruff_python_ast::ParameterWithDefault; use ruff_python_semantic::analyze::visibility::is_staticmethod; use ruff_python_trivia::textwrap::dedent; use ruff_source_file::NewlineWithTrailingNewline; @@ -1791,11 +1790,7 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: & 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 + for parameter in function .parameters .iter_non_variadic_params() .skip(usize::from( @@ -1803,7 +1798,7 @@ fn missing_args(checker: &mut Checker, docstring: &Docstring, docstrings_args: & && !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) { missing_arg_names.insert(arg_name.to_string()); } diff --git a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs index 7cb3ba88d8..6599e9ed20 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs @@ -104,17 +104,11 @@ pub(crate) fn no_self_use( } // Identify the `self` parameter. - let Some(parameter) = parameters - .posonlyargs - .iter() - .chain(¶meters.args) - .next() - .map(|param| ¶m.parameter) - else { + let Some(parameter) = parameters.posonlyargs.iter().chain(¶meters.args).next() else { return; }; - if parameter.name.as_str() != "self" { + if parameter.name() != "self" { return; } diff --git a/crates/ruff_linter/src/rules/pylint/rules/self_or_cls_assignment.rs b/crates/ruff_linter/src/rules/pylint/rules/self_or_cls_assignment.rs index 3e28dae9fb..2b27a8bfee 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/self_or_cls_assignment.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/self_or_cls_assignment.rs @@ -1,6 +1,6 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::{self as ast, Expr, ParameterWithDefault}; +use ruff_python_ast::{self as ast, Expr}; use ruff_python_semantic::analyze::function_type::{self as function_type, FunctionType}; use ruff_python_semantic::ScopeKind; use ruff_text_size::Ranged; @@ -82,10 +82,7 @@ pub(crate) fn self_or_cls_assignment(checker: &mut Checker, target: &Expr) { return; }; - let Some(ParameterWithDefault { - parameter: self_or_cls, - .. - }) = parameters + let Some(self_or_cls) = parameters .posonlyargs .first() .or_else(|| parameters.args.first()) @@ -102,7 +99,7 @@ pub(crate) fn self_or_cls_assignment(checker: &mut Checker, target: &Expr) { &checker.settings.pep8_naming.staticmethod_decorators, ); - let method_type = match (function_type, self_or_cls.name.as_str()) { + let method_type = match (function_type, self_or_cls.name().as_str()) { (FunctionType::Method { .. }, "self") => MethodType::Instance, (FunctionType::ClassMethod { .. }, "cls") => MethodType::Class, _ => return, 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 0bd87f6a01..e7c6bef1ca 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 @@ -68,12 +68,7 @@ pub(crate) fn too_many_arguments(checker: &mut Checker, function_def: &ast::Stmt let num_arguments = function_def .parameters .iter_non_variadic_params() - .filter(|arg| { - !checker - .settings - .dummy_variable_rgx - .is_match(&arg.parameter.name) - }) + .filter(|param| !checker.settings.dummy_variable_rgx.is_match(param.name())) .count(); if num_arguments <= checker.settings.pylint.max_args { diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_positional_arguments.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_positional_arguments.rs index 970ccbe56a..ddd2a5d363 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_positional_arguments.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_positional_arguments.rs @@ -72,12 +72,7 @@ pub(crate) fn too_many_positional_arguments( .posonlyargs .iter() .chain(&function_def.parameters.args) - .filter(|param| { - !checker - .settings - .dummy_variable_rgx - .is_match(¶m.parameter.name) - }) + .filter(|param| !checker.settings.dummy_variable_rgx.is_match(param.name())) .count(); if num_positional_args <= checker.settings.pylint.max_positional_args { diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/super_call_with_parameters.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/super_call_with_parameters.rs index 83680f35a8..2a6d7ee81a 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/super_call_with_parameters.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/super_call_with_parameters.rs @@ -1,6 +1,6 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::{self as ast, Expr, Parameter, ParameterWithDefault, Stmt}; +use ruff_python_ast::{self as ast, Expr, Stmt}; use ruff_text_size::{Ranged, TextSize}; use crate::checkers::ast::Checker; @@ -90,13 +90,7 @@ pub(crate) fn super_call_with_parameters(checker: &mut Checker, call: &ast::Expr }; // Extract the name of the first argument to the enclosing function. - let Some(ParameterWithDefault { - parameter: Parameter { - name: parent_arg, .. - }, - .. - }) = parent_parameters.args.first() - else { + let Some(parent_arg) = parent_parameters.args.first() else { return; }; @@ -122,7 +116,7 @@ pub(crate) fn super_call_with_parameters(checker: &mut Checker, call: &ast::Expr return; }; - if !(first_arg_id == parent_name.as_str() && second_arg_id == parent_arg.as_str()) { + if !(first_arg_id == parent_name.as_str() && second_arg_id == parent_arg.name().as_str()) { return; } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs index f22e5a4f55..8d802dd3ac 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep646_unpack.rs @@ -1,6 +1,6 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; -use ruff_python_ast::ExprSubscript; +use ruff_python_ast::{Expr, ExprSubscript}; use ruff_python_semantic::SemanticModel; use crate::{checkers::ast::Checker, settings::types::PythonVersion}; @@ -142,7 +142,7 @@ fn in_vararg(expr: &ExprSubscript, semantic: &SemanticModel) -> bool { .parameters .vararg .as_ref() - .and_then(|vararg| vararg.annotation.as_ref()) - .and_then(|annotation| annotation.as_subscript_expr()) + .and_then(|vararg| vararg.annotation()) + .and_then(Expr::as_subscript_expr) == Some(expr) } diff --git a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs index 5644cc2c72..23c6e7b3d7 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/reimplemented_operator.rs @@ -456,11 +456,11 @@ fn match_arguments( /// Returns `true` if the given argument is the "same" as the given expression. For example, if /// the argument has a default, it is not considered the same as any expression; if both match the /// same name, they are considered the same. -fn is_same_expression(arg: &ast::ParameterWithDefault, expr: &Expr) -> bool { - if arg.default.is_some() { +fn is_same_expression(param: &ast::ParameterWithDefault, expr: &Expr) -> bool { + if param.default.is_some() { false } else if let Expr::Name(name) = expr { - name.id == arg.parameter.name.as_str() + name.id == param.name().as_str() } else { false } 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 69af7d4bc1..744679a77a 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs @@ -6,7 +6,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::name::Name; -use ruff_python_ast::{self as ast, Expr, Operator, ParameterWithDefault, Parameters}; +use ruff_python_ast::{self as ast, Expr, Operator, Parameters}; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; @@ -163,21 +163,15 @@ fn generate_fix(checker: &Checker, conversion_type: ConversionType, expr: &Expr) /// RUF013 pub(crate) fn implicit_optional(checker: &mut Checker, parameters: &Parameters) { - for ParameterWithDefault { - parameter, - default, - range: _, - } in parameters.iter_non_variadic_params() - { - let Some(default) = default else { continue }; - if !default.is_none_literal_expr() { + for parameter in parameters.iter_non_variadic_params() { + let Some(Expr::NoneLiteral(_)) = parameter.default() else { continue; - } - let Some(annotation) = ¶meter.annotation else { + }; + let Some(annotation) = parameter.annotation() else { continue; }; - if let Expr::StringLiteral(string_expr) = annotation.as_ref() { + if let Expr::StringLiteral(string_expr) = annotation { // Quoted annotation. if let Ok(parsed_annotation) = checker.parse_type_annotation(string_expr) { let Some(expr) = type_hint_explicitly_allows_none( diff --git a/crates/ruff_linter/src/rules/ruff/rules/post_init_default.rs b/crates/ruff_linter/src/rules/ruff/rules/post_init_default.rs index 06cfb062f0..937d1ae2ac 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/post_init_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/post_init_default.rs @@ -104,20 +104,21 @@ pub(crate) fn post_init_default(checker: &mut Checker, function_def: &ast::StmtF let mut stopped_fixes = false; let mut diagnostics = vec![]; - for ast::ParameterWithDefault { - parameter, - default, - range: _, - } in function_def.parameters.iter_non_variadic_params() - { - let Some(default) = default else { + for parameter in function_def.parameters.iter_non_variadic_params() { + let Some(default) = parameter.default() else { continue; }; let mut diagnostic = Diagnostic::new(PostInitDefault, default.range()); if !stopped_fixes { diagnostic.try_set_fix(|| { - use_initvar(current_scope, function_def, parameter, default, checker) + use_initvar( + current_scope, + function_def, + ¶meter.parameter, + default, + checker, + ) }); // Need to stop fixes as soon as there is a parameter we cannot fix. // Otherwise, we risk a syntax error (a parameter without a default @@ -169,8 +170,7 @@ fn use_initvar( let line_ending = checker.stylist().line_ending().as_str(); if let Some(annotation) = ¶meter - .annotation - .as_deref() + .annotation() .map(|annotation| locator.slice(annotation)) { format!("{parameter_name}: {initvar_binding}[{annotation}] = {default}{line_ending}") diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 3727e43cd7..618fa5c71c 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -2654,6 +2654,16 @@ pub struct Parameter { pub annotation: Option>, } +impl Parameter { + pub const fn name(&self) -> &Identifier { + &self.name + } + + pub fn annotation(&self) -> Option<&Expr> { + self.annotation.as_deref() + } +} + /// See also [keyword](https://docs.python.org/3/library/ast.html#ast.keyword) #[derive(Clone, Debug, PartialEq)] pub struct Keyword { @@ -3147,6 +3157,20 @@ pub struct ParameterWithDefault { pub default: Option>, } +impl ParameterWithDefault { + pub fn default(&self) -> Option<&Expr> { + self.default.as_deref() + } + + pub const fn name(&self) -> &Identifier { + self.parameter.name() + } + + pub fn annotation(&self) -> Option<&Expr> { + self.parameter.annotation() + } +} + /// An AST node used to represent the arguments passed to a function call or class definition. /// /// For example, given: diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 6b3b07d966..8c8744f970 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -798,7 +798,7 @@ fn handle_parameter_comment<'a>( parameter: &'a Parameter, source: &str, ) -> CommentPlacement<'a> { - if parameter.annotation.as_deref().is_some() { + if parameter.annotation().is_some() { let colon = first_non_trivia_token(parameter.name.end(), source).expect( "A annotated parameter should have a colon following its name when it is valid syntax.", ); diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index d9c918c36b..49466a6534 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -1,6 +1,7 @@ //! Analysis rules for the `typing` module. use ruff_python_ast::helpers::{any_over_expr, is_const_false, map_subscript}; +use ruff_python_ast::identifier::Identifier; use ruff_python_ast::name::QualifiedName; use ruff_python_ast::{ self as ast, Expr, ExprCall, Int, Operator, ParameterWithDefault, Parameters, Stmt, StmtAssign, @@ -617,7 +618,7 @@ fn check_type(binding: &Binding, semantic: &SemanticModel) -> bo let Some(parameter) = find_parameter(parameters, binding) else { return false; }; - let Some(ref annotation) = parameter.parameter.annotation else { + let Some(annotation) = parameter.annotation() else { return false; }; T::match_annotation(annotation, semantic) @@ -1026,7 +1027,7 @@ fn find_parameter<'a>( ) -> Option<&'a ParameterWithDefault> { parameters .iter_non_variadic_params() - .find(|arg| arg.parameter.name.range() == binding.range()) + .find(|param| param.identifier() == binding.range()) } /// Return the [`QualifiedName`] of the value to which the given [`Expr`] is assigned, if any.