mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-26 20:09:22 +00:00
[pycodestyle
] Allow os.environ
modifications between imports (E402
) (#10066)
## Summary Allows, e.g.: ```python import os os.environ["WORLD_SIZE"] = "1" os.putenv("CUDA_VISIBLE_DEVICES", "4") import torch ``` For now, this is only allowed in preview. Closes https://github.com/astral-sh/ruff/issues/10059
This commit is contained in:
parent
7eafba2a4d
commit
4997c681f1
7 changed files with 77 additions and 1 deletions
7
crates/ruff_linter/resources/test/fixtures/pycodestyle/E402_2.py
vendored
Normal file
7
crates/ruff_linter/resources/test/fixtures/pycodestyle/E402_2.py
vendored
Normal file
|
@ -0,0 +1,7 @@
|
||||||
|
import os
|
||||||
|
|
||||||
|
os.environ["WORLD_SIZE"] = "1"
|
||||||
|
os.putenv("CUDA_VISIBLE_DEVICES", "4")
|
||||||
|
del os.environ["WORLD_SIZE"]
|
||||||
|
|
||||||
|
import torch
|
|
@ -374,7 +374,9 @@ where
|
||||||
|| helpers::is_assignment_to_a_dunder(stmt)
|
|| helpers::is_assignment_to_a_dunder(stmt)
|
||||||
|| helpers::in_nested_block(self.semantic.current_statements())
|
|| helpers::in_nested_block(self.semantic.current_statements())
|
||||||
|| imports::is_matplotlib_activation(stmt, self.semantic())
|
|| imports::is_matplotlib_activation(stmt, self.semantic())
|
||||||
|| imports::is_sys_path_modification(stmt, self.semantic()))
|
|| imports::is_sys_path_modification(stmt, self.semantic())
|
||||||
|
|| (self.settings.preview.is_enabled()
|
||||||
|
&& imports::is_os_environ_modification(stmt, self.semantic())))
|
||||||
{
|
{
|
||||||
self.semantic.flags |= SemanticModelFlags::IMPORT_BOUNDARY;
|
self.semantic.flags |= SemanticModelFlags::IMPORT_BOUNDARY;
|
||||||
}
|
}
|
||||||
|
|
|
@ -38,6 +38,7 @@ mod tests {
|
||||||
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E40.py"))]
|
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E40.py"))]
|
||||||
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_0.py"))]
|
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_0.py"))]
|
||||||
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_1.py"))]
|
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_1.py"))]
|
||||||
|
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_2.py"))]
|
||||||
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.ipynb"))]
|
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.ipynb"))]
|
||||||
#[test_case(Rule::MultipleImportsOnOneLine, Path::new("E40.py"))]
|
#[test_case(Rule::MultipleImportsOnOneLine, Path::new("E40.py"))]
|
||||||
#[test_case(Rule::MultipleStatementsOnOneLineColon, Path::new("E70.py"))]
|
#[test_case(Rule::MultipleStatementsOnOneLineColon, Path::new("E70.py"))]
|
||||||
|
@ -69,6 +70,7 @@ mod tests {
|
||||||
|
|
||||||
#[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))]
|
#[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))]
|
||||||
#[test_case(Rule::TypeComparison, Path::new("E721.py"))]
|
#[test_case(Rule::TypeComparison, Path::new("E721.py"))]
|
||||||
|
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_2.py"))]
|
||||||
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
|
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!(
|
let snapshot = format!(
|
||||||
"preview__{}_{}",
|
"preview__{}_{}",
|
||||||
|
|
|
@ -17,6 +17,9 @@ use crate::checkers::ast::Checker;
|
||||||
/// `sys.path.insert`, `sys.path.append`, and similar modifications between import
|
/// `sys.path.insert`, `sys.path.append`, and similar modifications between import
|
||||||
/// statements.
|
/// statements.
|
||||||
///
|
///
|
||||||
|
/// In [preview], this rule also allows `os.environ` modifications between import
|
||||||
|
/// statements.
|
||||||
|
///
|
||||||
/// ## Example
|
/// ## Example
|
||||||
/// ```python
|
/// ```python
|
||||||
/// "One string"
|
/// "One string"
|
||||||
|
@ -37,6 +40,7 @@ use crate::checkers::ast::Checker;
|
||||||
/// ```
|
/// ```
|
||||||
///
|
///
|
||||||
/// [PEP 8]: https://peps.python.org/pep-0008/#imports
|
/// [PEP 8]: https://peps.python.org/pep-0008/#imports
|
||||||
|
/// [preview]: https://docs.astral.sh/ruff/preview/
|
||||||
#[violation]
|
#[violation]
|
||||||
pub struct ModuleImportNotAtTopOfFile {
|
pub struct ModuleImportNotAtTopOfFile {
|
||||||
source_type: PySourceType,
|
source_type: PySourceType,
|
||||||
|
|
|
@ -0,0 +1,12 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
|
||||||
|
---
|
||||||
|
E402_2.py:7:1: E402 Module level import not at top of file
|
||||||
|
|
|
||||||
|
5 | del os.environ["WORLD_SIZE"]
|
||||||
|
6 |
|
||||||
|
7 | import torch
|
||||||
|
| ^^^^^^^^^^^^ E402
|
||||||
|
|
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
|
||||||
|
---
|
||||||
|
|
|
@ -1,3 +1,4 @@
|
||||||
|
use ruff_python_ast::helpers::map_subscript;
|
||||||
use ruff_python_ast::{self as ast, Expr, Stmt};
|
use ruff_python_ast::{self as ast, Expr, Stmt};
|
||||||
|
|
||||||
use crate::SemanticModel;
|
use crate::SemanticModel;
|
||||||
|
@ -36,6 +37,50 @@ pub fn is_sys_path_modification(stmt: &Stmt, semantic: &SemanticModel) -> bool {
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if a [`Stmt`] is an `os.environ` modification, as in:
|
||||||
|
/// ```python
|
||||||
|
/// import os
|
||||||
|
///
|
||||||
|
/// os.environ["CUDA_VISIBLE_DEVICES"] = "4"
|
||||||
|
/// ```
|
||||||
|
pub fn is_os_environ_modification(stmt: &Stmt, semantic: &SemanticModel) -> bool {
|
||||||
|
match stmt {
|
||||||
|
Stmt::Expr(ast::StmtExpr { value, .. }) => match value.as_ref() {
|
||||||
|
Expr::Call(ast::ExprCall { func, .. }) => semantic
|
||||||
|
.resolve_call_path(func.as_ref())
|
||||||
|
.is_some_and(|call_path| {
|
||||||
|
matches!(
|
||||||
|
call_path.as_slice(),
|
||||||
|
["os", "putenv" | "unsetenv"]
|
||||||
|
| [
|
||||||
|
"os",
|
||||||
|
"environ",
|
||||||
|
"update" | "pop" | "clear" | "setdefault" | "popitem"
|
||||||
|
]
|
||||||
|
)
|
||||||
|
}),
|
||||||
|
_ => false,
|
||||||
|
},
|
||||||
|
Stmt::Delete(ast::StmtDelete { targets, .. }) => targets.iter().any(|target| {
|
||||||
|
semantic
|
||||||
|
.resolve_call_path(map_subscript(target))
|
||||||
|
.is_some_and(|call_path| matches!(call_path.as_slice(), ["os", "environ"]))
|
||||||
|
}),
|
||||||
|
Stmt::Assign(ast::StmtAssign { targets, .. }) => targets.iter().any(|target| {
|
||||||
|
semantic
|
||||||
|
.resolve_call_path(map_subscript(target))
|
||||||
|
.is_some_and(|call_path| matches!(call_path.as_slice(), ["os", "environ"]))
|
||||||
|
}),
|
||||||
|
Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => semantic
|
||||||
|
.resolve_call_path(map_subscript(target))
|
||||||
|
.is_some_and(|call_path| matches!(call_path.as_slice(), ["os", "environ"])),
|
||||||
|
Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => semantic
|
||||||
|
.resolve_call_path(map_subscript(target))
|
||||||
|
.is_some_and(|call_path| matches!(call_path.as_slice(), ["os", "environ"])),
|
||||||
|
_ => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Returns `true` if a [`Stmt`] is a `matplotlib.use` activation, as in:
|
/// Returns `true` if a [`Stmt`] is a `matplotlib.use` activation, as in:
|
||||||
/// ```python
|
/// ```python
|
||||||
/// import matplotlib
|
/// import matplotlib
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue