Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect multiple type builtins in a union #363

Merged
merged 13 commits into from
Apr 11, 2023
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 52 additions & 9 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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())"])
Expand All @@ -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)
Expand All @@ -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):
Expand All @@ -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,
)


Expand Down Expand Up @@ -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)

Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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}'
38 changes: 36 additions & 2 deletions tests/union_duplicates.pyi
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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']".
Expand All @@ -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)

Expand All @@ -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)