Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[pylint] Implement shallow-copy-environ (W1507) #14241

Merged
merged 6 commits into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import copy
import os

copied_env = copy.copy(os.environ) # [shallow-copy-environ]
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
90 changes: 90 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/shallow_copy_environ.rs
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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]
2 changes: 2 additions & 0 deletions crates/ruff_python_semantic/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -1856,6 +1857,7 @@ bitflags! {
const CONTEXTVARS = 1 << 19;
const ANYIO = 1 << 20;
const FASTAPI = 1 << 21;
const COPY = 1 << 22;
}
}

Expand Down
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading