Skip to content

Commit

Permalink
Use top-level semantic detection for E402 (#6526)
Browse files Browse the repository at this point in the history
## Summary

Noticed in #6378. Given `import h;
import i`, we don't consider `import i` to be a "top-level" import for
E402 purposes, which is wrong. Similarly, we _do_ consider `import k` to
be a "top-level" import in:

```python
if __name__ == "__main__":
    import j; \
import k
```

Using the semantic detection, rather than relying on newline position,
fixes both cases.

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh authored Aug 12, 2023
1 parent c03e2ac commit b49c80f
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 19 deletions.
7 changes: 7 additions & 0 deletions crates/ruff/resources/test/fixtures/pycodestyle/E402.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,10 @@ def foo() -> None:

if __name__ == "__main__":
import g

import h; import i


if __name__ == "__main__":
import j; \
import k
12 changes: 2 additions & 10 deletions crates/ruff/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,11 +520,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pycodestyle::rules::multiple_imports_on_one_line(checker, stmt, names);
}
if checker.enabled(Rule::ModuleImportNotAtTopOfFile) {
pycodestyle::rules::module_import_not_at_top_of_file(
checker,
stmt,
checker.locator,
);
pycodestyle::rules::module_import_not_at_top_of_file(checker, stmt);
}
if checker.enabled(Rule::GlobalStatement) {
for name in names {
Expand Down Expand Up @@ -689,11 +685,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
let module = module.as_deref();
let level = level.map(|level| level.to_u32());
if checker.enabled(Rule::ModuleImportNotAtTopOfFile) {
pycodestyle::rules::module_import_not_at_top_of_file(
checker,
stmt,
checker.locator,
);
pycodestyle::rules::module_import_not_at_top_of_file(checker, stmt);
}
if checker.enabled(Rule::GlobalStatement) {
for name in names {
Expand Down
12 changes: 3 additions & 9 deletions crates/ruff/src/rules/pycodestyle/rules/imports.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use ruff_python_ast::{Alias, Ranged, Stmt};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_source_file::Locator;
use ruff_python_ast::{Alias, Ranged, Stmt};

use crate::checkers::ast::Checker;

Expand Down Expand Up @@ -81,12 +79,8 @@ pub(crate) fn multiple_imports_on_one_line(checker: &mut Checker, stmt: &Stmt, n
}

/// E402
pub(crate) fn module_import_not_at_top_of_file(
checker: &mut Checker,
stmt: &Stmt,
locator: &Locator,
) {
if checker.semantic().seen_import_boundary() && locator.is_at_start_of_line(stmt.start()) {
pub(crate) fn module_import_not_at_top_of_file(checker: &mut Checker, stmt: &Stmt) {
if checker.semantic().seen_import_boundary() && checker.semantic().at_top_level() {
checker
.diagnostics
.push(Diagnostic::new(ModuleImportNotAtTopOfFile, stmt.range()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,20 @@ E402.py:24:1: E402 Module level import not at top of file
| ^^^^^^^^ E402
|

E402.py:34:1: E402 Module level import not at top of file
|
32 | import g
33 |
34 | import h; import i
| ^^^^^^^^ E402
|

E402.py:34:11: E402 Module level import not at top of file
|
32 | import g
33 |
34 | import h; import i
| ^^^^^^^^ E402
|


0 comments on commit b49c80f

Please sign in to comment.