diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py index 6471613f9838ce..2a9c2d77871cce 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py @@ -37,3 +37,28 @@ def func(): # PYI055 x: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker + + +def convert_union(union: UnionType) -> _T | None: + converters: tuple[ + type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T], ... # PYI055 + ] = union.__args__ + ... + +def convert_union(union: UnionType) -> _T | None: + converters: tuple[ + Union[type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T]], ... # PYI055 + ] = union.__args__ + ... + +def convert_union(union: UnionType) -> _T | None: + converters: tuple[ + Union[type[_T] | type[Converter[_T]]] | Converter[_T] | Callable[[str], _T], ... # PYI055 + ] = union.__args__ + ... + +def convert_union(union: UnionType) -> _T | None: + converters: tuple[ + Union[type[_T] | type[Converter[_T]] | str] | Converter[_T] | Callable[[str], _T], ... # PYI055 + ] = union.__args__ + ... 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 75ba6ab9bc59fc..6d74fffd5b74e6 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 @@ -80,17 +80,24 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) } let mut type_exprs = Vec::new(); + let mut other_exprs = Vec::new(); 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); + let subscript = expr.as_subscript_expr(); + + if subscript.is_none() { + other_exprs.push(expr); + } else { + let unwrapped = subscript.unwrap(); + if checker + .semantic() + .resolve_call_path(unwrapped.value.as_ref()) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["" | "builtins", "type"])) + { + type_exprs.push(&unwrapped.slice); + } else { + other_exprs.push(expr); + } } }; @@ -113,55 +120,82 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr) if checker.semantic().is_builtin("type") { let content = if let Some(subscript) = subscript { - checker - .generator() - .expr(&Expr::Subscript(ast::ExprSubscript { - value: Box::new(Expr::Name(ast::ExprName { - id: "type".into(), - ctx: ExprContext::Load, - range: TextRange::default(), - })), - slice: Box::new(Expr::Subscript(ast::ExprSubscript { - value: subscript.value.clone(), - slice: Box::new(Expr::Tuple(ast::ExprTuple { - elts: type_members - .into_iter() - .map(|type_member| { - Expr::Name(ast::ExprName { - id: type_member, - ctx: ExprContext::Load, - range: TextRange::default(), - }) + let types = &Expr::Subscript(ast::ExprSubscript { + value: Box::new(Expr::Name(ast::ExprName { + id: "type".into(), + ctx: ExprContext::Load, + range: TextRange::default(), + })), + slice: Box::new(Expr::Subscript(ast::ExprSubscript { + value: subscript.value.clone(), + slice: Box::new(Expr::Tuple(ast::ExprTuple { + elts: type_members + .into_iter() + .map(|type_member| { + Expr::Name(ast::ExprName { + id: type_member, + ctx: ExprContext::Load, + range: TextRange::default(), }) - .collect(), - ctx: ExprContext::Load, - range: TextRange::default(), - })), + }) + .collect(), ctx: ExprContext::Load, range: TextRange::default(), })), ctx: ExprContext::Load, range: TextRange::default(), - })) - } else { - checker - .generator() - .expr(&Expr::Subscript(ast::ExprSubscript { - value: Box::new(Expr::Name(ast::ExprName { - id: "type".into(), + })), + ctx: ExprContext::Load, + range: TextRange::default(), + }); + + if other_exprs.is_empty() { + checker.generator().expr(types) + } else { + let mut exprs = Vec::new(); + exprs.push(types); + exprs.extend(other_exprs); + + let union = Expr::Subscript(ast::ExprSubscript { + value: subscript.value.clone(), + slice: Box::new(Expr::Tuple(ast::ExprTuple { + elts: exprs.into_iter().cloned().collect(), ctx: ExprContext::Load, range: TextRange::default(), })), - slice: Box::new(concatenate_bin_ors( - type_exprs - .clone() - .into_iter() - .map(std::convert::AsRef::as_ref) - .collect(), - )), ctx: ExprContext::Load, range: TextRange::default(), - })) + }); + + checker.generator().expr(&union) + } + } else { + let types = &Expr::Subscript(ast::ExprSubscript { + value: Box::new(Expr::Name(ast::ExprName { + id: "type".into(), + ctx: ExprContext::Load, + range: TextRange::default(), + })), + slice: Box::new(concatenate_bin_ors( + type_exprs + .clone() + .into_iter() + .map(std::convert::AsRef::as_ref) + .collect(), + )), + ctx: ExprContext::Load, + range: TextRange::default(), + }); + + if other_exprs.is_empty() { + checker.generator().expr(types) + } else { + let mut exprs = Vec::new(); + exprs.push(types); + exprs.extend(other_exprs); + + checker.generator().expr(&concatenate_bin_ors(exprs)) + } }; diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap index bdd2f92937d6c5..e93e79a12d48ad 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap @@ -54,5 +54,91 @@ PYI055.py:39:8: PYI055 [*] Multiple `type` members in a union. Combine them into 38 38 | # PYI055 39 |- x: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker 39 |+ x: type[Union[requests_mock.Mocker, httpretty, str]] = requests_mock.Mocker +40 40 | +41 41 | +42 42 | def convert_union(union: UnionType) -> _T | None: + +PYI055.py:44:9: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[_T | Converter[_T]]`. + | +42 | def convert_union(union: UnionType) -> _T | None: +43 | converters: tuple[ +44 | type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T], ... # PYI055 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +45 | ] = union.__args__ +46 | ... + | + = help: Combine multiple `type` members + +ℹ Safe fix +41 41 | +42 42 | def convert_union(union: UnionType) -> _T | None: +43 43 | converters: tuple[ +44 |- type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T], ... # PYI055 + 44 |+ type[_T | Converter[_T]] | Converter[_T] | Callable[[str], _T], ... # PYI055 +45 45 | ] = union.__args__ +46 46 | ... +47 47 | + +PYI055.py:50:15: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[_T | Converter[_T]]`. + | +48 | def convert_union(union: UnionType) -> _T | None: +49 | converters: tuple[ +50 | Union[type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T]], ... # PYI055 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +51 | ] = union.__args__ +52 | ... + | + = help: Combine multiple `type` members + +ℹ Safe fix +47 47 | +48 48 | def convert_union(union: UnionType) -> _T | None: +49 49 | converters: tuple[ +50 |- Union[type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T]], ... # PYI055 + 50 |+ Union[type[_T | Converter[_T]] | Converter[_T] | Callable[[str], _T]], ... # PYI055 +51 51 | ] = union.__args__ +52 52 | ... +53 53 | + +PYI055.py:56:15: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[_T | Converter[_T]]`. + | +54 | def convert_union(union: UnionType) -> _T | None: +55 | converters: tuple[ +56 | Union[type[_T] | type[Converter[_T]]] | Converter[_T] | Callable[[str], _T], ... # PYI055 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +57 | ] = union.__args__ +58 | ... + | + = help: Combine multiple `type` members + +ℹ Safe fix +53 53 | +54 54 | def convert_union(union: UnionType) -> _T | None: +55 55 | converters: tuple[ +56 |- Union[type[_T] | type[Converter[_T]]] | Converter[_T] | Callable[[str], _T], ... # PYI055 + 56 |+ Union[type[_T | Converter[_T]]] | Converter[_T] | Callable[[str], _T], ... # PYI055 +57 57 | ] = union.__args__ +58 58 | ... +59 59 | + +PYI055.py:62:15: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[_T | Converter[_T]]`. + | +60 | def convert_union(union: UnionType) -> _T | None: +61 | converters: tuple[ +62 | Union[type[_T] | type[Converter[_T]] | str] | Converter[_T] | Callable[[str], _T], ... # PYI055 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055 +63 | ] = union.__args__ +64 | ... + | + = help: Combine multiple `type` members + +ℹ Safe fix +59 59 | +60 60 | def convert_union(union: UnionType) -> _T | None: +61 61 | converters: tuple[ +62 |- Union[type[_T] | type[Converter[_T]] | str] | Converter[_T] | Callable[[str], _T], ... # PYI055 + 62 |+ Union[type[_T | Converter[_T]] | str] | Converter[_T] | Callable[[str], _T], ... # PYI055 +63 63 | ] = union.__args__ +64 64 | ... diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap index b845e5269adb81..0e41288be50738 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap @@ -120,7 +120,7 @@ PYI055.pyi:10:15: PYI055 [*] Multiple `type` members in a union. Combine them in 8 8 | z: Union[type[float, int], type[complex]] 9 9 | 10 |-def func(arg: type[int] | str | type[float]) -> None: ... - 10 |+def func(arg: type[int | float]) -> None: ... + 10 |+def func(arg: type[int | float] | str) -> None: ... 11 11 | 12 12 | # OK 13 13 | x: type[int, str, float]