Skip to content

Commit 49882fa

Browse files
[pyflakes] Avoid removing the format call when it would change behavior (F523) (#25320)
## Summary When every positional argument is unused, `F523` removes the entire `.format(...)` call. That changes behavior whenever the format string relies on the call: * Brace escapes: `"{{".format("!")` returns `"{"`, but `"{{"` is two characters. * Named placeholders: `"{x}".format("!")` raises `KeyError`, but `"{x}"` quietly succeeds. This detects these cases and avoids removing the entire `format` call. We also tried the "ideal" fix described on the issue of processing the string to handle escapes ourselves, but just preserving the call seemed like the better tradeoff in terms of complexity. ## Test plan New mdtests based on the examples from the issue Closes #15557 --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
1 parent f46ee08 commit 49882fa

2 files changed

Lines changed: 57 additions & 13 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# `string-dot-format-extra-positional-arguments` (`F523`)
2+
3+
```toml
4+
[lint]
5+
select = ["F523"]
6+
```
7+
8+
## Regression tests for [#15557]
9+
10+
These should both trigger the rule, but their fixes should not remove the `format` call. In the
11+
first case, `format` will handle escaping `{{` to `{`, while in the second case, removing the
12+
`format` call would suppress a `KeyError`:
13+
14+
```py
15+
print("{{".format("!")) # snapshot: string-dot-format-extra-positional-arguments
16+
print("{x}".format("!")) # snapshot: string-dot-format-extra-positional-arguments
17+
```
18+
19+
```snapshot
20+
error[F523]: `.format` call has unused arguments at position(s): 0
21+
--> src/mdtest_snippet.py:1:7
22+
|
23+
1 | print("{{".format("!")) # snapshot: string-dot-format-extra-positional-arguments
24+
| ^^^^^^^^^^^^^^^^
25+
|
26+
help: Remove extra positional arguments at position(s): 0
27+
- print("{{".format("!")) # snapshot: string-dot-format-extra-positional-arguments
28+
1 + print("{{".format()) # snapshot: string-dot-format-extra-positional-arguments
29+
2 | print("{x}".format("!")) # snapshot: string-dot-format-extra-positional-arguments
30+
31+
32+
error[F523]: `.format` call has unused arguments at position(s): 0
33+
--> src/mdtest_snippet.py:2:7
34+
|
35+
2 | print("{x}".format("!")) # snapshot: string-dot-format-extra-positional-arguments
36+
| ^^^^^^^^^^^^^^^^^
37+
|
38+
help: Remove extra positional arguments at position(s): 0
39+
1 | print("{{".format("!")) # snapshot: string-dot-format-extra-positional-arguments
40+
- print("{x}".format("!")) # snapshot: string-dot-format-extra-positional-arguments
41+
2 + print("{x}".format()) # snapshot: string-dot-format-extra-positional-arguments
42+
```
43+
44+
[#15557]: https://github.com/astral-sh/ruff/issues/15557

crates/ruff_linter/src/rules/pyflakes/fixes.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,18 @@ pub(crate) fn remove_unused_positional_arguments_from_format_call(
7373
// If we're removing _all_ arguments, we can remove the entire call.
7474
//
7575
// For example, `"Hello".format(", world!")` -> `"Hello"`, as opposed to `"Hello".format()`.
76-
if unused_arguments.len() == call.arguments.len() {
77-
if let Expr::Attribute(attribute) = &*call.func {
78-
return Ok(Edit::range_replacement(
79-
locator.slice(&*attribute.value).to_string(),
80-
call.range(),
81-
));
82-
}
76+
//
77+
// However, if the `format` call has other effects, like escaping `{{` to `{` or would raise a
78+
// `KeyError` for a missing field name, we preserve the empty call to avoid changing behavior.
79+
if unused_arguments.len() == call.arguments.len()
80+
&& let Expr::Attribute(attribute) = &*call.func
81+
&& let Expr::StringLiteral(string_expr) = &*attribute.value
82+
&& !string_expr.value.to_str().contains(['{', '}'])
83+
{
84+
return Ok(Edit::range_replacement(
85+
locator.slice(string_expr).to_string(),
86+
call.range(),
87+
));
8388
}
8489

8590
let source_code = locator.slice(call);
@@ -94,12 +99,7 @@ pub(crate) fn remove_unused_positional_arguments_from_format_call(
9499
!is_unused
95100
});
96101

97-
// If there are no arguments left, remove the parentheses.
98-
if call.args.is_empty() {
99-
Ok((*call.func).clone())
100-
} else {
101-
Ok(expression)
102-
}
102+
Ok(expression)
103103
})
104104
.map(|output| Edit::range_replacement(output, call.range()))
105105
}

0 commit comments

Comments
 (0)