Skip to content

Commit

Permalink
Rename Parameter#arg and ParameterWithDefault#def fields (#6255)
Browse files Browse the repository at this point in the history
## Summary

This PR renames...

- `Parameter#arg` to `Parameter#name`
- `ParameterWithDefault#def` to `ParameterWithDefault#parameter` (such
that `ParameterWithDefault` has a `default` and a `parameter`)

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh authored Aug 1, 2023
1 parent adc8bb7 commit 9c708d8
Show file tree
Hide file tree
Showing 56 changed files with 268 additions and 260 deletions.
4 changes: 2 additions & 2 deletions crates/ruff/src/checkers/ast/analyze/parameter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ use crate::rules::{flake8_builtins, pep8_naming, pycodestyle};
pub(crate) fn parameter(parameter: &Parameter, checker: &mut Checker) {
if checker.enabled(Rule::AmbiguousVariableName) {
if let Some(diagnostic) =
pycodestyle::rules::ambiguous_variable_name(&parameter.arg, parameter.range())
pycodestyle::rules::ambiguous_variable_name(&parameter.name, parameter.range())
{
checker.diagnostics.push(diagnostic);
}
}
if checker.enabled(Rule::InvalidArgumentName) {
if let Some(diagnostic) = pep8_naming::rules::invalid_argument_name(
&parameter.arg,
&parameter.name,
parameter,
&checker.settings.pep8_naming.ignore_names,
) {
Expand Down
12 changes: 6 additions & 6 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ where
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{
if let Some(expr) = &parameter_with_default.def.annotation {
if let Some(expr) = &parameter_with_default.parameter.annotation {
if runtime_annotation {
self.visit_runtime_annotation(expr);
} else {
Expand Down Expand Up @@ -896,7 +896,7 @@ where
// Visit the default arguments, but avoid the body, which will be deferred.
for ParameterWithDefault {
default,
def: _,
parameter: _,
range: _,
} in parameters
.posonlyargs
Expand Down Expand Up @@ -1298,16 +1298,16 @@ where
// Bind, but intentionally avoid walking default expressions, as we handle them
// upstream.
for parameter_with_default in &parameters.posonlyargs {
self.visit_parameter(&parameter_with_default.def);
self.visit_parameter(&parameter_with_default.parameter);
}
for parameter_with_default in &parameters.args {
self.visit_parameter(&parameter_with_default.def);
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.def);
self.visit_parameter(&parameter_with_default.parameter);
}
if let Some(arg) = &parameters.kwarg {
self.visit_parameter(arg);
Expand All @@ -1322,7 +1322,7 @@ where
// Bind, but intentionally avoid walking the annotation, as we handle it
// upstream.
self.add_binding(
&parameter.arg,
&parameter.name,
parameter.identifier(),
BindingKind::Argument,
BindingFlags::empty(),
Expand Down
39 changes: 21 additions & 18 deletions crates/ruff/src/rules/flake8_annotations/rules/definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ pub(crate) fn definition(

// ANN001, ANN401
for ParameterWithDefault {
def,
parameter,
default: _,
range: _,
} in arguments
Expand All @@ -542,26 +542,29 @@ pub(crate) fn definition(
)
{
// ANN401 for dynamically typed arguments
if let Some(annotation) = &def.annotation {
if let Some(annotation) = &parameter.annotation {
has_any_typed_arg = true;
if checker.enabled(Rule::AnyType) && !is_overridden {
check_dynamically_typed(
checker,
annotation,
|| def.arg.to_string(),
|| parameter.name.to_string(),
&mut diagnostics,
);
}
} else {
if !(checker.settings.flake8_annotations.suppress_dummy_args
&& checker.settings.dummy_variable_rgx.is_match(&def.arg))
&& checker
.settings
.dummy_variable_rgx
.is_match(&parameter.name))
{
if checker.enabled(Rule::MissingTypeFunctionArgument) {
diagnostics.push(Diagnostic::new(
MissingTypeFunctionArgument {
name: def.arg.to_string(),
name: parameter.name.to_string(),
},
def.range(),
parameter.range(),
));
}
}
Expand All @@ -574,18 +577,18 @@ pub(crate) fn definition(
has_any_typed_arg = true;
if !checker.settings.flake8_annotations.allow_star_arg_any {
if checker.enabled(Rule::AnyType) && !is_overridden {
let name = &arg.arg;
let name = &arg.name;
check_dynamically_typed(checker, expr, || format!("*{name}"), &mut diagnostics);
}
}
} else {
if !(checker.settings.flake8_annotations.suppress_dummy_args
&& checker.settings.dummy_variable_rgx.is_match(&arg.arg))
&& checker.settings.dummy_variable_rgx.is_match(&arg.name))
{
if checker.enabled(Rule::MissingTypeArgs) {
diagnostics.push(Diagnostic::new(
MissingTypeArgs {
name: arg.arg.to_string(),
name: arg.name.to_string(),
},
arg.range(),
));
Expand All @@ -600,7 +603,7 @@ pub(crate) fn definition(
has_any_typed_arg = true;
if !checker.settings.flake8_annotations.allow_star_arg_any {
if checker.enabled(Rule::AnyType) && !is_overridden {
let name = &arg.arg;
let name = &arg.name;
check_dynamically_typed(
checker,
expr,
Expand All @@ -611,12 +614,12 @@ pub(crate) fn definition(
}
} else {
if !(checker.settings.flake8_annotations.suppress_dummy_args
&& checker.settings.dummy_variable_rgx.is_match(&arg.arg))
&& checker.settings.dummy_variable_rgx.is_match(&arg.name))
{
if checker.enabled(Rule::MissingTypeKwargs) {
diagnostics.push(Diagnostic::new(
MissingTypeKwargs {
name: arg.arg.to_string(),
name: arg.name.to_string(),
},
arg.range(),
));
Expand All @@ -628,31 +631,31 @@ pub(crate) fn definition(
// ANN101, ANN102
if is_method && !visibility::is_staticmethod(cast::decorator_list(stmt), checker.semantic()) {
if let Some(ParameterWithDefault {
def,
parameter,
default: _,
range: _,
}) = arguments
.posonlyargs
.first()
.or_else(|| arguments.args.first())
{
if def.annotation.is_none() {
if parameter.annotation.is_none() {
if visibility::is_classmethod(cast::decorator_list(stmt), checker.semantic()) {
if checker.enabled(Rule::MissingTypeCls) {
diagnostics.push(Diagnostic::new(
MissingTypeCls {
name: def.arg.to_string(),
name: parameter.name.to_string(),
},
def.range(),
parameter.range(),
));
}
} else {
if checker.enabled(Rule::MissingTypeSelf) {
diagnostics.push(Diagnostic::new(
MissingTypeSelf {
name: def.arg.to_string(),
name: parameter.name.to_string(),
},
def.range(),
parameter.range(),
));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl Violation for HardcodedPasswordDefault {

fn check_password_kwarg(parameter: &Parameter, default: &Expr) -> Option<Diagnostic> {
string_literal(default).filter(|string| !string.is_empty())?;
let kwarg_name = &parameter.arg;
let kwarg_name = &parameter.name;
if !matches_password_name(kwarg_name) {
return None;
}
Expand All @@ -70,7 +70,7 @@ fn check_password_kwarg(parameter: &Parameter, default: &Expr) -> Option<Diagnos
/// S107
pub(crate) fn hardcoded_password_default(checker: &mut Checker, parameters: &Parameters) {
for ParameterWithDefault {
def,
parameter,
default,
range: _,
} in parameters
Expand All @@ -82,7 +82,7 @@ pub(crate) fn hardcoded_password_default(checker: &mut Checker, parameters: &Par
let Some(default) = default else {
continue;
};
if let Some(diagnostic) = check_password_kwarg(def, default) {
if let Some(diagnostic) = check_password_kwarg(parameter, default) {
checker.diagnostics.push(diagnostic);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ pub(crate) fn check_boolean_default_value_in_function_definition(
}

for ParameterWithDefault {
def: _,
parameter: _,
default,
range: _,
} in parameters.args.iter().chain(&parameters.posonlyargs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,15 @@ pub(crate) fn check_positional_boolean_in_def(
}

for ParameterWithDefault {
def,
parameter,
default: _,
range: _,
} in parameters.posonlyargs.iter().chain(&parameters.args)
{
if def.annotation.is_none() {
if parameter.annotation.is_none() {
continue;
}
let Some(expr) = &def.annotation else {
let Some(expr) = &parameter.annotation else {
continue;
};

Expand All @@ -122,7 +122,7 @@ pub(crate) fn check_positional_boolean_in_def(
}
checker.diagnostics.push(Diagnostic::new(
BooleanPositionalArgInFunctionDefinition,
def.range(),
parameter.range(),
));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub(crate) fn function_call_in_argument_default(checker: &mut Checker, parameter
let mut visitor = ArgumentDefaultVisitor::new(checker.semantic(), extend_immutable_calls);
for ParameterWithDefault {
default,
def: _,
parameter: _,
range: _,
} in parameters
.posonlyargs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ where
}) => {
visitor::walk_expr(self, body);
for ParameterWithDefault {
def,
parameter,
default: _,
range: _,
} in parameters
Expand All @@ -86,7 +86,7 @@ where
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{
self.names.remove(def.arg.as_str());
self.names.remove(parameter.name.as_str());
}
}
_ => visitor::walk_expr(self, expr),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl Violation for MutableArgumentDefault {
pub(crate) fn mutable_argument_default(checker: &mut Checker, parameters: &Parameters) {
// Scan in reverse order to right-align zip().
for ParameterWithDefault {
def,
parameter,
default,
range: _,
} in parameters
Expand All @@ -74,7 +74,7 @@ pub(crate) fn mutable_argument_default(checker: &mut Checker, parameters: &Param
};

if is_mutable_expr(default, checker.semantic())
&& !def
&& !parameter
.annotation
.as_ref()
.is_some_and(|expr| is_immutable_annotation(expr, checker.semantic()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ impl Violation for BuiltinArgumentShadowing {
/// A002
pub(crate) fn builtin_argument_shadowing(checker: &mut Checker, parameter: &Parameter) {
if shadows_builtin(
parameter.arg.as_str(),
parameter.name.as_str(),
&checker.settings.flake8_builtins.builtins_ignorelist,
) {
checker.diagnostics.push(Diagnostic::new(
BuiltinArgumentShadowing {
name: parameter.arg.to_string(),
name: parameter.name.to_string(),
},
parameter.range(),
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub(crate) fn any_eq_ne_annotation(checker: &mut Checker, name: &str, parameters
return;
}

let Some(annotation) = &parameters.args[1].def.annotation else {
let Some(annotation) = &parameters.args[1].parameter.annotation else {
return;
};

Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/rules/flake8_pyi/rules/exit_annotations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ fn check_positional_args(
];

for (arg, (error_info, predicate)) in positional_args.iter().skip(1).take(3).zip(validations) {
let Some(annotation) = arg.def.annotation.as_ref() else {
let Some(annotation) = arg.parameter.annotation.as_ref() else {
continue;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: &
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.filter_map(|arg| arg.def.annotation.as_ref())
.filter_map(|arg| arg.parameter.annotation.as_ref())
{
if checker.semantic().match_typing_expr(annotation, "NoReturn") {
checker.diagnostics.push(Diagnostic::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub(crate) fn redundant_numeric_union(checker: &mut Checker, parameters: &Parame
.iter()
.chain(parameters.posonlyargs.iter())
.chain(parameters.kwonlyargs.iter())
.filter_map(|arg| arg.def.annotation.as_ref())
.filter_map(|arg| arg.parameter.annotation.as_ref())
{
check_annotation(checker, annotation);
}
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ 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 {
def,
parameter,
default,
range: _,
} in parameters
Expand All @@ -414,7 +414,7 @@ pub(crate) fn typed_argument_simple_defaults(checker: &mut Checker, parameters:
let Some(default) = default else {
continue;
};
if def.annotation.is_some() {
if parameter.annotation.is_some() {
if !is_valid_default_value_with_annotation(
default,
true,
Expand All @@ -439,7 +439,7 @@ 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 {
def,
parameter,
default,
range: _,
} in parameters
Expand All @@ -451,7 +451,7 @@ pub(crate) fn argument_simple_defaults(checker: &mut Checker, parameters: &Param
let Some(default) = default else {
continue;
};
if def.annotation.is_none() {
if parameter.annotation.is_none() {
if !is_valid_default_value_with_annotation(
default,
true,
Expand Down
Loading

0 comments on commit 9c708d8

Please sign in to comment.