From 65848869d58aeeb12e1c77c4fc73ad0b4b941368 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 13 Jul 2024 15:45:35 -0400 Subject: [PATCH] [`refurb`] Make `list-reverse-copy` an unsafe fix (#12303) ## 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 https://github.com/astral-sh/ruff/issues/12267. --- .../src/rules/refurb/rules/list_reverse_copy.rs | 10 +++++++++- ...nter__rules__refurb__tests__FURB187_FURB187.py.snap | 6 +++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/refurb/rules/list_reverse_copy.rs b/crates/ruff_linter/src/rules/refurb/rules/list_reverse_copy.rs index 2f1d5ab53388f..0f1346fce9ecd 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/list_reverse_copy.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/list_reverse_copy.rs @@ -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] @@ -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(), ))), diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB187_FURB187.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB187_FURB187.py.snap index 43d0f5a1657d0..e4d08fd9eefbc 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB187_FURB187.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB187_FURB187.py.snap @@ -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 = [] @@ -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 = [] @@ -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 = []