diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py index 26d5f9d341957..041bb3d2b7391 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py @@ -38,3 +38,13 @@ def f(): def baz() -> pd.DataFrame | int: ... + + +def f(): + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from pandas import DataFrame + + def func(value: DataFrame): + ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index 27c9a6b7c79e2..66d0ad25273e9 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -59,6 +59,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { flake8_type_checking::helpers::is_valid_runtime_import( binding, &checker.semantic, + &checker.settings.flake8_type_checking, ) }) .collect() diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index 38309950ac6fc..f502d3519274e 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -1,10 +1,20 @@ +use crate::rules::flake8_type_checking::settings::Settings; +use anyhow::Result; +use ruff_diagnostics::Edit; use ruff_python_ast::call_path::from_qualified_name; use ruff_python_ast::helpers::{map_callable, map_subscript}; use ruff_python_ast::{self as ast, Expr}; -use ruff_python_semantic::{Binding, BindingId, BindingKind, SemanticModel}; +use ruff_python_codegen::Stylist; +use ruff_python_semantic::{Binding, BindingId, BindingKind, NodeId, SemanticModel}; +use ruff_source_file::Locator; +use ruff_text_size::Ranged; use rustc_hash::FxHashSet; -pub(crate) fn is_valid_runtime_import(binding: &Binding, semantic: &SemanticModel) -> bool { +pub(crate) fn is_valid_runtime_import( + binding: &Binding, + semantic: &SemanticModel, + settings: &Settings, +) -> bool { if matches!( binding.kind, BindingKind::Import(..) | BindingKind::FromImport(..) | BindingKind::SubmoduleImport(..) @@ -18,9 +28,10 @@ pub(crate) fn is_valid_runtime_import(binding: &Binding, semantic: &SemanticMode // willing to quote runtime-evaluated, but not runtime-required annotations. !(reference.in_type_checking_block() || reference.in_typing_only_annotation() - || reference.in_runtime_evaluated_annotation() || reference.in_complex_string_type_definition() - || reference.in_simple_string_type_definition()) + || reference.in_simple_string_type_definition() + || (settings.annotation_strategy.is_quote() + && reference.in_runtime_evaluated_annotation())) }) } else { false @@ -183,3 +194,60 @@ pub(crate) fn is_singledispatch_implementation( is_singledispatch_interface(function_def, semantic) }) } + +/// Wrap a type annotation in quotes. +/// +/// This requires more than just wrapping the reference itself in quotes. For example: +/// - When quoting `Series` in `Series[pd.Timestamp]`, we want `"Series[pd.Timestamp]"`. +/// - When quoting `kubernetes` in `kubernetes.SecurityContext`, we want `"kubernetes.SecurityContext"`. +/// - When quoting `Series` in `Series["pd.Timestamp"]`, we want `"Series[pd.Timestamp]"`. +/// - When quoting `Series` in `Series[Literal["pd.Timestamp"]]`, we want `"Series[Literal['pd.Timestamp']]"`. +pub(crate) fn quote_annotation( + node_id: NodeId, + semantic: &SemanticModel, + locator: &Locator, + stylist: &Stylist, +) -> Result { + let expr = semantic.expression(node_id); + if let Some(parent_id) = semantic.parent_expression_id(node_id) { + match semantic.expression(parent_id) { + Expr::Subscript(parent) => { + if expr == parent.value.as_ref() { + // If we're quoting the value of a subscript, we need to quote the entire + // expression. For example, when quoting `DataFrame` in `DataFrame[int]`, we + // should generate `"DataFrame[int]"`. + return quote_annotation(parent_id, semantic, locator, stylist); + } + } + Expr::Attribute(parent) => { + if expr == parent.value.as_ref() { + // If we're quoting the value of an attribute, we need to quote the entire + // expression. For example, when quoting `DataFrame` in `pd.DataFrame`, we + // should generate `"pd.DataFrame"`. + return quote_annotation(parent_id, semantic, locator, stylist); + } + } + Expr::BinOp(parent) => { + if parent.op.is_bit_or() { + // If we're quoting the left or right side of a binary operation, we need to + // quote the entire expression. For example, when quoting `DataFrame` in + // `DataFrame | Series`, we should generate `"DataFrame | Series"`. + return quote_annotation(parent_id, semantic, locator, stylist); + } + } + _ => {} + } + } + + let annotation = locator.slice(expr); + + // If the annotation already contains a quote, avoid attempting to re-quote it. + if annotation.contains('\'') || annotation.contains('"') { + return Err(anyhow::anyhow!("Annotation already contains a quote")); + } + + // If we're quoting a name, we need to quote the entire expression. + let quote = stylist.quote(); + let annotation = format!("{quote}{annotation}{quote}"); + Ok(Edit::range_replacement(annotation, expr.range())) +} diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/imports.rs b/crates/ruff_linter/src/rules/flake8_type_checking/imports.rs new file mode 100644 index 0000000000000..92c65cf275e1c --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/imports.rs @@ -0,0 +1,22 @@ +use ruff_python_semantic::{AnyImport, Binding, ResolvedReferenceId}; +use ruff_text_size::{Ranged, TextRange}; + +/// An import with its surrounding context. +pub(crate) struct ImportBinding<'a> { + /// The qualified name of the import (e.g., `typing.List` for `from typing import List`). + pub(crate) import: AnyImport<'a>, + /// The binding for the imported symbol. + pub(crate) binding: &'a Binding<'a>, + /// The first reference to the imported symbol. + pub(crate) reference_id: ResolvedReferenceId, + /// The trimmed range of the import (e.g., `List` in `from typing import List`). + pub(crate) range: TextRange, + /// The range of the import's parent statement. + pub(crate) parent_range: Option, +} + +impl Ranged for ImportBinding<'_> { + fn range(&self) -> TextRange { + self.range + } +} diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs index dd494af115e7f..754b7e8ae74ac 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -1,5 +1,6 @@ //! Rules from [flake8-type-checking](https://pypi.org/project/flake8-type-checking/). pub(crate) mod helpers; +mod imports; pub(crate) mod rules; pub mod settings; @@ -12,6 +13,7 @@ mod tests { use test_case::test_case; use crate::registry::{Linter, Rule}; + use crate::rules::flake8_type_checking::settings::AnnotationStrategy; use crate::test::{test_path, test_snippet}; use crate::{assert_messages, settings}; @@ -33,6 +35,7 @@ mod tests { #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_7.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_8.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TCH004_9.py"))] + #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))] #[test_case(Rule::TypingOnlyFirstPartyImport, Path::new("TCH001.py"))] #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("TCH003.py"))] #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("snapshot.py"))] @@ -52,6 +55,24 @@ mod tests { Ok(()) } + #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))] + #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("quote.py"))] + fn quote(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("quote_{}_{}", rule_code.as_ref(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("flake8_type_checking").join(path).as_path(), + &settings::LinterSettings { + flake8_type_checking: super::settings::Settings { + annotation_strategy: AnnotationStrategy::Quote, + ..Default::default() + }, + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("strict.py"))] fn strict(rule_code: Rule, path: &Path) -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index dea0f4007e0cd..6a766392f1403 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -5,13 +5,15 @@ use rustc_hash::FxHashMap; use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_semantic::{AnyImport, Imported, NodeId, ResolvedReferenceId, Scope}; -use ruff_text_size::{Ranged, TextRange}; +use ruff_python_semantic::{Imported, NodeId, Scope}; +use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::codes::Rule; use crate::fix; use crate::importer::ImportedMembers; +use crate::rules::flake8_type_checking::helpers::quote_annotation; +use crate::rules::flake8_type_checking::imports::ImportBinding; /// ## What it does /// Checks for runtime imports defined in a type-checking block. @@ -20,6 +22,11 @@ use crate::importer::ImportedMembers; /// The type-checking block is not executed at runtime, so the import will not /// be available at runtime. /// +/// If [`flake8-type-checking.annotation-strategy`] is set to `"quote"`, +/// annotations will be wrapped in quotes if doing so would enable the +/// corresponding import to remain in the type-checking block. +/// +/// /// ## Example /// ```python /// from typing import TYPE_CHECKING @@ -41,11 +48,15 @@ use crate::importer::ImportedMembers; /// foo.bar() /// ``` /// +/// ## Options +/// - `flake8-type-checking.annotation-strategy` +/// /// ## References /// - [PEP 535](https://peps.python.org/pep-0563/#runtime-annotation-resolution-and-type-checking) #[violation] pub struct RuntimeImportInTypeCheckingBlock { qualified_name: String, + strategy: Strategy, } impl Violation for RuntimeImportInTypeCheckingBlock { @@ -53,14 +64,26 @@ impl Violation for RuntimeImportInTypeCheckingBlock { #[derive_message_formats] fn message(&self) -> String { - let RuntimeImportInTypeCheckingBlock { qualified_name } = self; - format!( - "Move import `{qualified_name}` out of type-checking block. Import is used for more than type hinting." - ) + let Self { + qualified_name, + strategy, + } = self; + match strategy { + Strategy::MoveImport => format!( + "Move import `{qualified_name}` out of type-checking block. Import is used for more than type hinting." + ), + Strategy::QuoteUsages => format!( + "Quote references to `{qualified_name}`. Import is not available at runtime." + ), + } } fn fix_title(&self) -> Option { - Some("Move out of type-checking block".to_string()) + let Self { strategy, .. } = self; + match strategy { + Strategy::MoveImport => Some("Move out of type-checking block".to_string()), + Strategy::QuoteUsages => Some("Quote references".to_string()), + } } } @@ -71,7 +94,8 @@ pub(crate) fn runtime_import_in_type_checking_block( diagnostics: &mut Vec, ) { // Collect all runtime imports by statement. - let mut errors_by_statement: FxHashMap> = FxHashMap::default(); + let mut moves_by_statement: FxHashMap> = FxHashMap::default(); + let mut quotes_by_statement: FxHashMap> = FxHashMap::default(); let mut ignores_by_statement: FxHashMap> = FxHashMap::default(); for binding_id in scope.binding_ids() { @@ -101,6 +125,7 @@ pub(crate) fn runtime_import_in_type_checking_block( let import = ImportBinding { import, reference_id, + binding, range: binding.range(), parent_range: binding.parent_range(checker.semantic()), }; @@ -118,15 +143,32 @@ pub(crate) fn runtime_import_in_type_checking_block( .or_default() .push(import); } else { - errors_by_statement.entry(node_id).or_default().push(import); + // Determine whether the member should be fixed by moving the import out of the + // type-checking block, or by quoting its references. + if checker + .settings + .flake8_type_checking + .annotation_strategy + .is_quote() + && binding.references().all(|reference_id| { + let reference = checker.semantic().reference(reference_id); + reference.context().is_typing() + || reference.in_runtime_evaluated_annotation() + }) + { + println!("quote"); + quotes_by_statement.entry(node_id).or_default().push(import); + } else { + moves_by_statement.entry(node_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 (node_id, imports) in errors_by_statement { - let fix = fix_imports(checker, node_id, &imports).ok(); + for (node_id, imports) in moves_by_statement { + let fix = move_imports(checker, node_id, &imports).ok(); for ImportBinding { import, @@ -138,6 +180,36 @@ pub(crate) fn runtime_import_in_type_checking_block( let mut diagnostic = Diagnostic::new( RuntimeImportInTypeCheckingBlock { qualified_name: import.qualified_name(), + strategy: Strategy::MoveImport, + }, + 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); + } + } + + // Generate a diagnostic for every import, but share a fix across all imports within the same + // statement (excluding those that are ignored). + for (node_id, imports) in quotes_by_statement { + let fix = quote_imports(checker, node_id, &imports).ok(); + + for ImportBinding { + import, + range, + parent_range, + .. + } in imports + { + let mut diagnostic = Diagnostic::new( + RuntimeImportInTypeCheckingBlock { + qualified_name: import.qualified_name(), + strategy: Strategy::QuoteUsages, }, range, ); @@ -163,6 +235,7 @@ pub(crate) fn runtime_import_in_type_checking_block( let mut diagnostic = Diagnostic::new( RuntimeImportInTypeCheckingBlock { qualified_name: import.qualified_name(), + strategy: Strategy::MoveImport, }, range, ); @@ -173,26 +246,38 @@ pub(crate) fn runtime_import_in_type_checking_block( } } -/// A runtime-required import with its surrounding context. -struct ImportBinding<'a> { - /// The qualified name of the import (e.g., `typing.List` for `from typing import List`). - import: AnyImport<'a>, - /// The first reference to the imported symbol. - reference_id: ResolvedReferenceId, - /// The trimmed range of the import (e.g., `List` in `from typing import List`). - range: TextRange, - /// The range of the import's parent statement. - parent_range: Option, -} - -impl Ranged for ImportBinding<'_> { - fn range(&self) -> TextRange { - self.range - } +/// Generate a [`Fix`] to quote runtime usages for imports in a type-checking block. +fn quote_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) -> Result { + let mut quote_reference_edits = imports + .iter() + .flat_map(|ImportBinding { binding, .. }| { + binding.references.iter().filter_map(|reference_id| { + let reference = checker.semantic().reference(*reference_id); + if reference.context().is_runtime() { + Some(quote_annotation( + reference.node_id()?, + checker.semantic(), + checker.locator(), + checker.stylist(), + )) + } else { + None + } + }) + }) + .collect::>>()?; + let quote_reference_edit = quote_reference_edits + .pop() + .expect("Expected at least one reference"); + Ok( + Fix::unsafe_edits(quote_reference_edit, quote_reference_edits).isolate(Checker::isolation( + checker.semantic().parent_statement_id(node_id), + )), + ) } /// Generate a [`Fix`] to remove runtime imports from a type-checking block. -fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) -> Result { +fn move_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) -> Result { let statement = checker.semantic().statement(node_id); let parent = checker.semantic().parent_statement(node_id); @@ -236,3 +321,18 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) -> ), ) } + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Strategy { + /// The import should be moved out of the type-checking block. + /// + /// This is required when at least one reference to the symbol is in a runtime-required context. + /// For example, given `from foo import Bar`, `x = Bar()` would be runtime-required. + MoveImport, + /// All usages of the import should be wrapped in quotes. + /// + /// This is acceptable when all references to the symbol are in a runtime-evaluated, but not + /// runtime-required context. For example, given `from foo import Bar`, `x: Bar` would be + /// runtime-evaluated, but not runtime-required. + QuoteUsages, +} diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index 897469b7a43a4..b0184f4642d2b 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -3,20 +3,17 @@ use std::borrow::Cow; use anyhow::Result; use rustc_hash::FxHashMap; -use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit, Fix, FixAvailability, Violation}; +use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::Expr; -use ruff_python_codegen::Stylist; -use ruff_python_semantic::{ - AnyImport, Binding, Imported, NodeId, ResolvedReferenceId, Scope, SemanticModel, -}; -use ruff_source_file::Locator; -use ruff_text_size::{Ranged, TextRange}; +use ruff_python_semantic::{Binding, Imported, NodeId, Scope}; +use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::codes::Rule; use crate::fix; use crate::importer::ImportedMembers; +use crate::rules::flake8_type_checking::helpers::quote_annotation; +use crate::rules::flake8_type_checking::imports::ImportBinding; use crate::rules::isort::{categorize, ImportSection, ImportType}; /// ## What it does @@ -29,6 +26,10 @@ use crate::rules::isort::{categorize, ImportSection, ImportType}; /// instead be imported conditionally under an `if TYPE_CHECKING:` block to /// minimize runtime overhead. /// +/// If [`flake8-type-checking.annotation-strategy`] is set to `"quote"`, +/// annotations will be wrapped in quotes if doing so would enable the +/// corresponding import to be moved into an `if TYPE_CHECKING:` block. +/// /// If a class _requires_ that type annotations be available at runtime (as is /// the case for Pydantic, SQLAlchemy, and other libraries), consider using /// the [`flake8-type-checking.runtime-evaluated-base-classes`] and @@ -61,6 +62,7 @@ use crate::rules::isort::{categorize, ImportSection, ImportType}; /// ``` /// /// ## Options +/// - `flake8-type-checking.annotation-strategy` /// - `flake8-type-checking.runtime-evaluated-base-classes` /// - `flake8-type-checking.runtime-evaluated-decorators` /// @@ -97,6 +99,10 @@ impl Violation for TypingOnlyFirstPartyImport { /// instead be imported conditionally under an `if TYPE_CHECKING:` block to /// minimize runtime overhead. /// +/// If [`flake8-type-checking.annotation-strategy`] is set to `"quote"`, +/// annotations will be wrapped in quotes if doing so would enable the +/// corresponding import to be moved into an `if TYPE_CHECKING:` block. +/// /// If a class _requires_ that type annotations be available at runtime (as is /// the case for Pydantic, SQLAlchemy, and other libraries), consider using /// the [`flake8-type-checking.runtime-evaluated-base-classes`] and @@ -129,6 +135,7 @@ impl Violation for TypingOnlyFirstPartyImport { /// ``` /// /// ## Options +/// - `flake8-type-checking.annotation-strategy` /// - `flake8-type-checking.runtime-evaluated-base-classes` /// - `flake8-type-checking.runtime-evaluated-decorators` /// @@ -165,6 +172,10 @@ impl Violation for TypingOnlyThirdPartyImport { /// instead be imported conditionally under an `if TYPE_CHECKING:` block to /// minimize runtime overhead. /// +/// If [`flake8-type-checking.annotation-strategy`] is set to `"quote"`, +/// annotations will be wrapped in quotes if doing so would enable the +/// corresponding import to be moved into an `if TYPE_CHECKING:` block. +/// /// If a class _requires_ that type annotations be available at runtime (as is /// the case for Pydantic, SQLAlchemy, and other libraries), consider using /// the [`flake8-type-checking.runtime-evaluated-base-classes`] and @@ -197,6 +208,7 @@ impl Violation for TypingOnlyThirdPartyImport { /// ``` /// /// ## Options +/// - `flake8-type-checking.annotation-strategy` /// - `flake8-type-checking.runtime-evaluated-base-classes` /// - `flake8-type-checking.runtime-evaluated-decorators` /// @@ -267,9 +279,14 @@ pub(crate) fn typing_only_runtime_import( // quote. reference.in_type_checking_block() || reference.in_typing_only_annotation() - || reference.in_runtime_evaluated_annotation() || reference.in_complex_string_type_definition() || reference.in_simple_string_type_definition() + || (checker + .settings + .flake8_type_checking + .annotation_strategy + .is_quote() + && reference.in_runtime_evaluated_annotation()) }) { let qualified_name = import.qualified_name(); @@ -388,26 +405,6 @@ pub(crate) fn typing_only_runtime_import( } } -/// A runtime-required import with its surrounding context. -struct ImportBinding<'a> { - /// The qualified name of the import (e.g., `typing.List` for `from typing import List`). - import: AnyImport<'a>, - /// The binding for the imported symbol. - binding: &'a Binding<'a>, - /// The first reference to the imported symbol. - reference_id: ResolvedReferenceId, - /// The trimmed range of the import (e.g., `List` in `from typing import List`). - range: TextRange, - /// The range of the import's parent statement. - parent_range: Option, -} - -impl Ranged for ImportBinding<'_> { - fn range(&self) -> TextRange { - self.range - } -} - /// Return the [`Rule`] for the given import type. fn rule_for(import_type: ImportType) -> Rule { match import_type { @@ -527,60 +524,3 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) -> checker.semantic().parent_statement_id(node_id), ))) } - -/// Wrap a type annotation in quotes. -/// -/// This requires more than just wrapping the reference itself in quotes. For example: -/// - When quoting `Series` in `Series[pd.Timestamp]`, we want `"Series[pd.Timestamp]"`. -/// - When quoting `kubernetes` in `kubernetes.SecurityContext`, we want `"kubernetes.SecurityContext"`. -/// - When quoting `Series` in `Series["pd.Timestamp"]`, we want `"Series[pd.Timestamp]"`. -/// - When quoting `Series` in `Series[Literal["pd.Timestamp"]]`, we want `"Series[Literal['pd.Timestamp']]"`. -fn quote_annotation( - node_id: NodeId, - semantic: &SemanticModel, - locator: &Locator, - stylist: &Stylist, -) -> Result { - let expr = semantic.expression(node_id); - if let Some(parent_id) = semantic.parent_expression_id(node_id) { - match semantic.expression(parent_id) { - Expr::Subscript(parent) => { - if expr == parent.value.as_ref() { - // If we're quoting the value of a subscript, we need to quote the entire - // expression. For example, when quoting `DataFrame` in `DataFrame[int]`, we - // should generate `"DataFrame[int]"`. - return quote_annotation(parent_id, semantic, locator, stylist); - } - } - Expr::Attribute(parent) => { - if expr == parent.value.as_ref() { - // If we're quoting the value of an attribute, we need to quote the entire - // expression. For example, when quoting `DataFrame` in `pd.DataFrame`, we - // should generate `"pd.DataFrame"`. - return quote_annotation(parent_id, semantic, locator, stylist); - } - } - Expr::BinOp(parent) => { - if parent.op.is_bit_or() { - // If we're quoting the left or right side of a binary operation, we need to - // quote the entire expression. For example, when quoting `DataFrame` in - // `DataFrame | Series`, we should generate `"DataFrame | Series"`. - return quote_annotation(parent_id, semantic, locator, stylist); - } - } - _ => {} - } - } - - let annotation = locator.slice(expr); - - // If the annotation already contains a quote, avoid attempting to re-quote it. - if annotation.contains('\'') || annotation.contains('"') { - return Err(anyhow::anyhow!("Annotation already contains a quote")); - } - - // If we're quoting a name, we need to quote the entire expression. - let quote = stylist.quote(); - let annotation = format!("{quote}{annotation}{quote}"); - Ok(Edit::range_replacement(annotation, expr.range())) -} diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/settings.rs b/crates/ruff_linter/src/rules/flake8_type_checking/settings.rs index fa266694ce285..5380d16babb58 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/settings.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/settings.rs @@ -24,7 +24,9 @@ impl Default for Settings { } } -#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, CacheKey)] +#[derive( + Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, CacheKey, is_macro::Is, +)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] pub enum AnnotationStrategy { diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap new file mode 100644 index 0000000000000..7d935c57a185e --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_runtime-import-in-type-checking-block_quote.py.snap @@ -0,0 +1,22 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +quote.py:47:28: TCH004 [*] Quote references to `pandas.DataFrame`. Import is not available at runtime. + | +46 | if TYPE_CHECKING: +47 | from pandas import DataFrame + | ^^^^^^^^^ TCH004 +48 | +49 | def func(value: DataFrame): + | + = help: Quote references + +ℹ Unsafe fix +46 46 | if TYPE_CHECKING: +47 47 | from pandas import DataFrame +48 48 | +49 |- def func(value: DataFrame): + 49 |+ def func(value: "DataFrame"): +50 50 | ... + + diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote.py.snap new file mode 100644 index 0000000000000..14df699c48d49 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote.py.snap @@ -0,0 +1,158 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +quote.py:2:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block + | +1 | def f(): +2 | from pandas import DataFrame + | ^^^^^^^^^ TCH002 +3 | +4 | def baz() -> DataFrame: + | + = help: Move into type-checking block + +ℹ Unsafe fix +1 |-def f(): + 1 |+from typing import TYPE_CHECKING + 2 |+ + 3 |+if TYPE_CHECKING: +2 4 | from pandas import DataFrame + 5 |+def f(): +3 6 | +4 |- def baz() -> DataFrame: + 7 |+ def baz() -> "DataFrame": +5 8 | ... +6 9 | +7 10 | + +quote.py:9:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block + | + 8 | def f(): + 9 | from pandas import DataFrame + | ^^^^^^^^^ TCH002 +10 | +11 | def baz() -> DataFrame[int]: + | + = help: Move into type-checking block + +ℹ Unsafe fix + 1 |+from typing import TYPE_CHECKING + 2 |+ + 3 |+if TYPE_CHECKING: + 4 |+ from pandas import DataFrame +1 5 | def f(): +2 6 | from pandas import DataFrame +3 7 | +-------------------------------------------------------------------------------- +6 10 | +7 11 | +8 12 | def f(): +9 |- from pandas import DataFrame +10 13 | +11 |- def baz() -> DataFrame[int]: + 14 |+ def baz() -> "DataFrame[int]": +12 15 | ... +13 16 | +14 17 | + +quote.py:16:24: TCH002 Move third-party import `pandas.DataFrame` into a type-checking block + | +15 | def f(): +16 | from pandas import DataFrame + | ^^^^^^^^^ TCH002 +17 | +18 | def baz() -> DataFrame["int"]: + | + = help: Move into type-checking block + +quote.py:23:22: TCH002 [*] Move third-party import `pandas` into a type-checking block + | +22 | def f(): +23 | import pandas as pd + | ^^ TCH002 +24 | +25 | def baz() -> pd.DataFrame: + | + = help: Move into type-checking block + +ℹ Unsafe fix + 1 |+from typing import TYPE_CHECKING + 2 |+ + 3 |+if TYPE_CHECKING: + 4 |+ import pandas as pd +1 5 | def f(): +2 6 | from pandas import DataFrame +3 7 | +-------------------------------------------------------------------------------- +20 24 | +21 25 | +22 26 | def f(): +23 |- import pandas as pd +24 27 | +25 |- def baz() -> pd.DataFrame: + 28 |+ def baz() -> "pd.DataFrame": +26 29 | ... +27 30 | +28 31 | + +quote.py:30:22: TCH002 [*] Move third-party import `pandas` into a type-checking block + | +29 | def f(): +30 | import pandas as pd + | ^^ TCH002 +31 | +32 | def baz() -> pd.DataFrame.Extra: + | + = help: Move into type-checking block + +ℹ Unsafe fix + 1 |+from typing import TYPE_CHECKING + 2 |+ + 3 |+if TYPE_CHECKING: + 4 |+ import pandas as pd +1 5 | def f(): +2 6 | from pandas import DataFrame +3 7 | +-------------------------------------------------------------------------------- +27 31 | +28 32 | +29 33 | def f(): +30 |- import pandas as pd +31 34 | +32 |- def baz() -> pd.DataFrame.Extra: + 35 |+ def baz() -> "pd.DataFrame.Extra": +33 36 | ... +34 37 | +35 38 | + +quote.py:37:22: TCH002 [*] Move third-party import `pandas` into a type-checking block + | +36 | def f(): +37 | import pandas as pd + | ^^ TCH002 +38 | +39 | def baz() -> pd.DataFrame | int: + | + = help: Move into type-checking block + +ℹ Unsafe fix + 1 |+from typing import TYPE_CHECKING + 2 |+ + 3 |+if TYPE_CHECKING: + 4 |+ import pandas as pd +1 5 | def f(): +2 6 | from pandas import DataFrame +3 7 | +-------------------------------------------------------------------------------- +34 38 | +35 39 | +36 40 | def f(): +37 |- import pandas as pd +38 41 | +39 |- def baz() -> pd.DataFrame | int: + 42 |+ def baz() -> "pd.DataFrame | int": +40 43 | ... +41 44 | +42 45 | + + diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_quote.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_quote.py.snap new file mode 100644 index 0000000000000..73d0d948de4a0 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-import-in-type-checking-block_quote.py.snap @@ -0,0 +1,29 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +quote.py:47:28: TCH004 [*] Move import `pandas.DataFrame` out of type-checking block. Import is used for more than type hinting. + | +46 | if TYPE_CHECKING: +47 | from pandas import DataFrame + | ^^^^^^^^^ TCH004 +48 | +49 | def func(value: DataFrame): + | + = help: Move out of type-checking block + +ℹ Unsafe fix + 1 |+from pandas import DataFrame +1 2 | def f(): +2 3 | from pandas import DataFrame +3 4 | +-------------------------------------------------------------------------------- +44 45 | from typing import TYPE_CHECKING +45 46 | +46 47 | if TYPE_CHECKING: +47 |- from pandas import DataFrame + 48 |+ pass +48 49 | +49 50 | def func(value: DataFrame): +50 51 | ... + + diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-third-party-import_quote.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-third-party-import_quote.py.snap index cdd862fa0626a..6c5ead27428ce 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-third-party-import_quote.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-third-party-import_quote.py.snap @@ -1,156 +1,4 @@ --- source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs --- -quote.py:2:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block - | -1 | def f(): -2 | from pandas import DataFrame - | ^^^^^^^^^ TCH002 -3 | -4 | def baz() -> DataFrame: - | - = help: Move into type-checking block - -ℹ Unsafe fix -1 |-def f(): - 1 |+from typing import TYPE_CHECKING - 2 |+ - 3 |+if TYPE_CHECKING: -2 4 | from pandas import DataFrame - 5 |+def f(): -3 6 | -4 |- def baz() -> DataFrame: - 7 |+ def baz() -> "DataFrame": -5 8 | ... -6 9 | -7 10 | - -quote.py:9:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block - | - 8 | def f(): - 9 | from pandas import DataFrame - | ^^^^^^^^^ TCH002 -10 | -11 | def baz() -> DataFrame[int]: - | - = help: Move into type-checking block - -ℹ Unsafe fix - 1 |+from typing import TYPE_CHECKING - 2 |+ - 3 |+if TYPE_CHECKING: - 4 |+ from pandas import DataFrame -1 5 | def f(): -2 6 | from pandas import DataFrame -3 7 | --------------------------------------------------------------------------------- -6 10 | -7 11 | -8 12 | def f(): -9 |- from pandas import DataFrame -10 13 | -11 |- def baz() -> DataFrame[int]: - 14 |+ def baz() -> "DataFrame[int]": -12 15 | ... -13 16 | -14 17 | - -quote.py:16:24: TCH002 Move third-party import `pandas.DataFrame` into a type-checking block - | -15 | def f(): -16 | from pandas import DataFrame - | ^^^^^^^^^ TCH002 -17 | -18 | def baz() -> DataFrame["int"]: - | - = help: Move into type-checking block - -quote.py:23:22: TCH002 [*] Move third-party import `pandas` into a type-checking block - | -22 | def f(): -23 | import pandas as pd - | ^^ TCH002 -24 | -25 | def baz() -> pd.DataFrame: - | - = help: Move into type-checking block - -ℹ Unsafe fix - 1 |+from typing import TYPE_CHECKING - 2 |+ - 3 |+if TYPE_CHECKING: - 4 |+ import pandas as pd -1 5 | def f(): -2 6 | from pandas import DataFrame -3 7 | --------------------------------------------------------------------------------- -20 24 | -21 25 | -22 26 | def f(): -23 |- import pandas as pd -24 27 | -25 |- def baz() -> pd.DataFrame: - 28 |+ def baz() -> "pd.DataFrame": -26 29 | ... -27 30 | -28 31 | - -quote.py:30:22: TCH002 [*] Move third-party import `pandas` into a type-checking block - | -29 | def f(): -30 | import pandas as pd - | ^^ TCH002 -31 | -32 | def baz() -> pd.DataFrame.Extra: - | - = help: Move into type-checking block - -ℹ Unsafe fix - 1 |+from typing import TYPE_CHECKING - 2 |+ - 3 |+if TYPE_CHECKING: - 4 |+ import pandas as pd -1 5 | def f(): -2 6 | from pandas import DataFrame -3 7 | --------------------------------------------------------------------------------- -27 31 | -28 32 | -29 33 | def f(): -30 |- import pandas as pd -31 34 | -32 |- def baz() -> pd.DataFrame.Extra: - 35 |+ def baz() -> "pd.DataFrame.Extra": -33 36 | ... -34 37 | -35 38 | - -quote.py:37:22: TCH002 [*] Move third-party import `pandas` into a type-checking block - | -36 | def f(): -37 | import pandas as pd - | ^^ TCH002 -38 | -39 | def baz() -> pd.DataFrame | int: - | - = help: Move into type-checking block - -ℹ Unsafe fix - 1 |+from typing import TYPE_CHECKING - 2 |+ - 3 |+if TYPE_CHECKING: - 4 |+ import pandas as pd -1 5 | def f(): -2 6 | from pandas import DataFrame -3 7 | --------------------------------------------------------------------------------- -34 38 | -35 39 | -36 40 | def f(): -37 |- import pandas as pd -38 41 | -39 |- def baz() -> pd.DataFrame | int: - 42 |+ def baz() -> "pd.DataFrame | int": -40 43 | ... - diff --git a/pyproject.toml b/pyproject.toml index 981dca4ca291b..2d2b49ebe0807 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -99,3 +99,6 @@ version_files = [ "crates/ruff_shrinking/Cargo.toml", "scripts/benchmarks/pyproject.toml", ] + +[tool.ruff.flake8-type-checking] +annotation-strategy = "quote" diff --git a/ruff.schema.json b/ruff.schema.json index cbff63adcd807..afa8572f28e98 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -699,6 +699,31 @@ }, "additionalProperties": false, "definitions": { + "AnnotationStrategy": { + "oneOf": [ + { + "description": "Avoid changing the semantics of runtime-evaluated annotations (e.g., by quoting them, or inserting `from __future__ import annotations`). Imports will be classified as typing-only or runtime-required based exclusively on the existing type annotations.", + "type": "string", + "enum": [ + "preserve" + ] + }, + { + "description": "Quote runtime-evaluated annotations, if doing so would enable the corresponding import to be moved into an `if TYPE_CHECKING:` block.", + "type": "string", + "enum": [ + "quote" + ] + }, + { + "description": "Insert `from __future__ import annotations` at the top of the file, if doing so would enable an import to be moved into an `if TYPE_CHECKING:` block.", + "type": "string", + "enum": [ + "future" + ] + } + ] + }, "ApiBan": { "type": "object", "required": [ @@ -1184,6 +1209,17 @@ "Flake8TypeCheckingOptions": { "type": "object", "properties": { + "annotation-strategy": { + "description": "The strategy to use when analyzing type annotations that, by default, Python would evaluate at runtime.\n\nFor example, in the following, Python requires that `Sequence` be available at runtime, despite the fact that it's only used in a type annotation: ```python from collections.abc import Sequence\n\ndef func(value: Sequence[int]) -> None: ... ```\n\nIn other words, moving `from collections.abc import Sequence` into an `if TYPE_CHECKING:` block above would cause a runtime error.\n\nBy default, Ruff will respect such runtime evaluations (`\"preserve\"`), but supports two alternative strategies for handling them:\n\n- `\"quote\"`: Add quotes around the annotation (e.g., `\"Sequence[int]\"`), to prevent Python from evaluating it at runtime. Adding quotes around `Sequence` above will allow Ruff to move the import into an `if TYPE_CHECKING:` block. - `\"future\"`: Add a `from __future__ import annotations` import at the top of the file, which will cause Python to treat all annotations as strings, rather than evaluating them at runtime.\n\nSetting `annotation-strategy` to `\"quote\"` or `\"future\"` will allow Ruff to move more imports into type-checking blocks, but may cause issues with other tools that expect annotations to be evaluated at runtime.", + "anyOf": [ + { + "$ref": "#/definitions/AnnotationStrategy" + }, + { + "type": "null" + } + ] + }, "exempt-modules": { "description": "Exempt certain modules from needing to be moved into type-checking blocks.", "type": [