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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## 23.4.0

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]]`.

Other changes:
tomasr8 marked this conversation as resolved.
Show resolved Hide resolved
* Update error messages for Y019 and Y034 to recommend using
`typing_extensions.Self` rather than `_typeshed.Self`.

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]]`.
tomasr8 marked this conversation as resolved.
Show resolved Hide resolved

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
56 changes: 52 additions & 4 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ def unparse(node: ast.AST) -> str:

if sys.version_info >= (3, 9):
_LiteralMember: TypeAlias = ast.expr
_BuiltinsTypeMember: TypeAlias = ast.expr
else:
_LiteralMember: TypeAlias = Union[ast.expr, ast.slice]
_BuiltinsTypeMember: TypeAlias = Union[ast.expr, ast.slice]
tomasr8 marked this conversation as resolved.
Show resolved Hide resolved


class Error(NamedTuple):
Expand Down Expand Up @@ -364,6 +366,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 @@ -637,12 +640,15 @@ class UnionAnalysis(NamedTuple):
multiple_literals_in_union: bool
non_literals_in_union: bool
combined_literal_members: list[_LiteralMember]
multiple_builtins_types_in_union: bool
non_builtins_types_in_union: bool
combined_builtins_types: list[_BuiltinsTypeMember]
tomasr8 marked this conversation as resolved.
Show resolved Hide resolved


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]]')
tomasr8 marked this conversation as resolved.
Show resolved Hide resolved
>>> 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,21 @@ def _analyse_union(members: Sequence[ast.expr]) -> UnionAnalysis:
True
>>> unparse(ast.Tuple(analysis.combined_literal_members))
"('foo', 1)"
>>> analysis.multiple_builtins_types_in_union
True
>>> analysis.non_builtins_types_in_union
True
>>> unparse(ast.Tuple(analysis.combined_builtins_types))
'(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] = []
non_builtins_types_in_union = False
builtins_types_in_union: list[_BuiltinsTypeMember] = []

for member in members:
members_by_dump[ast.dump(member)].append(member)
Expand All @@ -678,6 +692,10 @@ 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):
builtins_types_in_union.append(member.slice)
else:
non_builtins_types_in_union = True

for literal in literals_in_union:
if isinstance(literal, ast.Tuple):
Expand All @@ -692,6 +710,9 @@ 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_builtins_types_in_union=len(builtins_types_in_union) >= 2,
non_builtins_types_in_union=non_builtins_types_in_union,
combined_builtins_types=builtins_types_in_union,
)


Expand Down Expand Up @@ -1246,7 +1267,7 @@ 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], parent: str) -> None:
first_union_member = members[0]
analysis = _analyse_union(members)

Expand All @@ -1258,6 +1279,10 @@ 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_builtins_types_in_union:
self._error_for_multiple_type_builtins_in_union(
first_union_member, analysis, parent
)
if self.visiting_arg.active:
self._check_for_redundant_numeric_unions(first_union_member, analysis)

Expand Down Expand Up @@ -1319,6 +1344,28 @@ def _error_for_multiple_literals_in_union(

self.error(first_union_member, Y030.format(suggestion=suggestion))

def _error_for_multiple_type_builtins_in_union(
self, first_union_member: ast.expr, analysis: UnionAnalysis, parent: str
) -> None:
# Union is the explicit Union type, e.g. Union[type[str], type[int]]
if parent == "Union":
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
type_slice = unparse(ast.Tuple(analysis.combined_builtins_types)).strip(
"()"
)
new_union = f"Union[{type_slice}]"
# Union using bit or, e.g. type[str] | type[int]
else:
new_union = " | ".join(
unparse(expr) for expr in analysis.combined_builtins_types
)

if analysis.non_builtins_types_in_union:
suggestion = f'Combine them into one, e.g. "type[{new_union}]".'
else:
suggestion = f'Use a single type expression, 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 +1386,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, parent="BitOr")

def visit_Subscript(self, node: ast.Subscript) -> None:
subscripted_object = node.value
Expand All @@ -1359,7 +1406,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, parent="Union")
self.visit(node)
elif parent == "Annotated":
# Allow literals, except in the first argument
Expand Down Expand Up @@ -2084,3 +2131,4 @@ def parse_options(
"Y054 Numeric literals with a string representation "
">10 characters long are not permitted"
)
Y055 = "Y055 Multiple type builtins in a union. {suggestion}"
tomasr8 marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 7 additions & 0 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 @@ -12,12 +12,16 @@ 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 builtins in a union. Use a single type expression, e.g. "type[int | str | float]".
def f7_pipe(x: type[int] | str | type[float]) -> None: ... # Y055 Multiple type builtins in a union. Combine them into one, e.g. "type[int | float]".

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 builtins in a union. Use a single type expression, e.g. "type[Union[int, str, float]]".
def f7_union(x: Union[type[int], str, type[float]]) -> None: ... # Y055 Multiple type builtins in a union. Combine them into one, e.g. "type[Union[int, float]]".

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 +40,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 +60,4 @@ 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 builtins in a union. Use a single type expression, e.g. "type[float | bytes]".