diff --git a/README.md b/README.md index d087ead03973d..660bdbdf22582 100644 --- a/README.md +++ b/README.md @@ -1202,6 +1202,9 @@ For more, see [flake8-type-checking](https://pypi.org/project/flake8-type-checki | Code | Name | Message | Fix | | ---- | ---- | ------- | --- | +| TYP001 | typing-only-first-party-import | Move application import `{}` into a type-checking block | | +| TYP002 | typing-only-third-party-import | Move third-party import `{}` into a type-checking block | | +| TYP003 | typing-only-standard-library-import | Move standard library import `{}` into a type-checking block | | | TYP004 | runtime-import-in-type-checking-block | Move import `{}` out of type-checking block. Import is used for more than type hinting. | | | TYP005 | empty-type-checking-block | Found empty type-checking block | | diff --git a/resources/test/fixtures/flake8_type_checking/TYP001.py b/resources/test/fixtures/flake8_type_checking/TYP001.py new file mode 100644 index 0000000000000..ce1cc042b5c0a --- /dev/null +++ b/resources/test/fixtures/flake8_type_checking/TYP001.py @@ -0,0 +1,28 @@ +"""Tests to determine first-party import classification. + +For typing-only import detection tests, see `TYP002.py`. +""" + + +def f(): + import TYP001 + + x: TYP001 + + +def f(): + import TYP001 + + print(TYP001) + + +def f(): + from . import TYP001 + + x: TYP001 + + +def f(): + from . import TYP001 + + print(TYP001) diff --git a/resources/test/fixtures/flake8_type_checking/TYP002.py b/resources/test/fixtures/flake8_type_checking/TYP002.py new file mode 100644 index 0000000000000..9ea3aa0cc3f1c --- /dev/null +++ b/resources/test/fixtures/flake8_type_checking/TYP002.py @@ -0,0 +1,148 @@ +"""Tests to determine accurate detection of typing-only imports.""" + + +def f(): + import pandas as pd # TYP002 + + x: pd.DataFrame + + +def f(): + from pandas import DataFrame # TYP002 + + x: DataFrame + + +def f(): + from pandas import DataFrame as df # TYP002 + + x: df + + +def f(): + import pandas as pd # TYP002 + + x: pd.DataFrame = 1 + + +def f(): + from pandas import DataFrame # TYP002 + + x: DataFrame = 2 + + +def f(): + from pandas import DataFrame as df # TYP002 + + x: df = 3 + + +def f(): + import pandas as pd # TYP002 + + x: "pd.DataFrame" = 1 + + +def f(): + import pandas as pd + + x = dict["pd.DataFrame", "pd.DataFrame"] # TYP002 + + +def f(): + import pandas as pd + + print(pd) + + +def f(): + from pandas import DataFrame + + print(DataFrame) + + +def f(): + from pandas import DataFrame + + def f(): + print(DataFrame) + + +def f(): + from typing import Dict, Any + + def example() -> Any: + return 1 + + x: Dict[int] = 20 + + +def f(): + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from typing import Dict + x: Dict[int] = 20 + + +def f(): + from pathlib import Path + + class ImportVisitor(ast.NodeTransformer): + def __init__(self, cwd: Path) -> None: + self.cwd = cwd + origin = Path(spec.origin) + + class ExampleClass: + def __init__(self): + self.cwd = Path(pandas.getcwd()) + + +def f(): + import pandas + + class Migration: + enum = pandas + + +def f(): + import pandas + + class Migration: + enum = pandas.EnumClass + + +def f(): + from typing import TYPE_CHECKING + + from pandas import y + + if TYPE_CHECKING: + _type = x + else: + _type = y + + +def f(): + from typing import TYPE_CHECKING + + from pandas import y + + if TYPE_CHECKING: + _type = x + elif True: + _type = y + + +def f(): + from typing import cast + + import pandas as pd + + x = cast(pd.DataFrame, 2) + + +def f(): + import pandas as pd + + x = dict[pd.DataFrame, pd.DataFrame] diff --git a/resources/test/fixtures/flake8_type_checking/TYP003.py b/resources/test/fixtures/flake8_type_checking/TYP003.py new file mode 100644 index 0000000000000..face13f3db089 --- /dev/null +++ b/resources/test/fixtures/flake8_type_checking/TYP003.py @@ -0,0 +1,16 @@ +"""Tests to determine standard library import classification. + +For typing-only import detection tests, see `TYP002.py`. +""" + + +def f(): + import os + + x: os + + +def f(): + import os + + print(os) diff --git a/ruff.schema.json b/ruff.schema.json index 6d9137d41d407..f991eca97de31 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1784,6 +1784,9 @@ "TYP", "TYP0", "TYP00", + "TYP001", + "TYP002", + "TYP003", "TYP004", "TYP005", "UP", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 011c0e58331df..ae69abe53d235 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -54,6 +54,7 @@ type DeferralContext<'a> = (Vec, Vec>); pub struct Checker<'a> { // Input data. path: &'a Path, + package: Option<&'a Path>, autofix: flags::Autofix, noqa: flags::Noqa, pub(crate) settings: &'a Settings, @@ -110,6 +111,7 @@ impl<'a> Checker<'a> { autofix: flags::Autofix, noqa: flags::Noqa, path: &'a Path, + package: Option<&'a Path>, locator: &'a Locator, style: &'a Stylist, indexer: &'a Indexer, @@ -120,6 +122,7 @@ impl<'a> Checker<'a> { autofix, noqa, path, + package, locator, stylist: style, indexer, @@ -320,8 +323,6 @@ where } } - let prev_in_type_checking_block = self.in_type_checking_block; - // Pre-visit. match &stmt.node { StmtKind::Global { names } => { @@ -1396,13 +1397,6 @@ where self.handle_node_load(target); } StmtKind::If { test, body, orelse } => { - if flake8_type_checking::helpers::is_type_checking_block(self, test) { - if self.settings.rules.enabled(&Rule::EmptyTypeCheckingBlock) { - flake8_type_checking::rules::empty_type_checking_block(self, test, body); - } - self.in_type_checking_block = true; - self.type_checking_blocks.push(stmt); - } if self.settings.rules.enabled(&Rule::IfTuple) { pyflakes::rules::if_tuple(self, stmt, test); } @@ -1859,9 +1853,27 @@ where } self.visit_expr(target); } + StmtKind::If { test, body, orelse } => { + self.visit_expr(test); + + if flake8_type_checking::helpers::is_type_checking_block(self, test) { + if self.settings.rules.enabled(&Rule::EmptyTypeCheckingBlock) { + flake8_type_checking::rules::empty_type_checking_block(self, test, body); + } + self.type_checking_blocks.push(stmt); + + let prev_in_type_checking_block = self.in_type_checking_block; + self.in_type_checking_block = true; + self.visit_body(body); + self.in_type_checking_block = prev_in_type_checking_block; + } else { + self.visit_body(body); + } + + self.visit_body(orelse); + } _ => visitor::walk_stmt(self, stmt), }; - self.in_type_checking_block = prev_in_type_checking_block; self.visible_scope = prev_visible_scope; // Post-visit. @@ -3823,7 +3835,11 @@ impl<'a> Checker<'a> { } if let Some(index) = scope.values.get(&id.as_str()) { - let context = if self.in_type_definition || self.in_type_checking_block { + let context = if self.in_type_checking_block + || self.in_annotation + || self.in_deferred_string_type_definition + || self.in_deferred_type_definition + { UsageContext::Typing } else { UsageContext::Runtime @@ -4290,18 +4306,30 @@ impl<'a> Checker<'a> { } fn check_dead_scopes(&mut self) { - if !self.settings.rules.enabled(&Rule::UnusedImport) - && !self.settings.rules.enabled(&Rule::ImportStarUsage) - && !self.settings.rules.enabled(&Rule::RedefinedWhileUnused) - && !self.settings.rules.enabled(&Rule::UndefinedExport) - && !self + if !(self.settings.rules.enabled(&Rule::UnusedImport) + || self.settings.rules.enabled(&Rule::ImportStarUsage) + || self.settings.rules.enabled(&Rule::RedefinedWhileUnused) + || self.settings.rules.enabled(&Rule::UndefinedExport) + || self .settings .rules .enabled(&Rule::GlobalVariableNotAssigned) - && !self + || self .settings .rules .enabled(&Rule::RuntimeImportInTypeCheckingBlock) + || self + .settings + .rules + .enabled(&Rule::TypingOnlyFirstPartyImport) + || self + .settings + .rules + .enabled(&Rule::TypingOnlyThirdPartyImport) + || self + .settings + .rules + .enabled(&Rule::TypingOnlyStandardLibraryImport)) { return; } @@ -4434,21 +4462,50 @@ impl<'a> Checker<'a> { } } - // TYP001 - for (_name, index) in scope - .values - .iter() - .chain(scope.overridden.iter().map(|(a, b)| (a, b))) + if self + .settings + .rules + .enabled(&Rule::RuntimeImportInTypeCheckingBlock) + || self + .settings + .rules + .enabled(&Rule::TypingOnlyFirstPartyImport) + || self + .settings + .rules + .enabled(&Rule::TypingOnlyThirdPartyImport) + || self + .settings + .rules + .enabled(&Rule::TypingOnlyStandardLibraryImport) { - let binding = &self.bindings[*index]; - - if let Some(diagnostic) = - flake8_type_checking::rules::runtime_import_in_type_checking_block( - binding, - &self.type_checking_blocks, - ) + for (.., index) in scope + .values + .iter() + .chain(scope.overridden.iter().map(|(a, b)| (a, b))) { - diagnostics.push(diagnostic); + let binding = &self.bindings[*index]; + + if let Some(diagnostic) = + flake8_type_checking::rules::runtime_import_in_type_checking_block( + binding, + &self.type_checking_blocks, + ) + { + diagnostics.push(diagnostic); + } + if let Some(diagnostic) = + flake8_type_checking::rules::typing_only_runtime_import( + binding, + &self.type_checking_blocks, + self.package, + self.settings, + ) + { + if self.settings.rules.enabled(diagnostic.kind.rule()) { + diagnostics.push(diagnostic); + } + } } } @@ -4944,6 +5001,7 @@ pub fn check_ast( autofix: flags::Autofix, noqa: flags::Noqa, path: &Path, + package: Option<&Path>, ) -> Vec { let mut checker = Checker::new( settings, @@ -4951,6 +5009,7 @@ pub fn check_ast( autofix, noqa, path, + package, locator, stylist, indexer, diff --git a/src/linter.rs b/src/linter.rs index 2a2bb9d3bdd33..f0a85fe0982eb 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -98,6 +98,7 @@ pub fn check_path( autofix, noqa, path, + package, )); } if use_imports { diff --git a/src/registry.rs b/src/registry.rs index 3b2e1629068ec..93843adb781f9 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -429,6 +429,9 @@ ruff_macros::define_rule_mapping!( EXE004 => rules::flake8_executable::rules::ShebangWhitespace, EXE005 => rules::flake8_executable::rules::ShebangNewline, // flake8-type-checking + TYP001 => rules::flake8_type_checking::rules::TypingOnlyFirstPartyImport, + TYP002 => rules::flake8_type_checking::rules::TypingOnlyThirdPartyImport, + TYP003 => rules::flake8_type_checking::rules::TypingOnlyStandardLibraryImport, TYP004 => rules::flake8_type_checking::rules::RuntimeImportInTypeCheckingBlock, TYP005 => rules::flake8_type_checking::rules::EmptyTypeCheckingBlock, // tryceratops diff --git a/src/rules/flake8_type_checking/mod.rs b/src/rules/flake8_type_checking/mod.rs index 78da85ed5d7ae..3532f2f618f7d 100644 --- a/src/rules/flake8_type_checking/mod.rs +++ b/src/rules/flake8_type_checking/mod.rs @@ -14,6 +14,9 @@ mod tests { use crate::registry::Rule; use crate::settings; + #[test_case(Rule::TypingOnlyFirstPartyImport, Path::new("TYP001.py"); "TYP001")] + #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("TYP002.py"); "TYP002")] + #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("TYP003.py"); "TYP003")] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TYP004_1.py"); "TYP004_1")] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TYP004_2.py"); "TYP004_2")] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TYP004_3.py"); "TYP004_3")] diff --git a/src/rules/flake8_type_checking/rules/mod.rs b/src/rules/flake8_type_checking/rules/mod.rs index 27c20f4a0c500..2b610f9833e8f 100644 --- a/src/rules/flake8_type_checking/rules/mod.rs +++ b/src/rules/flake8_type_checking/rules/mod.rs @@ -2,6 +2,11 @@ pub use empty_type_checking_block::{empty_type_checking_block, EmptyTypeChecking pub use runtime_import_in_type_checking_block::{ runtime_import_in_type_checking_block, RuntimeImportInTypeCheckingBlock, }; +pub use typing_only_runtime_import::{ + typing_only_runtime_import, TypingOnlyFirstPartyImport, TypingOnlyStandardLibraryImport, + TypingOnlyThirdPartyImport, +}; mod empty_type_checking_block; mod runtime_import_in_type_checking_block; +mod typing_only_runtime_import; diff --git a/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs new file mode 100644 index 0000000000000..716a0cd30b64e --- /dev/null +++ b/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -0,0 +1,126 @@ +use std::path::Path; + +use ruff_macros::derive_message_formats; +use rustpython_ast::Stmt; + +use crate::ast::types::{Binding, BindingKind, Range}; +use crate::define_violation; +use crate::registry::Diagnostic; +use crate::rules::isort::{categorize, ImportType}; +use crate::settings::Settings; +use crate::violation::Violation; + +define_violation!( + pub struct TypingOnlyFirstPartyImport { + pub full_name: String, + } +); +impl Violation for TypingOnlyFirstPartyImport { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Move application import `{}` into a type-checking block", + self.full_name + ) + } +} + +define_violation!( + pub struct TypingOnlyThirdPartyImport { + pub full_name: String, + } +); +impl Violation for TypingOnlyThirdPartyImport { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Move third-party import `{}` into a type-checking block", + self.full_name + ) + } +} + +define_violation!( + pub struct TypingOnlyStandardLibraryImport { + pub full_name: String, + } +); +impl Violation for TypingOnlyStandardLibraryImport { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Move standard library import `{}` into a type-checking block", + self.full_name + ) + } +} + +/// TYP001 +pub fn typing_only_runtime_import( + binding: &Binding, + blocks: &[&Stmt], + package: Option<&Path>, + settings: &Settings, +) -> Option { + let full_name = match &binding.kind { + BindingKind::Importation(.., full_name) => full_name, + BindingKind::FromImportation(.., full_name) => full_name.as_str(), + BindingKind::SubmoduleImportation(.., full_name) => full_name, + _ => return None, + }; + + let defined_in_type_checking = blocks + .iter() + .any(|block| Range::from_located(block).contains(&binding.range)); + if !defined_in_type_checking { + if binding.typing_usage.is_some() + && binding.runtime_usage.is_none() + && binding.synthetic_usage.is_none() + { + // Extract the module base and level from the full name. + // Ex) `foo.bar.baz` -> `foo`, `0` + // Ex) `.foo.bar.baz` -> `foo`, `1` + let module_base = full_name.split('.').next().unwrap(); + let level = full_name.chars().take_while(|c| *c == '.').count(); + + // Categorize the import. + match categorize( + module_base, + Some(&level), + &settings.src, + package, + &settings.isort.known_first_party, + &settings.isort.known_third_party, + &settings.isort.extra_standard_library, + ) { + ImportType::LocalFolder | ImportType::FirstParty => { + return Some(Diagnostic::new( + TypingOnlyFirstPartyImport { + full_name: full_name.to_string(), + }, + binding.range, + )); + } + ImportType::ThirdParty => { + return Some(Diagnostic::new( + TypingOnlyThirdPartyImport { + full_name: full_name.to_string(), + }, + binding.range, + )); + } + ImportType::StandardLibrary => { + return Some(Diagnostic::new( + TypingOnlyStandardLibraryImport { + full_name: full_name.to_string(), + }, + binding.range, + )); + } + ImportType::Future => unreachable!("`__future__` imports should be marked as used"), + } + } + } + + None +} diff --git a/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-first-party-import_TYP001.py.snap b/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-first-party-import_TYP001.py.snap new file mode 100644 index 0000000000000..b0a0bb7975312 --- /dev/null +++ b/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-first-party-import_TYP001.py.snap @@ -0,0 +1,16 @@ +--- +source: src/rules/flake8_type_checking/mod.rs +expression: diagnostics +--- +- kind: + TypingOnlyFirstPartyImport: + full_name: ".TYP001" + location: + row: 20 + column: 18 + end_location: + row: 20 + column: 24 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-standard-library-import_TYP003.py.snap b/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-standard-library-import_TYP003.py.snap new file mode 100644 index 0000000000000..b3967da79cbb7 --- /dev/null +++ b/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-standard-library-import_TYP003.py.snap @@ -0,0 +1,16 @@ +--- +source: src/rules/flake8_type_checking/mod.rs +expression: diagnostics +--- +- kind: + TypingOnlyStandardLibraryImport: + full_name: os + location: + row: 8 + column: 11 + end_location: + row: 8 + column: 13 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-third-party-import_TYP002.py.snap b/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-third-party-import_TYP002.py.snap new file mode 100644 index 0000000000000..1b02acf2e577f --- /dev/null +++ b/src/rules/flake8_type_checking/snapshots/ruff__rules__flake8_type_checking__tests__typing-only-third-party-import_TYP002.py.snap @@ -0,0 +1,93 @@ +--- +source: src/rules/flake8_type_checking/mod.rs +expression: diagnostics +--- +- kind: + TypingOnlyThirdPartyImport: + full_name: pandas + location: + row: 5 + column: 11 + end_location: + row: 5 + column: 23 + fix: ~ + parent: ~ +- kind: + TypingOnlyThirdPartyImport: + full_name: pandas.DataFrame + location: + row: 11 + column: 23 + end_location: + row: 11 + column: 32 + fix: ~ + parent: ~ +- kind: + TypingOnlyThirdPartyImport: + full_name: pandas.DataFrame + location: + row: 17 + column: 23 + end_location: + row: 17 + column: 38 + fix: ~ + parent: ~ +- kind: + TypingOnlyThirdPartyImport: + full_name: pandas + location: + row: 23 + column: 11 + end_location: + row: 23 + column: 23 + fix: ~ + parent: ~ +- kind: + TypingOnlyThirdPartyImport: + full_name: pandas.DataFrame + location: + row: 29 + column: 23 + end_location: + row: 29 + column: 32 + fix: ~ + parent: ~ +- kind: + TypingOnlyThirdPartyImport: + full_name: pandas.DataFrame + location: + row: 35 + column: 23 + end_location: + row: 35 + column: 38 + fix: ~ + parent: ~ +- kind: + TypingOnlyThirdPartyImport: + full_name: pandas + location: + row: 41 + column: 11 + end_location: + row: 41 + column: 23 + fix: ~ + parent: ~ +- kind: + TypingOnlyThirdPartyImport: + full_name: pandas + location: + row: 47 + column: 11 + end_location: + row: 47 + column: 23 + fix: ~ + parent: ~ + diff --git a/src/rules/isort/mod.rs b/src/rules/isort/mod.rs index a93ea51430079..262ee41228386 100644 --- a/src/rules/isort/mod.rs +++ b/src/rules/isort/mod.rs @@ -3,7 +3,7 @@ use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet}; use std::path::{Path, PathBuf}; -use categorize::{categorize, ImportType}; +pub use categorize::{categorize, ImportType}; use comments::Comment; use helpers::trailing_comma; use itertools::Either::{Left, Right};