Skip to content

Commit

Permalink
honor banned top level imports by TID253 in PLC0415. Fixes astral-sh#…
Browse files Browse the repository at this point in the history
  • Loading branch information
mishamsk committed Jan 21, 2025
1 parent 4cfa355 commit 4619702
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
def f():
# this should be allowed due to TID253 top-level ban
import foo_banned
from pkg import bar_banned

# this should still trigger an error due to multiple imports
from pkg import foo_allowed, bar_banned
47 changes: 45 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,21 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pycodestyle::rules::module_import_not_at_top_of_file(checker, stmt);
}
if checker.enabled(Rule::ImportOutsideTopLevel) {
pylint::rules::import_outside_top_level(checker, stmt);
pylint::rules::import_outside_top_level(
checker,
stmt,
None,
&names
.iter()
.map(|alias| {
flake8_tidy_imports::matchers::NameMatchPolicy::MatchNameOrParent(
flake8_tidy_imports::matchers::MatchNameOrParent {
module: &alias.name,
},
)
})
.collect(),
);
}
if checker.enabled(Rule::GlobalStatement) {
for name in names {
Expand Down Expand Up @@ -752,7 +766,36 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
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 let Some(module) = helpers::resolve_imported_module_path(
level,
module,
checker.module.qualified_name(),
) {
pylint::rules::import_outside_top_level(
checker,
stmt,
Some(
flake8_tidy_imports::matchers::NameMatchPolicy::MatchNameOrParent(
flake8_tidy_imports::matchers::MatchNameOrParent {
module: &module,
},
),
),
&names
.iter()
.map(|alias| {
flake8_tidy_imports::matchers::NameMatchPolicy::MatchName(
flake8_tidy_imports::matchers::MatchName {
module: &module,
member: &alias.name,
},
)
})
.collect(),
);
} else {
pylint::rules::import_outside_top_level(checker, stmt, None, &Vec::new());
}
}
if checker.enabled(Rule::GlobalStatement) {
for name in names {
Expand Down
25 changes: 24 additions & 1 deletion crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ mod tests {
use test_case::test_case;

use crate::registry::Rule;
use crate::rules::pylint;
use crate::rules::{flake8_tidy_imports, pylint};

use crate::settings::types::{PreviewMode, PythonVersion};
use crate::settings::LinterSettings;
Expand Down Expand Up @@ -412,6 +412,29 @@ mod tests {
Ok(())
}

#[test]
fn import_outside_top_level_with_banned() -> Result<()> {
let diagnostics = test_path(
Path::new("pylint/import_outside_top_level_with_banned.py"),
&LinterSettings {
preview: PreviewMode::Enabled,
flake8_tidy_imports: flake8_tidy_imports::settings::Settings {
banned_module_level_imports: vec![
"foo_banned".to_string(),
"pkg.bar_banned".to_string(),
],
..Default::default()
},
..LinterSettings::for_rules(vec![
Rule::BannedModuleLevelImports,
Rule::ImportOutsideTopLevel,
])
},
)?;
assert_messages!(diagnostics);
Ok(())
}

#[test_case(
Rule::RepeatedEqualityComparison,
Path::new("repeated_equality_comparison.py")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::Stmt;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::{
checkers::ast::Checker, codes::Rule, rules::flake8_tidy_imports::matchers::NameMatchPolicy,
};

/// ## What it does
/// Checks for `import` statements outside of a module's top-level scope, such
Expand Down Expand Up @@ -53,8 +55,38 @@ impl Violation for ImportOutsideTopLevel {
}

/// C0415
pub(crate) fn import_outside_top_level(checker: &mut Checker, stmt: &Stmt) {
if !checker.semantic().current_scope().kind.is_module() {
pub(crate) fn import_outside_top_level(
checker: &mut Checker,
stmt: &Stmt,
pkg: Option<NameMatchPolicy>,
modules: &Vec<NameMatchPolicy>,
) {
if !checker.semantic().current_scope().kind.is_module()
&& (!checker.enabled(Rule::BannedModuleLevelImports)
|| (pkg.is_some_and(|policy| {
policy
.find(
checker
.settings
.flake8_tidy_imports
.banned_module_level_imports
.iter()
.map(AsRef::as_ref),
)
.is_none()
}) && modules.iter().any(|policy| {
policy
.find(
checker
.settings
.flake8_tidy_imports
.banned_module_level_imports
.iter()
.map(AsRef::as_ref),
)
.is_none()
})))
{
checker
.diagnostics
.push(Diagnostic::new(ImportOutsideTopLevel, stmt.range()));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
import_outside_top_level_with_banned.py:7:5: PLC0415 `import` should be at the top-level of a file
|
6 | # this should still trigger an error due to multiple imports
7 | from pkg import foo_allowed, bar_banned
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0415
|

0 comments on commit 4619702

Please sign in to comment.