Skip to content

Commit

Permalink
Add Y062: Forbid duplicates in Literal[] slices (#449)
Browse files Browse the repository at this point in the history
Previous discussion for context:
#435 (comment)

Example:
`Literal[True, True, False, False]` - emits Y062 for both `True` and `False`.

This is how the new rule interacts with Y061:
- `Literal[None]`, `Literal[None, None]`, etc.. - Only Y061 is emitted
   and the suggestion is always just `None`
   (previously `Literal[None, None]` would suggest `Literal[] | None`)
- Combination of `None` and other duplicates
   e.g. `Literal[None, True, True]` - only Y062 is emitted
---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
  • Loading branch information
tomasr8 and AlexWaygood authored Nov 15, 2023
1 parent 7933e84 commit 70aa7a3
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 19 deletions.
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 causes Y062 to be emitted.
| 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"

0 comments on commit 70aa7a3

Please sign in to comment.