Skip to content

Commit aa59a37

Browse files
charliermarshnkxxll
authored andcommitted
[pycodestyle] Allow os.environ modifications between imports (E402) (astral-sh#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 astral-sh#10059
1 parent 0f7b6df commit aa59a37

File tree

7 files changed

+77
-1
lines changed

7 files changed

+77
-1
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import os
2+
3+
os.environ["WORLD_SIZE"] = "1"
4+
os.putenv("CUDA_VISIBLE_DEVICES", "4")
5+
del os.environ["WORLD_SIZE"]
6+
7+
import torch

crates/ruff_linter/src/checkers/ast/mod.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,9 @@ where
374374
|| helpers::is_assignment_to_a_dunder(stmt)
375375
|| helpers::in_nested_block(self.semantic.current_statements())
376376
|| imports::is_matplotlib_activation(stmt, self.semantic())
377-
|| imports::is_sys_path_modification(stmt, self.semantic()))
377+
|| imports::is_sys_path_modification(stmt, self.semantic())
378+
|| (self.settings.preview.is_enabled()
379+
&& imports::is_os_environ_modification(stmt, self.semantic())))
378380
{
379381
self.semantic.flags |= SemanticModelFlags::IMPORT_BOUNDARY;
380382
}

crates/ruff_linter/src/rules/pycodestyle/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ mod tests {
3838
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E40.py"))]
3939
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_0.py"))]
4040
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_1.py"))]
41+
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_2.py"))]
4142
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.ipynb"))]
4243
#[test_case(Rule::MultipleImportsOnOneLine, Path::new("E40.py"))]
4344
#[test_case(Rule::MultipleStatementsOnOneLineColon, Path::new("E70.py"))]
@@ -69,6 +70,7 @@ mod tests {
6970

7071
#[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))]
7172
#[test_case(Rule::TypeComparison, Path::new("E721.py"))]
73+
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_2.py"))]
7274
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
7375
let snapshot = format!(
7476
"preview__{}_{}",

crates/ruff_linter/src/rules/pycodestyle/rules/module_import_not_at_top_of_file.rs

+4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ use crate::checkers::ast::Checker;
1717
/// `sys.path.insert`, `sys.path.append`, and similar modifications between import
1818
/// statements.
1919
///
20+
/// In [preview], this rule also allows `os.environ` modifications between import
21+
/// statements.
22+
///
2023
/// ## Example
2124
/// ```python
2225
/// "One string"
@@ -37,6 +40,7 @@ use crate::checkers::ast::Checker;
3740
/// ```
3841
///
3942
/// [PEP 8]: https://peps.python.org/pep-0008/#imports
43+
/// [preview]: https://docs.astral.sh/ruff/preview/
4044
#[violation]
4145
pub struct ModuleImportNotAtTopOfFile {
4246
source_type: PySourceType,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
3+
---
4+
E402_2.py:7:1: E402 Module level import not at top of file
5+
|
6+
5 | del os.environ["WORLD_SIZE"]
7+
6 |
8+
7 | import torch
9+
| ^^^^^^^^^^^^ E402
10+
|
11+
12+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
3+
---
4+

crates/ruff_python_semantic/src/analyze/imports.rs

+45
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use ruff_python_ast::helpers::map_subscript;
12
use ruff_python_ast::{self as ast, Expr, Stmt};
23

34
use crate::SemanticModel;
@@ -36,6 +37,50 @@ pub fn is_sys_path_modification(stmt: &Stmt, semantic: &SemanticModel) -> bool {
3637
})
3738
}
3839

40+
/// Returns `true` if a [`Stmt`] is an `os.environ` modification, as in:
41+
/// ```python
42+
/// import os
43+
///
44+
/// os.environ["CUDA_VISIBLE_DEVICES"] = "4"
45+
/// ```
46+
pub fn is_os_environ_modification(stmt: &Stmt, semantic: &SemanticModel) -> bool {
47+
match stmt {
48+
Stmt::Expr(ast::StmtExpr { value, .. }) => match value.as_ref() {
49+
Expr::Call(ast::ExprCall { func, .. }) => semantic
50+
.resolve_call_path(func.as_ref())
51+
.is_some_and(|call_path| {
52+
matches!(
53+
call_path.as_slice(),
54+
["os", "putenv" | "unsetenv"]
55+
| [
56+
"os",
57+
"environ",
58+
"update" | "pop" | "clear" | "setdefault" | "popitem"
59+
]
60+
)
61+
}),
62+
_ => false,
63+
},
64+
Stmt::Delete(ast::StmtDelete { targets, .. }) => targets.iter().any(|target| {
65+
semantic
66+
.resolve_call_path(map_subscript(target))
67+
.is_some_and(|call_path| matches!(call_path.as_slice(), ["os", "environ"]))
68+
}),
69+
Stmt::Assign(ast::StmtAssign { targets, .. }) => targets.iter().any(|target| {
70+
semantic
71+
.resolve_call_path(map_subscript(target))
72+
.is_some_and(|call_path| matches!(call_path.as_slice(), ["os", "environ"]))
73+
}),
74+
Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => semantic
75+
.resolve_call_path(map_subscript(target))
76+
.is_some_and(|call_path| matches!(call_path.as_slice(), ["os", "environ"])),
77+
Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => semantic
78+
.resolve_call_path(map_subscript(target))
79+
.is_some_and(|call_path| matches!(call_path.as_slice(), ["os", "environ"])),
80+
_ => false,
81+
}
82+
}
83+
3984
/// Returns `true` if a [`Stmt`] is a `matplotlib.use` activation, as in:
4085
/// ```python
4186
/// import matplotlib

0 commit comments

Comments
 (0)