Add convenience helper methods for AST nodes representing function parameters (#15871)

Author: AlexWaygood

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);
 

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());

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,
         }

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(&param_name) {
+        let param_name = param.name();
+        if REMOVED_CONTEXT_KEYS.contains(&param_name.as_str()) {
             checker.diagnostics.push(Diagnostic::new(
                 Airflow3Removal {
                     deprecated: param_name.to_string(),
                     replacement: Replacement::None,
                 },
-                param.parameter.name.range(),
+                param_name.range(),
             ));
         }
     }

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)) =
-            (&parameter.parameter.annotation, &parameter.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: &parameter.parameter.name,
+                name: parameter.name(),
                 range: parameter.range,
             };
             seen_default = create_diagnostic(

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<Self> {
-        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, .. }) =
-            &parameter.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))

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) = &parameter.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(&parameter.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 {

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<Diagnos
 
 /// S107
 pub(crate) fn hardcoded_password_default(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 let Some(diagnostic) = check_password_kwarg(parameter, default) {
+        if let Some(diagnostic) = check_password_kwarg(&parameter.parameter, default) {
             checker.diagnostics.push(diagnostic);
         }
     }

crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_default_value_positional_argument.rs

@@ -1,9 +1,9 @@
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::identifier::Identifier;
 use ruff_python_ast::name::UnqualifiedName;
-use ruff_python_ast::{Decorator, ParameterWithDefault, Parameters};
+use ruff_python_ast::{Decorator, Expr, Parameters};
 use ruff_python_semantic::analyze::visibility;
-use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
 use crate::rules::flake8_boolean_trap::helpers::is_allowed_func_def;
@@ -115,16 +115,8 @@ pub(crate) fn boolean_default_value_positional_argument(
         return;
     }
 
-    for ParameterWithDefault {
-        parameter,
-        default,
-        range: _,
-    } in parameters.posonlyargs.iter().chain(&parameters.args)
-    {
-        if default
-            .as_ref()
-            .is_some_and(|default| default.is_boolean_literal_expr())
-        {
+    for param in parameters.posonlyargs.iter().chain(&parameters.args) {
+        if param.default().is_some_and(Expr::is_boolean_literal_expr) {
             // Allow Boolean defaults in setters.
             if decorator_list.iter().any(|decorator| {
                 UnqualifiedName::from_expr(&decorator.expression)
@@ -141,7 +133,7 @@ pub(crate) fn boolean_default_value_positional_argument(
 
             checker.diagnostics.push(Diagnostic::new(
                 BooleanDefaultValuePositionalArgument,
-                parameter.name.range(),
+                param.identifier(),
             ));
         }
     }

crates/ruff_linter/src/rules/flake8_boolean_trap/rules/boolean_type_hint_positional_argument.rs

@@ -1,11 +1,11 @@
 use ruff_diagnostics::Diagnostic;
 use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::identifier::Identifier;
 use ruff_python_ast::name::UnqualifiedName;
-use ruff_python_ast::{self as ast, Decorator, Expr, ParameterWithDefault, Parameters};
+use ruff_python_ast::{self as ast, Decorator, Expr, Parameters};
 use ruff_python_semantic::analyze::visibility;
 use ruff_python_semantic::SemanticModel;
-use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
 use crate::rules::flake8_boolean_trap::helpers::is_allowed_func_def;
@@ -124,13 +124,8 @@ pub(crate) fn boolean_type_hint_positional_argument(
         return;
     }
 
-    for ParameterWithDefault {
-        parameter,
-        default: _,
-        range: _,
-    } in parameters.posonlyargs.iter().chain(&parameters.args)
-    {
-        let Some(annotation) = parameter.annotation.as_ref() else {
+    for parameter in parameters.posonlyargs.iter().chain(&parameters.args) {
+        let Some(annotation) = parameter.annotation() else {
             continue;
         };
         if checker.settings.preview.is_enabled() {
@@ -164,7 +159,7 @@ pub(crate) fn boolean_type_hint_positional_argument(
 
         checker.diagnostics.push(Diagnostic::new(
             BooleanTypeHintPositionalArgument,
-            parameter.name.range(),
+            parameter.identifier(),
         ));
     }
 }

crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs

@@ -1,4 +1,4 @@
-use ruff_python_ast::{self as ast, Expr, ParameterWithDefault, Parameters};
+use ruff_python_ast::{self as ast, Expr, Parameters};
 use ruff_text_size::{Ranged, TextRange};
 
 use ruff_diagnostics::Violation;
@@ -139,17 +139,12 @@ pub(crate) fn function_call_in_argument_default(checker: &mut Checker, parameter
         .collect();
 
     let mut visitor = ArgumentDefaultVisitor::new(checker.semantic(), &extend_immutable_calls);
-    for ParameterWithDefault {
-        default,
-        parameter,
-        range: _,
-    } in parameters.iter_non_variadic_params()
-    {
-        if let Some(expr) = &default {
-            if !parameter.annotation.as_ref().is_some_and(|expr| {
+    for parameter in parameters.iter_non_variadic_params() {
+        if let Some(default) = parameter.default() {
+            if !parameter.annotation().is_some_and(|expr| {
                 is_immutable_annotation(expr, checker.semantic(), &extend_immutable_calls)
             }) {
-                visitor.visit_expr(expr);
+                visitor.visit_expr(default);
             }
         }
     }

crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs

@@ -1,4 +1,4 @@
-use ruff_python_ast::{self as ast, Expr, ParameterWithDefault};
+use ruff_python_ast::{self as ast, Expr};
 use rustc_hash::FxHashMap;
 
 use ruff_diagnostics::{Diagnostic, Violation};
@@ -101,13 +101,8 @@ impl<'a> 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());
                     }
                 }
             }