diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/shallow_copy_environ.py b/crates/ruff_linter/resources/test/fixtures/pylint/shallow_copy_environ.py index be6c0203bc..5048cd68a9 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/shallow_copy_environ.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/shallow_copy_environ.py @@ -2,3 +2,11 @@ import copy import os copied_env = copy.copy(os.environ) # [shallow-copy-environ] + + +# Test case where the proposed fix is wrong, i.e., unsafe fix +# Ref: https://github.com/astral-sh/ruff/issues/16274#event-16423475135 + +os.environ["X"] = "0" +env = copy.copy(os.environ) +os.environ["X"] = "1" diff --git a/crates/ruff_linter/src/rules/pylint/rules/shallow_copy_environ.rs b/crates/ruff_linter/src/rules/pylint/rules/shallow_copy_environ.rs index 7837c10478..af2bb08651 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/shallow_copy_environ.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/shallow_copy_environ.rs @@ -30,6 +30,13 @@ use crate::checkers::ast::Checker; /// env = os.environ.copy() /// ``` /// +/// ## Fix safety +/// +/// This rule's fix is marked as unsafe because replacing a shallow copy with a deep copy can lead +/// to unintended side effects. If the program modifies the shallow copy at some point, changing it +/// to a deep copy may prevent those modifications from affecting the original data, potentially +/// altering the program's behavior. +/// /// ## References /// - [Python documentation: `copy` — Shallow and deep copy operations](https://docs.python.org/3/library/copy.html) /// - [Python documentation: `os.environ`](https://docs.python.org/3/library/os.html#os.environ) @@ -82,7 +89,7 @@ pub(crate) fn shallow_copy_environ(checker: &Checker, call: &ast::ExprCall) { } let mut diagnostic = Diagnostic::new(ShallowCopyEnviron, call.range()); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( format!("{}.copy()", checker.locator().slice(arg)), call.range(), ))); diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1507_shallow_copy_environ.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1507_shallow_copy_environ.py.snap index c59f03fdf2..45472379ba 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1507_shallow_copy_environ.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1507_shallow_copy_environ.py.snap @@ -10,9 +10,29 @@ shallow_copy_environ.py:4:14: PLW1507 [*] Shallow copy of `os.environ` via `copy | = help: Replace with `os.environ.copy()` -ℹ Safe fix +ℹ Unsafe fix 1 1 | import copy 2 2 | import os 3 3 | 4 |-copied_env = copy.copy(os.environ) # [shallow-copy-environ] 4 |+copied_env = os.environ.copy() # [shallow-copy-environ] +5 5 | +6 6 | +7 7 | # Test case where the proposed fix is wrong, i.e., unsafe fix + +shallow_copy_environ.py:11:7: PLW1507 [*] Shallow copy of `os.environ` via `copy.copy(os.environ)` + | +10 | os.environ["X"] = "0" +11 | env = copy.copy(os.environ) + | ^^^^^^^^^^^^^^^^^^^^^ PLW1507 +12 | os.environ["X"] = "1" + | + = help: Replace with `os.environ.copy()` + +ℹ Unsafe fix +8 8 | # Ref: https://github.com/astral-sh/ruff/issues/16274#event-16423475135 +9 9 | +10 10 | os.environ["X"] = "0" +11 |-env = copy.copy(os.environ) + 11 |+env = os.environ.copy() +12 12 | os.environ["X"] = "1"