Skip to content

Commit 1fd852f

Browse files
danparizherntBre
danparizher
and
ntBre
authored
[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>
1 parent 5f3e086 commit 1fd852f

7 files changed

Lines changed: 71 additions & 25 deletions

File tree

crates/ruff_linter/resources/test/fixtures/ruff/RUF065.py renamed to crates/ruff_linter/resources/test/fixtures/ruff/RUF065_0.py

File renamed without changes.

‎crates/ruff_linter/resources/test/fixtures/ruff/RUF065_1.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import logging
2+
3+
# Test cases for str() that should NOT be flagged (issue #21315)
4+
# str() with no arguments - should not be flagged
5+
logging.warning("%s", str())
6+
7+
# str() with multiple arguments - should not be flagged
8+
logging.warning("%s", str(b"\xe2\x9a\xa0", "utf-8"))
9+
10+
# str() with starred arguments - should not be flagged
11+
logging.warning("%s", str(*(b"\xf0\x9f\x9a\xa7", "utf-8")))
12+
13+
# str() with keyword unpacking - should not be flagged
14+
logging.warning("%s", str(**{"object": b"\xf0\x9f\x9a\xa8", "encoding": "utf-8"}))
15+
16+
# str() with single keyword argument - should be flagged (equivalent to str("!"))
17+
logging.warning("%s", str(object="!"))
18+

‎crates/ruff_linter/src/rules/ruff/mod.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ mod tests {
112112
#[test_case(Rule::LegacyFormPytestRaises, Path::new("RUF061_warns.py"))]
113113
#[test_case(Rule::LegacyFormPytestRaises, Path::new("RUF061_deprecated_call.py"))]
114114
#[test_case(Rule::NonOctalPermissions, Path::new("RUF064.py"))]
115-
#[test_case(Rule::LoggingEagerConversion, Path::new("RUF065.py"))]
115+
#[test_case(Rule::LoggingEagerConversion, Path::new("RUF065_0.py"))]
116+
#[test_case(Rule::LoggingEagerConversion, Path::new("RUF065_1.py"))]
116117
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_0.py"))]
117118
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_1.py"))]
118119
#[test_case(Rule::InvalidRuleCode, Path::new("RUF102.py"))]

‎crates/ruff_linter/src/rules/ruff/rules/logging_eager_conversion.rs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,26 @@ pub(crate) fn logging_eager_conversion(checker: &Checker, call: &ast::ExprCall)
138138
.zip(call.arguments.args.iter().skip(msg_pos + 1))
139139
{
140140
// Check if the argument is a call to eagerly format a value
141-
if let Expr::Call(ast::ExprCall { func, .. }) = arg {
141+
if let Expr::Call(ast::ExprCall {
142+
func,
143+
arguments: str_call_args,
144+
..
145+
}) = arg
146+
{
142147
let CFormatType::String(format_conversion) = spec.format_type else {
143148
continue;
144149
};
145150

146151
// Check for various eager conversion patterns
147152
match format_conversion {
148153
// %s with str() - remove str() call
154+
// Only flag if str() has exactly one argument (positional or keyword) that is not unpacked
149155
FormatConversion::Str
150-
if checker.semantic().match_builtin_expr(func.as_ref(), "str") =>
156+
if checker.semantic().match_builtin_expr(func.as_ref(), "str")
157+
&& str_call_args.len() == 1
158+
&& str_call_args
159+
.find_argument("object", 0)
160+
.is_some_and(|arg| !arg.is_variadic()) =>
151161
{
152162
checker.report_diagnostic(
153163
LoggingEagerConversion {

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065.py.snap renamed to crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_0.py.snap

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
source: crates/ruff_linter/src/rules/ruff/mod.rs
33
---
44
RUF065 Unnecessary `str()` conversion when formatting with `%s`
5-
--> RUF065.py:4:26
5+
--> RUF065_0.py:4:26
66
|
77
3 | # %s + str()
88
4 | logging.info("Hello %s", str("World!"))
@@ -11,7 +11,7 @@ RUF065 Unnecessary `str()` conversion when formatting with `%s`
1111
|
1212

1313
RUF065 Unnecessary `str()` conversion when formatting with `%s`
14-
--> RUF065.py:5:39
14+
--> RUF065_0.py:5:39
1515
|
1616
3 | # %s + str()
1717
4 | logging.info("Hello %s", str("World!"))
@@ -22,7 +22,7 @@ RUF065 Unnecessary `str()` conversion when formatting with `%s`
2222
|
2323

2424
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
25-
--> RUF065.py:8:26
25+
--> RUF065_0.py:8:26
2626
|
2727
7 | # %s + repr()
2828
8 | logging.info("Hello %s", repr("World!"))
@@ -31,7 +31,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
3131
|
3232

3333
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
34-
--> RUF065.py:9:39
34+
--> RUF065_0.py:9:39
3535
|
3636
7 | # %s + repr()
3737
8 | logging.info("Hello %s", repr("World!"))
@@ -42,7 +42,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
4242
|
4343

4444
RUF065 Unnecessary `str()` conversion when formatting with `%s`
45-
--> RUF065.py:22:18
45+
--> RUF065_0.py:22:18
4646
|
4747
21 | # %s + str()
4848
22 | info("Hello %s", str("World!"))
@@ -51,7 +51,7 @@ RUF065 Unnecessary `str()` conversion when formatting with `%s`
5151
|
5252

5353
RUF065 Unnecessary `str()` conversion when formatting with `%s`
54-
--> RUF065.py:23:31
54+
--> RUF065_0.py:23:31
5555
|
5656
21 | # %s + str()
5757
22 | info("Hello %s", str("World!"))
@@ -62,7 +62,7 @@ RUF065 Unnecessary `str()` conversion when formatting with `%s`
6262
|
6363

6464
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
65-
--> RUF065.py:26:18
65+
--> RUF065_0.py:26:18
6666
|
6767
25 | # %s + repr()
6868
26 | info("Hello %s", repr("World!"))
@@ -71,7 +71,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
7171
|
7272

7373
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
74-
--> RUF065.py:27:31
74+
--> RUF065_0.py:27:31
7575
|
7676
25 | # %s + repr()
7777
26 | info("Hello %s", repr("World!"))
@@ -82,7 +82,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
8282
|
8383

8484
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
85-
--> RUF065.py:44:32
85+
--> RUF065_0.py:44:32
8686
|
8787
42 | logging.warning("Value: %r", repr(42))
8888
43 | logging.error("Error: %r", repr([1, 2, 3]))
@@ -92,7 +92,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
9292
|
9393

9494
RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` instead of `%s`
95-
--> RUF065.py:45:30
95+
--> RUF065_0.py:45:30
9696
|
9797
43 | logging.error("Error: %r", repr([1, 2, 3]))
9898
44 | logging.info("Debug info: %s", repr("test\nstring"))
@@ -103,7 +103,7 @@ RUF065 Unnecessary `repr()` conversion when formatting with `%s`. Use `%r` inste
103103
|
104104

105105
RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` instead of `%s`
106-
--> RUF065.py:48:27
106+
--> RUF065_0.py:48:27
107107
|
108108
47 | # %s + ascii()
109109
48 | logging.info("ASCII: %s", ascii("Hello\nWorld"))
@@ -112,7 +112,7 @@ RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` inst
112112
|
113113

114114
RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` instead of `%s`
115-
--> RUF065.py:49:30
115+
--> RUF065_0.py:49:30
116116
|
117117
47 | # %s + ascii()
118118
48 | logging.info("ASCII: %s", ascii("Hello\nWorld"))
@@ -123,7 +123,7 @@ RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` inst
123123
|
124124

125125
RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` instead of `%s`
126-
--> RUF065.py:52:27
126+
--> RUF065_0.py:52:27
127127
|
128128
51 | # %s + oct()
129129
52 | logging.info("Octal: %s", oct(42))
@@ -132,7 +132,7 @@ RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` inste
132132
|
133133

134134
RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` instead of `%s`
135-
--> RUF065.py:53:30
135+
--> RUF065_0.py:53:30
136136
|
137137
51 | # %s + oct()
138138
52 | logging.info("Octal: %s", oct(42))
@@ -143,7 +143,7 @@ RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` inste
143143
|
144144

145145
RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` instead of `%s`
146-
--> RUF065.py:56:25
146+
--> RUF065_0.py:56:25
147147
|
148148
55 | # %s + hex()
149149
56 | logging.info("Hex: %s", hex(42))
@@ -152,7 +152,7 @@ RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` inste
152152
|
153153

154154
RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` instead of `%s`
155-
--> RUF065.py:57:28
155+
--> RUF065_0.py:57:28
156156
|
157157
55 | # %s + hex()
158158
56 | logging.info("Hex: %s", hex(42))
@@ -161,7 +161,7 @@ RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` inste
161161
|
162162

163163
RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` instead of `%s`
164-
--> RUF065.py:63:19
164+
--> RUF065_0.py:63:19
165165
|
166166
61 | from logging import info, log
167167
62 |
@@ -171,7 +171,7 @@ RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` inst
171171
|
172172

173173
RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` instead of `%s`
174-
--> RUF065.py:64:32
174+
--> RUF065_0.py:64:32
175175
|
176176
63 | info("ASCII: %s", ascii("Hello\nWorld"))
177177
64 | log(logging.INFO, "ASCII: %s", ascii("test"))
@@ -181,7 +181,7 @@ RUF065 Unnecessary `ascii()` conversion when formatting with `%s`. Use `%a` inst
181181
|
182182

183183
RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` instead of `%s`
184-
--> RUF065.py:66:19
184+
--> RUF065_0.py:66:19
185185
|
186186
64 | log(logging.INFO, "ASCII: %s", ascii("test"))
187187
65 |
@@ -191,7 +191,7 @@ RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` inste
191191
|
192192

193193
RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` instead of `%s`
194-
--> RUF065.py:67:32
194+
--> RUF065_0.py:67:32
195195
|
196196
66 | info("Octal: %s", oct(42))
197197
67 | log(logging.INFO, "Octal: %s", oct(255))
@@ -201,7 +201,7 @@ RUF065 Unnecessary `oct()` conversion when formatting with `%s`. Use `%#o` inste
201201
|
202202

203203
RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` instead of `%s`
204-
--> RUF065.py:69:17
204+
--> RUF065_0.py:69:17
205205
|
206206
67 | log(logging.INFO, "Octal: %s", oct(255))
207207
68 |
@@ -211,7 +211,7 @@ RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` inste
211211
|
212212

213213
RUF065 Unnecessary `hex()` conversion when formatting with `%s`. Use `%#x` instead of `%s`
214-
--> RUF065.py:70:30
214+
--> RUF065_0.py:70:30
215215
|
216216
69 | info("Hex: %s", hex(42))
217217
70 | log(logging.INFO, "Hex: %s", hex(255))

‎crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF065_RUF065_1.py.snap‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
source: crates/ruff_linter/src/rules/ruff/mod.rs
3+
---
4+
RUF065 Unnecessary `str()` conversion when formatting with `%s`
5+
--> RUF065_1.py:17:23
6+
|
7+
16 | # str() with single keyword argument - should be flagged (equivalent to str("!"))
8+
17 | logging.warning("%s", str(object="!"))
9+
| ^^^^^^^^^^^^^^^
10+
|

‎crates/ruff_python_ast/src/nodes.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3269,6 +3269,13 @@ impl<'a> ArgOrKeyword<'a> {
32693269
ArgOrKeyword::Keyword(keyword) => &keyword.value,
32703270
}
32713271
}
3272+
3273+
pub const fn is_variadic(self) -> bool {
3274+
match self {
3275+
ArgOrKeyword::Arg(expr) => expr.is_starred_expr(),
3276+
ArgOrKeyword::Keyword(keyword) => keyword.arg.is_none(),
3277+
}
3278+
}
32723279
}
32733280

32743281
impl<'a> From<&'a Expr> for ArgOrKeyword<'a> {

0 commit comments

Comments
 (0)