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.
This commit is contained in:
Charlie Marsh 2023-12-07 13:35:55 -05:00 committed by GitHub
parent 96ae9fe685
commit b021ede481
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 110 additions and 22 deletions

View file

@ -19,21 +19,26 @@ if x > 0:
else:
import e
__some__magic = 1
import sys
sys.path.insert(0, "some/path")
import f
__some__magic = 1
def foo() -> None:
import e
if __name__ == "__main__":
import g
import h; import i
def foo() -> None:
import h
if __name__ == "__main__":
import j; \
import k
import i
import j; import k
if __name__ == "__main__":
import l; \
import m

View file

@ -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;
}

View file

@ -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!(

View file

@ -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"

View file

@ -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:29:1: E402 Module level import not at top of file
|
27 | __some__magic = 1
28 |
29 | import g
| ^^^^^^^^ E402
|
E402.py:34:1: 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:34:11: E402 Module level import not at top of file
E402.py:39:11: 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
|

View file

@ -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
|

View file

@ -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"
]
)
})
}

View file

@ -1,4 +1,5 @@
pub mod function_type;
pub mod imports;
pub mod logging;
pub mod type_inference;
pub mod typing;