Skip to content

Commit 99d4014

Browse files
[pyflakes] Mark F523 fix as unsafe when it removes a meaningful .format(...) call
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. Detect these cases and downgrade the fix applicability to `Unsafe`, so users opt in via `--unsafe-fixes` instead of having the autofix silently change runtime behavior. Side-effect detection keeps its existing behavior; the new check just widens the set of unsafe cases. Closes #15557
1 parent 767df43 commit 99d4014

3 files changed

Lines changed: 72 additions & 2 deletions

File tree

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,10 @@
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+
# These fixes must be unsafe: removing the `.format(...)` call entirely changes the
49+
# string ("{{" is two characters but `"{{".format()` is one), or suppresses a
50+
# KeyError raised by an unset named placeholder.
51+
"{{".format("!")
52+
"{x}".format("!")

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,14 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
920920
);
921921

922922
if is_contiguous_from_end(&missing, args) {
923+
// If every argument is being removed and the format string contains `{{`, `}}`,
924+
// or named placeholders, removing the entire `.format(...)` call changes the
925+
// string's observable behavior (`"{{".format()` returns `"{"`, but `"{{"` is
926+
// two characters; `"{x}".format()` raises `KeyError`, but `"{x}"` does not).
927+
// Mark such fixes as unsafe.
928+
let removes_format_call = missing.len() == args.len();
929+
let format_string_meaningful = removes_format_call
930+
&& format_string_has_braces_or_named_field(call, summary);
923931
diagnostic.try_set_fix(|| {
924932
let edit = remove_unused_positional_arguments_from_format_call(
925933
&missing,
@@ -929,8 +937,7 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
929937
)?;
930938
Ok(Fix::applicable_edit(
931939
edit,
932-
// Mark fix as unsafe if the `format` call contains an argument with side effect
933-
if side_effects {
940+
if side_effects || format_string_meaningful {
934941
Applicability::Unsafe
935942
} else {
936943
Applicability::Safe
@@ -940,6 +947,23 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
940947
}
941948
}
942949

950+
/// Returns `true` when the `format` call's target string literal contains either
951+
/// a brace-escape (`{{` / `}}`) or a named field. Both cases survive only as long
952+
/// as the `.format(...)` call survives, so removing the call entirely silently
953+
/// changes the resulting string or suppresses a `KeyError`.
954+
fn format_string_has_braces_or_named_field(call: &ast::ExprCall, summary: &FormatSummary) -> bool {
955+
if !summary.keywords.is_empty() {
956+
return true;
957+
}
958+
let Expr::Attribute(attribute) = &*call.func else {
959+
return false;
960+
};
961+
let Expr::StringLiteral(literal) = &*attribute.value else {
962+
return false;
963+
};
964+
literal.value.to_str().bytes().any(|b| b == b'{' || b == b'}')
965+
}
966+
943967
/// F524
944968
pub(crate) fn string_dot_format_missing_argument(
945969
checker: &Checker,

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,10 +332,49 @@ 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 | # These fixes must be unsafe: removing the `.format(...)` call entirely changes the
347+
348+
F523 [*] `.format` call has unused arguments at position(s): 0
349+
--> F523.py:51:1
350+
|
351+
49 | # string ("{{" is two characters but `"{{".format()` is one), or suppresses a
352+
50 | # KeyError raised by an unset named placeholder.
353+
51 | "{{".format("!")
354+
| ^^^^^^^^^^^^^^^^
355+
52 | "{x}".format("!")
356+
|
357+
help: Remove extra positional arguments at position(s): 0
358+
48 | # These fixes must be unsafe: removing the `.format(...)` call entirely changes the
359+
49 | # string ("{{" is two characters but `"{{".format()` is one), or suppresses a
360+
50 | # KeyError raised by an unset named placeholder.
361+
- "{{".format("!")
362+
51 + "{{"
363+
52 | "{x}".format("!")
364+
note: This is an unsafe fix and may change runtime behavior
365+
366+
F523 [*] `.format` call has unused arguments at position(s): 0
367+
--> F523.py:52:1
368+
|
369+
50 | # KeyError raised by an unset named placeholder.
370+
51 | "{{".format("!")
371+
52 | "{x}".format("!")
372+
| ^^^^^^^^^^^^^^^^^
373+
|
374+
help: Remove extra positional arguments at position(s): 0
375+
49 | # string ("{{" is two characters but `"{{".format()` is one), or suppresses a
376+
50 | # KeyError raised by an unset named placeholder.
377+
51 | "{{".format("!")
378+
- "{x}".format("!")
379+
52 + "{x}"
380+
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)