mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-04 18:58:04 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[pyflakes] Mark F504/F522/F523 autofix as unsafe if there's a call with side effect (#18839)

Browse source 
Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
Victor Hugo Gomes 2025-06-26 05:48:29 -03:00 • committed by GitHub
parent 170ccd80b4
commit 2362263d5e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 200 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

3
crates/ruff_linter/resources/test/fixtures/pyflakes/F504.py vendored
View file

@ -14,3 +14,6 @@ hidden = {"a": "!"}
"" % {
'test1': '', 'test2': '',
}
# https://github.com/astral-sh/ruff/issues/18806
"Hello, %(name)s" % {"greeting": print(1), "name": "World"}

8
crates/ruff_linter/resources/test/fixtures/pyflakes/F522.py vendored
View file

@ -4,3 +4,11 @@
"{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522
(''
.format(x=2)) # F522
# https://github.com/astral-sh/ruff/issues/18806
# The fix here is unsafe because the unused argument has side effect
"Hello, {name}".format(greeting=print(1), name="World")
# The fix here is safe because the unused argument has no side effect,
# even though the used argument has a side effect
"Hello, {name}".format(greeting="Pikachu", name=print(1))

8
crates/ruff_linter/resources/test/fixtures/pyflakes/F523.py vendored
View file

@ -35,3 +35,11 @@
# Removing the final argument.
"Hello".format("world")
"Hello".format("world", key="value")
# https://github.com/astral-sh/ruff/issues/18806
# The fix here is unsafe because the unused argument has side effect
"Hello, {0}".format("world", print(1))
# The fix here is safe because the unused argument has no side effect,
# even though the used argument has a side effect
"Hello, {0}".format(print(1), "Pikachu")

85
crates/ruff_linter/src/rules/pyflakes/rules/strings.rs
View file

@ -1,5 +1,7 @@
use std::string::ToString;
use ruff_diagnostics::Applicability;
use ruff_python_ast::helpers::contains_effect;
use rustc_hash::FxHashSet;
use ruff_macros::{ViolationMetadata, derive_message_formats};
@ -138,6 +140,16 @@ impl Violation for PercentFormatExpectedSequence {
/// "Hello, %(name)s" % {"name": "World"}
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe for mapping key
/// containing function calls with potential side effects,
/// because removing such arguments could change the behavior of the code.
///
/// For example, the fix would be marked as unsafe in the following case:
/// ```python
/// "Hello, %(name)s" % {"greeting": print(1), "name": "World"}
/// ```
///
/// ## References
/// - [Python documentation: `printf`-style String Formatting](https://docs.python.org/3/library/stdtypes.html#printf-style-string-formatting)
#[derive(ViolationMetadata)]
@ -379,6 +391,16 @@ impl Violation for StringDotFormatInvalidFormat {
/// "Hello, {name}".format(name="World")
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe if the unused keyword argument
/// contains a function call with potential side effects,
/// because removing such arguments could change the behavior of the code.
///
/// For example, the fix would be marked as unsafe in the following case:
/// ```python
/// "Hello, {name}".format(greeting=print(1), name="World")
/// ```
///
/// ## References
/// - [Python documentation: `str.format`](https://docs.python.org/3/library/stdtypes.html#str.format)
#[derive(ViolationMetadata)]
@ -420,6 +442,16 @@ impl Violation for StringDotFormatExtraNamedArguments {
/// "Hello, {0}".format("world")
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe if the unused positional argument
/// contains a function call with potential side effects,
/// because removing such arguments could change the behavior of the code.
///
/// For example, the fix would be marked as unsafe in the following case:
/// ```python
/// "Hello, {0}".format("world", print(1))
/// ```
///
/// ## References
/// - [Python documentation: `str.format`](https://docs.python.org/3/library/stdtypes.html#str.format)
#[derive(ViolationMetadata)]
@ -603,15 +635,24 @@ pub(crate) fn percent_format_extra_named_arguments(
PercentFormatExtraNamedArguments { missing: names },
location,
);
let indexes: Vec<usize> = missing.iter().map(|(index, _)| *index).collect();
diagnostic.try_set_fix(|| {
let indexes: Vec<usize> = missing.iter().map(|(index, _)| *index).collect();
let edit = remove_unused_format_arguments_from_dict(
&indexes,
dict,
checker.locator(),
checker.stylist(),
)?;
Ok(Fix::safe_edit(edit))
Ok(Fix::applicable_edit(
edit,
// Mark fix as unsafe if `dict` contains a call with side effect
if contains_effect(right, |id| checker.semantic().has_builtin_binding(id)) {
Applicability::Unsafe
} else {
Applicability::Safe
},
))
});
}
@ -734,16 +775,19 @@ pub(crate) fn string_dot_format_extra_named_arguments(
return;
}
let keywords = keywords
let keyword_names = keywords
.iter()
.filter_map(|Keyword { arg, .. }| arg.as_ref());
.filter_map(|Keyword { arg, value, .. }| Some((arg.as_ref()?, value)));
let missing: Vec<(usize, &Name)> = keywords
let mut side_effects = false;
let missing: Vec<(usize, &Name)> = keyword_names
.enumerate()
.filter_map(|(index, keyword)| {
.filter_map(|(index, (keyword, value))| {
if summary.keywords.contains(keyword.id()) {
None
} else {
side_effects |=
contains_effect(value, |id| checker.semantic().has_builtin_binding(id));
Some((index, &keyword.id))
}
})
@ -758,7 +802,7 @@ pub(crate) fn string_dot_format_extra_named_arguments(
StringDotFormatExtraNamedArguments { missing: names },
call.range(),
);
let indexes: Vec<usize> = missing.iter().map(|(index, _)| *index).collect();
let indexes: Vec<usize> = missing.into_iter().map(|(index, _)| index).collect();
diagnostic.try_set_fix(|| {
let edit = remove_unused_keyword_arguments_from_format_call(
&indexes,
@ -766,7 +810,16 @@ pub(crate) fn string_dot_format_extra_named_arguments(
checker.locator(),
checker.stylist(),
)?;
Ok(Fix::safe_edit(edit))
Ok(Fix::applicable_edit(
edit,
// Mark fix as unsafe if the `format` call contains an argument with side effect
if side_effects {
Applicability::Unsafe
} else {
Applicability::Safe
},
))
});
}
@ -802,13 +855,17 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
true
}
let mut side_effects = false;
let missing: Vec<usize> = args
.iter()
.enumerate()
.filter(|(i, arg)| {
!(arg.is_starred_expr() || summary.autos.contains(i) || summary.indices.contains(i))
})
.map(|(i, _)| i)
.map(|(i, arg)| {
side_effects |= contains_effect(arg, |id| checker.semantic().has_builtin_binding(id));
i
})
.collect();
if missing.is_empty() {
@ -833,7 +890,15 @@ pub(crate) fn string_dot_format_extra_positional_arguments(
checker.locator(),
checker.stylist(),
)?;
Ok(Fix::safe_edit(edit))
Ok(Fix::applicable_edit(
edit,
// Mark fix as unsafe if the `format` call contains an argument with side effect
if side_effects {
Applicability::Unsafe
} else {
Applicability::Safe
},
))
});
}
}

