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 new file mode 100644 index 0000000000000..be6c0203bcaf2 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/shallow_copy_environ.py @@ -0,0 +1,4 @@ +import copy +import os + +copied_env = copy.copy(os.environ) # [shallow-copy-environ] diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 6cdc7725c62e7..42d50c40e8d3b 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -817,6 +817,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::BadStrStripCall) { pylint::rules::bad_str_strip_call(checker, func, args); } + if checker.enabled(Rule::ShallowCopyEnviron) { + pylint::rules::shallow_copy_environ(checker, call); + } if checker.enabled(Rule::InvalidEnvvarDefault) { pylint::rules::invalid_envvar_default(checker, call); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index be27d11fc6b1d..8e1e90fd34514 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -283,6 +283,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "W0642") => (RuleGroup::Stable, rules::pylint::rules::SelfOrClsAssignment), (Pylint, "W0711") => (RuleGroup::Stable, rules::pylint::rules::BinaryOpException), (Pylint, "W1501") => (RuleGroup::Stable, rules::pylint::rules::BadOpenMode), + (Pylint, "W1507") => (RuleGroup::Preview, rules::pylint::rules::ShallowCopyEnviron), (Pylint, "W1508") => (RuleGroup::Stable, rules::pylint::rules::InvalidEnvvarDefault), (Pylint, "W1509") => (RuleGroup::Stable, rules::pylint::rules::SubprocessPopenPreexecFn), (Pylint, "W1510") => (RuleGroup::Stable, rules::pylint::rules::SubprocessRunWithoutCheck), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 09392c125d243..1ef1e7003f3e9 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -105,6 +105,7 @@ mod tests { Rule::InvalidCharacterBackspace, Path::new("invalid_characters_syntax_error.py") )] + #[test_case(Rule::ShallowCopyEnviron, Path::new("shallow_copy_environ.py"))] #[test_case(Rule::InvalidEnvvarDefault, Path::new("invalid_envvar_default.py"))] #[test_case(Rule::InvalidEnvvarValue, Path::new("invalid_envvar_value.py"))] #[test_case(Rule::IterationOverSet, Path::new("iteration_over_set.py"))] diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 753c7d9a439d9..08e6e868d0b2a 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -68,6 +68,7 @@ pub(crate) use repeated_keyword_argument::*; pub(crate) use return_in_init::*; pub(crate) use self_assigning_variable::*; pub(crate) use self_or_cls_assignment::*; +pub(crate) use shallow_copy_environ::*; pub(crate) use single_string_slots::*; pub(crate) use singledispatch_method::*; pub(crate) use singledispatchmethod_function::*; @@ -172,6 +173,7 @@ mod repeated_keyword_argument; mod return_in_init; mod self_assigning_variable; mod self_or_cls_assignment; +mod shallow_copy_environ; mod single_string_slots; mod singledispatch_method; mod singledispatchmethod_function; 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 new file mode 100644 index 0000000000000..84649db53e463 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/shallow_copy_environ.rs @@ -0,0 +1,90 @@ +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast}; +use ruff_python_semantic::Modules; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Check for shallow `os.environ` copies. +/// +/// ## Why is this bad? +/// `os.environ` is not a `dict` object, but rather, a proxy object. As such, mutating a shallow +/// copy of `os.environ` will also mutate the original object. +/// +/// See: [#15373] for more information. +/// +/// ## Example +/// ```python +/// import copy +/// import os +/// +/// env = copy.copy(os.environ) +/// ``` +/// +/// Use instead: +/// ```python +/// import os +/// +/// env = os.environ.copy() +/// ``` +/// +/// ## 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) +/// +/// [#15373]: https://bugs.python.org/issue15373 +#[violation] +pub struct ShallowCopyEnviron; + +impl AlwaysFixableViolation for ShallowCopyEnviron { + #[derive_message_formats] + fn message(&self) -> String { + "Shallow copy of `os.environ` via `copy.copy(os.environ)`".to_string() + } + + fn fix_title(&self) -> String { + "Replace with `os.environ.copy()`".to_string() + } +} + +/// PLW1507 +pub(crate) fn shallow_copy_environ(checker: &mut Checker, call: &ast::ExprCall) { + if !(checker.semantic().seen_module(Modules::OS) + && checker.semantic().seen_module(Modules::COPY)) + { + return; + } + + if !checker + .semantic() + .resolve_qualified_name(&call.func) + .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["copy", "copy"])) + { + return; + } + + if !call.arguments.keywords.is_empty() { + return; + } + + let [arg] = call.arguments.args.as_ref() else { + return; + }; + + if !checker + .semantic() + .resolve_qualified_name(arg) + .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["os", "environ"])) + { + return; + } + + let mut diagnostic = Diagnostic::new(ShallowCopyEnviron, call.range()); + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + format!("{}.copy()", checker.locator().slice(arg)), + call.range(), + ))); + checker.diagnostics.push(diagnostic); +} 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 new file mode 100644 index 0000000000000..40729240c30af --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1507_shallow_copy_environ.py.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +shallow_copy_environ.py:4:14: PLW1507 [*] Shallow copy of `os.environ` via `copy.copy(os.environ)` + | +2 | import os +3 | +4 | copied_env = copy.copy(os.environ) # [shallow-copy-environ] + | ^^^^^^^^^^^^^^^^^^^^^ PLW1507 + | + = help: Replace with `os.environ.copy()` + +ℹ Safe 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] diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 7c7967e80e36c..7e9fa5c73770c 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1266,6 +1266,7 @@ impl<'a> SemanticModel<'a> { "anyio" => self.seen.insert(Modules::ANYIO), "builtins" => self.seen.insert(Modules::BUILTINS), "collections" => self.seen.insert(Modules::COLLECTIONS), + "copy" => self.seen.insert(Modules::COPY), "contextvars" => self.seen.insert(Modules::CONTEXTVARS), "dataclasses" => self.seen.insert(Modules::DATACLASSES), "datetime" => self.seen.insert(Modules::DATETIME), @@ -1856,6 +1857,7 @@ bitflags! { const CONTEXTVARS = 1 << 19; const ANYIO = 1 << 20; const FASTAPI = 1 << 21; + const COPY = 1 << 22; } } diff --git a/ruff.schema.json b/ruff.schema.json index b1e24b4776030..2ae457b9acc11 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3601,6 +3601,7 @@ "PLW15", "PLW150", "PLW1501", + "PLW1507", "PLW1508", "PLW1509", "PLW151",