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

Enforce PEP-570 syntax in stubs #461

Merged
merged 6 commits into from
Mar 10, 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
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:
* Y063: Use [PEP 570 syntax](https://peps.python.org/pep-0570/) to mark
positional-only arguments, rather than
[the older Python 3.7-compatible syntax](https://peps.python.org/pep-0484/#positional-only-arguments)
described in PEP 484.

## 24.1.0

New error codes:
Expand Down
1 change: 1 addition & 0 deletions ERRORCODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ The following warnings are currently emitted by default:
| 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. | Style
| 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

## Warnings disabled by default

Expand Down
30 changes: 30 additions & 0 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -2089,6 +2089,34 @@ def check_self_typevars(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> N
return_annotation=return_annotation,
)

@staticmethod
def _is_positional_pre_570_argname(name: str) -> bool:
# https://peps.python.org/pep-0484/#positional-only-arguments
return name.startswith("__") and len(name) >= 3 and not name.endswith("__")

def _check_pep570_syntax_used_where_applicable(
self, node: ast.FunctionDef | ast.AsyncFunctionDef
) -> None:
if node.args.posonlyargs:
return
pos_or_kw_args = node.args.args
try:
first_param = pos_or_kw_args[0]
except IndexError:
return
if self.enclosing_class_ctx is None or any(
isinstance(decorator, ast.Name) and decorator.id == "staticmethod"
for decorator in node.decorator_list
):
uses_old_syntax = self._is_positional_pre_570_argname(first_param.arg)
else:
uses_old_syntax = self._is_positional_pre_570_argname(first_param.arg) or (
len(pos_or_kw_args) >= 2
and self._is_positional_pre_570_argname(pos_or_kw_args[1].arg)
)
if uses_old_syntax:
self.error(node, Y063)

def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None:
with self.in_function.enabled():
self.generic_visit(node)
Expand All @@ -2110,6 +2138,7 @@ def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None:
):
self.error(statement, Y010)

self._check_pep570_syntax_used_where_applicable(node)
if self.enclosing_class_ctx is not None:
self.check_self_typevars(node)

Expand Down Expand Up @@ -2336,6 +2365,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"
Y090 = (
'Y090 "{original}" means '
'"a tuple of length 1, in which the sole element is of type {typ!r}". '
Expand Down
10 changes: 7 additions & 3 deletions tests/exit_methods.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class GoodTwo:
async def __aexit__(self, typ: Type[BaseException] | None, *args: object) -> bool: ...

class GoodThree:
def __exit__(self, __typ: typing.Type[BaseException] | None, exc: BaseException | None, *args: object) -> None: ...
def __exit__(self, typ: typing.Type[BaseException] | None, /, exc: BaseException | None, *args: object) -> None: ...
async def __aexit__(self, typ: typing_extensions.Type[BaseException] | None, __exc: BaseException | None, *args: object) -> None: ...

class GoodFour:
Expand All @@ -41,7 +41,7 @@ class GoodSix:
def __aexit__(self, typ: Type[BaseException] | None, *args: _typeshed.Unused) -> Awaitable[None]: ...

class GoodSeven:
def __exit__(self, __typ: typing.Type[BaseException] | None, exc: BaseException | None, *args: _typeshed.Unused) -> bool: ...
def __exit__(self, typ: typing.Type[BaseException] | None, /, exc: BaseException | None, *args: _typeshed.Unused) -> bool: ...
def __aexit__(self, typ: type[BaseException] | None, exc: BaseException | None, tb: TracebackType | None, weird_extra_arg: int = ..., *args: Unused, **kwargs: Unused) -> Awaitable[None]: ...


Expand All @@ -56,8 +56,12 @@ class BadTwo:

class BadThree:
def __exit__(self, typ: type[BaseException], exc: BaseException | None, tb: TracebackType | None) -> None: ... # Y036 Badly defined __exit__ method: The first arg in an __exit__ method should be annotated with "type[BaseException] | None" or "object", not "type[BaseException]"
async def __aexit__(self, __typ: type[BaseException] | None, __exc: BaseException, __tb: TracebackType) -> bool | None: ... # Y036 Badly defined __aexit__ method: The second arg in an __aexit__ method should be annotated with "BaseException | None" or "object", not "BaseException" # Y036 Badly defined __aexit__ method: The third arg in an __aexit__ method should be annotated with "types.TracebackType | None" or "object", not "TracebackType"
async def __aexit__(self, typ: type[BaseException] | None, exc: BaseException, tb: TracebackType, /) -> bool | None: ... # Y036 Badly defined __aexit__ method: The second arg in an __aexit__ method should be annotated with "BaseException | None" or "object", not "BaseException" # Y036 Badly defined __aexit__ method: The third arg in an __aexit__ method should be annotated with "types.TracebackType | None" or "object", not "TracebackType"

class BadFour:
def __exit__(self, typ: BaseException | None, *args: list[str]) -> bool: ... # Y036 Badly defined __exit__ method: Star-args in an __exit__ method should be annotated with "object", not "list[str]" # Y036 Badly defined __exit__ method: The first arg in an __exit__ method should be annotated with "type[BaseException] | None" or "object", not "BaseException | None"
def __aexit__(self, *args: Any) -> Awaitable[None]: ... # Y036 Badly defined __aexit__ method: Star-args in an __aexit__ method should be annotated with "object", not "Any"

class ThisExistsToTestInteractionBetweenY036AndY063:
def __exit__(self, __typ, exc, tb, weird_extra_arg) -> None: ... # Y036 Badly defined __exit__ method: All arguments after the first 4 in an __exit__ method must have a default value # Y063 Use PEP-570 syntax to indicate positional-only arguments
async def __aexit__(self, __typ: type[BaseException] | None, __exc: BaseException, __tb: TracebackType) -> bool | None: ... # Y036 Badly defined __aexit__ method: The second arg in an __aexit__ method should be annotated with "BaseException | None" or "object", not "BaseException" # Y036 Badly defined __aexit__ method: The third arg in an __aexit__ method should be annotated with "types.TracebackType | None" or "object", not "TracebackType" # Y063 Use PEP-570 syntax to indicate positional-only arguments
64 changes: 64 additions & 0 deletions tests/pep570.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# See https://peps.python.org/pep-0484/#positional-only-arguments
# for the full details on which arguments using the older syntax should/shouldn't
# be considered positional-only arguments by type checkers.
from typing import Self

def bad(__x: int) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments
def also_bad(__x: int, __y: str) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments
def still_bad(__x_: int) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments

def no_args() -> None: ...
def okay(__x__: int) -> None: ...
# The first argument isn't positional-only, so logically the second can't be either:
def also_okay(x: int, __y: str) -> None: ...
def fine(x: bytes, /) -> None: ...
def no_idea_why_youd_do_this(__x: int, /, __y: str) -> None: ...
def cool(_x__: int) -> None: ...
def also_cool(x__: int) -> None: ...
def unclear_from_pep_484_if_this_is_positional_or_not(__: str) -> None: ...
def _(_: int) -> None: ...

class Foo:
def bad(__self) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments
@staticmethod
def bad2(__self) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments
def bad3(__self, __x: int) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments
def still_bad(self, __x_: int) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments
@staticmethod
def this_is_bad_too(__x: int) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments
@classmethod
def not_good(cls, __foo: int) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments

# The first non-self argument isn't positional-only, so logically the second can't be either:
def okay1(self, x: int, __y: int) -> None: ...
# Same here:
@staticmethod
def okay2(x: int, __y_: int) -> None: ...
@staticmethod
def no_args() -> int: ...
def okay3(__self__, __x__: int, __y: str) -> None: ...
def okay4(self, /) -> None: ...
def okay5(self, x: int, /) -> None: ...
def okay6(__self, /) -> None: ...
def cool(_self__: int) -> None: ...
def also_cool(self__: int) -> None: ...
def unclear_from_pep_484_if_this_is_positional_or_not(__: str) -> None: ...
def _(_: int) -> None: ...
@classmethod
def fine(cls, foo: int, /) -> None: ...

class Metaclass(type):
@classmethod
def __new__(mcls, __name: str, __bases: tuple[type, ...], __namespace: dict, **kwds) -> Self: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments

class Metaclass2(type):
@classmethod
def __new__(metacls, __name: str, __bases: tuple[type, ...], __namespace: dict, **kwds) -> Self: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments

class GoodMetaclass(type):
@classmethod
def __new__(mcls, name: str, bases: tuple[type, ...], namespace: dict, /, **kwds) -> Self: ...

class GoodMetaclass2(type):
@classmethod
def __new__(metacls, name: str, bases: tuple[type, ...], namespace: dict, /, **kwds) -> Self: ...
Loading