diff --git a/crates/ruff/src/rules/flake8_bandit/rules/bad_file_permissions.rs b/crates/ruff/src/rules/flake8_bandit/rules/bad_file_permissions.rs index 8cad152de742f..1d6d66423a414 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/bad_file_permissions.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/bad_file_permissions.rs @@ -4,7 +4,7 @@ use rustpython_parser::ast::{self, Constant, Expr, Keyword, Operator, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::CallPath; -use ruff_python_ast::helpers::SimpleCallArgs; +use ruff_python_ast::helpers::CallArguments; use ruff_python_semantic::SemanticModel; use crate::checkers::ast::Checker; @@ -61,7 +61,7 @@ pub(crate) fn bad_file_permissions( matches!(call_path.as_slice(), ["os", "chmod"]) }) { - let call_args = SimpleCallArgs::new(args, keywords); + let call_args = CallArguments::new(args, keywords); if let Some(mode_arg) = call_args.argument("mode", 1) { if let Some(int_value) = int_value(mode_arg, checker.semantic()) { if (int_value & WRITE_WORLD > 0) || (int_value & EXECUTE_GROUP > 0) { diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs b/crates/ruff/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs index 7a8385e8d5051..f46bac5329395 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/hashlib_insecure_hash_functions.rs @@ -2,7 +2,7 @@ use rustpython_parser::ast::{Expr, Keyword, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::{is_const_false, SimpleCallArgs}; +use ruff_python_ast::helpers::{find_keyword, is_const_false, CallArguments}; use crate::checkers::ast::Checker; @@ -78,15 +78,12 @@ pub(crate) fn hashlib_insecure_hash_functions( _ => None, }) { + if !is_used_for_security(keywords) { + return; + } match hashlib_call { HashlibCall::New => { - let call_args = SimpleCallArgs::new(args, keywords); - - if !is_used_for_security(&call_args) { - return; - } - - if let Some(name_arg) = call_args.argument("name", 0) { + if let Some(name_arg) = CallArguments::new(args, keywords).argument("name", 0) { if let Some(hash_func_name) = string_literal(name_arg) { // `hashlib.new` accepts both lowercase and uppercase names for hash // functions. @@ -105,12 +102,6 @@ pub(crate) fn hashlib_insecure_hash_functions( } } HashlibCall::WeakHash(func_name) => { - let call_args = SimpleCallArgs::new(args, keywords); - - if !is_used_for_security(&call_args) { - return; - } - checker.diagnostics.push(Diagnostic::new( HashlibInsecureHashFunction { string: (*func_name).to_string(), @@ -122,13 +113,12 @@ pub(crate) fn hashlib_insecure_hash_functions( } } -fn is_used_for_security(call_args: &SimpleCallArgs) -> bool { - match call_args.keyword_argument("usedforsecurity") { - Some(expr) => !is_const_false(expr), - _ => true, - } +fn is_used_for_security(keywords: &[Keyword]) -> bool { + find_keyword(keywords, "usedforsecurity") + .map_or(true, |keyword| !is_const_false(&keyword.value)) } +#[derive(Debug)] enum HashlibCall { New, WeakHash(&'static str), diff --git a/crates/ruff/src/rules/flake8_bandit/rules/snmp_weak_cryptography.rs b/crates/ruff/src/rules/flake8_bandit/rules/snmp_weak_cryptography.rs index 97093dd6cb197..26d5bae978694 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/snmp_weak_cryptography.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/snmp_weak_cryptography.rs @@ -2,7 +2,7 @@ use rustpython_parser::ast::{Expr, Keyword, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::SimpleCallArgs; +use ruff_python_ast::helpers::CallArguments; use crate::checkers::ast::Checker; @@ -36,8 +36,7 @@ impl Violation for SnmpWeakCryptography { #[derive_message_formats] fn message(&self) -> String { format!( - "You should not use SNMPv3 without encryption. `noAuthNoPriv` & `authNoPriv` is \ - insecure." + "You should not use SNMPv3 without encryption. `noAuthNoPriv` & `authNoPriv` is insecure." ) } } @@ -56,8 +55,7 @@ pub(crate) fn snmp_weak_cryptography( matches!(call_path.as_slice(), ["pysnmp", "hlapi", "UsmUserData"]) }) { - let call_args = SimpleCallArgs::new(args, keywords); - if call_args.len() < 3 { + if CallArguments::new(args, keywords).len() < 3 { checker .diagnostics .push(Diagnostic::new(SnmpWeakCryptography, func.range())); diff --git a/crates/ruff/src/rules/flake8_bandit/rules/unsafe_yaml_load.rs b/crates/ruff/src/rules/flake8_bandit/rules/unsafe_yaml_load.rs index 20face98c8f59..9cf1d37ba9bf9 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/unsafe_yaml_load.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/unsafe_yaml_load.rs @@ -2,7 +2,7 @@ use rustpython_parser::ast::{self, Expr, Keyword, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::SimpleCallArgs; +use ruff_python_ast::helpers::CallArguments; use crate::checkers::ast::Checker; @@ -73,7 +73,7 @@ pub(crate) fn unsafe_yaml_load( matches!(call_path.as_slice(), ["yaml", "load"]) }) { - let call_args = SimpleCallArgs::new(args, keywords); + let call_args = CallArguments::new(args, keywords); if let Some(loader_arg) = call_args.argument("Loader", 1) { if !checker .semantic() diff --git a/crates/ruff/src/rules/flake8_logging_format/rules/logging_call.rs b/crates/ruff/src/rules/flake8_logging_format/rules/logging_call.rs index bd5de7e19e8d5..85cc170b89036 100644 --- a/crates/ruff/src/rules/flake8_logging_format/rules/logging_call.rs +++ b/crates/ruff/src/rules/flake8_logging_format/rules/logging_call.rs @@ -2,7 +2,7 @@ use ruff_text_size::{TextRange, TextSize}; use rustpython_parser::ast::{self, Constant, Expr, Keyword, Operator, Ranged}; use ruff_diagnostics::{Diagnostic, Edit, Fix}; -use ruff_python_ast::helpers::{find_keyword, SimpleCallArgs}; +use ruff_python_ast::helpers::{find_keyword, CallArguments}; use ruff_python_semantic::analyze::logging; use ruff_python_stdlib::logging::LoggingLevel; @@ -164,7 +164,7 @@ pub(crate) fn logging_call( if let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func { if let Some(logging_call_type) = LoggingCallType::from_attribute(attr.as_str()) { - let call_args = SimpleCallArgs::new(args, keywords); + let call_args = CallArguments::new(args, keywords); let level_call_range = TextRange::new(value.end() + TextSize::from(1), func.end()); // G001 - G004 diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/fail.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/fail.rs index 979bc61bead76..c6fddc84ada77 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/fail.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/fail.rs @@ -2,7 +2,7 @@ use rustpython_parser::ast::{Expr, Keyword, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::SimpleCallArgs; +use ruff_python_ast::helpers::CallArguments; use crate::checkers::ast::Checker; @@ -58,7 +58,7 @@ impl Violation for PytestFailWithoutMessage { pub(crate) fn fail_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: &[Keyword]) { if is_pytest_fail(func, checker.semantic()) { - let call_args = SimpleCallArgs::new(args, keywords); + let call_args = CallArguments::new(args, keywords); // Allow either `pytest.fail(reason="...")` (introduced in pytest 7.0) or // `pytest.fail(msg="...")` (deprecated in pytest 7.0) diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/patch.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/patch.rs index 2e0951797837a..465ab7a161c5f 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/patch.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/patch.rs @@ -3,7 +3,7 @@ use rustpython_parser::ast::{self, Arguments, Expr, Keyword, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::collect_call_path; -use ruff_python_ast::helpers::{includes_arg_name, SimpleCallArgs}; +use ruff_python_ast::helpers::{find_keyword, includes_arg_name, CallArguments}; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; @@ -50,30 +50,30 @@ fn check_patch_call( keywords: &[Keyword], new_arg_number: usize, ) -> Option { - let simple_args = SimpleCallArgs::new(args, keywords); - if simple_args.keyword_argument("return_value").is_some() { + if find_keyword(keywords, "return_value").is_some() { return None; } - if let Some(Expr::Lambda(ast::ExprLambda { + let ast::ExprLambda { args, body, range: _, - })) = simple_args.argument("new", new_arg_number) - { - // Walk the lambda body. - let mut visitor = LambdaBodyVisitor { - arguments: args, - uses_args: false, - }; - visitor.visit_expr(body); + } = CallArguments::new(args, keywords) + .argument("new", new_arg_number)? + .as_lambda_expr()?; - if !visitor.uses_args { - return Some(Diagnostic::new(PytestPatchWithLambda, call.range())); - } - } + // Walk the lambda body. + let mut visitor = LambdaBodyVisitor { + arguments: args, + uses_args: false, + }; + visitor.visit_expr(body); - None + if visitor.uses_args { + None + } else { + Some(Diagnostic::new(PytestPatchWithLambda, call.range())) + } } pub(crate) fn patch_with_lambda( diff --git a/crates/ruff/src/rules/pylint/rules/logging.rs b/crates/ruff/src/rules/pylint/rules/logging.rs index b301530737601..d561bcfb3d726 100644 --- a/crates/ruff/src/rules/pylint/rules/logging.rs +++ b/crates/ruff/src/rules/pylint/rules/logging.rs @@ -2,7 +2,7 @@ use rustpython_parser::ast::{self, Constant, Expr, Keyword, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::SimpleCallArgs; +use ruff_python_ast::helpers::CallArguments; use ruff_python_semantic::analyze::logging; use ruff_python_stdlib::logging::LoggingLevel; @@ -102,10 +102,6 @@ pub(crate) fn logging_call( return; } - if !logging::is_logger_candidate(func, checker.semantic(), &checker.settings.logger_objects) { - return; - } - let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func else { return; }; @@ -114,7 +110,7 @@ pub(crate) fn logging_call( return; } - let call_args = SimpleCallArgs::new(args, keywords); + let call_args = CallArguments::new(args, keywords); let Some(Expr::Constant(ast::ExprConstant { value: Constant::Str(value), .. @@ -123,6 +119,10 @@ pub(crate) fn logging_call( return; }; + if !logging::is_logger_candidate(func, checker.semantic(), &checker.settings.logger_objects) { + return; + } + let Ok(summary) = CFormatSummary::try_from(value.as_str()) else { return; }; diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 7a934c60c614c..176845502779a 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -4,7 +4,6 @@ use std::path::Path; use num_traits::Zero; use ruff_text_size::{TextRange, TextSize}; -use rustc_hash::FxHashMap; use rustpython_ast::CmpOp; use rustpython_parser::ast::{ self, Arguments, Constant, ExceptHandler, Expr, Keyword, MatchCase, Pattern, Ranged, Stmt, @@ -1217,67 +1216,61 @@ pub fn is_docstring_stmt(stmt: &Stmt) -> bool { } } -/// A simple representation of a call's positional and keyword arguments. +/// A representation of a function call's positional and keyword arguments that ignores +/// starred expressions. #[derive(Default)] -pub struct SimpleCallArgs<'a> { - args: Vec<&'a Expr>, - kwargs: FxHashMap<&'a str, &'a Expr>, +pub struct CallArguments<'a> { + args: &'a [Expr], + keywords: &'a [Keyword], } -impl<'a> SimpleCallArgs<'a> { - pub fn new( - args: impl IntoIterator, - keywords: impl IntoIterator, - ) -> Self { - let args = args - .into_iter() - .take_while(|arg| !arg.is_starred_expr()) - .collect(); - - let kwargs = keywords - .into_iter() - .filter_map(|keyword| { - let node = keyword; - node.arg.as_ref().map(|arg| (arg.as_str(), &node.value)) - }) - .collect(); - - SimpleCallArgs { args, kwargs } +impl<'a> CallArguments<'a> { + pub fn new(args: &'a [Expr], keywords: &'a [Keyword]) -> Self { + Self { args, keywords } } - /// Get the argument with the given name. - /// If the argument is not found by name, return - /// `None`. - pub fn keyword_argument(&self, name: &str) -> Option<&'a Expr> { - self.kwargs.get(name).copied() - } - - /// Get the argument with the given name or position. - /// If the argument is not found with either name or position, return - /// `None`. + /// Get the argument with the given name or position, or `None` if no such + /// argument exists. pub fn argument(&self, name: &str, position: usize) -> Option<&'a Expr> { - self.keyword_argument(name) - .or_else(|| self.args.get(position).copied()) + self.keywords + .iter() + .find(|keyword| { + let Keyword { arg, .. } = keyword; + arg.as_ref().map_or(false, |arg| arg == name) + }) + .map(|keyword| &keyword.value) + .or_else(|| { + self.args + .iter() + .take_while(|expr| !expr.is_starred_expr()) + .nth(position) + }) } - /// Return the number of positional and keyword arguments. + /// Return the number of arguments. pub fn len(&self) -> usize { - self.args.len() + self.kwargs.len() + self.args.len() + self.keywords.len() + } + + /// Return `true` if there are no arguments. + pub fn is_empty(&self) -> bool { + self.len() == 0 } /// Return the number of positional arguments. pub fn num_args(&self) -> usize { - self.args.len() + self.args + .iter() + .take_while(|expr| !expr.is_starred_expr()) + .count() } /// Return the number of keyword arguments. pub fn num_kwargs(&self) -> usize { - self.kwargs.len() - } - - /// Return `true` if there are no positional or keyword arguments. - pub fn is_empty(&self) -> bool { - self.len() == 0 + self.keywords + .iter() + .filter(|keyword| keyword.arg.is_some()) + .count() } }