language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-04 18:58:26 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Generate one fix per statement for flake8-type-checking rules (#4915)

Browse source
This commit is contained in:
Charlie Marsh 2023-06-07 22:22:35 -04:00 • committed by GitHub
parent 5235977abc
commit 4b78141f6b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 733 additions and 306 deletions
Download patch file Download diff file Expand all files Collapse all files

25
crates/ruff/src/checkers/ast/mod.rs
View file

@ -5069,22 +5069,19 @@ impl<'a> Checker<'a> {
.copied()
.collect()
};
for binding_id in scope.binding_ids() {
let binding = &self.semantic_model.bindings[binding_id];
flake8_type_checking::rules::runtime_import_in_type_checking_block(
self,
binding,
&mut diagnostics,
);
flake8_type_checking::rules::runtime_import_in_type_checking_block(
self,
scope,
&mut diagnostics,
);
flake8_type_checking::rules::typing_only_runtime_import(
self,
binding,
&runtime_imports,
&mut diagnostics,
);
}
flake8_type_checking::rules::typing_only_runtime_import(
self,
scope,
&runtime_imports,
&mut diagnostics,
);
}
if self.enabled(Rule::UnusedImport) {

16
crates/ruff/src/importer/mod.rs
View file

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

42
crates/ruff/src/rules/flake8_type_checking/mod.rs
View file

@ -282,6 +282,48 @@ mod tests {
"#,
"import_from_type_checking_block"
)]
#[test_case(
r#"
from __future__ import annotations
from typing import TYPE_CHECKING
from pandas import (
DataFrame, # DataFrame
Series, # Series
)
def f(x: DataFrame, y: Series):
pass
"#,
"multiple_members"
)]
#[test_case(
r#"
from __future__ import annotations
from typing import TYPE_CHECKING
import os, sys
def f(x: os, y: sys):
pass
"#,
"multiple_modules_same_type"
)]
#[test_case(
r#"
from __future__ import annotations
from typing import TYPE_CHECKING
import os, pandas
def f(x: os, y: pandas):
pass
"#,
"multiple_modules_different_types"
)]
fn contents(contents: &str, snapshot: &str) {
let diagnostics = test_snippet(
contents,

222
crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs
View file

@ -1,11 +1,17 @@
use anyhow::Result;
use ruff_text_size::TextRange;
use rustc_hash::FxHashMap;
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::binding::Binding;
use ruff_python_semantic::node::NodeId;
use ruff_python_semantic::reference::ReferenceId;
use ruff_python_semantic::scope::Scope;
use crate::autofix;
use crate::checkers::ast::Checker;
use crate::importer::StmtImport;
use crate::registry::AsRule;
use crate::codes::Rule;
use crate::importer::StmtImports;
/// ## What it does
/// Checks for runtime imports defined in a type-checking block.
@ -61,72 +67,172 @@ impl Violation for RuntimeImportInTypeCheckingBlock {
/// TCH004
pub(crate) fn runtime_import_in_type_checking_block(
checker: &Checker,
binding: &Binding,
scope: &Scope,
diagnostics: &mut Vec<Diagnostic>,
) {
let Some(qualified_name) = binding.qualified_name() else {
return;
};
// Collect all runtime imports by statement.
let mut errors_by_statement: FxHashMap<NodeId, Vec<Import>> = FxHashMap::default();
let mut ignores_by_statement: FxHashMap<NodeId, Vec<Import>> = FxHashMap::default();
let Some(reference_id) = binding.references.first() else {
return;
};
for binding_id in scope.binding_ids() {
let binding = &checker.semantic_model().bindings[binding_id];
if binding.context.is_typing()
&& binding.references().any(|reference_id| {
checker
.semantic_model()
.references
.resolve(reference_id)
.context()
.is_runtime()
})
let Some(qualified_name) = binding.qualified_name() else {
continue;
};
let Some(reference_id) = binding.references.first().copied() else {
continue;
};
if binding.context.is_typing()
&& binding.references().any(|reference_id| {
checker
.semantic_model()
.references
.resolve(reference_id)
.context()
.is_runtime()
})
{
let Some(stmt_id) = binding.source else {
continue;
};
let import = Import {
qualified_name,
reference_id,
trimmed_range: binding.trimmed_range(checker.semantic_model(), checker.locator),
parent_range: binding.parent_range(checker.semantic_model()),
};
if checker.rule_is_ignored(
Rule::RuntimeImportInTypeCheckingBlock,
import.trimmed_range.start(),
) || import.parent_range.map_or(false, |parent_range| {
checker
.rule_is_ignored(Rule::RuntimeImportInTypeCheckingBlock, parent_range.start())
}) {
ignores_by_statement
.entry(stmt_id)
.or_default()
.push(import);
} else {
errors_by_statement.entry(stmt_id).or_default().push(import);
}
}
}
// Generate a diagnostic for every import, but share a fix across all imports within the same
// statement (excluding those that are ignored).
for (stmt_id, imports) in errors_by_statement {
let fix = if checker.patch(Rule::RuntimeImportInTypeCheckingBlock) {
fix_imports(checker, stmt_id, &imports).ok()
} else {
None
};
for Import {
qualified_name,
trimmed_range,
parent_range,
..
} in imports
{
let mut diagnostic = Diagnostic::new(
RuntimeImportInTypeCheckingBlock {
qualified_name: qualified_name.to_string(),
},
trimmed_range,
);
if let Some(range) = parent_range {
diagnostic.set_parent(range.start());
}
if let Some(fix) = fix.as_ref() {
diagnostic.set_fix(fix.clone());
}
diagnostics.push(diagnostic);
}
}
// Separately, generate a diagnostic for every _ignored_ import, to ensure that the
// suppression comments aren't marked as unused.
for Import {
qualified_name,
trimmed_range,
parent_range,
..
} in ignores_by_statement.into_values().flatten()
{
let mut diagnostic = Diagnostic::new(
RuntimeImportInTypeCheckingBlock {
qualified_name: qualified_name.to_string(),
},
binding.trimmed_range(checker.semantic_model(), checker.locator),
trimmed_range,
);
if let Some(range) = binding.parent_range(checker.semantic_model()) {
if let Some(range) = parent_range {
diagnostic.set_parent(range.start());
}
if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| {
// Step 1) Remove the import.
// SAFETY: All non-builtin bindings have a source.
let source = binding.source.unwrap();
let stmt = checker.semantic_model().stmts[source];
let parent = checker.semantic_model().stmts.parent(stmt);
let remove_import_edit = autofix::edits::remove_unused_imports(
std::iter::once(qualified_name),
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
)?;
// Step 2) Add the import to the top-level.
let reference = checker.semantic_model().references.resolve(*reference_id);
let add_import_edit = checker.importer.runtime_import_edit(
&StmtImport {
stmt,
qualified_name,
},
reference.range().start(),
)?;
Ok(
Fix::suggested_edits(remove_import_edit, add_import_edit.into_edits())
.isolate(checker.isolation(parent)),
)
});
}
if checker.enabled(diagnostic.kind.rule()) {
diagnostics.push(diagnostic);
}
diagnostics.push(diagnostic);
}
}
/// A runtime-required import with its surrounding context.
struct Import<'a> {
/// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
qualified_name: &'a str,
/// The first reference to the imported symbol.
reference_id: ReferenceId,
/// The trimmed range of the import (e.g., `List` in `from typing import List`).
trimmed_range: TextRange,
/// The range of the import's parent statement.
parent_range: Option<TextRange>,
}
/// Generate a [`Fix`] to remove runtime imports from a type-checking block.
fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result<Fix> {
let stmt = checker.semantic_model().stmts[stmt_id];
let parent = checker.semantic_model().stmts.parent(stmt);
let qualified_names: Vec<&str> = imports
.iter()
.map(|Import { qualified_name, .. }| *qualified_name)
.collect();
// Find the first reference across all imports.
let at = imports
.iter()
.map(|Import { reference_id, .. }| {
checker
.semantic_model()
.references
.resolve(*reference_id)
.range()
.start()
})
.min()
.expect("Expected at least one import");
// Step 1) Remove the import.
let remove_import_edit = autofix::edits::remove_unused_imports(
qualified_names.iter().copied(),
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
)?;
// Step 2) Add the import to the top-level.
let add_import_edit = checker.importer.runtime_import_edit(
&StmtImports {
stmt,
qualified_names,
},
at,
)?;
Ok(
Fix::suggested_edits(remove_import_edit, add_import_edit.into_edits())
.isolate(checker.isolation(parent)),
)
}

383
crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs
View file

@ -1,11 +1,18 @@
use anyhow::Result;
use ruff_text_size::TextRange;
use rustc_hash::FxHashMap;
use ruff_diagnostics::{AutofixKind, Diagnostic, DiagnosticKind, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::binding::Binding;
use ruff_python_semantic::node::NodeId;
use ruff_python_semantic::reference::ReferenceId;
use ruff_python_semantic::scope::Scope;
use crate::autofix;
use crate::checkers::ast::Checker;
use crate::importer::StmtImport;
use crate::registry::AsRule;
use crate::codes::Rule;
use crate::importer::StmtImports;
use crate::rules::isort::{categorize, ImportSection, ImportType};
/// ## What it does
@ -176,6 +183,211 @@ impl Violation for TypingOnlyStandardLibraryImport {
}
}
/// TCH001, TCH002, TCH003
pub(crate) fn typing_only_runtime_import(
checker: &Checker,
scope: &Scope,
runtime_imports: &[&Binding],
diagnostics: &mut Vec<Diagnostic>,
) {
// Collect all typing-only imports by statement and import type.
let mut errors_by_statement: FxHashMap<(NodeId, ImportType), Vec<Import>> =
FxHashMap::default();
let mut ignores_by_statement: FxHashMap<(NodeId, ImportType), Vec<Import>> =
FxHashMap::default();
for binding_id in scope.binding_ids() {
let binding = &checker.semantic_model().bindings[binding_id];
// If we're in un-strict mode, don't flag typing-only imports that are
// implicitly loaded by way of a valid runtime import.
if !checker.settings.flake8_type_checking.strict
&& runtime_imports
.iter()
.any(|import| is_implicit_import(binding, import))
{
continue;
}
let Some(qualified_name) = binding.qualified_name() else {
continue;
};
if is_exempt(
qualified_name,
&checker
.settings
.flake8_type_checking
.exempt_modules
.iter()
.map(String::as_str)
.collect::<Vec<_>>(),
) {
continue;
}
let Some(reference_id) = binding.references.first().copied() else {
continue;
};
if binding.context.is_runtime()
&& binding.references().all(|reference_id| {
checker
.semantic_model()
.references
.resolve(reference_id)
.context()
.is_typing()
})
{
// Extract the module base and level from the full name.
// Ex) `foo.bar.baz` -> `foo`, `0`
// Ex) `.foo.bar.baz` -> `foo`, `1`
let level = qualified_name
.chars()
.take_while(|c| *c == '.')
.count()
.try_into()
.unwrap();
// Categorize the import, using coarse-grained categorization.
let import_type = match categorize(
qualified_name,
Some(level),
&checker.settings.src,
checker.package(),
&checker.settings.isort.known_modules,
checker.settings.target_version,
) {
ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => {
ImportType::FirstParty
}
ImportSection::Known(ImportType::ThirdParty) | ImportSection::UserDefined(_) => {
ImportType::ThirdParty
}
ImportSection::Known(ImportType::StandardLibrary) => ImportType::StandardLibrary,
ImportSection::Known(ImportType::Future) => {
continue;
}
};
if !checker.enabled(rule_for(import_type)) {
continue;
}
let Some(stmt_id) = binding.source else {
continue;
};
let import = Import {
qualified_name,
reference_id,
trimmed_range: binding.trimmed_range(checker.semantic_model(), checker.locator),
parent_range: binding.parent_range(checker.semantic_model()),
};
if checker.rule_is_ignored(rule_for(import_type), import.trimmed_range.start())
|| import.parent_range.map_or(false, |parent_range| {
checker.rule_is_ignored(rule_for(import_type), parent_range.start())
})
{
ignores_by_statement
.entry((stmt_id, import_type))
.or_default()
.push(import);
} else {
errors_by_statement
.entry((stmt_id, import_type))
.or_default()
.push(import);
}
}
}
// Generate a diagnostic for every import, but share a fix across all imports within the same
// statement (excluding those that are ignored).
for ((stmt_id, import_type), imports) in errors_by_statement {
let fix = if checker.patch(rule_for(import_type)) {
fix_imports(checker, stmt_id, &imports).ok()
} else {
None
};
for Import {
qualified_name,
trimmed_range,
parent_range,
..
} in imports
{
let mut diagnostic = Diagnostic::new(
diagnostic_for(import_type, qualified_name.to_string()),
trimmed_range,
);
if let Some(range) = parent_range {
diagnostic.set_parent(range.start());
}
if let Some(fix) = fix.as_ref() {
diagnostic.set_fix(fix.clone());
}
diagnostics.push(diagnostic);
}
}
// Separately, generate a diagnostic for every _ignored_ import, to ensure that the
// suppression comments aren't marked as unused.
for ((_, import_type), imports) in ignores_by_statement {
for Import {
qualified_name,
trimmed_range,
parent_range,
..
} in imports
{
let mut diagnostic = Diagnostic::new(
diagnostic_for(import_type, qualified_name.to_string()),
trimmed_range,
);
if let Some(range) = parent_range {
diagnostic.set_parent(range.start());
}
diagnostics.push(diagnostic);
}
}
}
/// A runtime-required import with its surrounding context.
struct Import<'a> {
/// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
qualified_name: &'a str,
/// The first reference to the imported symbol.
reference_id: ReferenceId,
/// The trimmed range of the import (e.g., `List` in `from typing import List`).
trimmed_range: TextRange,
/// The range of the import's parent statement.
parent_range: Option<TextRange>,
}
/// Return the [`Rule`] for the given import type.
fn rule_for(import_type: ImportType) -> Rule {
match import_type {
ImportType::StandardLibrary => Rule::TypingOnlyStandardLibraryImport,
ImportType::ThirdParty => Rule::TypingOnlyThirdPartyImport,
ImportType::FirstParty => Rule::TypingOnlyFirstPartyImport,
_ => unreachable!("Unexpected import type"),
}
}
/// Return the [`Diagnostic`] for the given import type.
fn diagnostic_for(import_type: ImportType, qualified_name: String) -> DiagnosticKind {
match import_type {
ImportType::StandardLibrary => TypingOnlyStandardLibraryImport { qualified_name }.into(),
ImportType::ThirdParty => TypingOnlyThirdPartyImport { qualified_name }.into(),
ImportType::FirstParty => TypingOnlyFirstPartyImport { qualified_name }.into(),
_ => unreachable!("Unexpected import type"),
}
}
/// Return `true` if `this` is implicitly loaded via importing `that`.
fn is_implicit_import(this: &Binding, that: &Binding) -> bool {
let Some(this_module) = this.module_name() else {
@ -203,140 +415,51 @@ fn is_exempt(name: &str, exempt_modules: &[&str]) -> bool {
}
}
/// TCH001, TCH002, TCH003
pub(crate) fn typing_only_runtime_import(
checker: &Checker,
binding: &Binding,
runtime_imports: &[&Binding],
diagnostics: &mut Vec<Diagnostic>,
) {
// If we're in un-strict mode, don't flag typing-only imports that are
// implicitly loaded by way of a valid runtime import.
if !checker.settings.flake8_type_checking.strict
&& runtime_imports
.iter()
.any(|import| is_implicit_import(binding, import))
{
return;
}
/// Generate a [`Fix`] to remove typing-only imports from a runtime context.
fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result<Fix> {
let stmt = checker.semantic_model().stmts[stmt_id];
let parent = checker.semantic_model().stmts.parent(stmt);
let qualified_names: Vec<&str> = imports
.iter()
.map(|Import { qualified_name, .. }| *qualified_name)
.collect();
let Some(qualified_name) = binding.qualified_name() else {
return;
};
if is_exempt(
qualified_name,
&checker
.settings
.flake8_type_checking
.exempt_modules
.iter()
.map(String::as_str)
.collect::<Vec<_>>(),
) {
return;
}
let Some(reference_id) = binding.references.first() else {
return;
};
if binding.context.is_runtime()
&& binding.is_used()
&& binding.references().all(|reference_id| {
// Find the first reference across all imports.
let at = imports
.iter()
.map(|Import { reference_id, .. }| {
checker
.semantic_model()
.references
.resolve(reference_id)
.context()
.is_typing()
.resolve(*reference_id)
.range()
.start()
})
{
// Extract the module base and level from the full name.
// Ex) `foo.bar.baz` -> `foo`, `0`
// Ex) `.foo.bar.baz` -> `foo`, `1`
let level = qualified_name
.chars()
.take_while(|c| *c == '.')
.count()
.try_into()
.unwrap();
.min()
.expect("Expected at least one import");
// Categorize the import.
let kind: DiagnosticKind = match categorize(
qualified_name,
Some(level),
&checker.settings.src,
checker.package(),
&checker.settings.isort.known_modules,
checker.settings.target_version,
) {
ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => {
TypingOnlyFirstPartyImport {
qualified_name: qualified_name.to_string(),
}
.into()
}
ImportSection::Known(ImportType::ThirdParty) | ImportSection::UserDefined(_) => {
TypingOnlyThirdPartyImport {
qualified_name: qualified_name.to_string(),
}
.into()
}
ImportSection::Known(ImportType::StandardLibrary) => TypingOnlyStandardLibraryImport {
qualified_name: qualified_name.to_string(),
}
.into(),
// Step 1) Remove the import.
let remove_import_edit = autofix::edits::remove_unused_imports(
qualified_names.iter().copied(),
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
)?;
ImportSection::Known(ImportType::Future) => {
unreachable!("`__future__` imports should be marked as used")
}
};
// Step 2) Add the import to a `TYPE_CHECKING` block.
let add_import_edit = checker.importer.typing_import_edit(
&StmtImports {
stmt,
qualified_names,
},
at,
checker.semantic_model(),
)?;
let mut diagnostic = Diagnostic::new(
kind,
binding.trimmed_range(checker.semantic_model(), checker.locator),
);
if let Some(range) = binding.parent_range(checker.semantic_model()) {
diagnostic.set_parent(range.start());
}
if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| {
// Step 1) Remove the import.
// SAFETY: All non-builtin bindings have a source.
let source = binding.source.unwrap();
let stmt = checker.semantic_model().stmts[source];
let parent = checker.semantic_model().stmts.parent(stmt);
let remove_import_edit = autofix::edits::remove_unused_imports(
std::iter::once(qualified_name),
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
)?;
// Step 2) Add the import to a `TYPE_CHECKING` block.
let reference = checker.semantic_model().references.resolve(*reference_id);
let add_import_edit = checker.importer.typing_import_edit(
&StmtImport {
stmt,
qualified_name,
},
reference.range().start(),
checker.semantic_model(),
)?;
Ok(
Fix::suggested_edits(remove_import_edit, add_import_edit.into_edits())
.isolate(checker.isolation(parent)),
)
});
}
if checker.enabled(diagnostic.kind.rule()) {
diagnostics.push(diagnostic);
}
}
Ok(
Fix::suggested_edits(remove_import_edit, add_import_edit.into_edits())
.isolate(checker.isolation(parent)),
)
}

60
crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__multiple_members.snap Normal file
View file

@ -0,0 +1,60 @@
---
source: crates/ruff/src/rules/flake8_type_checking/mod.rs
---
<filename>:7:5: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block
|
7 | from pandas import (
8 | DataFrame, # DataFrame
| ^^^^^^^^^ TCH002
9 | Series, # Series
10 | )
|
= help: Move into type-checking block
ℹ Suggested fix
3 3 |
4 4 | from typing import TYPE_CHECKING
5 5 |
6 |-from pandas import (
7 |- DataFrame, # DataFrame
8 |- Series, # Series
9 |-)
10 6 |
7 |+if TYPE_CHECKING:
8 |+ from pandas import (
9 |+ DataFrame, # DataFrame
10 |+ Series, # Series
11 |+ )
12 |+
11 13 | def f(x: DataFrame, y: Series):
12 14 | pass
<filename>:8:5: TCH002 [*] Move third-party import `pandas.Series` into a type-checking block
|
8 | from pandas import (
9 | DataFrame, # DataFrame
10 | Series, # Series
| ^^^^^^ TCH002
11 | )
|
= help: Move into type-checking block
ℹ Suggested fix
3 3 |
4 4 | from typing import TYPE_CHECKING
5 5 |
6 |-from pandas import (
7 |- DataFrame, # DataFrame
8 |- Series, # Series
9 |-)
10 6 |
7 |+if TYPE_CHECKING:
8 |+ from pandas import (
9 |+ DataFrame, # DataFrame
10 |+ Series, # Series
11 |+ )
12 |+
11 13 | def f(x: DataFrame, y: Series):
12 14 | pass

52
crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__multiple_modules_different_types.snap Normal file
View file

@ -0,0 +1,52 @@
---
source: crates/ruff/src/rules/flake8_type_checking/mod.rs
---
<filename>:6:8: TCH003 [*] Move standard library import `os` into a type-checking block
|
6 | from typing import TYPE_CHECKING
7 |
8 | import os, pandas
| ^^ TCH003
9 |
10 | def f(x: os, y: pandas):
|
= help: Move into type-checking block
ℹ Suggested fix
3 3 |
4 4 | from typing import TYPE_CHECKING
5 5 |
6 |-import os, pandas
6 |+import pandas
7 |+
8 |+if TYPE_CHECKING:
9 |+ import os
7 10 |
8 11 | def f(x: os, y: pandas):
9 12 | pass
<filename>:6:12: TCH002 [*] Move third-party import `pandas` into a type-checking block
|
6 | from typing import TYPE_CHECKING
7 |
8 | import os, pandas
| ^^^^^^ TCH002
9 |
10 | def f(x: os, y: pandas):
|
= help: Move into type-checking block
ℹ Suggested fix
3 3 |
4 4 | from typing import TYPE_CHECKING
5 5 |
6 |-import os, pandas
6 |+import os
7 |+
8 |+if TYPE_CHECKING:
9 |+ import pandas
7 10 |
8 11 | def f(x: os, y: pandas):
9 12 | pass

50
crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__multiple_modules_same_type.snap Normal file
View file

@ -0,0 +1,50 @@
---
source: crates/ruff/src/rules/flake8_type_checking/mod.rs
---
<filename>:6:8: TCH003 [*] Move standard library import `os` into a type-checking block
|
6 | from typing import TYPE_CHECKING
7 |
8 | import os, sys
| ^^ TCH003
9 |
10 | def f(x: os, y: sys):
|
= help: Move into type-checking block
ℹ Suggested fix
3 3 |
4 4 | from typing import TYPE_CHECKING
5 5 |
6 |-import os, sys
7 6 |
7 |+if TYPE_CHECKING:
8 |+ import os, sys
9 |+
8 10 | def f(x: os, y: sys):
9 11 | pass
<filename>:6:12: TCH003 [*] Move standard library import `sys` into a type-checking block
|
6 | from typing import TYPE_CHECKING
7 |
8 | import os, sys
| ^^^ TCH003
9 |
10 | def f(x: os, y: sys):
|
= help: Move into type-checking block
ℹ Suggested fix
3 3 |
4 4 | from typing import TYPE_CHECKING
5 5 |
6 |-import os, sys
7 6 |
7 |+if TYPE_CHECKING:
8 |+ import os, sys
9 |+
8 10 | def f(x: os, y: sys):
9 11 | pass

12
crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_TCH004_5.py.snap
View file

@ -11,11 +11,11 @@ TCH004_5.py:4:24: TCH004 [*] Move import `typing.List` out of type-checking bloc
ℹ Suggested fix
1 1 | from typing import TYPE_CHECKING
2 |+from typing import List
2 |+from typing import List, Sequence, Set
2 3 |
3 4 | if TYPE_CHECKING:
4 |- from typing import List, Sequence, Set
5 |+ from typing import Sequence, Set
5 |+ pass
5 6 |
6 7 |
7 8 | def example(a: List[int], /, b: Sequence[int], *, c: Set[int]):
@ -30,11 +30,11 @@ TCH004_5.py:4:30: TCH004 [*] Move import `typing.Sequence` out of type-checking
ℹ Suggested fix
1 1 | from typing import TYPE_CHECKING
2 |+from typing import Sequence
2 |+from typing import List, Sequence, Set
2 3 |
3 4 | if TYPE_CHECKING:
4 |- from typing import List, Sequence, Set
5 |+ from typing import List, Set
5 |+ pass
5 6 |
6 7 |
7 8 | def example(a: List[int], /, b: Sequence[int], *, c: Set[int]):
@ -49,11 +49,11 @@ TCH004_5.py:4:40: TCH004 [*] Move import `typing.Set` out of type-checking block
ℹ Suggested fix
1 1 | from typing import TYPE_CHECKING
2 |+from typing import Set
2 |+from typing import List, Sequence, Set
2 3 |
3 4 | if TYPE_CHECKING:
4 |- from typing import List, Sequence, Set
5 |+ from typing import List, Sequence
5 |+ pass
5 6 |
6 7 |
7 8 | def example(a: List[int], /, b: Sequence[int], *, c: Set[int]):

8
crates/ruff/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_TCH004_9.py.snap
View file

@ -13,11 +13,11 @@ TCH004_9.py:4:24: TCH004 [*] Move import `typing.Tuple` out of type-checking blo
ℹ Suggested fix
1 1 | from typing import TYPE_CHECKING
2 |+from typing import Tuple
2 |+from typing import Tuple, List
2 3 |
3 4 | if TYPE_CHECKING:
4 |- from typing import Tuple, List, Dict
5 |+ from typing import List, Dict
5 |+ from typing import Dict
5 6 |
6 7 | x: Tuple
7 8 |
@ -34,11 +34,11 @@ TCH004_9.py:4:31: TCH004 [*] Move import `typing.List` out of type-checking bloc
ℹ Suggested fix
1 1 | from typing import TYPE_CHECKING
2 |+from typing import List
2 |+from typing import Tuple, List
2 3 |
3 4 | if TYPE_CHECKING:
4 |- from typing import Tuple, List, Dict
5 |+ from typing import Tuple, Dict
5 |+ from typing import Dict
5 6 |
6 7 | x: Tuple
7 8 |

169
crates/ruff/src/rules/pyflakes/rules/unused_import.rs
View file

@ -1,9 +1,8 @@
use itertools::Itertools;
use anyhow::Result;
use ruff_text_size::TextRange;
use rustc_hash::FxHashMap;
use rustpython_parser::ast::Ranged;
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, IsolationLevel, Violation};
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::binding::Exceptions;
use ruff_python_semantic::node::NodeId;
@ -82,8 +81,7 @@ impl Violation for UnusedImport {
}
Some(UnusedImportContext::Init) => {
format!(
"`{name}` imported but unused; consider adding to `__all__` or using a redundant \
alias"
"`{name}` imported but unused; consider adding to `__all__` or using a redundant alias"
)
}
None => format!("`{name}` imported but unused"),
@ -100,13 +98,10 @@ impl Violation for UnusedImport {
}
}
type SpannedName<'a> = (&'a str, TextRange);
type BindingContext = (NodeId, Option<NodeId>, Exceptions);
pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut Vec<Diagnostic>) {
// Collect all unused imports by statement.
let mut unused: FxHashMap<BindingContext, Vec<SpannedName>> = FxHashMap::default();
let mut ignored: FxHashMap<BindingContext, Vec<SpannedName>> = FxHashMap::default();
let mut unused: FxHashMap<(NodeId, Exceptions), Vec<Import>> = FxHashMap::default();
let mut ignored: FxHashMap<(NodeId, Exceptions), Vec<Import>> = FxHashMap::default();
for binding_id in scope.binding_ids() {
let binding = &checker.semantic_model().bindings[binding_id];
@ -119,67 +114,55 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
continue;
};
let stmt_id = binding.source.unwrap();
let parent_id = checker.semantic_model().stmts.parent_id(stmt_id);
let exceptions = binding.exceptions;
let diagnostic_offset = binding.range.start();
let stmt = &checker.semantic_model().stmts[stmt_id];
let parent_offset = if stmt.is_import_from_stmt() {
Some(stmt.start())
} else {
None
let Some(stmt_id) = binding.source else {
continue;
};
if checker.rule_is_ignored(Rule::UnusedImport, diagnostic_offset)
|| parent_offset.map_or(false, |parent_offset| {
checker.rule_is_ignored(Rule::UnusedImport, parent_offset)
let import = Import {
qualified_name,
trimmed_range: binding.trimmed_range(checker.semantic_model(), checker.locator),
parent_range: binding.parent_range(checker.semantic_model()),
};
if checker.rule_is_ignored(Rule::UnusedImport, import.trimmed_range.start())
|| import.parent_range.map_or(false, |parent_range| {
checker.rule_is_ignored(Rule::UnusedImport, parent_range.start())
})
{
ignored
.entry((stmt_id, parent_id, exceptions))
.entry((stmt_id, binding.exceptions))
.or_default()
.push((qualified_name, binding.range));
.push(import);
} else {
unused
.entry((stmt_id, parent_id, exceptions))
.entry((stmt_id, binding.exceptions))
.or_default()
.push((qualified_name, binding.range));
.push(import);
}
}
let in_init =
checker.settings.ignore_init_module_imports && checker.path().ends_with("__init__.py");
// Generate a diagnostic for every unused import, but share a fix across all unused imports
// within the same statement (excluding those that are ignored).
for ((stmt_id, parent_id, exceptions), unused_imports) in unused
.into_iter()
.sorted_by_key(|((defined_by, ..), ..)| *defined_by)
{
let stmt = checker.semantic_model().stmts[stmt_id];
let parent = parent_id.map(|parent_id| checker.semantic_model().stmts[parent_id]);
let multiple = unused_imports.len() > 1;
// Generate a diagnostic for every import, but share a fix across all imports within the same
// statement (excluding those that are ignored).
for ((stmt_id, exceptions), imports) in unused {
let in_except_handler =
exceptions.intersects(Exceptions::MODULE_NOT_FOUND_ERROR | Exceptions::IMPORT_ERROR);
let multiple = imports.len() > 1;
let fix = if !in_init && !in_except_handler && checker.patch(Rule::UnusedImport) {
autofix::edits::remove_unused_imports(
unused_imports
.iter()
.map(|(qualified_name, _)| *qualified_name),
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
)
.ok()
fix_imports(checker, stmt_id, &imports).ok()
} else {
None
};
for (qualified_name, range) in unused_imports {
for Import {
qualified_name,
trimmed_range,
parent_range,
} in imports
{
let mut diagnostic = Diagnostic::new(
UnusedImport {
name: qualified_name.to_string(),
@ -192,52 +175,66 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
},
multiple,
},
range,
trimmed_range,
);
if stmt.is_import_from_stmt() {
diagnostic.set_parent(stmt.start());
if let Some(range) = parent_range {
diagnostic.set_parent(range.start());
}
if let Some(edit) = fix.as_ref() {
diagnostic.set_fix(Fix::automatic(edit.clone()).isolate(
parent_id.map_or(IsolationLevel::default(), |node_id| {
IsolationLevel::Group(node_id.into())
}),
));
if !in_except_handler {
if let Some(fix) = fix.as_ref() {
diagnostic.set_fix(fix.clone());
}
}
diagnostics.push(diagnostic);
}
}
// Separately, generate a diagnostic for every _ignored_ unused import, but don't bother
// creating a fix. We have to generate these diagnostics, even though they'll be ignored later
// on, so that the suppression comments themselves aren't marked as unnecessary.
for ((stmt_id, .., exceptions), unused_imports) in ignored
.into_iter()
.sorted_by_key(|((stmt_id, ..), ..)| *stmt_id)
// Separately, generate a diagnostic for every _ignored_ import, to ensure that the
// suppression comments aren't marked as unused.
for Import {
qualified_name,
trimmed_range,
parent_range,
} in ignored.into_values().flatten()
{
let stmt = checker.semantic_model().stmts[stmt_id];
let multiple = unused_imports.len() > 1;
let in_except_handler =
exceptions.intersects(Exceptions::MODULE_NOT_FOUND_ERROR | Exceptions::IMPORT_ERROR);
for (qualified_name, range) in unused_imports {
let mut diagnostic = Diagnostic::new(
UnusedImport {
name: qualified_name.to_string(),
context: if in_except_handler {
Some(UnusedImportContext::ExceptHandler)
} else if in_init {
Some(UnusedImportContext::Init)
} else {
None
},
multiple,
},
range,
);
if stmt.is_import_from_stmt() {
diagnostic.set_parent(stmt.start());
}
diagnostics.push(diagnostic);
let mut diagnostic = Diagnostic::new(
UnusedImport {
name: qualified_name.to_string(),
context: None,
multiple: false,
},
trimmed_range,
);
if let Some(range) = parent_range {
diagnostic.set_parent(range.start());
}
diagnostics.push(diagnostic);
}
}
/// An unused import with its surrounding context.
struct Import<'a> {
/// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
qualified_name: &'a str,
/// The trimmed range of the import (e.g., `List` in `from typing import List`).
trimmed_range: TextRange,
/// The range of the import's parent statement.
parent_range: Option<TextRange>,
}
/// Generate a [`Fix`] to remove unused imports from a statement.
fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result<Fix> {
let stmt = checker.semantic_model().stmts[stmt_id];
let parent = checker.semantic_model().stmts.parent(stmt);
let edit = autofix::edits::remove_unused_imports(
imports
.iter()
.map(|Import { qualified_name, .. }| *qualified_name),
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
)?;
Ok(Fix::automatic(edit).isolate(checker.isolation(parent)))
}