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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Change Log

## Unreleased

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

## 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
89 changes: 69 additions & 20 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,19 @@ def _is_type_or_Type(node: ast.expr) -> bool:
return cls_name in {"type", "Type"}


def _is_None(node: ast.expr) -> bool:
return isinstance(node, ast.Constant) and node.value is None
if sys.version_info >= (3, 9):

def _is_None(node: ast.expr) -> bool:
return isinstance(node, ast.Constant) and node.value is None

else:

def _is_None(node: Union[ast.expr, ast.slice]) -> bool:
if isinstance(node, ast.Constant):
return node.value is None
elif isinstance(node, ast.Index):
return node.value is None
return False


class ExitArgAnalysis(NamedTuple):
Expand Down Expand Up @@ -671,6 +682,41 @@ 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)
if _is_None(member):
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 +1471,26 @@ 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():
if len(member_list) > 1 and not _is_None(member_list[0]):
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 +2306,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
8 changes: 8 additions & 0 deletions tests/literals.pyi
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
from typing import Literal

Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | 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"

# If both Y061 and Y062 would be emitted, only emit Y062
Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"