Skip to content

Commit 0c706a0

Browse files
[pyflakes] Implement the ideal F523 fix when the .format call has no remaining replacement fields
When dropping every positional argument leaves a format string with only `{{` / `}}` brace escapes, the rewrite now removes the `.format(...)` call and reduces the escapes to single braces so the result matches what `.format()` would have produced. When the string still has replacement fields (auto, indexed, or named), the empty `.format()` call is kept so the runtime behaviour, including any `KeyError`, is preserved. Both branches are safe fixes again.
1 parent 767df43 commit 0c706a0

4 files changed

Lines changed: 192 additions & 16 deletions

File tree

crates/ruff_linter/resources/test/fixtures/pyflakes/F523.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,14 @@
4343
# The fix here is safe because the unused argument has no side effect,
4444
# even though the used argument has a side effect
4545
"Hello, {0}".format(print(1), "Pikachu")
46+
47+
# https://github.com/astral-sh/ruff/issues/15557
48+
# When dropping every argument leaves a string with only `{{`/`}}` escapes, the
49+
# `.format(...)` call is removed and the escapes are reduced to single braces.
50+
# When the string still has replacement fields, the empty `.format()` call is
51+
# kept so the `KeyError` (or other failure) at runtime is preserved.
52+
"{{".format("!")
53+
"{x}".format("!")
54+
"{{}}".format("!")
55+
"{{0}}".format("!")
56+
"{x}".format("!", x=1)

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

Lines changed: 84 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use ruff_text_size::Ranged;
1010
use crate::Edit;
1111
use crate::Locator;
1212
use crate::cst::matchers::{match_call_mut, match_dict, transform_expression};
13+
use crate::rules::pyflakes::format::FormatSummary;
1314

1415
/// Generate a [`Edit`] to remove unused keys from format dict.
1516
pub(super) fn remove_unused_format_arguments_from_dict(
@@ -64,22 +65,29 @@ pub(super) fn remove_unused_keyword_arguments_from_format_call(
6465
}
6566

6667
/// Generate a [`Edit`] to remove unused positional arguments from a `format` call.
68+
///
69+
/// When the rewrite would leave `.format(...)` with zero arguments and the
70+
/// format string has no replacement fields, the call itself is dropped and any
71+
/// `{{` / `}}` escapes in the literal are reduced to single braces so the
72+
/// result matches what `.format()` would have produced. When the string still
73+
/// has replacement fields (or the literal cannot be unescaped textually) the
74+
/// empty `.format()` call is kept so that runtime behaviour, including any
75+
/// `KeyError`, is preserved.
6776
pub(crate) fn remove_unused_positional_arguments_from_format_call(
6877
unused_arguments: &[usize],
6978
call: &ast::ExprCall,
79+
summary: &FormatSummary,
7080
locator: &Locator,
7181
stylist: &Stylist,
7282
) -> Result<Edit> {
73-
// If we're removing _all_ arguments, we can remove the entire call.
74-
//
75-
// 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-
}
83+
if unused_arguments.len() == call.arguments.len()
84+
&& summary.autos.is_empty()
85+
&& summary.indices.is_empty()
86+
&& summary.keywords.is_empty()
87+
&& let Expr::Attribute(attribute) = &*call.func
88+
&& let Some(replacement) = literal_source_without_format_call(&attribute.value, locator)
89+
{
90+
return Ok(Edit::range_replacement(replacement, call.range()));
8391
}
8492

8593
let source_code = locator.slice(call);
@@ -94,16 +102,76 @@ pub(crate) fn remove_unused_positional_arguments_from_format_call(
94102
!is_unused
95103
});
96104

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-
}
105+
Ok(expression)
103106
})
104107
.map(|output| Edit::range_replacement(output, call.range()))
105108
}
106109

