Skip to content

Commit

Permalink
Add convenience methods for iterating over all parameter nodes in a f…
Browse files Browse the repository at this point in the history 
…unction (#11174)
  • Loading branch information
@AlexWaygood
AlexWaygood authored Apr 29, 2024
1 parent 8a887da commit 87929ad
Show file tree
Hide file tree
Showing 27 changed files with 396 additions and 445 deletions.
83 changes: 14 additions & 69 deletions crates/ruff_linter/src/checkers/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ use std::path::Path;
use itertools::Itertools;
use log::debug;
use ruff_python_ast::{
self as ast, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext, FStringElement,
Keyword, MatchCase, Parameter, ParameterWithDefault, Parameters, Pattern, Stmt, Suite, UnaryOp,
self as ast, AnyParameterRef, Comprehension, ElifElseClause, ExceptHandler, Expr, ExprContext,
FStringElement, Keyword, MatchCase, Parameter, Parameters, Pattern, Stmt, Suite, UnaryOp,
};
use ruff_text_size::{Ranged, TextRange, TextSize};

Expand Down Expand Up @@ -604,15 +604,11 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.visit_type_params(type_params);
}

for parameter_with_default in parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
{
if let Some(expr) = &parameter_with_default.parameter.annotation {
if singledispatch {
for parameter in &**parameters {
if let Some(expr) = parameter.annotation() {
if singledispatch && !parameter.is_variadic() {
self.visit_runtime_required_annotation(expr);
singledispatch = false;
} else {
match annotation {
AnnotationContext::RuntimeRequired => {
Expand All @@ -625,41 +621,10 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.visit_annotation(expr);
}
}
};
}
if let Some(expr) = &parameter_with_default.default {
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);
}
}
if let Some(expr) = parameter.default() {
self.visit_expr(expr);
}
}
for expr in returns {
Expand Down Expand Up @@ -1043,19 +1008,11 @@ impl<'a> Visitor<'a> for Checker<'a> {
) => {
// Visit the default arguments, but avoid the body, which will be deferred.
if let Some(parameters) = parameters {
for ParameterWithDefault {
default,
parameter: _,
range: _,
} in parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
for default in parameters
.iter_non_variadic_params()
.filter_map(|param| param.default.as_deref())
{
if let Some(expr) = &default {
self.visit_expr(expr);
}
self.visit_expr(default);
}
}

Expand Down Expand Up @@ -1483,20 +1440,8 @@ impl<'a> Visitor<'a> for Checker<'a> {
// Step 1: Binding.
// Bind, but intentionally avoid walking default expressions, as we handle them
// upstream.
for parameter_with_default in &parameters.posonlyargs {
self.visit_parameter(&parameter_with_default.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);
for parameter in parameters.iter().map(AnyParameterRef::as_parameter) {
self.visit_parameter(parameter);
}

// Step 4: Analysis
Expand Down
18 changes: 5 additions & 13 deletions crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ fn is_stub_function(function_def: &ast::StmtFunctionDef, checker: &Checker) -> b
}

/// Generate flake8-annotation checks for a given `Definition`.
/// ANN001, ANN401
pub(crate) fn definition(
checker: &Checker,
definition: &Definition,
Expand Down Expand Up @@ -615,23 +616,14 @@ pub(crate) fn definition(

let is_overridden = visibility::is_override(decorator_list, checker.semantic());

// ANN001, ANN401
// If this is a non-static method, skip `cls` or `self`.
for ParameterWithDefault {
parameter,
default: _,
range: _,
} in parameters
.posonlyargs
.iter()
.chain(&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()),
),
)
{
} 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 {
has_any_typed_arg = true;
Expand Down
6 changes: 1 addition & 5 deletions crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_password_default.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,7 @@ pub(crate) fn hardcoded_password_default(checker: &mut Checker, parameters: &Par
parameter,
default,
range: _,
} in parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
} in parameters.iter_non_variadic_params()
{
let Some(default) = default else {
continue;
Expand Down
61 changes: 26 additions & 35 deletions crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_bad_defaults.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,44 +49,35 @@ impl Violation for SslWithBadDefaults {

/// S503
pub(crate) fn ssl_with_bad_defaults(checker: &mut Checker, function_def: &StmtFunctionDef) {
function_def
for default in function_def
.parameters
.posonlyargs
.iter()
.chain(
function_def
.parameters
.args
.iter()
.chain(function_def.parameters.kwonlyargs.iter()),
)
.for_each(|param| {
if let Some(default) = &param.default {
match default.as_ref() {
Expr::Name(ast::ExprName { id, range, .. }) => {
if is_insecure_protocol(id.as_str()) {
checker.diagnostics.push(Diagnostic::new(
SslWithBadDefaults {
protocol: id.to_string(),
},
*range,
));
}
}
Expr::Attribute(ast::ExprAttribute { attr, range, .. }) => {
if is_insecure_protocol(attr.as_str()) {
checker.diagnostics.push(Diagnostic::new(
SslWithBadDefaults {
protocol: attr.to_string(),
},
*range,
));
}
}
_ => {}
.iter_non_variadic_params()
.filter_map(|param| param.default.as_deref())
{
match default {
Expr::Name(ast::ExprName { id, range, .. }) => {
if is_insecure_protocol(id.as_str()) {
checker.diagnostics.push(Diagnostic::new(
SslWithBadDefaults {
protocol: id.to_string(),
},
*range,
));
}
}
});
Expr::Attribute(ast::ExprAttribute { attr, range, .. }) => {
if is_insecure_protocol(attr.as_str()) {
checker.diagnostics.push(Diagnostic::new(
SslWithBadDefaults {
protocol: attr.to_string(),
},
*range,
));
}
}
_ => {}
}
}
}

/// Returns `true` if the given protocol name is insecure.
Expand Down
6 changes: 1 addition & 5 deletions crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,7 @@ pub(crate) fn function_call_in_argument_default(checker: &mut Checker, parameter
default,
parameter,
range: _,
} in parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
} in parameters.iter_non_variadic_params()
{
if let Some(expr) = &default {
if !parameter.annotation.as_ref().is_some_and(|expr| {
Expand Down
6 changes: 1 addition & 5 deletions crates/ruff_linter/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,7 @@ impl<'a> Visitor<'a> for NameFinder<'a> {
parameter,
default: _,
range: _,
} in parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
} in parameters.iter_non_variadic_params()
{
self.names.remove(parameter.name.as_str());
}
Expand Down
7 changes: 1 addition & 6 deletions crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,7 @@ pub(crate) fn mutable_argument_default(checker: &mut Checker, function_def: &ast
parameter,
default,
range: _,
} in function_def
.parameters
.posonlyargs
.iter()
.chain(&function_def.parameters.args)
.chain(&function_def.parameters.kwonlyargs)
} in function_def.parameters.iter_non_variadic_params()
{
let Some(default) = default else {
continue;
Expand Down
15 changes: 3 additions & 12 deletions crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,7 @@ pub(crate) fn unnecessary_map(
if parameters.as_ref().is_some_and(|parameters| {
late_binding(parameters, body)
|| parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.iter_non_variadic_params()
.any(|param| param.default.is_some())
|| parameters.vararg.is_some()
|| parameters.kwarg.is_some()
Expand Down Expand Up @@ -152,10 +149,7 @@ pub(crate) fn unnecessary_map(
if parameters.as_ref().is_some_and(|parameters| {
late_binding(parameters, body)
|| parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.iter_non_variadic_params()
.any(|param| param.default.is_some())
|| parameters.vararg.is_some()
|| parameters.kwarg.is_some()
Expand Down Expand Up @@ -207,10 +201,7 @@ pub(crate) fn unnecessary_map(
if parameters.as_ref().is_some_and(|parameters| {
late_binding(parameters, body)
|| parameters
.posonlyargs
.iter()
.chain(&parameters.args)
.chain(&parameters.kwonlyargs)
.iter_non_variadic_params()
.any(|param| param.default.is_some())
|| parameters.vararg.is_some()
|| parameters.kwarg.is_some()
Expand Down
48 changes: 13 additions & 35 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::fmt;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, Parameters};
use ruff_python_ast::{AnyParameterRef, Parameters};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -58,43 +58,21 @@ pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: &
// Ex) def func(arg: NoReturn): ...
// Ex) def func(arg: NoReturn, /): ...
// Ex) def func(*, arg: NoReturn): ...
for annotation in parameters
.posonlyargs
.iter()
.chain(&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): ...
if let Some(arg) = &parameters.vararg {
if let Some(annotation) = &arg.annotation {
check_no_return_argument_annotation(checker, annotation);
}
}

// Ex) def func(**kwargs: NoReturn): ...
if let Some(arg) = &parameters.kwarg {
if let Some(annotation) = &arg.annotation {
check_no_return_argument_annotation(checker, annotation);
}
}
}

fn check_no_return_argument_annotation(checker: &mut Checker, annotation: &Expr) {
if checker.semantic().match_typing_expr(annotation, "NoReturn") {
checker.diagnostics.push(Diagnostic::new(
NoReturnArgumentAnnotationInStub {
module: if checker.settings.target_version >= Py311 {
TypingModule::Typing
} else {
TypingModule::TypingExtensions
for annotation in parameters.iter().filter_map(AnyParameterRef::annotation) {
if checker.semantic().match_typing_expr(annotation, "NoReturn") {
checker.diagnostics.push(Diagnostic::new(
NoReturnArgumentAnnotationInStub {
module: if checker.settings.target_version >= Py311 {
TypingModule::Typing
} else {
TypingModule::TypingExtensions
},
},
},
annotation.range(),
));
annotation.range(),
));
}
}
}

Expand Down
Loading

0 comments on commit 87929ad

Please sign in to comment.