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

Warn on Callable[<parameter_list>, None] and Callable[<parameter_list>, Any] as parameter annotations #412

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
21 changes: 13 additions & 8 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@

## Unreleased

* Introduce Y090, which warns if you have an annotation such as `tuple[int]` or
`Tuple[int]`. These mean "a tuple of length 1, in which the sole element is
of type `int`". This is sometimes what you want, but more usually you'll want
`tuple[int, ...]`, which means "a tuple of arbitrary (possibly 0) length, in
which all elements are of type `int`".

This error code is disabled by default due to the risk of false-positive
errors. To enable it, use the `--extend-select=Y090` option.
* Introduce several codes which are disabled by default due to the risk of
false-positive errors. To enable these lints, use the
`--extend-select={code1,code2,...}` option.
* Y090, which warns if you have an annotation such as `tuple[int]` or
`Tuple[int]`. These mean "a tuple of length 1, in which the sole element is
of type `int`". This is sometimes what you want, but more usually you'll want
`tuple[int, ...]`, which means "a tuple of arbitrary (possibly 0) length, in
which all elements are of type `int`".
* Y091, which warns if `Callable[<parameter_list>, Any]` or
`Callable[<parameter_list>, None]` is used as an argument annotation in a
function or method. Most of the time, the returned object from callbacks like
these is ignored at runtime, so the annotation you're probably looking for is
`Callable[<parameter_list>, object]`.

## 23.6.0

Expand Down
1 change: 1 addition & 0 deletions ERRORCODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,4 @@ recommend only using `--extend-select`, never `--select`.
| Code | Description
|------|------------
| Y090 | `tuple[int]` means "a tuple of length 1, in which the sole element is of type `int`". Consider using `tuple[int, ...]` instead, which means "a tuple of arbitrary (possibly 0) length, in which all elements are of type `int`".
| Y091 | `Callable[<parameter_list>, None]` or `Callable[<parameter_list>, Any]` is generally a mistake in the context of a parameter annotation for a function or method. The returned value from a callback is generally ignored at runtime, so `Callable[<parameter_list>, object]` is usually a better annotation in this kind of situation.
17 changes: 15 additions & 2 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -1369,8 +1369,9 @@ def _Y090_error(self, node: ast.Subscript) -> None:

def visit_Subscript(self, node: ast.Subscript) -> None:
subscripted_object = node.value
interesting_modules = _TYPING_MODULES | {"builtins", "collections.abc"}
subscripted_object_name = _get_name_of_class_if_from_modules(
subscripted_object, modules=_TYPING_MODULES | {"builtins"}
subscripted_object, modules=interesting_modules
)
self.visit(subscripted_object)
if subscripted_object_name == "Literal":
Expand Down Expand Up @@ -1398,6 +1399,14 @@ def _visit_slice_tuple(self, node: ast.Tuple, parent: str | None) -> None:
self.visit(elt)
else:
self.visit(node)
elif parent == "Callable":
self.visit(node)
if self.visiting_arg.active and len(node.elts) == 2:
return_annotation = node.elts[1]
if _is_None(return_annotation):
self.error(node, Y091.format(bad_return="None"))
elif _is_Any(return_annotation):
self.error(node, Y091.format(bad_return="Any"))
else:
self.visit(node)

Expand Down Expand Up @@ -2129,5 +2138,9 @@ def parse_options(options: argparse.Namespace) -> None:
'"a tuple of length 1, in which the sole element is of type {typ!r}". '
'Perhaps you meant "{new}"?'
)
Y091 = (
'Y091 "Callable" in argument annotations '
'should generally have "object" for the second parameter, not "{bad_return}"'
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
)

DISABLED_BY_DEFAULT = ["Y090"]
DISABLED_BY_DEFAULT = ["Y090", "Y091"]
29 changes: 29 additions & 0 deletions tests/bad_callbacks.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# flags: --extend-select=Y091

import collections.abc
import typing
from collections.abc import Callable
from typing import Any

from typing_extensions import TypeAlias

def bad(
a: Callable[[], None], # Y091 "Callable" in argument annotations should generally have "object" for the second parameter, not "None"
b: Callable[..., Any], # Y091 "Callable" in argument annotations should generally have "object" for the second parameter, not "Any"
c: typing.Callable[[str, int], typing.Any], # Y022 Use "collections.abc.Callable" instead of "typing.Callable" (PEP 585 syntax) # Y091 "Callable" in argument annotations should generally have "object" for the second parameter, not "Any"
d: collections.abc.Callable[[None], None], # Y091 "Callable" in argument annotations should generally have "object" for the second parameter, not "None"
e: int | str | tuple[Callable[[], None], int, str], # Y091 "Callable" in argument annotations should generally have "object" for the second parameter, not "None"
) -> None: ...

def good(
a: Callable[[], object],
b: Callable[..., object],
c: typing.Callable, # Y022 Use "collections.abc.Callable" instead of "typing.Callable" (PEP 585 syntax)
d: collections.abc.Callable[[None], object],
) -> None: ...

def also_good() -> Callable[[], None]: ...

_TypeAlias: TypeAlias = collections.abc.Callable[[int, str], Any]
x: _TypeAlias
y: typing.Callable[[], None] # Y022 Use "collections.abc.Callable" instead of "typing.Callable" (PEP 585 syntax)