From 6927424a56f087f924ab46e624e77db35e5af3c2 Mon Sep 17 00:00:00 2001 From: Daverball Date: Thu, 21 Nov 2024 12:11:55 +0100 Subject: [PATCH 1/4] [`flake8-type-checking`] Adds implementation for TC006 --- .../fixtures/flake8_type_checking/TC006.py | 62 ++++++++ crates/ruff_linter/src/checkers/ast/mod.rs | 4 + crates/ruff_linter/src/codes.rs | 1 + .../src/rules/flake8_type_checking/helpers.rs | 17 +++ .../src/rules/flake8_type_checking/mod.rs | 15 ++ .../rules/flake8_type_checking/rules/mod.rs | 2 + .../rules/runtime_cast_value.rs | 69 +++++++++ ..._preview__runtime-cast-value_TC006.py.snap | 138 ++++++++++++++++++ ruff.schema.json | 1 + 9 files changed, 309 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC006.py create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__preview__runtime-cast-value_TC006.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC006.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC006.py new file mode 100644 index 0000000000000..5a26d361df72d --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC006.py @@ -0,0 +1,62 @@ +def f(): + from typing import cast + + cast(int, 3.0) # TC006 + + +def f(): + from typing import cast + + cast(list[tuple[bool | float | int | str]], 3.0) # TC006 + + +def f(): + from typing import Union, cast + + cast(list[tuple[Union[bool, float, int, str]]], 3.0) # TC006 + + +def f(): + from typing import cast + + cast("int", 3.0) # OK + + +def f(): + from typing import cast + + cast("list[tuple[bool | float | int | str]]", 3.0) # OK + + +def f(): + from typing import Union, cast + + cast("list[tuple[Union[bool, float, int, str]]]", 3.0) # OK + + +def f(): + from typing import cast as typecast + + typecast(int, 3.0) # TC006 + + +def f(): + import typing + + typing.cast(int, 3.0) # TC006 + + +def f(): + import typing as t + + t.cast(int, 3.0) # TC006 + + +def f(): + from typing import cast + + cast( + int # TC006 (unsafe, because it will get rid of this comment) + | None, + 3.0 + ) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 2421565b6b25a..1f02f66f3dccb 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -1280,6 +1280,10 @@ impl<'a> Visitor<'a> for Checker<'a> { let mut args = arguments.args.iter(); if let Some(arg) = args.next() { self.visit_type_definition(arg); + + if self.enabled(Rule::RuntimeCastValue) { + flake8_type_checking::rules::runtime_cast_value(self, arg); + } } for arg in args { self.visit_expr(arg); diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 42dae07c8da2f..20db1269bc6c7 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -856,6 +856,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8TypeChecking, "003") => (RuleGroup::Stable, rules::flake8_type_checking::rules::TypingOnlyStandardLibraryImport), (Flake8TypeChecking, "004") => (RuleGroup::Stable, rules::flake8_type_checking::rules::RuntimeImportInTypeCheckingBlock), (Flake8TypeChecking, "005") => (RuleGroup::Stable, rules::flake8_type_checking::rules::EmptyTypeCheckingBlock), + (Flake8TypeChecking, "006") => (RuleGroup::Preview, rules::flake8_type_checking::rules::RuntimeCastValue), (Flake8TypeChecking, "010") => (RuleGroup::Stable, rules::flake8_type_checking::rules::RuntimeStringUnion), // tryceratops 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 7da64c8bf75cb..d0f3cea3747a5 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -269,6 +269,23 @@ pub(crate) fn quote_annotation( } } + quote_type_expression(expr, semantic, stylist) +} + +/// Wrap a type expression in quotes. +/// +/// This function assumes that the callee already expanded expression components +/// to the minimum acceptable range for quoting, i.e. the parent node may not be +/// a [`Expr::Subscript`], [`Expr::Attribute`], `[Expr::Call]` or `[Expr::BinOp]`. +/// +/// In most cases you want to call [`quote_annotation`] instead, which provides +/// that guarantee by expanding the expression before calling into this function. +pub(crate) fn quote_type_expression( + expr: &Expr, + semantic: &SemanticModel, + stylist: &Stylist, +) -> Result { + // Quote the entire expression. let quote = stylist.quote(); let mut quote_annotator = QuoteAnnotator::new(semantic, stylist); quote_annotator.visit_expr(expr); 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 2e903ca965bd2..3a3492bbe27bf 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -13,6 +13,7 @@ mod tests { use test_case::test_case; use crate::registry::{Linter, Rule}; + use crate::settings::types::PreviewMode; use crate::test::{test_path, test_snippet}; use crate::{assert_messages, settings}; @@ -62,6 +63,20 @@ mod tests { Ok(()) } + #[test_case(Rule::RuntimeCastValue, Path::new("TC006.py"))] + fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("preview__{}_{}", rule_code.as_ref(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("flake8_type_checking").join(path).as_path(), + &settings::LinterSettings { + preview: PreviewMode::Enabled, + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("quote.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote2.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/mod.rs index 1f94e927c46c6..f3a41b66a802c 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/mod.rs @@ -1,9 +1,11 @@ pub(crate) use empty_type_checking_block::*; +pub(crate) use runtime_cast_value::*; pub(crate) use runtime_import_in_type_checking_block::*; pub(crate) use runtime_string_union::*; pub(crate) use typing_only_runtime_import::*; mod empty_type_checking_block; +mod runtime_cast_value; mod runtime_import_in_type_checking_block; mod runtime_string_union; mod typing_only_runtime_import; diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs new file mode 100644 index 0000000000000..3d1bd1a256b1b --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs @@ -0,0 +1,69 @@ +use ruff_python_ast::Expr; + +use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::rules::flake8_type_checking::helpers::quote_type_expression; + +/// ## What it does +/// Checks for an unquoted type expression in `typing.cast()` calls. +/// +/// ## Why is this bad? +/// `typing.cast()` does not do anything at runtime, so the time spent +/// on evaluating the type expression is wasted. +/// +/// ## Example +/// ```python +/// from typing import cast +/// +/// x = cast(dict[str, int], foo) +/// ``` +/// +/// Use instead: +/// ```python +/// from typing import cast +/// +/// x = cast("dict[str, int]", foo) +/// ``` +/// +/// ## Fix safety +/// This fix is safe as long as the type expression doesn't span multiple +/// lines and includes comments on any of the lines apart from the last one. +#[violation] +pub struct RuntimeCastValue; + +impl Violation for RuntimeCastValue { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + "Add quotes to type expression in `typing.cast()`".to_string() + } + + fn fix_title(&self) -> Option { + Some("Add quotes".to_string()) + } +} + +/// TC006 +pub(crate) fn runtime_cast_value(checker: &mut Checker, type_expr: &Expr) { + if type_expr.is_string_literal_expr() { + return; + } + + let mut diagnostic = Diagnostic::new(RuntimeCastValue, type_expr.range()); + let edit = quote_type_expression(type_expr, checker.semantic(), checker.stylist()).ok(); + if let Some(edit) = edit.as_ref() { + if checker + .comment_ranges() + .has_comments(type_expr, checker.source()) + { + diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); + } else { + diagnostic.set_fix(Fix::safe_edit(edit.clone())); + } + } + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__preview__runtime-cast-value_TC006.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__preview__runtime-cast-value_TC006.py.snap new file mode 100644 index 0000000000000..65831f702bca4 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__preview__runtime-cast-value_TC006.py.snap @@ -0,0 +1,138 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +TC006.py:4:10: TC006 [*] Add quotes to type expression in `typing.cast()` + | +2 | from typing import cast +3 | +4 | cast(int, 3.0) # TC006 + | ^^^ TC006 + | + = help: Add quotes + +ℹ Safe fix +1 1 | def f(): +2 2 | from typing import cast +3 3 | +4 |- cast(int, 3.0) # TC006 + 4 |+ cast("int", 3.0) # TC006 +5 5 | +6 6 | +7 7 | def f(): + +TC006.py:10:10: TC006 [*] Add quotes to type expression in `typing.cast()` + | + 8 | from typing import cast + 9 | +10 | cast(list[tuple[bool | float | int | str]], 3.0) # TC006 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TC006 + | + = help: Add quotes + +ℹ Safe fix +7 7 | def f(): +8 8 | from typing import cast +9 9 | +10 |- cast(list[tuple[bool | float | int | str]], 3.0) # TC006 + 10 |+ cast("list[tuple[bool | float | int | str]]", 3.0) # TC006 +11 11 | +12 12 | +13 13 | def f(): + +TC006.py:16:10: TC006 [*] Add quotes to type expression in `typing.cast()` + | +14 | from typing import Union, cast +15 | +16 | cast(list[tuple[Union[bool, float, int, str]]], 3.0) # TC006 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TC006 + | + = help: Add quotes + +ℹ Safe fix +13 13 | def f(): +14 14 | from typing import Union, cast +15 15 | +16 |- cast(list[tuple[Union[bool, float, int, str]]], 3.0) # TC006 + 16 |+ cast("list[tuple[Union[bool, float, int, str]]]", 3.0) # TC006 +17 17 | +18 18 | +19 19 | def f(): + +TC006.py:40:14: TC006 [*] Add quotes to type expression in `typing.cast()` + | +38 | from typing import cast as typecast +39 | +40 | typecast(int, 3.0) # TC006 + | ^^^ TC006 + | + = help: Add quotes + +ℹ Safe fix +37 37 | def f(): +38 38 | from typing import cast as typecast +39 39 | +40 |- typecast(int, 3.0) # TC006 + 40 |+ typecast("int", 3.0) # TC006 +41 41 | +42 42 | +43 43 | def f(): + +TC006.py:46:17: TC006 [*] Add quotes to type expression in `typing.cast()` + | +44 | import typing +45 | +46 | typing.cast(int, 3.0) # TC006 + | ^^^ TC006 + | + = help: Add quotes + +ℹ Safe fix +43 43 | def f(): +44 44 | import typing +45 45 | +46 |- typing.cast(int, 3.0) # TC006 + 46 |+ typing.cast("int", 3.0) # TC006 +47 47 | +48 48 | +49 49 | def f(): + +TC006.py:52:12: TC006 [*] Add quotes to type expression in `typing.cast()` + | +50 | import typing as t +51 | +52 | t.cast(int, 3.0) # TC006 + | ^^^ TC006 + | + = help: Add quotes + +ℹ Safe fix +49 49 | def f(): +50 50 | import typing as t +51 51 | +52 |- t.cast(int, 3.0) # TC006 + 52 |+ t.cast("int", 3.0) # TC006 +53 53 | +54 54 | +55 55 | def f(): + +TC006.py:59:9: TC006 [*] Add quotes to type expression in `typing.cast()` + | +58 | cast( +59 | int # TC006 (unsafe, because it will get rid of this comment) + | _________^ +60 | | | None, + | |______________^ TC006 +61 | 3.0 +62 | ) + | + = help: Add quotes + +ℹ Unsafe fix +56 56 | from typing import cast +57 57 | +58 58 | cast( +59 |- int # TC006 (unsafe, because it will get rid of this comment) +60 |- | None, + 59 |+ "int | None", +61 60 | 3.0 +62 61 | ) diff --git a/ruff.schema.json b/ruff.schema.json index 48a44c9f6bc7b..436fec6ad47cc 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4000,6 +4000,7 @@ "TC003", "TC004", "TC005", + "TC006", "TC01", "TC010", "TD", From 80a735eda5569ccc14f568da46f106caa6c39d82 Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Fri, 22 Nov 2024 14:37:38 +0100 Subject: [PATCH 2/4] Update crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs Co-authored-by: Micha Reiser --- .../rules/flake8_type_checking/rules/runtime_cast_value.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs index 3d1bd1a256b1b..94dc641736866 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs @@ -55,14 +55,14 @@ pub(crate) fn runtime_cast_value(checker: &mut Checker, type_expr: &Expr) { let mut diagnostic = Diagnostic::new(RuntimeCastValue, type_expr.range()); let edit = quote_type_expression(type_expr, checker.semantic(), checker.stylist()).ok(); - if let Some(edit) = edit.as_ref() { + if let Some(edit) = edit { if checker .comment_ranges() .has_comments(type_expr, checker.source()) { - diagnostic.set_fix(Fix::unsafe_edit(edit.clone())); + diagnostic.set_fix(Fix::unsafe_edit(edit)); } else { - diagnostic.set_fix(Fix::safe_edit(edit.clone())); + diagnostic.set_fix(Fix::safe_edit(edit)); } } checker.diagnostics.push(diagnostic); From 4fd8d2242a2efe363fd25bfe38b135dcf96f5295 Mon Sep 17 00:00:00 2001 From: Daverball Date: Fri, 22 Nov 2024 14:42:38 +0100 Subject: [PATCH 3/4] Applies remaining suggestions from code review --- .../test/fixtures/flake8_type_checking/TC006.py | 2 +- .../src/rules/flake8_type_checking/mod.rs | 15 +-------------- ...king__tests__runtime-cast-value_TC006.py.snap} | 8 ++++---- 3 files changed, 6 insertions(+), 19 deletions(-) rename crates/ruff_linter/src/rules/flake8_type_checking/snapshots/{ruff_linter__rules__flake8_type_checking__tests__preview__runtime-cast-value_TC006.py.snap => ruff_linter__rules__flake8_type_checking__tests__runtime-cast-value_TC006.py.snap} (93%) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC006.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC006.py index 5a26d361df72d..1e45cec95abf6 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC006.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC006.py @@ -49,7 +49,7 @@ def f(): def f(): import typing as t - t.cast(int, 3.0) # TC006 + t.cast(t.Literal["3.0", '3'], 3.0) # TC006 def f(): 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 3a3492bbe27bf..e973f6af4e81d 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -18,6 +18,7 @@ mod tests { use crate::{assert_messages, settings}; #[test_case(Rule::EmptyTypeCheckingBlock, Path::new("TC005.py"))] + #[test_case(Rule::RuntimeCastValue, Path::new("TC006.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TC004_1.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TC004_10.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("TC004_11.py"))] @@ -63,20 +64,6 @@ mod tests { Ok(()) } - #[test_case(Rule::RuntimeCastValue, Path::new("TC006.py"))] - fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { - let snapshot = format!("preview__{}_{}", rule_code.as_ref(), path.to_string_lossy()); - let diagnostics = test_path( - Path::new("flake8_type_checking").join(path).as_path(), - &settings::LinterSettings { - preview: PreviewMode::Enabled, - ..settings::LinterSettings::for_rule(rule_code) - }, - )?; - assert_messages!(snapshot, diagnostics); - Ok(()) - } - #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote.py"))] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("quote.py"))] #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote2.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__preview__runtime-cast-value_TC006.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-cast-value_TC006.py.snap similarity index 93% rename from crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__preview__runtime-cast-value_TC006.py.snap rename to crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-cast-value_TC006.py.snap index 65831f702bca4..63660e31919d8 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__preview__runtime-cast-value_TC006.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__runtime-cast-value_TC006.py.snap @@ -100,8 +100,8 @@ TC006.py:52:12: TC006 [*] Add quotes to type expression in `typing.cast()` | 50 | import typing as t 51 | -52 | t.cast(int, 3.0) # TC006 - | ^^^ TC006 +52 | t.cast(t.Literal["3.0", '3'], 3.0) # TC006 + | ^^^^^^^^^^^^^^^^^^^^^ TC006 | = help: Add quotes @@ -109,8 +109,8 @@ TC006.py:52:12: TC006 [*] Add quotes to type expression in `typing.cast()` 49 49 | def f(): 50 50 | import typing as t 51 51 | -52 |- t.cast(int, 3.0) # TC006 - 52 |+ t.cast("int", 3.0) # TC006 +52 |- t.cast(t.Literal["3.0", '3'], 3.0) # TC006 + 52 |+ t.cast("t.Literal['3.0', '3']", 3.0) # TC006 53 53 | 54 54 | 55 55 | def f(): From c72f00872cebecabb3e3797c94184f9b7ddc38cb Mon Sep 17 00:00:00 2001 From: Daverball Date: Fri, 22 Nov 2024 14:46:42 +0100 Subject: [PATCH 4/4] Removes unused import --- crates/ruff_linter/src/rules/flake8_type_checking/mod.rs | 1 - 1 file changed, 1 deletion(-) 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 e973f6af4e81d..c4ee167951053 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -13,7 +13,6 @@ mod tests { use test_case::test_case; use crate::registry::{Linter, Rule}; - use crate::settings::types::PreviewMode; use crate::test::{test_path, test_snippet}; use crate::{assert_messages, settings};