Avoid allocations in SimpleCallArgs (#6021)

## Summary

My intuition is that it's faster to do these checks as-needed rather
than allocation new hash maps and vectors for the arguments. (We
typically only query once anyway.)

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

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,
         })
     {
-        match hashlib_call {
-            HashlibCall::New => {
-                let call_args = SimpleCallArgs::new(args, keywords);
-
-                if !is_used_for_security(&call_args) {
+        if !is_used_for_security(keywords) {
             return;
         }
-
-                if let Some(name_arg) = call_args.argument("name", 0) {
+        match hashlib_call {
+            HashlibCall::New => {
+                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),

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

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

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

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)

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,17 +50,18 @@ fn check_patch_call(
     keywords: &[Keyword],
     new_arg_number: usize,
 ) -> Option<Diagnostic> {
-    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)
-    {
+    } = CallArguments::new(args, keywords)
+        .argument("new", new_arg_number)?
+        .as_lambda_expr()?;
+
     // Walk the lambda body.
     let mut visitor = LambdaBodyVisitor {
         arguments: args,
@@ -68,12 +69,11 @@ fn check_patch_call(
     };
     visitor.visit_expr(body);
 
-        if !visitor.uses_args {
-            return Some(Diagnostic::new(PytestPatchWithLambda, call.range()));
-        }
-    }
-
+    if visitor.uses_args {
         None
+    } else {
+        Some(Diagnostic::new(PytestPatchWithLambda, call.range()))
+    }
 }
 
 pub(crate) fn patch_with_lambda(

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

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<Item = &'a Expr>,
-        keywords: impl IntoIterator<Item = &'a Keyword>,
-    ) -> 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()
     }
 }