110+
/// Returns the source text to substitute for `expr.format(...)` when the
111+
/// `.format(...)` call is being dropped because it has no replacement fields
112+
/// and no remaining arguments.
113+
///
114+
/// Returns `None` when the rewrite cannot be done safely, in which case the
115+
/// caller should keep the `.format(...)` call (with no args) instead.
116+
///
117+
/// If the literal's decoded value has no braces, `.format()` is a no-op on it
118+
/// and the original source can be reused verbatim. When the value contains
119+
/// `{{` / `}}` escapes, those are reduced to single braces, but only when the
120+
/// literal's source content equals its decoded value: an escape such as
121+
/// `\x7b` could otherwise interact with a neighbouring literal brace and
122+
/// silently change the resulting string.
123+
fn literal_source_without_format_call(expr: &Expr, locator: &Locator) -> Option<String> {
124+
let Expr::StringLiteral(string_expr) = expr else {
125+
return None;
126+
};
127+
if string_expr.value.is_implicit_concatenated() {
128+
return None;
129+
}
130+
let literal = string_expr.value.iter().next()?;
131+
132+
if !literal.value.contains(['{', '}']) {
133+
// `.format()` would not have changed anything in the resulting
134+
// string, so reuse the original source as-is.
135+
return Some(locator.slice(string_expr).to_string());
136+
}
137+
138+
let content_range = literal.content_range();
139+
let source_content = locator.slice(content_range);
140+
141+
// The textual `{{` -> `{` rewrite below is only safe when the source
142+
// content matches the decoded value: otherwise a Python escape sequence
143+
// such as `\x7b` could combine with a neighbouring literal brace and
144+
// change the resulting string.
145+
if source_content != &*literal.value {
146+
return None;
147+
}
148+
149+
let unescaped = unescape_format_braces(source_content);
150+
let prefix_and_opener = locator.slice(ruff_text_size::TextRange::new(
151+
literal.start(),
152+
content_range.start(),
153+
));
154+
let closer = locator.slice(ruff_text_size::TextRange::new(
155+
content_range.end(),
156+
literal.end(),
157+
));
158+
159+
Some(format!("{prefix_and_opener}{unescaped}{closer}"))
160+
}
161+
162+
/// Replaces every `{{` with `{` and every `}}` with `}` in `text`.
163+
fn unescape_format_braces(text: &str) -> String {
164+
let mut out = String::with_capacity(text.len());
165+
let mut chars = text.chars().peekable();
166+
while let Some(ch) = chars.next() {
167+
if (ch == '{' || ch == '}') && chars.peek() == Some(&ch) {
168+
chars.next();
169+
}
170+
out.push(ch);
171+
}
172+
out
173+
}
174+
107175
/// Generate a [`Edit`] to remove the binding from an exception handler.
108176
pub(crate) fn remove_exception_handler_assignment(
109177
bound_exception: &Binding,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,7 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
924924
let edit = remove_unused_positional_arguments_from_format_call(
925925
&missing,
926926
call,
927+
summary,
927928
checker.locator(),
928929
checker.stylist(),
929930
)?;

crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F523_F523.py.snap

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,10 +332,106 @@ F523 [*] `.format` call has unused arguments at position(s): 1
332332
44 | # even though the used argument has a side effect
333333
45 | "Hello, {0}".format(print(1), "Pikachu")
334334
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
335+
46 |
336+
47 | # https://github.com/astral-sh/ruff/issues/15557
335337
|
336338
help: Remove extra positional arguments at position(s): 1
337339
42 |
338340
43 | # The fix here is safe because the unused argument has no side effect,
339341
44 | # even though the used argument has a side effect
340342
- "Hello, {0}".format(print(1), "Pikachu")
341343
45 + "Hello, {0}".format(print(1), )
344+
46 |
345+
47 | # https://github.com/astral-sh/ruff/issues/15557
346+
48 | # When dropping every argument leaves a string with only `{{`/`}}` escapes, the
347+
348+
F523 [*] `.format` call has unused arguments at position(s): 0
349+
--> F523.py:52:1
350+
|
351+
50 | # When the string still has replacement fields, the empty `.format()` call is
352+
51 | # kept so the `KeyError` (or other failure) at runtime is preserved.
353+
52 | "{{".format("!")
354+
| ^^^^^^^^^^^^^^^^
355+
53 | "{x}".format("!")
356+
54 | "{{}}".format("!")
357+
|
358+
help: Remove extra positional arguments at position(s): 0
359+
49 | # `.format(...)` call is removed and the escapes are reduced to single braces.
360+
50 | # When the string still has replacement fields, the empty `.format()` call is
361+
51 | # kept so the `KeyError` (or other failure) at runtime is preserved.
362+
- "{{".format("!")
363+
52 + "{"
364+
53 | "{x}".format("!")
365+
54 | "{{}}".format("!")
366+
55 | "{{0}}".format("!")
367+
368+
F523 [*] `.format` call has unused arguments at position(s): 0
369+
--> F523.py:53:1
370+
|
371+
51 | # kept so the `KeyError` (or other failure) at runtime is preserved.
372+
52 | "{{".format("!")
373+
53 | "{x}".format("!")
374+
| ^^^^^^^^^^^^^^^^^
375+
54 | "{{}}".format("!")
376+
55 | "{{0}}".format("!")
377+
|
378+
help: Remove extra positional arguments at position(s): 0
379+
50 | # When the string still has replacement fields, the empty `.format()` call is
380+
51 | # kept so the `KeyError` (or other failure) at runtime is preserved.
381+
52 | "{{".format("!")
382+
- "{x}".format("!")
383+
53 + "{x}".format()
384+
54 | "{{}}".format("!")
385+
55 | "{{0}}".format("!")
386+
56 | "{x}".format("!", x=1)
387+
388+
F523 [*] `.format` call has unused arguments at position(s): 0
389+
--> F523.py:54:1
390+
|
391+
52 | "{{".format("!")
392+
53 | "{x}".format("!")
393+
54 | "{{}}".format("!")
394+
| ^^^^^^^^^^^^^^^^^^
395+
55 | "{{0}}".format("!")
396+
56 | "{x}".format("!", x=1)
397+
|
398+
help: Remove extra positional arguments at position(s): 0
399+
51 | # kept so the `KeyError` (or other failure) at runtime is preserved.
400+
52 | "{{".format("!")
401+
53 | "{x}".format("!")
402+
- "{{}}".format("!")
403+
54 + "{}"
404+
55 | "{{0}}".format("!")
405+
56 | "{x}".format("!", x=1)
406+
407+
F523 [*] `.format` call has unused arguments at position(s): 0
408+
--> F523.py:55:1
409+
|
410+
53 | "{x}".format("!")
411+
54 | "{{}}".format("!")
412+
55 | "{{0}}".format("!")
413+
| ^^^^^^^^^^^^^^^^^^^
414+
56 | "{x}".format("!", x=1)
415+
|
416+
help: Remove extra positional arguments at position(s): 0
417+
52 | "{{".format("!")
418+
53 | "{x}".format("!")
419+
54 | "{{}}".format("!")
420+
- "{{0}}".format("!")
421+
55 + "{0}"
422+
56 | "{x}".format("!", x=1)
423+
424+
F523 [*] `.format` call has unused arguments at position(s): 0
425+
--> F523.py:56:1
426+
|
427+
54 | "{{}}".format("!")
428+
55 | "{{0}}".format("!")
429+
56 | "{x}".format("!", x=1)
430+
| ^^^^^^^^^^^^^^^^^^^^^^
431+
|
432+
help: Remove extra positional arguments at position(s): 0
433+
53 | "{x}".format("!")
434+
54 | "{{}}".format("!")
435+
55 | "{{0}}".format("!")
436+
- "{x}".format("!", x=1)
437+
56 + "{x}".format(x=1)

0 commit comments

Comments
 (0)