Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement pylint import-outside-toplevel rule (C0415) #5180

Merged
merged 1 commit into from
Oct 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from typing import TYPE_CHECKING

# Verify that statements nested in conditionals (such as top-level type-checking blocks)
# are still considered top-level
if TYPE_CHECKING:
import string


def import_in_function():
import symtable # [import-outside-toplevel]
import os, sys # [import-outside-toplevel]
import time as thyme # [import-outside-toplevel]
import random as rand, socket as sock # [import-outside-toplevel]
from collections import defaultdict # [import-outside-toplevel]
from math import sin as sign, cos as cosplay # [import-outside-toplevel]


class ClassWithImports:
import tokenize # [import-outside-toplevel]

def __init__(self):
import trace # [import-outside-toplevel]
6 changes: 6 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::ModuleImportNotAtTopOfFile) {
pycodestyle::rules::module_import_not_at_top_of_file(checker, stmt);
}
if checker.enabled(Rule::ImportOutsideTopLevel) {
pylint::rules::import_outside_top_level(checker, stmt);
}
if checker.enabled(Rule::GlobalStatement) {
for name in names {
if let Some(asname) = name.asname.as_ref() {
Expand Down Expand Up @@ -706,6 +709,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::ModuleImportNotAtTopOfFile) {
pycodestyle::rules::module_import_not_at_top_of_file(checker, stmt);
}
if checker.enabled(Rule::ImportOutsideTopLevel) {
pylint::rules::import_outside_top_level(checker, stmt);
}
if checker.enabled(Rule::GlobalStatement) {
for name in names {
if let Some(asname) = name.asname.as_ref() {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "C0205") => (RuleGroup::Stable, rules::pylint::rules::SingleStringSlots),
(Pylint, "C0208") => (RuleGroup::Stable, rules::pylint::rules::IterationOverSet),
(Pylint, "C0414") => (RuleGroup::Stable, rules::pylint::rules::UselessImportAlias),
(Pylint, "C0415") => (RuleGroup::Preview, rules::pylint::rules::ImportOutsideTopLevel),
(Pylint, "C2401") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiName),
(Pylint, "C2403") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiImportName),
#[allow(deprecated)]
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ mod tests {
Rule::GlobalVariableNotAssigned,
Path::new("global_variable_not_assigned.py")
)]
#[test_case(Rule::ImportOutsideTopLevel, Path::new("import_outside_top_level.py"))]
#[test_case(Rule::ImportSelf, Path::new("import_self/module.py"))]
#[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"))]
#[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Stmt;
use ruff_text_size::Ranged;

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

/// ## What it does
/// Checks for `import` statements outside of a module's top-level scope, such
/// as within a function or class definition.
///
/// ## Why is this bad?
/// [PEP 8] recommends placing imports not only at the top-level of a module,
/// but at the very top of the file, "just after any module comments and
/// docstrings, and before module globals and constants."
///
/// `import` statements have effects that are global in scope; defining them at
/// the top level has a number of benefits. For example, it makes it easier to
/// identify the dependencies of a module, and ensures that any invalid imports
/// are caught regardless of whether a specific function is called or class is
/// instantiated.
///
/// An import statement would typically be placed within a function only to
/// avoid a circular dependency, to defer a costly module load, or to avoid
/// loading a dependency altogether in a certain runtime environment.
///
/// ## Example
/// ```python
/// def print_python_version():
/// import platform
///
/// print(python.python_version())
/// ```
///
/// Use instead:
/// ```python
/// import platform
///
///
/// def print_python_version():
/// print(python.python_version())
/// ```
///
/// [PEP 8]: https://peps.python.org/pep-0008/#imports
#[violation]
pub struct ImportOutsideTopLevel;

impl Violation for ImportOutsideTopLevel {
#[derive_message_formats]
fn message(&self) -> String {
format!("`import` should be at the top-level of a file")
}
}

/// C0415
pub(crate) fn import_outside_top_level(checker: &mut Checker, stmt: &Stmt) {
if !checker.semantic().current_scope().kind.is_module() {
checker
.diagnostics
.push(Diagnostic::new(ImportOutsideTopLevel, stmt.range()));
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub(crate) use eq_without_hash::*;
pub(crate) use global_at_module_level::*;
pub(crate) use global_statement::*;
pub(crate) use global_variable_not_assigned::*;
pub(crate) use import_outside_top_level::*;
pub(crate) use import_self::*;
pub(crate) use invalid_all_format::*;
pub(crate) use invalid_all_object::*;
Expand Down Expand Up @@ -87,6 +88,7 @@ mod eq_without_hash;
mod global_at_module_level;
mod global_statement;
mod global_variable_not_assigned;
mod import_outside_top_level;
mod import_self;
mod invalid_all_format;
mod invalid_all_object;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
import_outside_top_level.py:10:5: PLC0415 `import` should be at the top-level of a file
|
9 | def import_in_function():
10 | import symtable # [import-outside-toplevel]
| ^^^^^^^^^^^^^^^ PLC0415
11 | import os, sys # [import-outside-toplevel]
12 | import time as thyme # [import-outside-toplevel]
|

import_outside_top_level.py:11:5: PLC0415 `import` should be at the top-level of a file
|
9 | def import_in_function():
10 | import symtable # [import-outside-toplevel]
11 | import os, sys # [import-outside-toplevel]
| ^^^^^^^^^^^^^^ PLC0415
12 | import time as thyme # [import-outside-toplevel]
13 | import random as rand, socket as sock # [import-outside-toplevel]
|

import_outside_top_level.py:12:5: PLC0415 `import` should be at the top-level of a file
|
10 | import symtable # [import-outside-toplevel]
11 | import os, sys # [import-outside-toplevel]
12 | import time as thyme # [import-outside-toplevel]
| ^^^^^^^^^^^^^^^^^^^^ PLC0415
13 | import random as rand, socket as sock # [import-outside-toplevel]
14 | from collections import defaultdict # [import-outside-toplevel]
|

import_outside_top_level.py:13:5: PLC0415 `import` should be at the top-level of a file
|
11 | import os, sys # [import-outside-toplevel]
12 | import time as thyme # [import-outside-toplevel]
13 | import random as rand, socket as sock # [import-outside-toplevel]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0415
14 | from collections import defaultdict # [import-outside-toplevel]
15 | from math import sin as sign, cos as cosplay # [import-outside-toplevel]
|

import_outside_top_level.py:14:5: PLC0415 `import` should be at the top-level of a file
|
12 | import time as thyme # [import-outside-toplevel]
13 | import random as rand, socket as sock # [import-outside-toplevel]
14 | from collections import defaultdict # [import-outside-toplevel]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0415
15 | from math import sin as sign, cos as cosplay # [import-outside-toplevel]
|

import_outside_top_level.py:15:5: PLC0415 `import` should be at the top-level of a file
|
13 | import random as rand, socket as sock # [import-outside-toplevel]
14 | from collections import defaultdict # [import-outside-toplevel]
15 | from math import sin as sign, cos as cosplay # [import-outside-toplevel]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0415
|

import_outside_top_level.py:19:5: PLC0415 `import` should be at the top-level of a file
|
18 | class ClassWithImports:
19 | import tokenize # [import-outside-toplevel]
| ^^^^^^^^^^^^^^^ PLC0415
20 |
21 | def __init__(self):
|

import_outside_top_level.py:22:9: PLC0415 `import` should be at the top-level of a file
|
21 | def __init__(self):
22 | import trace # [import-outside-toplevel]
| ^^^^^^^^^^^^ PLC0415
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading