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

Add Y062: Forbid duplicates in Literal[] slices #449

Merged
merged 11 commits into from
Nov 15, 2023
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Change Log

## Unreleased

New error codes:
* Y062: Disallow duplicate elements inside `Literal[]` slices.

Other changes:
* Y061 is no longer emitted in situations where Y062 would also be emitted.

## 23.11.0

New error codes:
Expand Down
3 changes: 2 additions & 1 deletion ERRORCODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ The following warnings are currently emitted by default:
| Y058 | Use `Iterator` rather than `Generator` as the return value for simple `__iter__` methods, and `AsyncIterator` rather than `AsyncGenerator` as the return value for simple `__aiter__` methods. Using `(Async)Iterator` for these methods is simpler and more elegant, and reflects the fact that the precise kind of iterator returned from an `__iter__` method is usually an implementation detail that could change at any time, and should not be relied upon.
| Y059 | `Generic[]` should always be the last base class, if it is present in a class's bases tuple. At runtime, if `Generic[]` is not the final class in a the bases tuple, this [can cause the class creation to fail](https://github.com/python/cpython/issues/106102). In a stub file, however, this rule is enforced purely for stylistic consistency.
| Y060 | Redundant inheritance from `Generic[]`. For example, `class Foo(Iterable[_T], Generic[_T]): ...` can be written more simply as `class Foo(Iterable[_T]): ...`.<br><br>To avoid false-positive errors, and to avoid complexity in the implementation, this check is deliberately conservative: it only flags classes where all subscripted bases have identical code inside their subscript slices.
| Y061 | Do not use `None` inside a `Literal[]` slice. For example, use `Literal["foo"] \| None` instead of `Literal["foo", None]`. While both are legal according to [PEP 586](https://peps.python.org/pep-0586/), the former is preferred for stylistic consistency.
| Y061 | Do not use `None` inside a `Literal[]` slice. For example, use `Literal["foo"] \| None` instead of `Literal["foo", None]`. While both are legal according to [PEP 586](https://peps.python.org/pep-0586/), the former is preferred for stylistic consistency. Note that this warning is not emitted if Y062 is emitted for the same `Literal[]` slice. For example, `Literal[None, None, True, True]` only emits Y062.
| Y062 | `Literal[]` slices shouldn't contain duplicates, e.g. `Literal[True, True]` is not allowed.

## Warnings disabled by default

Expand Down
78 changes: 60 additions & 18 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,43 @@ def _analyse_union(members: Sequence[ast.expr]) -> UnionAnalysis:
)


class TypingLiteralAnalysis(NamedTuple):
members_by_dump: defaultdict[str, list[_SliceContents]]
members_without_none: list[_SliceContents]
none_members: list[_SliceContents]
contains_only_none: bool


def _analyse_typing_Literal(node: ast.Subscript) -> TypingLiteralAnalysis:
"""Return a tuple providing analysis of a `typing.Literal` slice."""

members: Sequence[_SliceContents]
members_by_dump: defaultdict[str, list[_SliceContents]] = defaultdict(list)
members_without_none: list[_SliceContents] = []
none_members: list[_SliceContents] = []

if isinstance(node.slice, ast.Tuple):
members = node.slice.elts
else:
members = [node.slice]

for member in members:
members_by_dump[ast.dump(member)].append(member)
# https://github.com/PyCQA/flake8-pyi/pull/449#discussion_r1391804472
# TODO: Remove the `type: ignore` when we drop support for py38
if _is_None(member): # type: ignore[arg-type,unused-ignore]
none_members.append(member)
else:
members_without_none.append(member)

return TypingLiteralAnalysis(
members_by_dump=members_by_dump,
members_without_none=members_without_none,
none_members=none_members,
contains_only_none=bool(none_members and not members_without_none),
)


_ALLOWED_MATH_ATTRIBUTES_IN_DEFAULTS = frozenset(
{"math.inf", "math.nan", "math.e", "math.pi", "math.tau"}
)
Expand Down Expand Up @@ -1425,24 +1462,28 @@ def visit_Subscript(self, node: ast.Subscript) -> None:
self._Y090_error(node)

def _visit_typing_Literal(self, node: ast.Subscript) -> None:
if isinstance(node.slice, ast.Constant) and _is_None(node.slice):
# Special case for `Literal[None]`
self.error(node.slice, Y061.format(suggestion="None"))
elif isinstance(node.slice, ast.Tuple):
elts = node.slice.elts
for i, elt in enumerate(elts):
if _is_None(elt):
elts_without_none = elts[:i] + [
elt for elt in elts[i + 1 :] if not _is_None(elt)
]
if len(elts_without_none) == 1:
new_literal_slice = unparse(elts_without_none[0])
else:
new_slice_node = ast.Tuple(elts=elts_without_none)
new_literal_slice = unparse(new_slice_node).strip("()")
suggestion = f"Literal[{new_literal_slice}] | None"
self.error(elt, Y061.format(suggestion=suggestion))
break # Only report the first `None`
analysis = _analyse_typing_Literal(node)

Y062_encountered = False
for member_list in analysis.members_by_dump.values():
# https://github.com/PyCQA/flake8-pyi/pull/449#discussion_r1391804472
# TODO: Remove the `type: ignore` when we drop support for py38
if len(member_list) > 1 and not _is_None(member_list[0]): # type: ignore[arg-type,unused-ignore]
Y062_encountered = True
self.error(member_list[1], Y062.format(unparse(member_list[1])))

if not Y062_encountered:
if analysis.contains_only_none:
self.error(node.slice, Y061.format(suggestion="None"))
elif analysis.none_members:
if len(analysis.members_without_none) == 1:
new_literal_slice = unparse(analysis.members_without_none[0])
else:
new_slice_node = ast.Tuple(elts=analysis.members_without_none)
new_literal_slice = unparse(new_slice_node).strip("()")
suggestion = f"Literal[{new_literal_slice}] | None"
self.error(analysis.none_members[0], Y061.format(suggestion=suggestion))

self.visit(node.slice)

def _visit_slice_tuple(self, node: ast.Tuple, parent: str | None) -> None:
Expand Down Expand Up @@ -2258,6 +2299,7 @@ def parse_options(options: argparse.Namespace) -> None:
"class would be inferred as generic anyway"
)
Y061 = 'Y061 None inside "Literal[]" expression. Replace with "{suggestion}"'
Y062 = 'Y062 Duplicate "Literal[]" member "{}"'
Y090 = (
'Y090 "{original}" means '
'"a tuple of length 1, in which the sole element is of type {typ!r}". '
Expand Down
18 changes: 18 additions & 0 deletions tests/literals.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,22 @@ from typing import Literal

Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"

Literal[True, True] # Y062 Duplicate "Literal[]" member "True"
Literal[True, True, True] # Y062 Duplicate "Literal[]" member "True"
Literal[True, False, True, False] # Y062 Duplicate "Literal[]" member "True" # Y062 Duplicate "Literal[]" member "False"

###
# The following rules here are slightly subtle,
# but make sense when it comes to giving the best suggestions to users of flake8-pyi.
###

# If Y061 and Y062 both apply, but all the duplicate members are None,
# only emit Y061...
Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"

# ... but if Y061 and Y062 both apply
# and there are no None members in the Literal[] slice,
# only emit Y062:
Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"