19
crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F504_F504.py.snap
View file

@ -89,6 +89,8 @@ F504.py:14:1: F504 [*] `%`-format string has unused named argument(s): test1, te
15 | | 'test1': '', 'test2': '',
16 | | }
| |_^ F504
17 |
18 | # https://github.com/astral-sh/ruff/issues/18806
|
= help: Remove extra named arguments: test1, test2
@ -99,3 +101,20 @@ F504.py:14:1: F504 [*] `%`-format string has unused named argument(s): test1, te
15 |- 'test1': '', 16 |- 'test2': '',
15 |+
17 16 | }
18 17 |
19 18 | # https://github.com/astral-sh/ruff/issues/18806
F504.py:20:1: F504 [*] `%`-format string has unused named argument(s): greeting
|
19 | # https://github.com/astral-sh/ruff/issues/18806
20 | "Hello, %(name)s" % {"greeting": print(1), "name": "World"}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F504
|
= help: Remove extra named arguments: greeting
ℹ Unsafe fix
17 17 | }
18 18 |
19 19 | # https://github.com/astral-sh/ruff/issues/18806
20 |-"Hello, %(name)s" % {"greeting": print(1), "name": "World"}
20 |+"Hello, %(name)s" % {"name": "World"}

44
crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F522_F522.py.snap
View file

