diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI041.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI041.py new file mode 100644 index 0000000000000..d1349b3ee3a0f --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI041.py @@ -0,0 +1,47 @@ +from typing import ( + Union, +) + +from typing_extensions import ( + TypeAlias, +) + +TA0: TypeAlias = int +TA1: TypeAlias = int | float | bool +TA2: TypeAlias = Union[int, float, bool] + + +def good1(arg: int) -> int | bool: + ... + + +def good2(arg: int, arg2: int | bool) -> None: + ... + + +def f0(arg1: float | int) -> None: + ... + + +def f1(arg1: float, *, arg2: float | list[str] | type[bool] | complex) -> None: + ... + + +def f2(arg1: int, /, arg2: int | int | float) -> None: + ... + + +def f3(arg1: int, *args: Union[int | int | float]) -> None: + ... + + +async def f4(**kwargs: int | int | float) -> None: + ... + + +class Foo: + def good(self, arg: int) -> None: + ... + + def bad(self, arg: int | float | complex) -> None: + ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI041.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI041.pyi new file mode 100644 index 0000000000000..07c2bd3fd67a3 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI041.pyi @@ -0,0 +1,39 @@ +from typing import ( + Union, +) + +from typing_extensions import ( + TypeAlias, +) + +# Type aliases not flagged +TA0: TypeAlias = int +TA1: TypeAlias = int | float | bool +TA2: TypeAlias = Union[int, float, bool] + + +def good1(arg: int) -> int | bool: ... + + +def good2(arg: int, arg2: int | bool) -> None: ... + + +def f0(arg1: float | int) -> None: ... # PYI041 + + +def f1(arg1: float, *, arg2: float | list[str] | type[bool] | complex) -> None: ... # PYI041 + + +def f2(arg1: int, /, arg2: int | int | float) -> None: ... # PYI041 + + +def f3(arg1: int, *args: Union[int | int | float]) -> None: ... # PYI041 + + +async def f4(**kwargs: int | int | float) -> None: ... # PYI041 + + +class Foo: + def good(self, arg: int) -> None: ... + + def bad(self, arg: int | float | complex) -> None: ... # PYI041 diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 930bcc35c7b8f..3d3921f025eaa 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -443,6 +443,9 @@ where args, ); } + if self.enabled(Rule::RedundantNumericUnion) { + flake8_pyi::rules::redundant_numeric_union(self, args); + } } if self.enabled(Rule::DunderFunctionName) { if let Some(diagnostic) = pep8_naming::rules::dunder_function_name( diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index b8b95250ad618..3a2d13a41e1d6 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -639,6 +639,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "034") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NonSelfReturnType), (Flake8Pyi, "035") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnassignedSpecialVariableInStub), (Flake8Pyi, "036") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::BadExitAnnotation), + (Flake8Pyi, "041") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::RedundantNumericUnion), (Flake8Pyi, "042") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::SnakeCaseTypeAlias), (Flake8Pyi, "043") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::TSuffixedTypeAlias), (Flake8Pyi, "044") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::FutureAnnotationsInStub), diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index 6b8e6e4264fed..b931f17c0b9b2 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -49,6 +49,8 @@ mod tests { #[test_case(Rule::PassStatementStubBody, Path::new("PYI009.pyi"))] #[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.py"))] #[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.pyi"))] + #[test_case(Rule::RedundantNumericUnion, Path::new("PYI041.py"))] + #[test_case(Rule::RedundantNumericUnion, Path::new("PYI041.pyi"))] #[test_case(Rule::SnakeCaseTypeAlias, Path::new("PYI042.py"))] #[test_case(Rule::SnakeCaseTypeAlias, Path::new("PYI042.pyi"))] #[test_case(Rule::UnassignedSpecialVariableInStub, Path::new("PYI035.py"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 5fda64cb32913..f16fde0d9e855 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -16,6 +16,7 @@ pub(crate) use pass_in_class_body::*; pub(crate) use pass_statement_stub_body::*; pub(crate) use prefix_type_params::*; pub(crate) use quoted_annotation_in_stub::*; +pub(crate) use redundant_numeric_union::*; pub(crate) use simple_defaults::*; pub(crate) use str_or_repr_defined_in_stub::*; pub(crate) use string_or_bytes_too_long::*; @@ -45,6 +46,7 @@ mod pass_in_class_body; mod pass_statement_stub_body; mod prefix_type_params; mod quoted_annotation_in_stub; +mod redundant_numeric_union; mod simple_defaults; mod str_or_repr_defined_in_stub; mod string_or_bytes_too_long; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/redundant_numeric_union.rs b/crates/ruff/src/rules/flake8_pyi/rules/redundant_numeric_union.rs new file mode 100644 index 0000000000000..740c9e342a593 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/redundant_numeric_union.rs @@ -0,0 +1,133 @@ +use rustpython_parser::ast::{Arguments, Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; +use crate::rules::flake8_pyi::helpers::traverse_union; + +/// ## What it does +/// Checks for union annotations that contain redundant numeric types (e.g., +/// `int | float`). +/// +/// ## Why is this bad? +/// In Python, `int` is a subtype of `float`, and `float` is a subtype of +/// `complex`. As such, a union that includes both `int` and `float` is +/// redundant, as it is equivalent to a union that only includes `float`. +/// +/// For more, see [PEP 3141], which defines Python's "numeric tower". +/// +/// Unions with redundant elements are less readable than unions without them. +/// +/// ## Example +/// ```python +/// def foo(x: float | int) -> None: +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(x: float) -> None: +/// ... +/// ``` +/// +/// ## References +/// - [Python documentation: The numeric tower](https://docs.python.org/3/library/numbers.html#the-numeric-tower) +/// +/// [PEP 3141]: https://peps.python.org/pep-3141/ +#[violation] +pub struct RedundantNumericUnion { + redundancy: Redundancy, +} + +impl Violation for RedundantNumericUnion { + #[derive_message_formats] + fn message(&self) -> String { + let (subtype, supertype) = match self.redundancy { + Redundancy::FloatComplex => ("float", "complex"), + Redundancy::IntComplex => ("int", "complex"), + Redundancy::IntFloat => ("int", "float"), + }; + format!("Use `{supertype}` instead of `{subtype} | {supertype}`") + } +} + +/// PYI041 +pub(crate) fn redundant_numeric_union(checker: &mut Checker, args: &Arguments) { + for annotation in args + .args + .iter() + .chain(args.posonlyargs.iter()) + .chain(args.kwonlyargs.iter()) + .filter_map(|arg| arg.def.annotation.as_ref()) + { + check_annotation(checker, annotation); + } + + // If annotations on `args` or `kwargs` are flagged by this rule, the annotations themselves + // are not accurate, but check them anyway. It's possible that flagging them will help the user + // realize they're incorrect. + for annotation in args + .vararg + .iter() + .chain(args.kwarg.iter()) + .filter_map(|arg| arg.annotation.as_ref()) + { + check_annotation(checker, annotation); + } +} + +#[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; + + let mut func = |expr: &Expr, _parent: Option<&Expr>| { + let Some(call_path) = checker.semantic().resolve_call_path(expr) else { + return; + }; + + match call_path.as_slice() { + ["" | "builtins", "int"] => has_int = true, + ["" | "builtins", "float"] => has_float = true, + ["" | "builtins", "complex"] => has_complex = true, + _ => (), + } + }; + + traverse_union(&mut func, checker.semantic(), annotation, None); + + if has_complex { + if has_float { + checker.diagnostics.push(Diagnostic::new( + RedundantNumericUnion { + redundancy: Redundancy::FloatComplex, + }, + annotation.range(), + )); + } + + if has_int { + checker.diagnostics.push(Diagnostic::new( + RedundantNumericUnion { + redundancy: Redundancy::IntComplex, + }, + annotation.range(), + )); + } + } else if has_float && has_int { + checker.diagnostics.push(Diagnostic::new( + RedundantNumericUnion { + redundancy: Redundancy::IntFloat, + }, + annotation.range(), + )); + } +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI041_PYI041.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI041_PYI041.py.snap new file mode 100644 index 0000000000000..d1aa2e9116558 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI041_PYI041.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI041_PYI041.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI041_PYI041.pyi.snap new file mode 100644 index 0000000000000..f89a27a632992 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI041_PYI041.pyi.snap @@ -0,0 +1,44 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI041.pyi:21:14: PYI041 Use `float` instead of `int | float` + | +21 | def f0(arg1: float | int) -> None: ... # PYI041 + | ^^^^^^^^^^^ PYI041 + | + +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 + | + +PYI041.pyi:27:28: PYI041 Use `float` instead of `int | float` + | +27 | def f2(arg1: int, /, arg2: int | int | float) -> None: ... # PYI041 + | ^^^^^^^^^^^^^^^^^ PYI041 + | + +PYI041.pyi:33:24: PYI041 Use `float` instead of `int | float` + | +33 | async def f4(**kwargs: int | int | float) -> None: ... # PYI041 + | ^^^^^^^^^^^^^^^^^ PYI041 + | + +PYI041.pyi:39:24: PYI041 Use `complex` instead of `float | complex` + | +37 | def good(self, arg: int) -> None: ... +38 | +39 | def bad(self, arg: int | float | complex) -> None: ... # PYI041 + | ^^^^^^^^^^^^^^^^^^^^^ PYI041 + | + +PYI041.pyi:39:24: PYI041 Use `complex` instead of `int | complex` + | +37 | def good(self, arg: int) -> None: ... +38 | +39 | def bad(self, arg: int | float | complex) -> None: ... # PYI041 + | ^^^^^^^^^^^^^^^^^^^^^ PYI041 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index ac8dfa68b5696..353136f6944ff 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2357,6 +2357,7 @@ "PYI035", "PYI036", "PYI04", + "PYI041", "PYI042", "PYI043", "PYI044",