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

New rule: prefer x: Final = 42 over x: Final[Literal[42]] #469

Merged
merged 3 commits into from
Mar 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ERRORCODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ The following warnings are currently emitted by default:
| 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. | Style
| Y062 | `Literal[]` slices shouldn't contain duplicates, e.g. `Literal[True, True]` is not allowed. | Redundant code
| Y063 | Use [PEP 570 syntax](https://peps.python.org/pep-0570/) (e.g. `def foo(x: int, /) -> None: ...`) to denote positional-only arguments, rather than [the older Python 3.7-compatible syntax described in PEP 484](https://peps.python.org/pep-0484/#positional-only-arguments) (`def foo(__x: int) -> None: ...`, etc.). | Style
| Y064 | Use simpler syntax to define final literal types. For example, use `x: Final = 42` instead of `x: Final[Literal[42]]`. | Style

## Warnings disabled by default

Expand Down
43 changes: 41 additions & 2 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,7 @@ class PyiVisitor(ast.NodeVisitor):
long_strings_allowed: NestingCounter
in_function: NestingCounter
visiting_arg: NestingCounter
Y061_suppressed: NestingCounter

# This is only relevant for visiting classes
enclosing_class_ctx: EnclosingClassContext | None = None
Expand All @@ -1058,6 +1059,7 @@ def __init__(self, filename: str) -> None:
self.long_strings_allowed = NestingCounter()
self.in_function = NestingCounter()
self.visiting_arg = NestingCounter()
self.Y061_suppressed = NestingCounter()

def __repr__(self) -> str:
return f"{self.__class__.__name__}(filename={self.filename!r})"
Expand Down Expand Up @@ -1318,7 +1320,14 @@ def visit_AnnAssign(self, node: ast.AnnAssign) -> None:
)

self.visit(node_target)
self.visit(node_annotation)

Y064_encountered = self._check_for_Y064_violations(node)
if Y064_encountered:
with self.Y061_suppressed.enabled():
self.visit(node_annotation)
else:
self.visit(node_annotation)

if node_value is not None:
if is_typealias:
self.visit(node_value)
Expand Down Expand Up @@ -1354,6 +1363,35 @@ def visit_TypeAlias(self, node: ast.TypeAlias) -> None:
self.generic_visit(node)
self._check_typealias(node=node, alias_name=node.name.id)

def _check_for_Y064_violations(self, node: ast.AnnAssign) -> bool:
annotation = node.annotation

if node.value or not isinstance(annotation, ast.Subscript):
return False

value = annotation.value
slice_ = annotation.slice

if (
_is_Final(value)
and isinstance(slice_, ast.Subscript)
and _is_Literal(slice_.value)
and isinstance(slice_.slice, ast.Constant)
):
final = ast.Name(id="Final", ctx=ast.Load())
suggestion = ast.AnnAssign(
target=node.target,
annotation=final,
value=slice_.slice,
simple=node.simple,
)
self.error(
node,
Y064.format(suggestion=unparse(suggestion), original=unparse(node)),
)
return True
return False

def _check_union_members(
self, members: Sequence[ast.expr], is_pep_604_union: bool
) -> None:
Expand Down Expand Up @@ -1513,7 +1551,7 @@ def _visit_typing_Literal(self, node: ast.Subscript) -> None:
Y062_encountered = True
self.error(member_list[1], Y062.format(unparse(member_list[1])))

if not Y062_encountered:
if not Y062_encountered and not self.Y061_suppressed.active:
if analysis.contains_only_none:
self.error(node.slice, Y061.format(suggestion="None"))
elif analysis.none_members:
Expand Down Expand Up @@ -2366,6 +2404,7 @@ def parse_options(options: argparse.Namespace) -> None:
Y061 = 'Y061 None inside "Literal[]" expression. Replace with "{suggestion}"'
Y062 = 'Y062 Duplicate "Literal[]" member "{}"'
Y063 = "Y063 Use PEP-570 syntax to indicate positional-only arguments"
Y064 = 'Y064 Use "{suggestion}" instead of "{original}"'
Y090 = (
'Y090 "{original}" means '
'"a tuple of length 1, in which the sole element is of type {typ!r}". '
Expand Down
7 changes: 6 additions & 1 deletion tests/literals.pyi
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Literal
from typing import Final, Literal

Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
Expand All @@ -21,3 +21,8 @@ Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replac
# and there are no None members in the Literal[] slice,
# only emit Y062:
Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"

x: Final[Literal[True]] # Y064 Use "x: Final = True" instead of "x: Final[Literal[True]]"
# If Y061 and Y064 both apply, only emit Y064
y: Final[Literal[None]] # Y064 Use "y: Final = None" instead of "y: Final[Literal[None]]"
z: Final[Literal[True, False]]
Loading