diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI055.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI055.py new file mode 100644 index 0000000000..84b0caf2ad --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI055.py @@ -0,0 +1,20 @@ +import builtins +from typing import Union + + +w: builtins.type[int] | builtins.type[str] | builtins.type[complex] +x: type[int] | type[str] | type[float] +y: builtins.type[int] | type[str] | builtins.type[complex] +z: Union[type[float], type[complex]] +z: Union[type[float, int], type[complex]] + + +def func(arg: type[int] | str | type[float]) -> None: ... + +# OK +x: type[int, str, float] +y: builtins.type[int, str, complex] +z: Union[float, complex] + + +def func(arg: type[int, float] | str) -> None: ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI055.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI055.pyi new file mode 100644 index 0000000000..84b0caf2ad --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI055.pyi @@ -0,0 +1,20 @@ +import builtins +from typing import Union + + +w: builtins.type[int] | builtins.type[str] | builtins.type[complex] +x: type[int] | type[str] | type[float] +y: builtins.type[int] | type[str] | builtins.type[complex] +z: Union[type[float], type[complex]] +z: Union[type[float, int], type[complex]] + + +def func(arg: type[int] | str | type[float]) -> None: ... + +# OK +x: type[int, str, float] +y: builtins.type[int, str, complex] +z: Union[float, complex] + + +def func(arg: type[int, float] | str) -> None: ... diff --git a/crates/ruff/src/checkers/ast/analyze/expression.rs b/crates/ruff/src/checkers/ast/analyze/expression.rs index d064e086db..b8fb6e5bd7 100644 --- a/crates/ruff/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff/src/checkers/ast/analyze/expression.rs @@ -78,6 +78,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { Rule::UnnecessaryLiteralUnion, Rule::DuplicateUnionMember, Rule::RedundantLiteralUnion, + Rule::UnnecessaryTypeUnion, ]) { // Avoid duplicate checks if the parent is an `Union[...]` since these rules // traverse nested unions. @@ -96,9 +97,12 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::DuplicateUnionMember) { flake8_pyi::rules::duplicate_union_member(checker, expr); } - if checker.is_stub && checker.enabled(Rule::RedundantLiteralUnion) { + if checker.enabled(Rule::RedundantLiteralUnion) { flake8_pyi::rules::redundant_literal_union(checker, expr); } + if checker.enabled(Rule::UnnecessaryTypeUnion) { + flake8_pyi::rules::unnecessary_type_union(checker, expr); + } } } @@ -179,9 +183,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { && checker.semantic.in_annotation() && !checker.settings.pyupgrade.keep_runtime_typing { - flake8_future_annotations::rules::future_rewritable_type_annotation( - checker, expr, - ); + flake8_future_annotations::rules::future_rewritable_type_annotation(checker, expr); } } if checker.enabled(Rule::NonPEP585Annotation) { @@ -387,9 +389,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { .enabled(Rule::StringDotFormatExtraPositionalArguments) { pyflakes::rules::string_dot_format_extra_positional_arguments( - checker, - &summary, args, location, - ); + checker, &summary, args, location, + ); } if checker.enabled(Rule::StringDotFormatMissingArguments) { pyflakes::rules::string_dot_format_missing_argument( @@ -1075,34 +1076,31 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { ); } } + + // Avoid duplicate checks if the parent is an `|` since these rules + // traverse nested unions. + let is_unchecked_union = !matches!( + checker.semantic.expr_parent(), + Some(Expr::BinOp(ast::ExprBinOp { + op: Operator::BitOr, + .. + })) + ); if checker.enabled(Rule::DuplicateUnionMember) && checker.semantic.in_type_definition() - // Avoid duplicate checks if the parent is an `|` - && !matches!( - checker.semantic.expr_parent(), - Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..})) - ) + && is_unchecked_union { flake8_pyi::rules::duplicate_union_member(checker, expr); } - if checker.enabled(Rule::UnnecessaryLiteralUnion) - // Avoid duplicate checks if the parent is an `|` - && !matches!( - checker.semantic.expr_parent(), - Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..})) - ) - { + if checker.enabled(Rule::UnnecessaryLiteralUnion) && is_unchecked_union { flake8_pyi::rules::unnecessary_literal_union(checker, expr); } - if checker.enabled(Rule::RedundantLiteralUnion) - // Avoid duplicate checks if the parent is an `|` - && !matches!( - checker.semantic.expr_parent(), - Some(Expr::BinOp(ast::ExprBinOp { op: Operator::BitOr, ..})) - ) - { + if checker.enabled(Rule::RedundantLiteralUnion) && is_unchecked_union { flake8_pyi::rules::redundant_literal_union(checker, expr); } + if checker.enabled(Rule::UnnecessaryTypeUnion) && is_unchecked_union { + flake8_pyi::rules::unnecessary_type_union(checker, expr); + } } Expr::UnaryOp(ast::ExprUnaryOp { op, diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index f908c2f88f..5e3cfe478a 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -665,6 +665,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "052") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnannotatedAssignmentInStub), (Flake8Pyi, "054") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NumericLiteralTooLong), (Flake8Pyi, "053") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::StringOrBytesTooLong), + (Flake8Pyi, "055") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnnecessaryTypeUnion), (Flake8Pyi, "056") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnsupportedMethodCallOnAll), // flake8-pytest-style diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index 259b02c21a..d565839f61 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -104,6 +104,8 @@ mod tests { #[test_case(Rule::UnusedPrivateTypedDict, Path::new("PYI049.pyi"))] #[test_case(Rule::RedundantLiteralUnion, Path::new("PYI051.py"))] #[test_case(Rule::RedundantLiteralUnion, Path::new("PYI051.pyi"))] + #[test_case(Rule::UnnecessaryTypeUnion, Path::new("PYI055.py"))] + #[test_case(Rule::UnnecessaryTypeUnion, Path::new("PYI055.pyi"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index e340427d04..98ed9c01d5 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -28,6 +28,7 @@ pub(crate) use type_alias_naming::*; pub(crate) use type_comment_in_stub::*; pub(crate) use unaliased_collections_abc_set_import::*; pub(crate) use unnecessary_literal_union::*; +pub(crate) use unnecessary_type_union::*; pub(crate) use unrecognized_platform::*; pub(crate) use unrecognized_version_info::*; pub(crate) use unsupported_method_call_on_all::*; @@ -63,6 +64,7 @@ mod type_alias_naming; mod type_comment_in_stub; mod unaliased_collections_abc_set_import; mod unnecessary_literal_union; +mod unnecessary_type_union; mod unrecognized_platform; mod unrecognized_version_info; mod unsupported_method_call_on_all; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unnecessary_type_union.rs b/crates/ruff/src/rules/flake8_pyi/rules/unnecessary_type_union.rs new file mode 100644 index 0000000000..488d7c9c8e --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/unnecessary_type_union.rs @@ -0,0 +1,82 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{Expr, Ranged}; + +use crate::{checkers::ast::Checker, rules::flake8_pyi::helpers::traverse_union}; + +/// ## What it does +/// Checks for the presence of multiple `type`s in a union. +/// +/// ## Why is this bad? +/// The `type` built-in function accepts unions, and it is +/// clearer to explicitly specify them as a single `type`. +/// +/// ## Example +/// ```python +/// field: type[int] | type[float] +/// ``` +/// +/// Use instead: +/// ```python +/// field: type[int | float] +/// ``` +#[violation] +pub struct UnnecessaryTypeUnion { + members: Vec, + is_pep604_union: bool, +} + +impl Violation for UnnecessaryTypeUnion { + #[derive_message_formats] + fn message(&self) -> String { + let union_str = if self.is_pep604_union { + format!("{}", self.members.join(" | ")) + } else { + format!("Union[{}]", self.members.join(", ")) + }; + + format!( + "Multiple `type` members in a union. Combine them into one, e.g., `type[{union_str}]`." + ) + } +} + +/// PYI055 +pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) { + let mut type_exprs = Vec::new(); + + // Check if `union` is a PEP604 union (e.g. `float | int`) or a `typing.Union[float, int]` + let is_pep604_union = !union.as_subscript_expr().is_some_and(|subscript| { + checker + .semantic() + .match_typing_expr(&subscript.value, "Union") + }); + + let mut collect_type_exprs = |expr: &'a Expr, _| { + let Some(subscript) = expr.as_subscript_expr() else { + return; + }; + if checker + .semantic() + .resolve_call_path(subscript.value.as_ref()) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["" | "builtins", "type"])) + { + type_exprs.push(&subscript.slice); + } + }; + + traverse_union(&mut collect_type_exprs, checker.semantic(), union, None); + + if type_exprs.len() > 1 { + checker.diagnostics.push(Diagnostic::new( + UnnecessaryTypeUnion { + members: type_exprs + .into_iter() + .map(|type_expr| checker.locator().slice(type_expr.range()).to_string()) + .collect(), + is_pep604_union, + }, + union.range(), + )); + } +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI051_PYI051.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI051_PYI051.py.snap index e825128300..3c6c91602c 100644 --- a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI051_PYI051.py.snap +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI051_PYI051.py.snap @@ -11,6 +11,62 @@ PYI051.py:4:18: PYI051 `Literal["foo"]` is redundant in a union with `str` 6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] | +PYI051.py:5:37: PYI051 `Literal[b"bar"]` is redundant in a union with `bytes` + | +4 | A: str | Literal["foo"] +5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] + | ^^^^^^ PYI051 +6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] +7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] + | + +PYI051.py:5:45: PYI051 `Literal[b"foo"]` is redundant in a union with `bytes` + | +4 | A: str | Literal["foo"] +5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] + | ^^^^^^ PYI051 +6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] +7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] + | + +PYI051.py:6:37: PYI051 `Literal[5]` is redundant in a union with `int` + | +4 | A: str | Literal["foo"] +5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] +6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] + | ^ PYI051 +7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] + | + +PYI051.py:6:67: PYI051 `Literal["foo"]` is redundant in a union with `str` + | +4 | A: str | Literal["foo"] +5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] +6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] + | ^^^^^ PYI051 +7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] + | + +PYI051.py:7:37: PYI051 `Literal[b"str_bytes"]` is redundant in a union with `bytes` + | +5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] +6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] +7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] + | ^^^^^^^^^^^^ PYI051 +8 | +9 | def func(x: complex | Literal[1J], y: Union[Literal[3.14], float]): ... + | + +PYI051.py:7:51: PYI051 `Literal[42]` is redundant in a union with `int` + | +5 | B: TypeAlias = typing.Union[Literal[b"bar", b"foo"], bytes, str] +6 | C: TypeAlias = typing.Union[Literal[5], int, typing.Union[Literal["foo"], str]] +7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] + | ^^ PYI051 +8 | +9 | def func(x: complex | Literal[1J], y: Union[Literal[3.14], float]): ... + | + PYI051.py:9:31: PYI051 `Literal[1J]` is redundant in a union with `complex` | 7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] @@ -21,4 +77,14 @@ PYI051.py:9:31: PYI051 `Literal[1J]` is redundant in a union with `complex` 11 | # OK | +PYI051.py:9:53: PYI051 `Literal[3.14]` is redundant in a union with `float` + | + 7 | D: TypeAlias = typing.Union[Literal[b"str_bytes", 42], bytes, int] + 8 | + 9 | def func(x: complex | Literal[1J], y: Union[Literal[3.14], float]): ... + | ^^^^ PYI051 +10 | +11 | # OK + | + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI055_PYI055.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI055_PYI055.py.snap new file mode 100644 index 0000000000..78622d6f93 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI055_PYI055.py.snap @@ -0,0 +1,56 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI055.py:5:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | complex]`. + | +5 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +6 | x: type[int] | type[str] | type[float] +7 | y: builtins.type[int] | type[str] | builtins.type[complex] + | + +PYI055.py:6:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | float]`. + | +5 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] +6 | x: type[int] | type[str] | type[float] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +7 | y: builtins.type[int] | type[str] | builtins.type[complex] +8 | z: Union[type[float], type[complex]] + | + +PYI055.py:7:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | complex]`. + | +5 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] +6 | x: type[int] | type[str] | type[float] +7 | y: builtins.type[int] | type[str] | builtins.type[complex] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +8 | z: Union[type[float], type[complex]] +9 | z: Union[type[float, int], type[complex]] + | + +PYI055.py:8:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[Union[float, complex]]`. + | +6 | x: type[int] | type[str] | type[float] +7 | y: builtins.type[int] | type[str] | builtins.type[complex] +8 | z: Union[type[float], type[complex]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +9 | z: Union[type[float, int], type[complex]] + | + +PYI055.py:9:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[Union[float, int, complex]]`. + | +7 | y: builtins.type[int] | type[str] | builtins.type[complex] +8 | z: Union[type[float], type[complex]] +9 | z: Union[type[float, int], type[complex]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 + | + +PYI055.py:12:15: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | float]`. + | +12 | def func(arg: type[int] | str | type[float]) -> None: ... + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +13 | +14 | # OK + | + + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap new file mode 100644 index 0000000000..f0ab0f3f53 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap @@ -0,0 +1,56 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI055.pyi:5:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | complex]`. + | +5 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +6 | x: type[int] | type[str] | type[float] +7 | y: builtins.type[int] | type[str] | builtins.type[complex] + | + +PYI055.pyi:6:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | float]`. + | +5 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] +6 | x: type[int] | type[str] | type[float] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +7 | y: builtins.type[int] | type[str] | builtins.type[complex] +8 | z: Union[type[float], type[complex]] + | + +PYI055.pyi:7:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | str | complex]`. + | +5 | w: builtins.type[int] | builtins.type[str] | builtins.type[complex] +6 | x: type[int] | type[str] | type[float] +7 | y: builtins.type[int] | type[str] | builtins.type[complex] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +8 | z: Union[type[float], type[complex]] +9 | z: Union[type[float, int], type[complex]] + | + +PYI055.pyi:8:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[Union[float, complex]]`. + | +6 | x: type[int] | type[str] | type[float] +7 | y: builtins.type[int] | type[str] | builtins.type[complex] +8 | z: Union[type[float], type[complex]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +9 | z: Union[type[float, int], type[complex]] + | + +PYI055.pyi:9:4: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[Union[float, int, complex]]`. + | +7 | y: builtins.type[int] | type[str] | builtins.type[complex] +8 | z: Union[type[float], type[complex]] +9 | z: Union[type[float, int], type[complex]] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 + | + +PYI055.pyi:12:15: PYI055 Multiple `type` members in a union. Combine them into one, e.g., `type[int | float]`. + | +12 | def func(arg: type[int] | str | type[float]) -> None: ... + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +13 | +14 | # OK + | + + diff --git a/ruff.schema.json b/ruff.schema.json index fc25a95e01..2c5bfc3090 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2414,6 +2414,7 @@ "PYI052", "PYI053", "PYI054", + "PYI055", "PYI056", "Q", "Q0",