mirror of https://github.com/astral-sh/ruff
[`ruff`] Ignore `str()` when not used for simple conversion (`RUF065`) (#21330)
## Summary Fixed RUF065 (`logging-eager-conversion`) to only flag `str()` calls when they perform a simple conversion that can be safely removed. The rule now ignores `str()` calls with no arguments, multiple arguments, starred arguments, or keyword unpacking, preventing false positives. Fixes #21315 ## Problem Analysis The RUF065 rule was incorrectly flagging all `str()` calls in logging statements, even when `str()` was performing actual conversion work beyond simple type coercion. Specifically, the rule flagged: - `str()` with no arguments - which returns an empty string - `str(b"data", "utf-8")` with multiple arguments - which performs encoding conversion - `str(*args)` with starred arguments - which unpacks arguments - `str(**kwargs)` with keyword unpacking - which passes keyword arguments These cases cannot be safely removed because `str()` is doing meaningful work (encoding conversion, argument unpacking, etc.), not just redundant type conversion. The root cause was that the rule only checked if the function was `str()` without validating the call signature. It didn't distinguish between simple `str(value)` conversions (which can be removed) and more complex `str()` calls that perform actual work. ## Approach The fix adds validation to the `str()` detection logic in `logging_eager_conversion.rs`: 1. **Check argument count**: Only flag `str()` calls with exactly one positional argument (`str_call_args.args.len() == 1`) 2. **Check for starred arguments**: Ensure the single argument is not starred (`!str_call_args.args[0].is_starred_expr()`) 3. **Check for keyword arguments**: Ensure there are no keyword arguments (`str_call_args.keywords.is_empty()`) This ensures the rule only flags cases like `str(value)` where `str()` is truly redundant and can be removed, while ignoring cases where `str()` performs actual conversion work. The fix maintains backward compatibility - all existing valid test cases continue to be flagged correctly, while the new edge cases are properly ignored. --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
parent
5f3e086ee4
commit
1fd852fb3f
|
|
@ -0,0 +1,18 @@
|
|||
import logging
|
||||
|
||||
# Test cases for str() that should NOT be flagged (issue #21315)
|
||||
# str() with no arguments - should not be flagged
|
||||
logging.warning("%s", str())
|
||||
|
||||
# str() with multiple arguments - should not be flagged
|
||||
logging.warning("%s", str(b"\xe2\x9a\xa0", "utf-8"))
|
||||
|
||||
# str() with starred arguments - should not be flagged
|
||||
logging.warning("%s", str(*(b"\xf0\x9f\x9a\xa7", "utf-8")))
|
||||
|
||||
# str() with keyword unpacking - should not be flagged
|
||||
logging.warning("%s", str(**{"object": b"\xf0\x9f\x9a\xa8", "encoding": "utf-8"}))
|
||||
|
||||
# str() with single keyword argument - should be flagged (equivalent to str("!"))
|
||||
logging.warning("%s", str(object="!"))
|
||||
|
||||
|
|
@ -112,7 +112,8 @@ mod tests {
|
|||
#[test_case(Rule::LegacyFormPytestRaises, Path::new("RUF061_warns.py"))]
|
||||
#[test_case(Rule::LegacyFormPytestRaises, Path::new("RUF061_deprecated_call.py"))]
|
||||
#[test_case(Rule::NonOctalPermissions, Path::new("RUF064.py"))]
|
||||
#[test_case(Rule::LoggingEagerConversion, Path::new("RUF065.py"))]
|
||||
#[test_case(Rule::LoggingEagerConversion, Path::new("RUF065_0.py"))]
|
||||
#[test_case(Rule::LoggingEagerConversion, Path::new("RUF065_1.py"))]
|
||||
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_0.py"))]
|
||||
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_1.py"))]
|
||||
#[test_case(Rule::InvalidRuleCode, Path::new("RUF102.py"))]
|
||||
|
|
|
|||
|
|
@ -138,7 +138,12 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall)
|
|||
.zip(call.arguments.args.iter().skip(msg_pos + 1))
|
||||
{
|
||||
// Check if the argument is a call to eagerly format a value
|
||||
if let Expr::Call(ast::ExprCall { func, .. }) = arg {
|
||||
if let Expr::Call(ast::ExprCall {
|
||||
func,
|
||||
arguments: str_call_args,
|
||||
..
|
||||
}) = arg
|
||||
{
|
||||
let CFormatType::String(format_conversion) = spec.format_type else {
|
||||
continue;
|
||||
};
|
||||
|
|
@ -146,8 +151,13 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall)
|
|||
// Check for various eager conversion patterns
|
||||
match format_conversion {
|
||||
// %s with str() - remove str() call
|
||||
// Only flag if str() has exactly one argument (positional or keyword) that is not unpacked
|
||||
FormatConversion::Str
|
||||
if checker.semantic().match_builtin_expr(func.as_ref(), "str") =>
|
||||
if checker.semantic().match_builtin_expr(func.as_ref(), "str")
|
||||
&& str_call_args.len() == 1
|
||||
&& str_call_args
|
||||
.find_argument("object", 0)
|
||||
.is_some_and(|arg| !arg.is_variadic()) =>
|
||||
{
|
||||
checker.report_diagnostic(
|
||||
LoggingEagerConversion {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
source: crates/ruff_linter/src/rules/ruff/mod.rs
|
||||
---
|
||||
RUF065 Unnecessary `str()` conversion when formatting with `%s`
|
||||
--> RUF065.py:4:26
|
||||
--> RUF065_0.py:4:26
|
||||
|
|
||||
3 | # %s + str()
|
||||
4 | logging.info("Hello %s", str("World!"))
|
||||
|
|
@ -11,7 +11,7 @@ RUF065 Unnecessary `str()` conversion when formatting with `%s`
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `str()` conversion when formatting with `%s`
|
||||
--> RUF065.py:5:39
|
||||
--> RUF065_0.py:5:39
|
||||
|
|
||||
3 | # %s + str()
|
||||
4 | logging.info("Hello %s", str("World!"))
|
||||
|
|
@ -22,7 +22,7 @@ RUF065 Unnecessary `str()` conversion when formatting with `%s`
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
|
||||
--> RUF065.py:8:26
|
||||
--> RUF065_0.py:8:26
|
||||
|
|
||||
7 | # %s + repr()
|
||||
8 | logging.info("Hello %s", repr("World!"))
|
||||
|
|
@ -31,7 +31,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
|
||||
--> RUF065.py:9:39
|
||||
--> RUF065_0.py:9:39
|
||||
|
|
||||
7 | # %s + repr()
|
||||
8 | logging.info("Hello %s", repr("World!"))
|
||||
|
|
@ -42,7 +42,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `str()` conversion when formatting with `%s`
|
||||
--> RUF065.py:22:18
|
||||
--> RUF065_0.py:22:18
|
||||
|
|
||||
21 | # %s + str()
|
||||
22 | info("Hello %s", str("World!"))
|
||||
|
|
@ -51,7 +51,7 @@ RUF065 Unnecessary `str()` conversion when formatting with `%s`
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `str()` conversion when formatting with `%s`
|
||||
--> RUF065.py:23:31
|
||||
--> RUF065_0.py:23:31
|
||||
|
|
||||
21 | # %s + str()
|
||||
22 | info("Hello %s", str("World!"))
|
||||
|
|
@ -62,7 +62,7 @@ RUF065 Unnecessary `str()` conversion when formatting with `%s`
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
|
||||
--> RUF065.py:26:18
|
||||
--> RUF065_0.py:26:18
|
||||
|
|
||||
25 | # %s + repr()
|
||||
26 | info("Hello %s", repr("World!"))
|
||||
|
|
@ -71,7 +71,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
|
||||
--> RUF065.py:27:31
|
||||
--> RUF065_0.py:27:31
|
||||
|
|
||||
25 | # %s + repr()
|
||||
26 | info("Hello %s", repr("World!"))
|
||||
|
|
@ -82,7 +82,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
|
||||
--> RUF065.py:44:32
|
||||
--> RUF065_0.py:44:32
|
||||
|
|
||||
42 | logging.warning("Value: %r", repr(42))
|
||||
43 | logging.error("Error: %r", repr([1, 2, 3]))
|
||||
|
|
@ -92,7 +92,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
|
||||
--> RUF065.py:45:30
|
||||
--> RUF065_0.py:45:30
|
||||
|
|
||||
43 | logging.error("Error: %r", repr([1, 2, 3]))
|
||||
44 | logging.info("Debug info: %s", repr("test\nstring"))
|
||||
|
|
@ -103,7 +103,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` instead of `%s`
|
||||
--> RUF065.py:48:27
|
||||
--> RUF065_0.py:48:27
|
||||
|
|
||||
47 | # %s + ascii()
|
||||
48 | logging.info("ASCII: %s", ascii("Hello\nWorld"))
|
||||
|
|
@ -112,7 +112,7 @@ RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` inst
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` instead of `%s`
|
||||
--> RUF065.py:49:30
|
||||
--> RUF065_0.py:49:30
|
||||
|
|
||||
47 | # %s + ascii()
|
||||
48 | logging.info("ASCII: %s", ascii("Hello\nWorld"))
|
||||
|
|
@ -123,7 +123,7 @@ RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` inst
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` instead of `%s`
|
||||
--> RUF065.py:52:27
|
||||
--> RUF065_0.py:52:27
|
||||
|
|
||||
51 | # %s + oct()
|
||||
52 | logging.info("Octal: %s", oct(42))
|
||||
|
|
@ -132,7 +132,7 @@ RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` instead of `%s`
|
||||
--> RUF065.py:53:30
|
||||
--> RUF065_0.py:53:30
|
||||
|
|
||||
51 | # %s + oct()
|
||||
52 | logging.info("Octal: %s", oct(42))
|
||||
|
|
@ -143,7 +143,7 @@ RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` instead of `%s`
|
||||
--> RUF065.py:56:25
|
||||
--> RUF065_0.py:56:25
|
||||
|
|
||||
55 | # %s + hex()
|
||||
56 | logging.info("Hex: %s", hex(42))
|
||||
|
|
@ -152,7 +152,7 @@ RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` instead of `%s`
|
||||
--> RUF065.py:57:28
|
||||
--> RUF065_0.py:57:28
|
||||
|
|
||||
55 | # %s + hex()
|
||||
56 | logging.info("Hex: %s", hex(42))
|
||||
|
|
@ -161,7 +161,7 @@ RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` instead of `%s`
|
||||
--> RUF065.py:63:19
|
||||
--> RUF065_0.py:63:19
|
||||
|
|
||||
61 | from logging import info, log
|
||||
62 |
|
||||
|
|
@ -171,7 +171,7 @@ RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` inst
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` instead of `%s`
|
||||
--> RUF065.py:64:32
|
||||
--> RUF065_0.py:64:32
|
||||
|
|
||||
63 | info("ASCII: %s", ascii("Hello\nWorld"))
|
||||
64 | log(logging.INFO, "ASCII: %s", ascii("test"))
|
||||
|
|
@ -181,7 +181,7 @@ RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` inst
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` instead of `%s`
|
||||
--> RUF065.py:66:19
|
||||
--> RUF065_0.py:66:19
|
||||
|
|
||||
64 | log(logging.INFO, "ASCII: %s", ascii("test"))
|
||||
65 |
|
||||
|
|
@ -191,7 +191,7 @@ RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` instead of `%s`
|
||||
--> RUF065.py:67:32
|
||||
--> RUF065_0.py:67:32
|
||||
|
|
||||
66 | info("Octal: %s", oct(42))
|
||||
67 | log(logging.INFO, "Octal: %s", oct(255))
|
||||
|
|
@ -201,7 +201,7 @@ RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` instead of `%s`
|
||||
--> RUF065.py:69:17
|
||||
--> RUF065_0.py:69:17
|
||||
|
|
||||
67 | log(logging.INFO, "Octal: %s", oct(255))
|
||||
68 |
|
||||
|
|
@ -211,7 +211,7 @@ RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` inste
|
|||
|
|
||||
|
||||
RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` instead of `%s`
|
||||
--> RUF065.py:70:30
|
||||
--> RUF065_0.py:70:30
|
||||
|
|
||||
69 | info("Hex: %s", hex(42))
|
||||
70 | log(logging.INFO, "Hex: %s", hex(255))
|
||||
|
|
@ -0,0 +1,10 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/ruff/mod.rs
|
||||
---
|
||||
RUF065 Unnecessary `str()` conversion when formatting with `%s`
|
||||
--> RUF065_1.py:17:23
|
||||
|
|
||||
16 | # str() with single keyword argument - should be flagged (equivalent to str("!"))
|
||||
17 | logging.warning("%s", str(object="!"))
|
||||
| ^^^^^^^^^^^^^^^
|
||||
|
|
||||
|
|
@ -3269,6 +3269,13 @@ impl<'a> ArgOrKeyword<'a> {
|
|||
ArgOrKeyword::Keyword(keyword) => &keyword.value,
|
||||
}
|
||||
}
|
||||
|
||||
pub const fn is_variadic(self) -> bool {
|
||||
match self {
|
||||
ArgOrKeyword::Arg(expr) => expr.is_starred_expr(),
|
||||
ArgOrKeyword::Keyword(keyword) => keyword.arg.is_none(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a> From<&'a Expr> for ArgOrKeyword<'a> {
|
||||
|
|
|
|||
Loading…
Reference in New Issue