From 395bb3124702f363af5922d0906c2b241ba93584 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 9 Aug 2023 16:39:10 -0400 Subject: [PATCH] Improve counting of message arguments when msg is provided as a keyword (#6456) Closes https://github.com/astral-sh/ruff/issues/6454. --- .../fixtures/pylint/logging_too_few_args.py | 4 ++++ .../fixtures/pylint/logging_too_many_args.py | 4 ++++ crates/ruff/src/rules/pylint/rules/logging.rs | 4 ++-- crates/ruff_python_ast/src/nodes.rs | 22 +++++++++---------- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pylint/logging_too_few_args.py b/crates/ruff/resources/test/fixtures/pylint/logging_too_few_args.py index 2fc6f49ab5..65f021ea42 100644 --- a/crates/ruff/resources/test/fixtures/pylint/logging_too_few_args.py +++ b/crates/ruff/resources/test/fixtures/pylint/logging_too_few_args.py @@ -19,6 +19,10 @@ logging.error("Example log %s, %s", "foo", "bar", "baz", **kwargs) # do not handle keyword arguments logging.error("%(objects)d modifications: %(modifications)d errors: %(errors)d") +logging.info(msg="Hello %s") + +logging.info(msg="Hello %s %s") + import warning warning.warning("Hello %s %s", "World!") diff --git a/crates/ruff/resources/test/fixtures/pylint/logging_too_many_args.py b/crates/ruff/resources/test/fixtures/pylint/logging_too_many_args.py index c087b05c4e..c64641eaed 100644 --- a/crates/ruff/resources/test/fixtures/pylint/logging_too_many_args.py +++ b/crates/ruff/resources/test/fixtures/pylint/logging_too_many_args.py @@ -15,6 +15,10 @@ logging.error("Example log %s, %s", "foo", "bar", "baz", **kwargs) # do not handle keyword arguments logging.error("%(objects)d modifications: %(modifications)d errors: %(errors)d", {"objects": 1, "modifications": 1, "errors": 1}) +logging.info(msg="Hello") + +logging.info(msg="Hello", something="else") + import warning warning.warning("Hello %s", "World!", "again") diff --git a/crates/ruff/src/rules/pylint/rules/logging.rs b/crates/ruff/src/rules/pylint/rules/logging.rs index 5d2f0604ac..5f778ea5d6 100644 --- a/crates/ruff/src/rules/pylint/rules/logging.rs +++ b/crates/ruff/src/rules/pylint/rules/logging.rs @@ -111,8 +111,8 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) { let Some(Expr::Constant(ast::ExprConstant { value: Constant::Str(value), .. - })) = call.arguments.find_argument("msg", 0) - else { + })) = call.arguments + .find_positional( 0) else { return; }; diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 46efa5b098..5011ba76d9 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -2126,23 +2126,21 @@ impl Arguments { }) } + /// Return the positional argument at the given index, or `None` if no such argument exists. + pub fn find_positional(&self, position: usize) -> Option<&Expr> { + self.args + .iter() + .take_while(|expr| !expr.is_starred_expr()) + .nth(position) + } + /// Return the argument with the given name or at the given position, or `None` if no such /// argument exists. Used to retrieve arguments that can be provided _either_ as keyword or /// positional arguments. pub fn find_argument(&self, name: &str, position: usize) -> Option<&Expr> { - self.keywords - .iter() - .find(|keyword| { - let Keyword { arg, .. } = keyword; - arg.as_ref().is_some_and(|arg| arg == name) - }) + self.find_keyword(name) .map(|keyword| &keyword.value) - .or_else(|| { - self.args - .iter() - .take_while(|expr| !expr.is_starred_expr()) - .nth(position) - }) + .or_else(|| self.find_positional(position)) } }