Detect multiple type[Foo] in a union (#363)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Change Log
 
+## Unreleased
+
+New error codes:
+* Y055: Unions of the form `type[X] | type[Y]` can be simplified to `type[X | Y]`.
+  Similarly, `Union[type[X], type[Y]]` can be simplified to `type[Union[X, Y]]`.
+
 ## 23.4.0
 
 * Update error messages for Y019 and Y034 to recommend using

README.md

@@ -87,6 +87,7 @@ currently emitted:
 | Y052 | Y052 disallows assignments to constant values where the assignment does not have a type annotation. For example, `x = 0` in the global namespace is ambiguous in a stub, as there are four different types that could be inferred for the variable `x`: `int`, `Final[int]`, `Literal[0]`, or `Final[Literal[0]]`. Enum members are excluded from this check, as are various special assignments such as `__all__` and `__match_args__`.
 | Y053 | Only string and bytes literals <=50 characters long are permitted.
 | Y054 | Only numeric literals with a string representation <=10 characters long are permitted.
+| Y055 | Unions of the form `type[X] \| type[Y]` can be simplified to `type[X \| Y]`. Similarly, `Union[type[X], type[Y]]` can be simplified to `type[Union[X, Y]]`.
 
 Note that several error codes recommend using types from `typing_extensions` or
 `_typeshed`. Strictly speaking, these packages are not part of the standard

pyi.py

@@ -45,9 +45,9 @@ def unparse(node: ast.AST) -> str:
 FLAKE8_MAJOR_VERSION = flake8.__version_info__[0]
 
 if sys.version_info >= (3, 9):
-    _LiteralMember: TypeAlias = ast.expr
+    _SliceContents: TypeAlias = ast.expr
 else:
-    _LiteralMember: TypeAlias = Union[ast.expr, ast.slice]
+    _SliceContents: TypeAlias = Union[ast.expr, ast.slice]
 
 
 class Error(NamedTuple):
@@ -364,6 +364,7 @@ def _is_object(node: ast.AST | None, name: str, *, from_: Container[str]) -> boo
 _is_Self = partial(_is_object, name="Self", from_=({"_typeshed"} | _TYPING_MODULES))
 _is_TracebackType = partial(_is_object, name="TracebackType", from_={"types"})
 _is_builtins_object = partial(_is_object, name="object", from_={"builtins"})
+_is_builtins_type = partial(_is_object, name="type", from_={"builtins"})
 _is_Unused = partial(_is_object, name="Unused", from_={"_typeshed"})
 _is_Iterable = partial(_is_object, name="Iterable", from_={"typing", "collections.abc"})
 _is_AsyncIterable = partial(
@@ -636,13 +637,18 @@ class UnionAnalysis(NamedTuple):
     builtins_classes_in_union: set[str]
     multiple_literals_in_union: bool
     non_literals_in_union: bool
-    combined_literal_members: list[_LiteralMember]
+    combined_literal_members: list[_SliceContents]
+    # type subscript == type[Foo]
+    multiple_type_subscripts_in_union: bool
+    combined_type_subscripts: list[_SliceContents]
 
 
 def _analyse_union(members: Sequence[ast.expr]) -> UnionAnalysis:
     """Return a tuple providing analysis of a given sequence of union members.
 
-    >>> union = _ast_node_for('Union[int, memoryview, memoryview, Literal["foo"], Literal[1]]')
+    >>> union = _ast_node_for(
+    ...     'Union[int, memoryview, memoryview, Literal["foo"], Literal[1], type[float], type[str]]'
+    ... )
     >>> members = union.slice.elts if sys.version_info >= (3, 9) else union.slice.value.elts
     >>> analysis = _analyse_union(members)
     >>> len(analysis.members_by_dump["Name(id='memoryview', ctx=Load())"])
@@ -659,13 +665,18 @@ def _analyse_union(members: Sequence[ast.expr]) -> UnionAnalysis:
     True
     >>> unparse(ast.Tuple(analysis.combined_literal_members))
     "('foo', 1)"
+    >>> analysis.multiple_type_subscripts_in_union
+    True
+    >>> unparse(ast.Tuple(analysis.combined_type_subscripts))
+    '(float, str)'
     """
 
     non_literals_in_union = False
     members_by_dump: defaultdict[str, list[ast.expr]] = defaultdict(list)
     builtins_classes_in_union: set[str] = set()
     literals_in_union = []
-    combined_literal_members: list[_LiteralMember] = []
+    combined_literal_members: list[_SliceContents] = []
+    type_subscripts_in_union: list[_SliceContents] = []
 
     for member in members:
         members_by_dump[ast.dump(member)].append(member)
@@ -678,6 +689,8 @@ def _analyse_union(members: Sequence[ast.expr]) -> UnionAnalysis:
             literals_in_union.append(member.slice)
         else:
             non_literals_in_union = True
+        if isinstance(member, ast.Subscript) and _is_builtins_type(member.value):
+            type_subscripts_in_union.append(member.slice)
 
     for literal in literals_in_union:
         if isinstance(literal, ast.Tuple):
@@ -692,6 +705,8 @@ def _analyse_union(members: Sequence[ast.expr]) -> UnionAnalysis:
         multiple_literals_in_union=len(literals_in_union) >= 2,
         non_literals_in_union=non_literals_in_union,
         combined_literal_members=combined_literal_members,
+        multiple_type_subscripts_in_union=len(type_subscripts_in_union) >= 2,
+        combined_type_subscripts=type_subscripts_in_union,
     )
 
 
@@ -1246,7 +1261,9 @@ def visit_AnnAssign(self, node: ast.AnnAssign) -> None:
         if node_value and not _is_valid_default_value_with_annotation(node_value):
             self.error(node, Y015)
 
-    def _check_union_members(self, members: Sequence[ast.expr]) -> None:
+    def _check_union_members(
+        self, members: Sequence[ast.expr], is_pep_604_union: bool
+    ) -> None:
         first_union_member = members[0]
         analysis = _analyse_union(members)
 
@@ -1258,12 +1275,16 @@ def _check_union_members(self, members: Sequence[ast.expr]) -> None:
             self._check_for_Y051_violations(analysis)
             if analysis.multiple_literals_in_union:
                 self._error_for_multiple_literals_in_union(first_union_member, analysis)
+            elif analysis.multiple_type_subscripts_in_union:
+                self._error_for_multiple_type_subscripts_in_union(
+                    first_union_member, analysis, is_pep_604_union
+                )
             if self.visiting_arg.active:
                 self._check_for_redundant_numeric_unions(first_union_member, analysis)
 
     def _check_for_Y051_violations(self, analysis: UnionAnalysis) -> None:
         """Search for redundant unions fitting the pattern `str | Literal["foo"]`, etc."""
-        literal_classes_present: defaultdict[str, list[_LiteralMember]]
+        literal_classes_present: defaultdict[str, list[_SliceContents]]
         literal_classes_present = defaultdict(list)
         for literal in analysis.combined_literal_members:
             if isinstance(literal, ast.Str):
@@ -1319,6 +1340,27 @@ def _error_for_multiple_literals_in_union(
 
         self.error(first_union_member, Y030.format(suggestion=suggestion))
 
+    def _error_for_multiple_type_subscripts_in_union(
+        self,
+        first_union_member: ast.expr,
+        analysis: UnionAnalysis,
+        is_pep_604_union: bool,
+    ) -> None:
+        # Union using bit or, e.g. type[str] | type[int]
+        if is_pep_604_union:
+            new_union = " | ".join(
+                unparse(expr) for expr in analysis.combined_type_subscripts
+            )
+        # Union is the explicit Union type, e.g. Union[type[str], type[int]]
+        else:
+            type_slice = unparse(ast.Tuple(analysis.combined_type_subscripts)).strip(
+                "()"
+            )
+            new_union = f"Union[{type_slice}]"
+
+        suggestion = f'Combine them into one, e.g. "type[{new_union}]".'
+        self.error(first_union_member, Y055.format(suggestion=suggestion))
+
     def visit_BinOp(self, node: ast.BinOp) -> None:
         if not isinstance(node.op, ast.BitOr):
             self.generic_visit(node)
@@ -1339,7 +1381,7 @@ def visit_BinOp(self, node: ast.BinOp) -> None:
         for member in members:
             self.visit(member)
 
-        self._check_union_members(members)
+        self._check_union_members(members, is_pep_604_union=True)
 
     def visit_Subscript(self, node: ast.Subscript) -> None:
         subscripted_object = node.value
@@ -1359,7 +1401,7 @@ def visit_Subscript(self, node: ast.Subscript) -> None:
 
     def _visit_slice_tuple(self, node: ast.Tuple, parent: str | None) -> None:
         if parent == "Union":
-            self._check_union_members(node.elts)
+            self._check_union_members(node.elts, is_pep_604_union=False)
             self.visit(node)
         elif parent == "Annotated":
             # Allow literals, except in the first argument
@@ -2084,3 +2126,4 @@ def parse_options(
     "Y054 Numeric literals with a string representation "
     ">10 characters long are not permitted"
 )
+Y055 = 'Y055 Multiple "type[Foo]" members in a union. {suggestion}'

tests/union_duplicates.pyi

@@ -2,22 +2,51 @@
 import builtins
 import typing
 from collections.abc import Mapping
-from typing import Union
+from typing import (  # Y022 Use "type[MyClass]" instead of "typing.Type[MyClass]" (PEP 585 syntax)
+    Type,
+    Union,
+)
 
 import typing_extensions
-from typing_extensions import Literal, TypeAlias
+from typing_extensions import (  # Y022 Use "type[MyClass]" instead of "typing_extensions.Type[MyClass]" (PEP 585 syntax)
+    Literal,
+    Type as Type_,
+    TypeAlias,
+)
 
 def f1_pipe(x: int | str) -> None: ...
 def f2_pipe(x: int | int) -> None: ...  # Y016 Duplicate union member "int"
 def f3_pipe(x: None | int | int) -> None: ...  # Y016 Duplicate union member "int"
 def f4_pipe(x: int | None | int) -> None: ...  # Y016 Duplicate union member "int"
 def f5_pipe(x: int | int | None) -> None: ...  # Y016 Duplicate union member "int"
+def f6_pipe(x: type[int] | type[str] | type[float]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[int | str | float]".
+def f7_pipe(x: type[int] | str | type[float]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[int | float]".
+def f8_pipe(x: builtins.type[int] | builtins.type[str] | builtins.type[float]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[int | str | float]".
+def f9_pipe(x: builtins.type[int] | str | builtins.type[float]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[int | float]".
+def f10_pipe(x: type[int] | builtins.type[float]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[int | float]".
+# typing.Type and typing_extensions.Type are intentionally excluded from Y055
+# The following type annotations should not generate any Y055 errors
+def f11_pipe(x: Type[int] | Type[str]) -> None: ...
+def f12_pipe(x: typing.Type[int] | typing.Type[str]) -> None: ...  # Y022 Use "type[MyClass]" instead of "typing.Type[MyClass]" (PEP 585 syntax)  # Y022 Use "type[MyClass]" instead of "typing.Type[MyClass]" (PEP 585 syntax)
+def f13_pipe(x: Type_[int] | Type_[str]) -> None: ...
+def f14_pipe(x: typing_extensions.Type[int] | typing_extensions.Type[str]) -> None: ...  # Y022 Use "type[MyClass]" instead of "typing_extensions.Type[MyClass]" (PEP 585 syntax)  # Y022 Use "type[MyClass]" instead of "typing_extensions.Type[MyClass]" (PEP 585 syntax)
 
 def f1_union(x: Union[int, str]) -> None: ...
 def f2_union(x: Union[int, int]) -> None: ...  # Y016 Duplicate union member "int"
 def f3_union(x: Union[None, int, int]) -> None: ...  # Y016 Duplicate union member "int"
 def f4_union(x: typing.Union[int, None, int]) -> None: ...  # Y016 Duplicate union member "int"
 def f5_union(x: typing.Union[int, int, None]) -> None: ...  # Y016 Duplicate union member "int"
+def f6_union(x: Union[type[int], type[str], type[float]]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[Union[int, str, float]]".
+def f7_union(x: Union[type[int], str, type[float]]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[Union[int, float]]".
+def f8_union(x: Union[builtins.type[int], builtins.type[str], builtins.type[float]]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[Union[int, str, float]]".
+def f9_union(x: Union[builtins.type[int], str, builtins.type[float]]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[Union[int, float]]".
+def f10_union(x: Union[type[int], builtins.type[float]]) -> None: ...  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[Union[int, float]]".
+# typing.Type and typing_extensions.Type are intentionally excluded from Y055
+# The following type annotations should not generate any Y055 errors
+def f11_union(x: Union[Type[int], Type[str]]) -> None: ...
+def f12_union(x: Union[typing.Type[int], typing.Type[str]]) -> None: ...  # Y022 Use "type[MyClass]" instead of "typing.Type[MyClass]" (PEP 585 syntax) # Y022 Use "type[MyClass]" instead of "typing.Type[MyClass]" (PEP 585 syntax)
+def f13_union(x: Union[Type_[int], Type_[str]]) -> None: ...
+def f14_union(x: Union[typing_extensions.Type[int], typing_extensions.Type[str]]) -> None: ...  # Y022 Use "type[MyClass]" instead of "typing_extensions.Type[MyClass]" (PEP 585 syntax) # Y022 Use "type[MyClass]" instead of "typing_extensions.Type[MyClass]" (PEP 585 syntax)
 
 just_literals_subscript_union: Union[Literal[1], typing.Literal[2]]  # Y030 Multiple Literal members in a union. Use a single Literal, e.g. "Literal[1, 2]".
 mixed_subscript_union: Union[bytes, Literal['foo'], typing_extensions.Literal['bar']]  # Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal['foo', 'bar']".
@@ -36,6 +65,8 @@ c: Union[builtins.complex, memoryview, slice, int]  # No error here, Y041 only a
 
 # Don't error with Y041 here, the two error messages combined are quite confusing
 def foo(d: int | int | float) -> None: ...  # Y016 Duplicate union member "int"
+# Don't error with Y055 here either
+def baz(d: type[int] | type[int]) -> None: ...  # Y016 Duplicate union member "type[int]"
 
 def bar(f: Literal["foo"] | Literal["bar"] | int | float | builtins.bool) -> None: ...  # Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal['foo', 'bar']".  # Y041 Use "float" instead of "int | float" (see "The numeric tower" in PEP 484)
 
@@ -54,3 +85,6 @@ class Four:
 DupesHereSoNoY051: TypeAlias = int | int | Literal[42]  # Y016 Duplicate union member "int"
 NightmareAlias1 = int | float | Literal[4, b"bar"] | Literal["foo"]  # Y026 Use typing_extensions.TypeAlias for type aliases, e.g. "NightmareAlias1: TypeAlias = int | float | Literal[4, b'bar'] | Literal['foo']"  # Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal[4, b'bar', 'foo']".  # Y051 "Literal[4]" is redundant in a union with "int"
 nightmare_alias2: TypeAlias = int | float | Literal[True, 4] | Literal["foo"]  # Y042 Type aliases should use the CamelCase naming convention  # Y030 Multiple Literal members in a union. Combine them into one, e.g. "Literal[True, 4, 'foo']".  # Y051 "Literal[4]" is redundant in a union with "int"
+DoublyNestedAlias: TypeAlias = Union[type[str], type[float] | type[bytes]]  # Y055 Multiple "type[Foo]" members in a union. Combine them into one, e.g. "type[float | bytes]".
+# typing.Type and typing_extensions.Type are intentionally excluded from Y055
+DoublyNestedAlias2: TypeAlias = Union[Type[str], typing.Type[float], Type_[bytes], typing_extensions.Type[complex]]  # Y022 Use "type[MyClass]" instead of "typing.Type[MyClass]" (PEP 585 syntax) # Y022 Use "type[MyClass]" instead of "typing_extensions.Type[MyClass]" (PEP 585 syntax)