From b021ede481d8a2e693fa6e6adca8dbd70d57d303 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 7 Dec 2023 13:35:55 -0500 Subject: [PATCH] Allow `sys.path` modifications between imports (#9047) ## Summary It's common to interleave a `sys.path` modification between imports at the top of a file. This is a frequent cause of `# noqa: E402` false positives, as seen in the ecosystem checks. This PR modifies E402 to omit such modifications when determining the "import boundary". (We could consider linting against `sys.path` modifications, but that should be a separate rule.) Closes: https://github.com/astral-sh/ruff/issues/5557. --- .../test/fixtures/pycodestyle/E402.py | 17 ++++++--- crates/ruff_linter/src/checkers/ast/mod.rs | 10 +++-- .../ruff_linter/src/rules/pycodestyle/mod.rs | 1 + .../src/rules/pycodestyle/rules/imports.rs | 4 ++ ...les__pycodestyle__tests__E402_E402.py.snap | 34 +++++++++++------ ...destyle__tests__preview__E402_E402.py.snap | 28 ++++++++++++++ .../src/analyze/imports.rs | 37 +++++++++++++++++++ .../ruff_python_semantic/src/analyze/mod.rs | 1 + 8 files changed, 110 insertions(+), 22 deletions(-) create mode 100644 crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E402_E402.py.snap create mode 100644 crates/ruff_python_semantic/src/analyze/imports.rs diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E402.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E402.py index d5943407852a2..8910ff6007e5f 100644 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E402.py +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E402.py @@ -19,21 +19,26 @@ else: import e -__some__magic = 1 +import sys +sys.path.insert(0, "some/path") import f +__some__magic = 1 + +import g + def foo() -> None: - import e + import h if __name__ == "__main__": - import g + import i -import h; import i +import j; import k if __name__ == "__main__": - import j; \ -import k + import l; \ +import m diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 91b9a96deaa98..6c347af7d1844 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -49,7 +49,7 @@ use ruff_python_ast::{helpers, str, visitor, PySourceType}; use ruff_python_codegen::{Generator, Quote, Stylist}; use ruff_python_index::Indexer; use ruff_python_parser::typing::{parse_type_annotation, AnnotationKind}; -use ruff_python_semantic::analyze::{typing, visibility}; +use ruff_python_semantic::analyze::{imports, typing, visibility}; use ruff_python_semantic::{ BindingFlags, BindingId, BindingKind, Exceptions, Export, FromImport, Globals, Import, Module, ModuleKind, NodeId, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags, Snapshot, @@ -303,9 +303,11 @@ where } _ => { self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY; - if !self.semantic.seen_import_boundary() - && !helpers::is_assignment_to_a_dunder(stmt) - && !helpers::in_nested_block(self.semantic.current_statements()) + if !(self.semantic.seen_import_boundary() + || helpers::is_assignment_to_a_dunder(stmt) + || helpers::in_nested_block(self.semantic.current_statements()) + || self.settings.preview.is_enabled() + && imports::is_sys_path_modification(stmt, self.semantic())) { self.semantic.flags |= SemanticModelFlags::IMPORT_BOUNDARY; } diff --git a/crates/ruff_linter/src/rules/pycodestyle/mod.rs b/crates/ruff_linter/src/rules/pycodestyle/mod.rs index 3f6aa412bf179..008118ceeb48d 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/mod.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/mod.rs @@ -65,6 +65,7 @@ mod tests { } #[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))] + #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.py"))] #[test_case(Rule::TypeComparison, Path::new("E721.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/imports.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/imports.rs index 443e044498b75..b85dc9eba8fb5 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/imports.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/imports.rs @@ -41,6 +41,10 @@ impl Violation for MultipleImportsOnOneLine { /// According to [PEP 8], "imports are always put at the top of the file, just after any /// module comments and docstrings, and before module globals and constants." /// +/// In [preview], this rule makes an exception for `sys.path` modifications, +/// allowing for `sys.path.insert`, `sys.path.append`, and similar +/// modifications between import statements. +/// /// ## Example /// ```python /// "One string" diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E402_E402.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E402_E402.py.snap index 8c15438eb55af..44d7f5870ac56 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E402_E402.py.snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E402_E402.py.snap @@ -1,27 +1,37 @@ --- source: crates/ruff_linter/src/rules/pycodestyle/mod.rs --- -E402.py:24:1: E402 Module level import not at top of file +E402.py:25:1: E402 Module level import not at top of file | -22 | __some__magic = 1 -23 | -24 | import f +23 | sys.path.insert(0, "some/path") +24 | +25 | import f | ^^^^^^^^ E402 +26 | +27 | __some__magic = 1 | -E402.py:34:1: E402 Module level import not at top of file +E402.py:29:1: E402 Module level import not at top of file | -32 | import g -33 | -34 | import h; import i +27 | __some__magic = 1 +28 | +29 | import g | ^^^^^^^^ E402 | -E402.py:34:11: E402 Module level import not at top of file +E402.py:39:1: E402 Module level import not at top of file | -32 | import g -33 | -34 | import h; import i +37 | import i +38 | +39 | import j; import k + | ^^^^^^^^ E402 + | + +E402.py:39:11: E402 Module level import not at top of file + | +37 | import i +38 | +39 | import j; import k | ^^^^^^^^ E402 | diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E402_E402.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E402_E402.py.snap new file mode 100644 index 0000000000000..cf3a61872dd85 --- /dev/null +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__preview__E402_E402.py.snap @@ -0,0 +1,28 @@ +--- +source: crates/ruff_linter/src/rules/pycodestyle/mod.rs +--- +E402.py:29:1: E402 Module level import not at top of file + | +27 | __some__magic = 1 +28 | +29 | import g + | ^^^^^^^^ E402 + | + +E402.py:39:1: E402 Module level import not at top of file + | +37 | import i +38 | +39 | import j; import k + | ^^^^^^^^ E402 + | + +E402.py:39:11: E402 Module level import not at top of file + | +37 | import i +38 | +39 | import j; import k + | ^^^^^^^^ E402 + | + + diff --git a/crates/ruff_python_semantic/src/analyze/imports.rs b/crates/ruff_python_semantic/src/analyze/imports.rs new file mode 100644 index 0000000000000..cc9d219775b2f --- /dev/null +++ b/crates/ruff_python_semantic/src/analyze/imports.rs @@ -0,0 +1,37 @@ +use ruff_python_ast::{self as ast, Expr, Stmt}; + +use crate::SemanticModel; + +/// Returns `true` if a [`Stmt`] is a `sys.path` modification, as in: +/// ```python +/// import sys +/// +/// sys.path.append("../") +/// ``` +pub fn is_sys_path_modification(stmt: &Stmt, semantic: &SemanticModel) -> bool { + let Stmt::Expr(ast::StmtExpr { value, range: _ }) = stmt else { + return false; + }; + let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() else { + return false; + }; + semantic + .resolve_call_path(func.as_ref()) + .is_some_and(|call_path| { + matches!( + call_path.as_slice(), + [ + "sys", + "path", + "append" + | "insert" + | "extend" + | "remove" + | "pop" + | "clear" + | "reverse" + | "sort" + ] + ) + }) +} diff --git a/crates/ruff_python_semantic/src/analyze/mod.rs b/crates/ruff_python_semantic/src/analyze/mod.rs index 941309a526c26..f0fb3844e30c6 100644 --- a/crates/ruff_python_semantic/src/analyze/mod.rs +++ b/crates/ruff_python_semantic/src/analyze/mod.rs @@ -1,4 +1,5 @@ pub mod function_type; +pub mod imports; pub mod logging; pub mod type_inference; pub mod typing;