diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.py index d1349b3ee3a0f..8675cc5464904 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.py @@ -39,9 +39,38 @@ async def f4(**kwargs: int | int | float) -> None: ... +def f5( + arg: Union[ # comment + float, # another + complex, int] + ) -> None: + ... + +def f6( + arg: ( + int | # comment + float | # another + complex + ) + ) -> None: + ... + + class Foo: def good(self, arg: int) -> None: ... def bad(self, arg: int | float | complex) -> None: ... + + def bad2(self, arg: int | Union[float, complex]) -> None: + ... + + def bad3(self, arg: Union[Union[float, complex], int]) -> None: + ... + + def bad4(self, arg: Union[float | complex, int]) -> None: + ... + + def bad5(self, arg: int | (float | complex)) -> None: + ... diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.pyi index 07c2bd3fd67a3..f45d741a5d81b 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041.pyi @@ -32,8 +32,29 @@ def f3(arg1: int, *args: Union[int | int | float]) -> None: ... # PYI041 async def f4(**kwargs: int | int | float) -> None: ... # PYI041 +def f5( + arg: Union[ # comment + float, # another + complex, int] + ) -> None: ... # PYI041 + +def f6( + arg: ( + int | # comment + float | # another + complex + ) + ) -> None: ... # PYI041 class Foo: def good(self, arg: int) -> None: ... def bad(self, arg: int | float | complex) -> None: ... # PYI041 + + def bad2(self, arg: int | Union[float, complex]) -> None: ... # PYI041 + + def bad3(self, arg: Union[Union[float, complex], int]) -> None: ... # PYI041 + + def bad4(self, arg: Union[float | complex, int]) -> None: ... # PYI041 + + def bad5(self, arg: int | (float | complex)) -> None: ... # PYI041 diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs index 5b2c19fd2cc61..cc67e93ad0bfb 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs @@ -30,8 +30,8 @@ use crate::checkers::ast::Checker; /// ## Fix safety /// This rule's fix is marked as safe, unless the type annotation contains comments. /// -/// Note that the fix will flatten nested literals into a single top-level -/// literal. +/// Note that while the fix may flatten nested literals into a single top-level literal, +/// the semantics of the annotation will remain unchanged. /// /// ## References /// - [Python documentation: `typing.Literal`](https://docs.python.org/3/library/typing.html#typing.Literal) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs index 39251a474170b..508fdbb7fff79 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs @@ -1,10 +1,17 @@ -use ruff_diagnostics::{Diagnostic, Violation}; +use bitflags::bitflags; + +use anyhow::Result; + +use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{AnyParameterRef, Expr, Parameters}; +use ruff_python_ast::{ + name::Name, AnyParameterRef, Expr, ExprBinOp, ExprContext, ExprName, ExprSubscript, ExprTuple, + Operator, Parameters, +}; use ruff_python_semantic::analyze::typing::traverse_union; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; -use crate::checkers::ast::Checker; +use crate::{checkers::ast::Checker, importer::ImportRequest}; /// ## What it does /// Checks for parameter annotations that contain redundant unions between @@ -37,6 +44,12 @@ use crate::checkers::ast::Checker; /// def foo(x: float | str) -> None: ... /// ``` /// +/// ## Fix safety +/// This rule's fix is marked as safe, unless the type annotation contains comments. +/// +/// Note that while the fix may flatten nested unions into a single top-level union, +/// the semantics of the annotation will remain unchanged. +/// /// ## References /// - [Python documentation: The numeric tower](https://docs.python.org/3/library/numbers.html#the-numeric-tower) /// - [PEP 484: The numeric tower](https://peps.python.org/pep-0484/#the-numeric-tower) @@ -48,15 +61,23 @@ pub struct RedundantNumericUnion { } impl Violation for RedundantNumericUnion { + // Always fixable, but currently under preview. + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { let (subtype, supertype) = match self.redundancy { + Redundancy::IntFloatComplex => ("int | float", "complex"), Redundancy::FloatComplex => ("float", "complex"), Redundancy::IntComplex => ("int", "complex"), Redundancy::IntFloat => ("int", "float"), }; format!("Use `{supertype}` instead of `{subtype} | {supertype}`") } + + fn fix_title(&self) -> Option { + Some("Remove redundant type".to_string()) + } } /// PYI041 @@ -66,57 +87,210 @@ pub(crate) fn redundant_numeric_union(checker: &mut Checker, parameters: &Parame } } -#[derive(Debug, Clone, Copy, Eq, PartialEq)] -enum Redundancy { - FloatComplex, - IntComplex, - IntFloat, -} - -fn check_annotation(checker: &mut Checker, annotation: &Expr) { - let mut has_float = false; - let mut has_complex = false; - let mut has_int = false; +fn check_annotation<'a>(checker: &mut Checker, annotation: &'a Expr) { + let mut numeric_flags = NumericFlags::empty(); let mut find_numeric_type = |expr: &Expr, _parent: &Expr| { let Some(builtin_type) = checker.semantic().resolve_builtin_symbol(expr) else { return; }; - match builtin_type { - "int" => has_int = true, - "float" => has_float = true, - "complex" => has_complex = true, - _ => {} - } + numeric_flags.seen_builtin_type(builtin_type); }; + // Traverse the union, and remember which numeric types are found. traverse_union(&mut find_numeric_type, checker.semantic(), annotation); - if has_complex { - if has_float { - checker.diagnostics.push(Diagnostic::new( - RedundantNumericUnion { - redundancy: Redundancy::FloatComplex, - }, - annotation.range(), - )); + let Some(redundancy) = Redundancy::from_numeric_flags(numeric_flags) else { + return; + }; + + // Traverse the union a second time to construct the fix. + let mut necessary_nodes: Vec<&Expr> = Vec::new(); + + let mut union_type = UnionKind::TypingUnion; + let mut remove_numeric_type = |expr: &'a Expr, parent: &'a Expr| { + let Some(builtin_type) = checker.semantic().resolve_builtin_symbol(expr) else { + // Keep type annotations that are not numeric. + necessary_nodes.push(expr); + return; + }; + + if matches!(parent, Expr::BinOp(_)) { + union_type = UnionKind::PEP604; } - if has_int { - checker.diagnostics.push(Diagnostic::new( - RedundantNumericUnion { - redundancy: Redundancy::IntComplex, - }, - annotation.range(), - )); + // `int` is always dropped, since `float` or `complex` must be present. + // `float` is only dropped if `complex`` is present. + if (builtin_type == "float" && !numeric_flags.contains(NumericFlags::COMPLEX)) + || (builtin_type != "float" && builtin_type != "int") + { + necessary_nodes.push(expr); + } + }; + + // Traverse the union a second time to construct a [`Fix`]. + traverse_union(&mut remove_numeric_type, checker.semantic(), annotation); + + let mut diagnostic = Diagnostic::new(RedundantNumericUnion { redundancy }, annotation.range()); + if checker.settings.preview.is_enabled() { + // Mark [`Fix`] as unsafe when comments are in range. + let applicability = if checker.comment_ranges().intersects(annotation.range()) { + Applicability::Unsafe + } else { + Applicability::Safe + }; + + // Generate the flattened fix once. + let fix = if let &[edit_expr] = necessary_nodes.as_slice() { + // Generate a [`Fix`] for a single type expression, e.g. `int`. + Fix::applicable_edit( + Edit::range_replacement(checker.generator().expr(edit_expr), annotation.range()), + applicability, + ) + } else { + match union_type { + UnionKind::PEP604 => { + generate_pep604_fix(checker, necessary_nodes, annotation, applicability) + } + UnionKind::TypingUnion => { + generate_union_fix(checker, necessary_nodes, annotation, applicability) + .ok() + .unwrap() + } + } + }; + diagnostic.set_fix(fix); + }; + + checker.diagnostics.push(diagnostic); +} + +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +enum Redundancy { + IntFloatComplex, + FloatComplex, + IntComplex, + IntFloat, +} + +impl Redundancy { + pub(super) fn from_numeric_flags(numeric_flags: NumericFlags) -> Option { + if numeric_flags == NumericFlags::INT | NumericFlags::FLOAT | NumericFlags::COMPLEX { + Some(Self::IntFloatComplex) + } else if numeric_flags == NumericFlags::FLOAT | NumericFlags::COMPLEX { + Some(Self::FloatComplex) + } else if numeric_flags == NumericFlags::INT | NumericFlags::COMPLEX { + Some(Self::IntComplex) + } else if numeric_flags == NumericFlags::FLOAT | NumericFlags::INT { + Some(Self::IntFloat) + } else { + None } - } else if has_float && has_int { - checker.diagnostics.push(Diagnostic::new( - RedundantNumericUnion { - redundancy: Redundancy::IntFloat, - }, - annotation.range(), - )); } } + +bitflags! { + #[derive(Copy, Clone, Debug, PartialEq, Eq)] + pub(super) struct NumericFlags: u8 { + /// `int` + const INT = 1 << 0; + /// `float` + const FLOAT = 1 << 1; + /// `complex` + const COMPLEX = 1 << 2; + } +} + +impl NumericFlags { + pub(super) fn seen_builtin_type(&mut self, name: &str) { + let flag: NumericFlags = match name { + "int" => NumericFlags::INT, + "float" => NumericFlags::FLOAT, + "complex" => NumericFlags::COMPLEX, + _ => { + return; + } + }; + self.insert(flag); + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum UnionKind { + /// E.g., `typing.Union[int, str]` + TypingUnion, + /// E.g., `int | str` + PEP604, +} + +// Generate a [`Fix`] for two or more type expressions, e.g. `int | float | complex`. +fn generate_pep604_fix( + checker: &Checker, + nodes: Vec<&Expr>, + annotation: &Expr, + applicability: Applicability, +) -> Fix { + debug_assert!(nodes.len() >= 2, "At least two nodes required"); + + let new_expr = nodes + .into_iter() + .fold(None, |acc: Option, right: &Expr| { + if let Some(left) = acc { + Some(Expr::BinOp(ExprBinOp { + left: Box::new(left), + op: Operator::BitOr, + right: Box::new(right.clone()), + range: TextRange::default(), + })) + } else { + Some(right.clone()) + } + }) + .unwrap(); + + Fix::applicable_edit( + Edit::range_replacement(checker.generator().expr(&new_expr), annotation.range()), + applicability, + ) +} + +// Generate a [`Fix`] for two or more type expresisons, e.g. `typing.Union[int, float, complex]`. +fn generate_union_fix( + checker: &Checker, + nodes: Vec<&Expr>, + annotation: &Expr, + applicability: Applicability, +) -> Result { + debug_assert!(nodes.len() >= 2, "At least two nodes required"); + + // Request `typing.Union` + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import_from("typing", "Union"), + annotation.start(), + checker.semantic(), + )?; + + // Construct the expression as `Subscript[typing.Union, Tuple[expr, [expr, ...]]]` + let new_expr = Expr::Subscript(ExprSubscript { + range: TextRange::default(), + value: Box::new(Expr::Name(ExprName { + id: Name::new(binding), + ctx: ExprContext::Store, + range: TextRange::default(), + })), + slice: Box::new(Expr::Tuple(ExprTuple { + elts: nodes.into_iter().cloned().collect(), + range: TextRange::default(), + ctx: ExprContext::Load, + parenthesized: false, + })), + ctx: ExprContext::Load, + }); + + Ok(Fix::applicable_edits( + Edit::range_replacement(checker.generator().expr(&new_expr), annotation.range()), + [import_edit], + applicability, + )) +} diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs index 459d37a649b19..0a498264c856d 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs @@ -27,12 +27,10 @@ use crate::checkers::ast::Checker; /// ``` /// /// ## Fix safety +/// This rule's fix is marked as safe, unless the type annotation contains comments. /// -/// This rule's fix is marked as safe in most cases; however, the fix will -/// flatten nested unions type expressions into a single top-level union. -/// -/// The fix is marked as unsafe when comments are present within the type -/// expression. +/// Note that while the fix may flatten nested unions into a single top-level union, +/// the semantics of the annotation will remain unchanged. #[violation] pub struct UnnecessaryTypeUnion { members: Vec, diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI041_PYI041.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI041_PYI041.py.snap index dba3e9a548cc1..4e3c5c893017d 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI041_PYI041.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI041_PYI041.py.snap @@ -1,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs -snapshot_kind: text --- PYI041.py:22:14: PYI041 Use `float` instead of `int | float` | @@ -8,6 +7,7 @@ PYI041.py:22:14: PYI041 Use `float` instead of `int | float` | ^^^^^^^^^^^ PYI041 23 | ... | + = help: Remove redundant type PYI041.py:26:30: PYI041 Use `complex` instead of `float | complex` | @@ -15,6 +15,7 @@ PYI041.py:26:30: PYI041 Use `complex` instead of `float | complex` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI041 27 | ... | + = help: Remove redundant type PYI041.py:30:28: PYI041 Use `float` instead of `int | float` | @@ -22,6 +23,7 @@ PYI041.py:30:28: PYI041 Use `float` instead of `int | float` | ^^^^^^^^^^^^^^^^^ PYI041 31 | ... | + = help: Remove redundant type PYI041.py:38:24: PYI041 Use `float` instead of `int | float` | @@ -29,21 +31,81 @@ PYI041.py:38:24: PYI041 Use `float` instead of `int | float` | ^^^^^^^^^^^^^^^^^ PYI041 39 | ... | + = help: Remove redundant type -PYI041.py:46:24: PYI041 Use `complex` instead of `float | complex` +PYI041.py:43:10: PYI041 Use `complex` instead of `int | float | complex` | -44 | ... -45 | -46 | def bad(self, arg: int | float | complex) -> None: - | ^^^^^^^^^^^^^^^^^^^^^ PYI041 -47 | ... +42 | def f5( +43 | arg: Union[ # comment + | __________^ +44 | | float, # another +45 | | complex, int] + | |_____________________^ PYI041 +46 | ) -> None: +47 | ... + | + = help: Remove redundant type + +PYI041.py:51:9: PYI041 Use `complex` instead of `int | float | complex` + | +49 | def f6( +50 | arg: ( +51 | int | # comment + | _________^ +52 | | float | # another +53 | | complex + | |_______________^ PYI041 +54 | ) +55 | ) -> None: | + = help: Remove redundant type -PYI041.py:46:24: PYI041 Use `complex` instead of `int | complex` +PYI041.py:63:24: PYI041 Use `complex` instead of `int | float | complex` | -44 | ... -45 | -46 | def bad(self, arg: int | float | complex) -> None: +61 | ... +62 | +63 | def bad(self, arg: int | float | complex) -> None: | ^^^^^^^^^^^^^^^^^^^^^ PYI041 -47 | ... +64 | ... + | + = help: Remove redundant type + +PYI041.py:66:25: PYI041 Use `complex` instead of `int | float | complex` + | +64 | ... +65 | +66 | def bad2(self, arg: int | Union[float, complex]) -> None: + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI041 +67 | ... + | + = help: Remove redundant type + +PYI041.py:69:25: PYI041 Use `complex` instead of `int | float | complex` + | +67 | ... +68 | +69 | def bad3(self, arg: Union[Union[float, complex], int]) -> None: + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI041 +70 | ... + | + = help: Remove redundant type + +PYI041.py:72:25: PYI041 Use `complex` instead of `int | float | complex` + | +70 | ... +71 | +72 | def bad4(self, arg: Union[float | complex, int]) -> None: + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI041 +73 | ... + | + = help: Remove redundant type + +PYI041.py:75:25: PYI041 Use `complex` instead of `int | float | complex` + | +73 | ... +74 | +75 | def bad5(self, arg: int | (float | complex)) -> None: + | ^^^^^^^^^^^^^^^^^^^^^^^ PYI041 +76 | ... | + = help: Remove redundant type diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI041_PYI041.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI041_PYI041.pyi.snap index e75e5add98720..479a879ed72d7 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI041_PYI041.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI041_PYI041.pyi.snap @@ -1,43 +1,111 @@ --- source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs -snapshot_kind: text --- PYI041.pyi:21:14: PYI041 Use `float` instead of `int | float` | 21 | def f0(arg1: float | int) -> None: ... # PYI041 | ^^^^^^^^^^^ PYI041 | + = help: Remove redundant type PYI041.pyi:24:30: PYI041 Use `complex` instead of `float | complex` | 24 | def f1(arg1: float, *, arg2: float | list[str] | type[bool] | complex) -> None: ... # PYI041 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI041 | + = help: Remove redundant type PYI041.pyi:27:28: PYI041 Use `float` instead of `int | float` | 27 | def f2(arg1: int, /, arg2: int | int | float) -> None: ... # PYI041 | ^^^^^^^^^^^^^^^^^ PYI041 | + = help: Remove redundant type PYI041.pyi:33:24: PYI041 Use `float` instead of `int | float` | 33 | async def f4(**kwargs: int | int | float) -> None: ... # PYI041 | ^^^^^^^^^^^^^^^^^ PYI041 +34 | +35 | def f5( | + = help: Remove redundant type -PYI041.pyi:39:24: PYI041 Use `complex` instead of `float | complex` +PYI041.pyi:36:10: PYI041 Use `complex` instead of `int | float | complex` | -37 | def good(self, arg: int) -> None: ... -38 | -39 | def bad(self, arg: int | float | complex) -> None: ... # PYI041 - | ^^^^^^^^^^^^^^^^^^^^^ PYI041 +35 | def f5( +36 | arg: Union[ # comment + | __________^ +37 | | float, # another +38 | | complex, int] + | |_____________________^ PYI041 +39 | ) -> None: ... # PYI041 + | + = help: Remove redundant type + +PYI041.pyi:43:9: PYI041 Use `complex` instead of `int | float | complex` + | +41 | def f6( +42 | arg: ( +43 | int | # comment + | _________^ +44 | | float | # another +45 | | complex + | |_______________^ PYI041 +46 | ) +47 | ) -> None: ... # PYI041 | + = help: Remove redundant type -PYI041.pyi:39:24: PYI041 Use `complex` instead of `int | complex` +PYI041.pyi:52:24: PYI041 Use `complex` instead of `int | float | complex` | -37 | def good(self, arg: int) -> None: ... -38 | -39 | def bad(self, arg: int | float | complex) -> None: ... # PYI041 +50 | def good(self, arg: int) -> None: ... +51 | +52 | def bad(self, arg: int | float | complex) -> None: ... # PYI041 | ^^^^^^^^^^^^^^^^^^^^^ PYI041 +53 | +54 | def bad2(self, arg: int | Union[float, complex]) -> None: ... # PYI041 + | + = help: Remove redundant type + +PYI041.pyi:54:25: PYI041 Use `complex` instead of `int | float | complex` + | +52 | def bad(self, arg: int | float | complex) -> None: ... # PYI041 +53 | +54 | def bad2(self, arg: int | Union[float, complex]) -> None: ... # PYI041 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI041 +55 | +56 | def bad3(self, arg: Union[Union[float, complex], int]) -> None: ... # PYI041 + | + = help: Remove redundant type + +PYI041.pyi:56:25: PYI041 Use `complex` instead of `int | float | complex` + | +54 | def bad2(self, arg: int | Union[float, complex]) -> None: ... # PYI041 +55 | +56 | def bad3(self, arg: Union[Union[float, complex], int]) -> None: ... # PYI041 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI041 +57 | +58 | def bad4(self, arg: Union[float | complex, int]) -> None: ... # PYI041 + | + = help: Remove redundant type + +PYI041.pyi:58:25: PYI041 Use `complex` instead of `int | float | complex` + | +56 | def bad3(self, arg: Union[Union[float, complex], int]) -> None: ... # PYI041 +57 | +58 | def bad4(self, arg: Union[float | complex, int]) -> None: ... # PYI041 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI041 +59 | +60 | def bad5(self, arg: int | (float | complex)) -> None: ... # PYI041 + | + = help: Remove redundant type + +PYI041.pyi:60:25: PYI041 Use `complex` instead of `int | float | complex` + | +58 | def bad4(self, arg: Union[float | complex, int]) -> None: ... # PYI041 +59 | +60 | def bad5(self, arg: int | (float | complex)) -> None: ... # PYI041 + | ^^^^^^^^^^^^^^^^^^^^^^^ PYI041 | + = help: Remove redundant type