@ -1,6 +1,5 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
snapshot_kind: text
---
F522.py:1:1: F522 [*] `.format` call has unused named argument(s): bar
|
@ -55,6 +54,7 @@ F522.py:4:1: F522 [*] `.format` call has unused named argument(s): eggs, ham
4 |+"{bar:{spam}}".format(bar=2, spam=3, ) # F522
5 5 | (''
6 6 | .format(x=2)) # F522
7 7 |
F522.py:5:2: F522 [*] `.format` call has unused named argument(s): x
|
@ -64,6 +64,8 @@ F522.py:5:2: F522 [*] `.format` call has unused named argument(s): x
| __^
6 | | .format(x=2)) # F522
| |_____________^ F522
7 |
8 | # https://github.com/astral-sh/ruff/issues/18806
|
= help: Remove extra named arguments: x
@ -73,3 +75,43 @@ F522.py:5:2: F522 [*] `.format` call has unused named argument(s): x
5 5 | (''
6 |- .format(x=2)) # F522
6 |+ .format()) # F522
7 7 |
8 8 | # https://github.com/astral-sh/ruff/issues/18806
9 9 | # The fix here is unsafe because the unused argument has side effect
F522.py:10:1: F522 [*] `.format` call has unused named argument(s): greeting
|
8 | # https://github.com/astral-sh/ruff/issues/18806
9 | # The fix here is unsafe because the unused argument has side effect
10 | "Hello, {name}".format(greeting=print(1), name="World")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F522
11 |
12 | # The fix here is safe because the unused argument has no side effect,
|
= help: Remove extra named arguments: greeting
ℹ Unsafe fix
7 7 |
8 8 | # https://github.com/astral-sh/ruff/issues/18806
9 9 | # The fix here is unsafe because the unused argument has side effect
10 |-"Hello, {name}".format(greeting=print(1), name="World")
10 |+"Hello, {name}".format(name="World")
11 11 |
12 12 | # The fix here is safe because the unused argument has no side effect,
13 13 | # even though the used argument has a side effect
F522.py:14:1: F522 [*] `.format` call has unused named argument(s): greeting
|
12 | # The fix here is safe because the unused argument has no side effect,
13 | # even though the used argument has a side effect
14 | "Hello, {name}".format(greeting="Pikachu", name=print(1))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F522
|
= help: Remove extra named arguments: greeting
ℹ Safe fix
11 11 |
12 12 | # The fix here is safe because the unused argument has no side effect,
13 13 | # even though the used argument has a side effect
14 |-"Hello, {name}".format(greeting="Pikachu", name=print(1))
14 |+"Hello, {name}".format(name=print(1))

44
crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F523_F523.py.snap
View file

@ -286,6 +286,8 @@ F523.py:36:1: F523 [*] `.format` call has unused arguments at position(s): 0
36 |-"Hello".format("world")
36 |+"Hello"
37 37 | "Hello".format("world", key="value")
38 38 |
39 39 | # https://github.com/astral-sh/ruff/issues/18806
F523.py:37:1: F523 [*] `.format` call has unused arguments at position(s): 0
|
@ -293,6 +295,8 @@ F523.py:37:1: F523 [*] `.format` call has unused arguments at position(s): 0
36 | "Hello".format("world")
37 | "Hello".format("world", key="value")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F523
38 |
39 | # https://github.com/astral-sh/ruff/issues/18806
|
= help: Remove extra positional arguments at position(s): 0
@ -302,3 +306,43 @@ F523.py:37:1: F523 [*] `.format` call has unused arguments at position(s): 0
36 36 | "Hello".format("world")
37 |-"Hello".format("world", key="value")
37 |+"Hello".format(key="value")
38 38 |
39 39 | # https://github.com/astral-sh/ruff/issues/18806
40 40 | # The fix here is unsafe because the unused argument has side effect
F523.py:41:1: F523 [*] `.format` call has unused arguments at position(s): 1
|
39 | # https://github.com/astral-sh/ruff/issues/18806
40 | # The fix here is unsafe because the unused argument has side effect
41 | "Hello, {0}".format("world", print(1))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F523
42 |
43 | # The fix here is safe because the unused argument has no side effect,
|
= help: Remove extra positional arguments at position(s): 1
ℹ Unsafe fix
38 38 |
39 39 | # https://github.com/astral-sh/ruff/issues/18806
40 40 | # The fix here is unsafe because the unused argument has side effect
41 |-"Hello, {0}".format("world", print(1))
41 |+"Hello, {0}".format("world", )
42 42 |
43 43 | # The fix here is safe because the unused argument has no side effect,
44 44 | # even though the used argument has a side effect
F523.py:45:1: F523 [*] `.format` call has unused arguments at position(s): 1
|
43 | # The fix here is safe because the unused argument has no side effect,
44 | # even though the used argument has a side effect
45 | "Hello, {0}".format(print(1), "Pikachu")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F523
|
= help: Remove extra positional arguments at position(s): 1
ℹ Safe fix
42 42 |
43 43 | # The fix here is safe because the unused argument has no side effect,
44 44 | # even though the used argument has a side effect
45 |-"Hello, {0}".format(print(1), "Pikachu")
45 |+"Hello, {0}".format(print(1), )