Some abstractions

This commit is contained in:
Charlie Marsh 2023-07-27 20:19:39 -04:00
parent 3eec7de6e4
commit ac0f51ef4f
18 changed files with 215 additions and 272 deletions

View File

@ -33,7 +33,7 @@ impl<'a, T: Codegen<'a>> CodegenStylist<'a> for T {
/// ///
/// Returns `Ok(None)` if the statement is empty after removing the imports. /// Returns `Ok(None)` if the statement is empty after removing the imports.
pub(crate) fn remove_imports<'a>( pub(crate) fn remove_imports<'a>(
imports: impl Iterator<Item = &'a str>, member_names: impl Iterator<Item = &'a str>,
stmt: &Stmt, stmt: &Stmt,
locator: &Locator, locator: &Locator,
stylist: &Stylist, stylist: &Stylist,
@ -45,27 +45,20 @@ pub(crate) fn remove_imports<'a>(
bail!("Expected Statement::Simple"); bail!("Expected Statement::Simple");
}; };
let (aliases, import_module) = match body.body.first_mut() { let aliases = match body.body.first_mut() {
Some(SmallStatement::Import(import_body)) => (&mut import_body.names, None), Some(SmallStatement::Import(import_body)) => &mut import_body.names,
Some(SmallStatement::ImportFrom(import_body)) => { Some(SmallStatement::ImportFrom(import_body)) => {
if let ImportNames::Aliases(names) = &mut import_body.names { if let ImportNames::Aliases(names) = &mut import_body.names {
( names
names,
Some((&import_body.relative, import_body.module.as_ref())),
)
} else if let ImportNames::Star(..) = &import_body.names { } else if let ImportNames::Star(..) = &import_body.names {
// Special-case: if the import is a `from ... import *`, then we delete the // Special-case: if the import is a `from ... import *`, then we delete the
// entire statement. // entire statement.
let mut found_star = false; let mut found_star = false;
for import in imports { for member in member_names {
let qualified_name = match import_body.module.as_ref() { if member == "*" {
Some(module_name) => format!("{}.*", compose_module_path(module_name)),
None => "*".to_string(),
};
if import == qualified_name {
found_star = true; found_star = true;
} else { } else {
bail!("Expected \"*\" for unused import (got: \"{}\")", import); bail!("Expected \"*\" for unused import (got: \"{}\")", member);
} }
} }
if !found_star { if !found_star {
@ -82,31 +75,10 @@ pub(crate) fn remove_imports<'a>(
// Preserve the trailing comma (or not) from the last entry. // Preserve the trailing comma (or not) from the last entry.
let trailing_comma = aliases.last().and_then(|alias| alias.comma.clone()); let trailing_comma = aliases.last().and_then(|alias| alias.comma.clone());
for import in imports { for member in member_names {
// If it's an import-from, we only care about members. let alias_index = aliases
let alias_index = aliases.iter().position(|alias| { .iter()
let qualified_name = match import_module { .position(|alias| member == compose_module_path(&alias.name));
Some((relative, module)) => {
let module = module.map(compose_module_path);
let member = compose_module_path(&alias.name);
let mut qualified_name = String::with_capacity(
relative.len() + module.as_ref().map_or(0, String::len) + member.len() + 1,
);
for _ in 0..relative.len() {
qualified_name.push('.');
}
if let Some(module) = module {
qualified_name.push_str(&module);
qualified_name.push('.');
}
qualified_name.push_str(&member);
qualified_name
}
None => compose_module_path(&alias.name),
};
qualified_name == import
});
if let Some(index) = alias_index { if let Some(index) = alias_index {
aliases.remove(index); aliases.remove(index);
} }
@ -140,7 +112,7 @@ pub(crate) fn remove_imports<'a>(
/// ///
/// Returns the modified import statement. /// Returns the modified import statement.
pub(crate) fn retain_imports( pub(crate) fn retain_imports(
imports: &[&str], member_names: &[&str],
stmt: &Stmt, stmt: &Stmt,
locator: &Locator, locator: &Locator,
stylist: &Stylist, stylist: &Stylist,
@ -152,14 +124,11 @@ pub(crate) fn retain_imports(
bail!("Expected Statement::Simple"); bail!("Expected Statement::Simple");
}; };
let (aliases, import_module) = match body.body.first_mut() { let aliases = match body.body.first_mut() {
Some(SmallStatement::Import(import_body)) => (&mut import_body.names, None), Some(SmallStatement::Import(import_body)) => &mut import_body.names,
Some(SmallStatement::ImportFrom(import_body)) => { Some(SmallStatement::ImportFrom(import_body)) => {
if let ImportNames::Aliases(names) = &mut import_body.names { if let ImportNames::Aliases(names) = &mut import_body.names {
( names
names,
Some((&import_body.relative, import_body.module.as_ref())),
)
} else { } else {
bail!("Expected: ImportNames::Aliases"); bail!("Expected: ImportNames::Aliases");
} }
@ -171,28 +140,9 @@ pub(crate) fn retain_imports(
let trailing_comma = aliases.last().and_then(|alias| alias.comma.clone()); let trailing_comma = aliases.last().and_then(|alias| alias.comma.clone());
aliases.retain(|alias| { aliases.retain(|alias| {
imports.iter().any(|import| { member_names
let qualified_name = match import_module { .iter()
Some((relative, module)) => { .any(|member| *member == compose_module_path(&alias.name))
let module = module.map(compose_module_path);
let member = compose_module_path(&alias.name);
let mut qualified_name = String::with_capacity(
relative.len() + module.as_ref().map_or(0, String::len) + member.len() + 1,
);
for _ in 0..relative.len() {
qualified_name.push('.');
}
if let Some(module) = module {
qualified_name.push_str(&module);
qualified_name.push('.');
}
qualified_name.push_str(&member);
qualified_name
}
None => compose_module_path(&alias.name),
};
qualified_name == *import
})
}); });
// But avoid destroying any trailing comments. // But avoid destroying any trailing comments.

View File

@ -55,14 +55,14 @@ pub(crate) fn delete_stmt(
/// Generate a `Fix` to remove the specified imports from an `import` statement. /// Generate a `Fix` to remove the specified imports from an `import` statement.
pub(crate) fn remove_unused_imports<'a>( pub(crate) fn remove_unused_imports<'a>(
unused_imports: impl Iterator<Item = &'a str>, member_names: impl Iterator<Item = &'a str>,
stmt: &Stmt, stmt: &Stmt,
parent: Option<&Stmt>, parent: Option<&Stmt>,
locator: &Locator, locator: &Locator,
stylist: &Stylist, stylist: &Stylist,
indexer: &Indexer, indexer: &Indexer,
) -> Result<Edit> { ) -> Result<Edit> {
match codemods::remove_imports(unused_imports, stmt, locator, stylist)? { match codemods::remove_imports(member_names, stmt, locator, stylist)? {
None => Ok(delete_stmt(stmt, parent, locator, indexer)), None => Ok(delete_stmt(stmt, parent, locator, indexer)),
Some(content) => Ok(Edit::range_replacement(content, stmt.range())), Some(content) => Ok(Edit::range_replacement(content, stmt.range())),
} }

View File

@ -87,13 +87,13 @@ impl<'a> Importer<'a> {
/// import statement. /// import statement.
pub(crate) fn runtime_import_edit( pub(crate) fn runtime_import_edit(
&self, &self,
import: &StmtImports, import: &ImportedMembers,
at: TextSize, at: TextSize,
) -> Result<RuntimeImportEdit> { ) -> Result<RuntimeImportEdit> {
// Generate the modified import statement. // Generate the modified import statement.
let content = autofix::codemods::retain_imports( let content = autofix::codemods::retain_imports(
&import.qualified_names, &import.names,
import.stmt, import.statement,
self.locator, self.locator,
self.stylist, self.stylist,
)?; )?;
@ -118,14 +118,14 @@ impl<'a> Importer<'a> {
/// `TYPE_CHECKING` block. /// `TYPE_CHECKING` block.
pub(crate) fn typing_import_edit( pub(crate) fn typing_import_edit(
&self, &self,
import: &StmtImports, import: &ImportedMembers,
at: TextSize, at: TextSize,
semantic: &SemanticModel, semantic: &SemanticModel,
) -> Result<TypingImportEdit> { ) -> Result<TypingImportEdit> {
// Generate the modified import statement. // Generate the modified import statement.
let content = autofix::codemods::retain_imports( let content = autofix::codemods::retain_imports(
&import.qualified_names, &import.names,
import.stmt, import.statement,
self.locator, self.locator,
self.stylist, self.stylist,
)?; )?;
@ -446,11 +446,11 @@ impl<'a> ImportRequest<'a> {
} }
/// An existing list of module or member imports, located within an import statement. /// An existing list of module or member imports, located within an import statement.
pub(crate) struct StmtImports<'a> { pub(crate) struct ImportedMembers<'a> {
/// The import statement. /// The import statement.
pub(crate) stmt: &'a Stmt, pub(crate) statement: &'a Stmt,
/// The "qualified names" of the imported modules or members. /// The "names" of the imported members.
pub(crate) qualified_names: Vec<&'a str>, pub(crate) names: Vec<&'a str>,
} }
/// The result of an [`Importer::get_or_import_symbol`] call. /// The result of an [`Importer::get_or_import_symbol`] call.

View File

@ -1,15 +1,16 @@
use anyhow::Result; use anyhow::Result;
use ruff_text_size::TextRange; use ruff_text_size::TextRange;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use std::borrow::Cow;
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation}; use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::{NodeId, ResolvedReferenceId, Scope}; use ruff_python_semantic::{AnyImport, NodeId, ResolvedReferenceId, Scope};
use crate::autofix; use crate::autofix;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::codes::Rule; use crate::codes::Rule;
use crate::importer::StmtImports; use crate::importer::ImportedMembers;
/// ## What it does /// ## What it does
/// Checks for runtime imports defined in a type-checking block. /// Checks for runtime imports defined in a type-checking block.
@ -51,7 +52,7 @@ impl Violation for RuntimeImportInTypeCheckingBlock {
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
let RuntimeImportInTypeCheckingBlock { qualified_name, .. } = self; let RuntimeImportInTypeCheckingBlock { qualified_name } = self;
format!( format!(
"Move import `{qualified_name}` out of type-checking block. Import is used for more than type hinting." "Move import `{qualified_name}` out of type-checking block. Import is used for more than type hinting."
) )
@ -69,13 +70,13 @@ pub(crate) fn runtime_import_in_type_checking_block(
diagnostics: &mut Vec<Diagnostic>, diagnostics: &mut Vec<Diagnostic>,
) { ) {
// Collect all runtime imports by statement. // Collect all runtime imports by statement.
let mut errors_by_statement: FxHashMap<NodeId, Vec<Import>> = FxHashMap::default(); let mut errors_by_statement: FxHashMap<NodeId, Vec<ImportBinding>> = FxHashMap::default();
let mut ignores_by_statement: FxHashMap<NodeId, Vec<Import>> = FxHashMap::default(); let mut ignores_by_statement: FxHashMap<NodeId, Vec<ImportBinding>> = FxHashMap::default();
for binding_id in scope.binding_ids() { for binding_id in scope.binding_ids() {
let binding = checker.semantic().binding(binding_id); let binding = checker.semantic().binding(binding_id);
let Some(call_path) = binding.call_path() else { let Some(import) = binding.as_any_import() else {
continue; continue;
}; };
@ -96,8 +97,8 @@ pub(crate) fn runtime_import_in_type_checking_block(
continue; continue;
}; };
let import = Import { let import = ImportBinding {
call_path, import,
reference_id, reference_id,
range: binding.range, range: binding.range,
parent_range: binding.parent_range(checker.semantic()), parent_range: binding.parent_range(checker.semantic()),
@ -130,8 +131,8 @@ pub(crate) fn runtime_import_in_type_checking_block(
None None
}; };
for Import { for ImportBinding {
call_path, import,
range, range,
parent_range, parent_range,
.. ..
@ -139,7 +140,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
{ {
let mut diagnostic = Diagnostic::new( let mut diagnostic = Diagnostic::new(
RuntimeImportInTypeCheckingBlock { RuntimeImportInTypeCheckingBlock {
qualified_name: call_path.join("."), qualified_name: import.qualified_name(),
}, },
range, range,
); );
@ -155,8 +156,8 @@ pub(crate) fn runtime_import_in_type_checking_block(
// Separately, generate a diagnostic for every _ignored_ import, to ensure that the // Separately, generate a diagnostic for every _ignored_ import, to ensure that the
// suppression comments aren't marked as unused. // suppression comments aren't marked as unused.
for Import { for ImportBinding {
call_path, import,
range, range,
parent_range, parent_range,
.. ..
@ -164,7 +165,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
{ {
let mut diagnostic = Diagnostic::new( let mut diagnostic = Diagnostic::new(
RuntimeImportInTypeCheckingBlock { RuntimeImportInTypeCheckingBlock {
qualified_name: call_path.join("."), qualified_name: import.qualified_name(),
}, },
range, range,
); );
@ -176,9 +177,9 @@ pub(crate) fn runtime_import_in_type_checking_block(
} }
/// A runtime-required import with its surrounding context. /// A runtime-required import with its surrounding context.
struct Import<'a> { struct ImportBinding<'a> {
/// The qualified name of the import (e.g., `typing.List` for `from typing import List`). /// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
call_path: &'a [&'a str], import: AnyImport<'a>,
/// The first reference to the imported symbol. /// The first reference to the imported symbol.
reference_id: ResolvedReferenceId, reference_id: ResolvedReferenceId,
/// The trimmed range of the import (e.g., `List` in `from typing import List`). /// The trimmed range of the import (e.g., `List` in `from typing import List`).
@ -188,29 +189,26 @@ struct Import<'a> {
} }
/// Generate a [`Fix`] to remove runtime imports from a type-checking block. /// Generate a [`Fix`] to remove runtime imports from a type-checking block.
fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result<Fix> { fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[ImportBinding]) -> Result<Fix> {
let stmt = checker.semantic().stmts[stmt_id]; let stmt = checker.semantic().stmts[stmt_id];
let parent = checker.semantic().stmts.parent(stmt); let parent = checker.semantic().stmts.parent(stmt);
let qualified_names: Vec<String> = imports let member_names: Vec<Cow<'_, str>> = imports
.iter() .iter()
.map(|Import { call_path, .. }| call_path.join(".")) .map(|ImportBinding { import, .. }| import.member_name())
.collect(); .collect();
// Find the first reference across all imports. // Find the first reference across all imports.
let at = imports let at = imports
.iter() .iter()
.map(|Import { reference_id, .. }| { .map(|ImportBinding { reference_id, .. }| {
checker.semantic().reference(*reference_id).range().start() checker.semantic().reference(*reference_id).range().start()
}) })
.min() .min()
.expect("Expected at least one import"); .expect("Expected at least one import");
// Step 1) Remove the import. // Step 1) Remove the import.
// This could be a mix of imports...
// One framing could be: what's the name of the symbol being imported (e.g., `List` in `from typing import List`,
// or `foo` in `import foo`, or `foo.bar` in `import foo.bar`).
let remove_import_edit = autofix::edits::remove_unused_imports( let remove_import_edit = autofix::edits::remove_unused_imports(
qualified_names.iter().map(|name| name.as_str()), member_names.iter().map(AsRef::as_ref),
stmt, stmt,
parent, parent,
checker.locator(), checker.locator(),
@ -220,9 +218,9 @@ fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result
// Step 2) Add the import to the top-level. // Step 2) Add the import to the top-level.
let add_import_edit = checker.importer().runtime_import_edit( let add_import_edit = checker.importer().runtime_import_edit(
&StmtImports { &ImportedMembers {
stmt, statement: stmt,
qualified_names: qualified_names.iter().map(|name| name.as_str()).collect(), names: member_names.iter().map(AsRef::as_ref).collect(),
}, },
at, at,
)?; )?;

View File

@ -1,15 +1,17 @@
use std::borrow::Cow;
use anyhow::Result; use anyhow::Result;
use ruff_text_size::TextRange;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use ruff_diagnostics::{AutofixKind, Diagnostic, DiagnosticKind, Fix, Violation}; use ruff_diagnostics::{AutofixKind, Diagnostic, DiagnosticKind, Fix, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::{Binding, NodeId, ResolvedReferenceId, Scope}; use ruff_python_semantic::{AnyImport, Binding, NodeId, ResolvedReferenceId, Scope};
use ruff_text_size::TextRange;
use crate::autofix; use crate::autofix;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::codes::Rule; use crate::codes::Rule;
use crate::importer::StmtImports; use crate::importer::ImportedMembers;
use crate::rules::isort::{categorize, ImportSection, ImportType}; use crate::rules::isort::{categorize, ImportSection, ImportType};
/// ## What it does /// ## What it does
@ -188,9 +190,9 @@ pub(crate) fn typing_only_runtime_import(
diagnostics: &mut Vec<Diagnostic>, diagnostics: &mut Vec<Diagnostic>,
) { ) {
// Collect all typing-only imports by statement and import type. // Collect all typing-only imports by statement and import type.
let mut errors_by_statement: FxHashMap<(NodeId, ImportType), Vec<Import>> = let mut errors_by_statement: FxHashMap<(NodeId, ImportType), Vec<ImportBinding>> =
FxHashMap::default(); FxHashMap::default();
let mut ignores_by_statement: FxHashMap<(NodeId, ImportType), Vec<Import>> = let mut ignores_by_statement: FxHashMap<(NodeId, ImportType), Vec<ImportBinding>> =
FxHashMap::default(); FxHashMap::default();
for binding_id in scope.binding_ids() { for binding_id in scope.binding_ids() {
@ -206,23 +208,10 @@ pub(crate) fn typing_only_runtime_import(
continue; continue;
} }
let Some(qualified_name) = binding.qualified_name() else { let Some(import) = binding.as_any_import() else {
continue; continue;
}; };
if is_exempt(
qualified_name.as_str(),
&checker
.settings
.flake8_type_checking
.exempt_modules
.iter()
.map(String::as_str)
.collect::<Vec<_>>(),
) {
continue;
}
let Some(reference_id) = binding.references.first().copied() else { let Some(reference_id) = binding.references.first().copied() else {
continue; continue;
}; };
@ -236,20 +225,25 @@ pub(crate) fn typing_only_runtime_import(
.is_typing() .is_typing()
}) })
{ {
// Extract the module base and level from the full name. let qualified_name = import.qualified_name();
// Ex) `foo.bar.baz` -> `foo`, `0`
// Ex) `.foo.bar.baz` -> `foo`, `1` if is_exempt(
let level = qualified_name qualified_name.as_str(),
.chars() &checker
.take_while(|c| *c == '.') .settings
.count() .flake8_type_checking
.try_into() .exempt_modules
.unwrap(); .iter()
.map(String::as_str)
.collect::<Vec<_>>(),
) {
continue;
}
// Categorize the import, using coarse-grained categorization. // Categorize the import, using coarse-grained categorization.
let import_type = match categorize( let import_type = match categorize(
qualified_name.as_str(), qualified_name.as_str(),
Some(level), None,
&checker.settings.src, &checker.settings.src,
checker.package(), checker.package(),
&checker.settings.isort.known_modules, &checker.settings.isort.known_modules,
@ -275,8 +269,8 @@ pub(crate) fn typing_only_runtime_import(
continue; continue;
}; };
let import = Import { let import = ImportBinding {
qualified_name, import,
reference_id, reference_id,
range: binding.range, range: binding.range,
parent_range: binding.parent_range(checker.semantic()), parent_range: binding.parent_range(checker.semantic()),
@ -309,17 +303,15 @@ pub(crate) fn typing_only_runtime_import(
None None
}; };
for Import { for ImportBinding {
qualified_name, import,
range, range,
parent_range, parent_range,
.. ..
} in imports } in imports
{ {
let mut diagnostic = Diagnostic::new( let mut diagnostic =
diagnostic_for(import_type, qualified_name.to_string()), Diagnostic::new(diagnostic_for(import_type, import.qualified_name()), range);
range,
);
if let Some(range) = parent_range { if let Some(range) = parent_range {
diagnostic.set_parent(range.start()); diagnostic.set_parent(range.start());
} }
@ -333,17 +325,15 @@ pub(crate) fn typing_only_runtime_import(
// Separately, generate a diagnostic for every _ignored_ import, to ensure that the // Separately, generate a diagnostic for every _ignored_ import, to ensure that the
// suppression comments aren't marked as unused. // suppression comments aren't marked as unused.
for ((_, import_type), imports) in ignores_by_statement { for ((_, import_type), imports) in ignores_by_statement {
for Import { for ImportBinding {
qualified_name, import,
range, range,
parent_range, parent_range,
.. ..
} in imports } in imports
{ {
let mut diagnostic = Diagnostic::new( let mut diagnostic =
diagnostic_for(import_type, qualified_name.to_string()), Diagnostic::new(diagnostic_for(import_type, import.qualified_name()), range);
range,
);
if let Some(range) = parent_range { if let Some(range) = parent_range {
diagnostic.set_parent(range.start()); diagnostic.set_parent(range.start());
} }
@ -353,9 +343,9 @@ pub(crate) fn typing_only_runtime_import(
} }
/// A runtime-required import with its surrounding context. /// A runtime-required import with its surrounding context.
struct Import { struct ImportBinding<'a> {
/// The qualified name of the import (e.g., `typing.List` for `from typing import List`). /// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
qualified_name: String, import: AnyImport<'a>,
/// The first reference to the imported symbol. /// The first reference to the imported symbol.
reference_id: ResolvedReferenceId, reference_id: ResolvedReferenceId,
/// The trimmed range of the import (e.g., `List` in `from typing import List`). /// The trimmed range of the import (e.g., `List` in `from typing import List`).
@ -412,18 +402,18 @@ fn is_exempt(name: &str, exempt_modules: &[&str]) -> bool {
} }
/// Generate a [`Fix`] to remove typing-only imports from a runtime context. /// Generate a [`Fix`] to remove typing-only imports from a runtime context.
fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result<Fix> { fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[ImportBinding]) -> Result<Fix> {
let stmt = checker.semantic().stmts[stmt_id]; let stmt = checker.semantic().stmts[stmt_id];
let parent = checker.semantic().stmts.parent(stmt); let parent = checker.semantic().stmts.parent(stmt);
let qualified_names: Vec<&str> = imports let member_names: Vec<Cow<'_, str>> = imports
.iter() .iter()
.map(|Import { qualified_name, .. }| qualified_name.as_str()) .map(|ImportBinding { import, .. }| import.member_name())
.collect(); .collect();
// Find the first reference across all imports. // Find the first reference across all imports.
let at = imports let at = imports
.iter() .iter()
.map(|Import { reference_id, .. }| { .map(|ImportBinding { reference_id, .. }| {
checker.semantic().reference(*reference_id).range().start() checker.semantic().reference(*reference_id).range().start()
}) })
.min() .min()
@ -431,7 +421,7 @@ fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result
// Step 1) Remove the import. // Step 1) Remove the import.
let remove_import_edit = autofix::edits::remove_unused_imports( let remove_import_edit = autofix::edits::remove_unused_imports(
qualified_names.iter().copied(), member_names.iter().map(AsRef::as_ref),
stmt, stmt,
parent, parent,
checker.locator(), checker.locator(),
@ -441,9 +431,9 @@ fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result
// Step 2) Add the import to a `TYPE_CHECKING` block. // Step 2) Add the import to a `TYPE_CHECKING` block.
let add_import_edit = checker.importer().typing_import_edit( let add_import_edit = checker.importer().typing_import_edit(
&StmtImports { &ImportedMembers {
stmt, statement: stmt,
qualified_names, names: member_names.iter().map(AsRef::as_ref).collect(),
}, },
at, at,
checker.semantic(), checker.semantic(),

View File

@ -1,5 +0,0 @@
---
source: crates/ruff/src/rules/flake8_type_checking/mod.rs
assertion_line: 43
---

View File

@ -65,7 +65,7 @@ pub(crate) fn inplace_argument(
.first() .first()
.and_then(|module| checker.semantic().find_binding(module)) .and_then(|module| checker.semantic().find_binding(module))
.map_or(false, |binding| { .map_or(false, |binding| {
if let BindingKind::Import(Import { call_path, .. }) = &binding.kind { if let BindingKind::Import(Import { call_path }) = &binding.kind {
matches!(call_path.as_ref(), ["pandas"]) matches!(call_path.as_ref(), ["pandas"])
} else { } else {
false false

View File

@ -1,10 +1,11 @@
use anyhow::Result; use anyhow::Result;
use ruff_text_size::TextRange;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use std::borrow::Cow;
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation}; use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::{Exceptions, NodeId, Scope}; use ruff_python_semantic::{AnyImport, Exceptions, NodeId, Scope};
use ruff_text_size::TextRange;
use crate::autofix; use crate::autofix;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -97,8 +98,8 @@ impl Violation for UnusedImport {
pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut Vec<Diagnostic>) { pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut Vec<Diagnostic>) {
// Collect all unused imports by statement. // Collect all unused imports by statement.
let mut unused: FxHashMap<(NodeId, Exceptions), Vec<Import>> = FxHashMap::default(); let mut unused: FxHashMap<(NodeId, Exceptions), Vec<ImportBinding>> = FxHashMap::default();
let mut ignored: FxHashMap<(NodeId, Exceptions), Vec<Import>> = FxHashMap::default(); let mut ignored: FxHashMap<(NodeId, Exceptions), Vec<ImportBinding>> = FxHashMap::default();
for binding_id in scope.binding_ids() { for binding_id in scope.binding_ids() {
let binding = checker.semantic().binding(binding_id); let binding = checker.semantic().binding(binding_id);
@ -111,7 +112,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
continue; continue;
} }
let Some(member_name) = binding.member_name() else { let Some(import) = binding.as_any_import() else {
continue; continue;
}; };
@ -119,8 +120,8 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
continue; continue;
}; };
let import = Import { let import = ImportBinding {
member_name, import,
range: binding.range, range: binding.range,
parent_range: binding.parent_range(checker.semantic()), parent_range: binding.parent_range(checker.semantic()),
}; };
@ -158,15 +159,15 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
None None
}; };
for Import { for ImportBinding {
member_name, import,
range, range,
parent_range, parent_range,
} in imports } in imports
{ {
let mut diagnostic = Diagnostic::new( let mut diagnostic = Diagnostic::new(
UnusedImport { UnusedImport {
name: member_name.to_string(), name: import.qualified_name(),
context: if in_except_handler { context: if in_except_handler {
Some(UnusedImportContext::ExceptHandler) Some(UnusedImportContext::ExceptHandler)
} else if in_init { } else if in_init {
@ -192,15 +193,15 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
// Separately, generate a diagnostic for every _ignored_ import, to ensure that the // Separately, generate a diagnostic for every _ignored_ import, to ensure that the
// suppression comments aren't marked as unused. // suppression comments aren't marked as unused.
for Import { for ImportBinding {
member_name, import,
range, range,
parent_range, parent_range,
} in ignored.into_values().flatten() } in ignored.into_values().flatten()
{ {
let mut diagnostic = Diagnostic::new( let mut diagnostic = Diagnostic::new(
UnusedImport { UnusedImport {
name: member_name.to_string(), name: import.qualified_name(),
context: None, context: None,
multiple: false, multiple: false,
}, },
@ -214,9 +215,9 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
} }
/// An unused import with its surrounding context. /// An unused import with its surrounding context.
struct Import { struct ImportBinding<'a> {
/// The qualified name of the import (e.g., `typing.List` for `from typing import List`). /// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
member_name: String, import: AnyImport<'a>,
/// The trimmed range of the import (e.g., `List` in `from typing import List`). /// The trimmed range of the import (e.g., `List` in `from typing import List`).
range: TextRange, range: TextRange,
/// The range of the import's parent statement. /// The range of the import's parent statement.
@ -224,13 +225,17 @@ struct Import {
} }
/// Generate a [`Fix`] to remove unused imports from a statement. /// Generate a [`Fix`] to remove unused imports from a statement.
fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result<Fix> { fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[ImportBinding]) -> Result<Fix> {
let stmt = checker.semantic().stmts[stmt_id]; let stmt = checker.semantic().stmts[stmt_id];
let parent = checker.semantic().stmts.parent(stmt); let parent = checker.semantic().stmts.parent(stmt);
let member_names: Vec<Cow<'_, str>> = imports
.iter()
.map(|ImportBinding { import, .. }| import.member_name())
.collect();
let edit = autofix::edits::remove_unused_imports( let edit = autofix::edits::remove_unused_imports(
imports member_names.iter().map(AsRef::as_ref),
.iter()
.map(|Import { member_name, .. }| member_name.as_str()),
stmt, stmt,
parent, parent,
checker.locator(), checker.locator(),

View File

@ -1,7 +1,7 @@
--- ---
source: crates/ruff/src/rules/pyflakes/mod.rs source: crates/ruff/src/rules/pyflakes/mod.rs
--- ---
F401_6.py:7:25: F401 [*] `.background.BackgroundTasks` imported but unused F401_6.py:7:25: F401 [*] `pyflakes.background.BackgroundTasks` imported but unused
| |
6 | # F401 `background.BackgroundTasks` imported but unused 6 | # F401 `background.BackgroundTasks` imported but unused
7 | from .background import BackgroundTasks 7 | from .background import BackgroundTasks
@ -9,7 +9,7 @@ F401_6.py:7:25: F401 [*] `.background.BackgroundTasks` imported but unused
8 | 8 |
9 | # F401 `datastructures.UploadFile` imported but unused 9 | # F401 `datastructures.UploadFile` imported but unused
| |
= help: Remove unused import: `.background.BackgroundTasks` = help: Remove unused import: `pyflakes.background.BackgroundTasks`
Fix Fix
4 4 | from .applications import FastAPI as FastAPI 4 4 | from .applications import FastAPI as FastAPI
@ -20,7 +20,7 @@ F401_6.py:7:25: F401 [*] `.background.BackgroundTasks` imported but unused
9 8 | # F401 `datastructures.UploadFile` imported but unused 9 8 | # F401 `datastructures.UploadFile` imported but unused
10 9 | from .datastructures import UploadFile as FileUpload 10 9 | from .datastructures import UploadFile as FileUpload
F401_6.py:10:43: F401 [*] `.datastructures.UploadFile` imported but unused F401_6.py:10:43: F401 [*] `pyflakes.datastructures.UploadFile` imported but unused
| |
9 | # F401 `datastructures.UploadFile` imported but unused 9 | # F401 `datastructures.UploadFile` imported but unused
10 | from .datastructures import UploadFile as FileUpload 10 | from .datastructures import UploadFile as FileUpload
@ -28,7 +28,7 @@ F401_6.py:10:43: F401 [*] `.datastructures.UploadFile` imported but unused
11 | 11 |
12 | # OK 12 | # OK
| |
= help: Remove unused import: `.datastructures.UploadFile` = help: Remove unused import: `pyflakes.datastructures.UploadFile`
Fix Fix
7 7 | from .background import BackgroundTasks 7 7 | from .background import BackgroundTasks

View File

@ -1,62 +0,0 @@
---
source: crates/ruff/src/rules/pyflakes/mod.rs
assertion_line: 147
---
F401_6.py:7:25: F401 [*] `pyflakes.background.BackgroundTasks` imported but unused
|
6 | # F401 `background.BackgroundTasks` imported but unused
7 | from .background import BackgroundTasks
| ^^^^^^^^^^^^^^^ F401
8 |
9 | # F401 `datastructures.UploadFile` imported but unused
|
= help: Remove unused import: `pyflakes.background.BackgroundTasks`
Fix
F401_6.py:10:43: F401 [*] `pyflakes.datastructures.UploadFile` imported but unused
|
9 | # F401 `datastructures.UploadFile` imported but unused
10 | from .datastructures import UploadFile as FileUpload
| ^^^^^^^^^^ F401
11 |
12 | # OK
|
= help: Remove unused import: `pyflakes.datastructures.UploadFile`
Fix
F401_6.py:16:8: F401 [*] `background` imported but unused
|
15 | # F401 `background` imported but unused
16 | import background
| ^^^^^^^^^^ F401
17 |
18 | # F401 `datastructures` imported but unused
|
= help: Remove unused import: `background`
Fix
13 13 | import applications as applications
14 14 |
15 15 | # F401 `background` imported but unused
16 |-import background
17 16 |
18 17 | # F401 `datastructures` imported but unused
19 18 | import datastructures as structures
F401_6.py:19:26: F401 [*] `datastructures` imported but unused
|
18 | # F401 `datastructures` imported but unused
19 | import datastructures as structures
| ^^^^^^^^^^ F401
|
= help: Remove unused import: `datastructures`
Fix
16 16 | import background
17 17 |
18 18 | # F401 `datastructures` imported but unused
19 |-import datastructures as structures

View File

@ -57,8 +57,6 @@ impl AlwaysAutofixableViolation for OSErrorAlias {
/// Return `true` if an [`Expr`] is an alias of `OSError`. /// Return `true` if an [`Expr`] is an alias of `OSError`.
fn is_alias(expr: &Expr, semantic: &SemanticModel) -> bool { fn is_alias(expr: &Expr, semantic: &SemanticModel) -> bool {
semantic.resolve_call_path(expr).map_or(false, |call_path| { semantic.resolve_call_path(expr).map_or(false, |call_path| {
println!("expr: {:?}", expr);
println!("call_path: {:?}", call_path);
matches!( matches!(
call_path.as_slice(), call_path.as_slice(),
["", "EnvironmentError" | "IOError" | "WindowsError"] ["", "EnvironmentError" | "IOError" | "WindowsError"]

View File

@ -124,13 +124,11 @@ pub(crate) fn unnecessary_builtin_import(
diagnostic.try_set_fix(|| { diagnostic.try_set_fix(|| {
let stmt = checker.semantic().stmt(); let stmt = checker.semantic().stmt();
let parent = checker.semantic().stmt_parent(); let parent = checker.semantic().stmt_parent();
let unused_imports: Vec<String> = unused_imports
.iter()
// These are always ImportFrom.
.map(|alias| format!("{module}.{}", alias.name))
.collect();
let edit = autofix::edits::remove_unused_imports( let edit = autofix::edits::remove_unused_imports(
unused_imports.iter().map(String::as_str), unused_imports
.iter()
.map(|alias| &alias.name)
.map(ruff_python_ast::Identifier::as_str),
stmt, stmt,
parent, parent,
checker.locator(), checker.locator(),

View File

@ -111,14 +111,13 @@ pub(crate) fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, name
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| { diagnostic.try_set_fix(|| {
let unused_imports: Vec<String> = unused_imports
.iter()
.map(|alias| format!("__future__.{}", alias.name))
.collect();
let stmt = checker.semantic().stmt(); let stmt = checker.semantic().stmt();
let parent = checker.semantic().stmt_parent(); let parent = checker.semantic().stmt_parent();
let edit = autofix::edits::remove_unused_imports( let edit = autofix::edits::remove_unused_imports(
unused_imports.iter().map(String::as_str), unused_imports
.iter()
.map(|alias| &alias.name)
.map(ruff_python_ast::Identifier::as_str),
stmt, stmt,
parent, parent,
checker.locator(), checker.locator(),

View File

@ -278,12 +278,31 @@ pub fn compose_call_path(expr: &Expr) -> Option<String> {
/// Format a call path for display. /// Format a call path for display.
pub fn format_call_path(call_path: &[&str]) -> String { pub fn format_call_path(call_path: &[&str]) -> String {
if call_path if call_path.first().map_or(false, |first| first.is_empty()) {
.first() // If the first segment is empty, the `CallPath` is that of a builtin.
.expect("Unable to format empty call path") // Ex) `["", "bool"]` -> `"bool"`
.is_empty()
{
call_path[1..].join(".") call_path[1..].join(".")
} else if call_path
.first()
.map_or(false, |first| matches!(*first, "."))
{
// If the call path is dot-prefixed, it's an unresolved relative import.
// Ex) `[".foo", "bar"]` -> `".foo.bar"`
let mut formatted = String::new();
let mut iter = call_path.iter();
for segment in iter.by_ref() {
if matches!(*segment, ".") {
formatted.push('.');
} else {
formatted.push_str(segment);
break;
}
}
for segment in iter {
formatted.push('.');
formatted.push_str(segment);
}
formatted
} else { } else {
call_path.join(".") call_path.join(".")
} }

View File

@ -940,7 +940,7 @@ pub fn literal_path<'a>(
+ 1, + 1,
); );
// Remove segments based on the number of dots. // Include the dots
if let Some(level) = level { if let Some(level) = level {
if level > 0 { if level > 0 {
for _ in 0..level { for _ in 0..level {

View File

@ -238,6 +238,60 @@ impl<'a> Binding<'a> {
} }
}) })
} }
pub fn as_any_import(&'a self) -> Option<AnyImport<'a>> {
match &self.kind {
BindingKind::Import(import) => Some(AnyImport::Import(import)),
BindingKind::SubmoduleImport(import) => Some(AnyImport::SubmoduleImport(import)),
BindingKind::FromImport(import) => Some(AnyImport::FromImport(import)),
_ => None,
}
}
}
pub enum AnyImport<'a> {
Import(&'a Import<'a>),
SubmoduleImport(&'a SubmoduleImport<'a>),
FromImport(&'a FromImport<'a>),
}
impl<'a> AnyImport<'a> {
/// Returns the fully-qualified symbol name, if this symbol was imported from another module.
pub fn call_path(&self) -> &[&str] {
match self {
Self::Import(Import { call_path }) => call_path.as_ref(),
Self::SubmoduleImport(SubmoduleImport { call_path }) => call_path.as_ref(),
Self::FromImport(FromImport { call_path }) => call_path.as_ref(),
}
}
/// Returns the fully-qualified symbol name, if this symbol was imported from another module.
pub fn qualified_name(&self) -> String {
format_call_path(self.call_path())
}
/// Returns the fully-qualified name of the module from which this symbol was imported, if this
/// symbol was imported from another module.
pub fn module_name(&self) -> &[&str] {
match self {
Self::Import(Import { call_path }) => &call_path[..1],
Self::SubmoduleImport(SubmoduleImport { call_path }) => &call_path[..1],
Self::FromImport(FromImport { call_path }) => &call_path[..call_path.len() - 1],
}
}
/// Returns the fully-qualified symbol name, if this symbol was imported from another module.
pub fn member_name(&self) -> Cow<'a, str> {
match self {
Self::Import(Import { call_path }) => Cow::Owned(format_call_path(call_path)),
Self::SubmoduleImport(SubmoduleImport { call_path }) => {
Cow::Owned(format_call_path(call_path))
}
Self::FromImport(FromImport { call_path }) => {
Cow::Borrowed(call_path[call_path.len() - 1])
}
}
}
} }
bitflags! { bitflags! {

View File

@ -664,6 +664,7 @@ impl<'a> SemanticModel<'a> {
module: &str, module: &str,
member: &str, member: &str,
) -> Option<ImportedName> { ) -> Option<ImportedName> {
// TODO(charlie): Pass in a slice.
let module_path: Vec<&str> = module.split('.').collect(); let module_path: Vec<&str> = module.split('.').collect();
self.scopes().enumerate().find_map(|(scope_index, scope)| { self.scopes().enumerate().find_map(|(scope_index, scope)| {
scope.bindings().find_map(|(name, binding_id)| { scope.bindings().find_map(|(name, binding_id)| {

2
foo.py
View File

@ -1,2 +0,0 @@
# F401 `datastructures.UploadFile` imported but unused
from .datastructures import UploadFile as FileUpload