Skip to content
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,3 @@
import concurrent.futures as futures

1
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import concurrent.futures as futures

1
23 changes: 23 additions & 0 deletions crates/ruff_linter/src/rules/isort/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,29 @@ mod tests {
Ok(())
}

#[test_case(Path::new("plr0402_skip.py"))]
fn plr0402_skips_required_imports(path: &Path) -> Result<()> {
let snapshot = format!("plr0402_skips_required_imports_{}", path.to_string_lossy());
let diagnostics = test_path(
Path::new("isort/required_imports").join(path).as_path(),
&LinterSettings {
src: vec![test_resource_path("fixtures/isort")],
isort: super::settings::Settings {
required_imports: BTreeSet::from_iter([NameImport::Import(
ModuleNameImport::alias(
"concurrent.futures".to_string(),
"futures".to_string(),
),
)]),
..super::settings::Settings::default()
},
..LinterSettings::for_rules([Rule::MissingRequiredImport, Rule::ManualFromImport])
},
)?;
assert_diagnostics!(snapshot, diagnostics);
Ok(())
}

#[test_case(Path::new("from_first.py"))]
fn from_first(path: &Path) -> Result<()> {
let snapshot = format!("from_first_{}", path.to_string_lossy());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/isort/mod.rs
---

10 changes: 10 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/manual_import_from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use ruff_text_size::{Ranged, TextRange};
use ruff_macros::{ViolationMetadata, derive_message_formats};

use crate::checkers::ast::Checker;
use crate::rules::pyupgrade::rules::is_import_required_by_isort;
use crate::{Edit, Fix, FixAvailability, Violation};

/// ## What it does
Expand Down Expand Up @@ -58,6 +59,15 @@ pub(crate) fn manual_from_import(checker: &Checker, stmt: &Stmt, alias: &Alias,
return;
}

// Skip if this import is required by isort to prevent infinite loops with I002
if is_import_required_by_isort(
&checker.settings().isort.required_imports,
stmt.into(),
alias,
) {
return;
}

let mut diagnostic = checker.report_diagnostic(
ManualFromImport {
module: module.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use itertools::{Itertools, chain};
use ruff_python_semantic::NodeId;

use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::name::{QualifiedName, QualifiedNameBuilder};
use ruff_python_ast::{self as ast, Alias, Stmt, StmtRef};
use ruff_python_semantic::{NameImport, Scope};
use ruff_text_size::Ranged;
Expand Down Expand Up @@ -95,20 +96,35 @@ pub(crate) fn is_import_required_by_isort(
stmt: StmtRef,
alias: &Alias,
) -> bool {
let segments: &[&str] = match stmt {
match stmt {
StmtRef::ImportFrom(ast::StmtImportFrom {
module: Some(module),
..
}) => &[module.as_str(), alias.name.as_str()],
StmtRef::ImportFrom(ast::StmtImportFrom { module: None, .. }) | StmtRef::Import(_) => {
&[alias.name.as_str()]
}) => {
let mut builder = QualifiedNameBuilder::with_capacity(module.split('.').count() + 1);
builder.extend(module.split('.'));
builder.push(alias.name.as_str());
let qualified = builder.build();

required_imports
.iter()
.any(|required_import| required_import.qualified_name() == qualified)
}
_ => return false,
};
StmtRef::ImportFrom(ast::StmtImportFrom { module: None, .. })
| StmtRef::Import(ast::StmtImport { .. }) => {
let name = alias.name.as_str();
let qualified = if name.contains('.') {
QualifiedName::from_dotted_name(name)
} else {
QualifiedName::user_defined(name)
};

required_imports
.iter()
.any(|required_import| required_import.qualified_name().segments() == segments)
required_imports
.iter()
.any(|required_import| required_import.qualified_name() == qualified)
}
_ => false,
}
}

/// UP010
Expand Down
Loading