Skip to content

Commit

Permalink
[refurb] Make list-reverse-copy an unsafe fix (#12303)
Browse files Browse the repository at this point in the history
## Summary

I don't know that there's more to do here. We could consider not raising
the violation at all for arguments, but that would have some false
negatives and could also be surprising to users.

Closes #12267.
  • Loading branch information
charliermarsh authored Jul 13, 2024
1 parent 456d6a2 commit 6584886
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
10 changes: 9 additions & 1 deletion crates/ruff_linter/src/rules/refurb/rules/list_reverse_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ use crate::checkers::ast::Checker;
/// l.reverse()
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe, as calling `.reverse()` on a list
/// will mutate the list in-place, unlike `reversed`, which creates a new list
/// and leaves the original list unchanged.
///
/// If the list is referenced elsewhere, this could lead to unexpected
/// behavior.
///
/// ## References
/// - [Python documentation: More on Lists](https://docs.python.org/3/tutorial/datastructures.html#more-on-lists)
#[violation]
Expand Down Expand Up @@ -88,7 +96,7 @@ pub(crate) fn list_assign_reversed(checker: &mut Checker, assign: &StmtAssign) {
},
assign.range(),
)
.with_fix(Fix::safe_edit(Edit::range_replacement(
.with_fix(Fix::unsafe_edit(Edit::range_replacement(
format!("{}.reverse()", target_expr.id),
assign.range(),
))),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ FURB187.py:6:5: FURB187 [*] Use of assignment of `reversed` on list `l`
|
= help: Replace with `l.reverse()`

Safe fix
Unsafe fix
3 3 |
4 4 | def a():
5 5 | l = []
Expand All @@ -29,7 +29,7 @@ FURB187.py:11:5: FURB187 [*] Use of assignment of `reversed` on list `l`
|
= help: Replace with `l.reverse()`

Safe fix
Unsafe fix
8 8 |
9 9 | def b():
10 10 | l = []
Expand All @@ -48,7 +48,7 @@ FURB187.py:16:5: FURB187 [*] Use of assignment of `reversed` on list `l`
|
= help: Replace with `l.reverse()`

Safe fix
Unsafe fix
13 13 |
14 14 | def c():
15 15 | l = []
Expand Down

0 comments on commit 6584886

Please sign in to comment.