diff --git a/CHANGELOG.md b/CHANGELOG.md index aedaa20f..275c55e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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: diff --git a/ERRORCODES.md b/ERRORCODES.md index a9db5b7b..1e0bd31d 100644 --- a/ERRORCODES.md +++ b/ERRORCODES.md @@ -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]): ...`.

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 diff --git a/pyi.py b/pyi.py index 1164bf5d..971bde3d 100644 --- a/pyi.py +++ b/pyi.py @@ -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"} ) @@ -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: @@ -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}". ' diff --git a/tests/literals.pyi b/tests/literals.pyi index a619881f..57124c07 100644 --- a/tests/literals.pyi +++ b/tests/literals.pyi @@ -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"