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

Check for bare Incomplete annotations #475

Merged
merged 6 commits into from
Mar 30, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Change Log

New error codes:
* Y065: Don't use bare `Incomplete` in argument and return annotations.

Bugfixes:
* Y090: Fix false positive for `tuple[Unpack[Ts]]`.

Expand Down
1 change: 1 addition & 0 deletions ERRORCODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ The following warnings are currently emitted by default:
| 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
| Y065 | Don't use bare `Incomplete` in argument and return annotations. Instead, leave them unannotated. Omitting an annotation entirely from a function will cause some type checkers to view the parameter or return type as "untyped"; this may result in stricter type-checking on code that makes use of the stubbed function. | Style

## Warnings disabled by default

Expand Down
7 changes: 7 additions & 0 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ def _is_object(node: ast.AST | None, name: str, *, from_: Container[str]) -> boo
_is_builtins_object = partial(_is_object, name="object", from_={"builtins"})
_is_builtins_type = partial(_is_object, name="type", from_={"builtins"})
_is_Unused = partial(_is_object, name="Unused", from_={"_typeshed"})
_is_Incomplete = partial(_is_object, name="Incomplete", from_={"_typeshed"})
_is_Iterable = partial(_is_object, name="Iterable", from_=_TYPING_OR_COLLECTIONS_ABC)
_is_AsyncIterable = partial(
_is_object, name="AsyncIterable", from_=_TYPING_OR_COLLECTIONS_ABC
Expand Down Expand Up @@ -2162,6 +2163,9 @@ def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None:
with self.in_function.enabled():
self.generic_visit(node)

if node.name != "__getattr__" and node.returns and _is_Incomplete(node.returns):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_is_Incomplete accepts ast.AST | None, so I don't think we need this check here:

Suggested change
if node.name != "__getattr__" and node.returns and _is_Incomplete(node.returns):
if node.name != "__getattr__" and _is_Incomplete(node.returns):

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's necessary to make mypy happy, as self.error() only accepts an AST.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. Maybe we could make _is_object return TypeGuard[ast.AST]. But what you have is also fine.

self.error(node.returns, Y065.format(what="return type"))

body = node.body
if len(body) > 1:
self.error(body[1], Y048)
Expand All @@ -2186,6 +2190,8 @@ def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None:
def visit_arg(self, node: ast.arg) -> None:
if _is_NoReturn(node.annotation):
self.error(node, Y050)
if _is_Incomplete(node.annotation):
self.error(node, Y065.format(what=f'parameter "{node.arg}"'))
with self.visiting_arg.enabled():
self.generic_visit(node)

Expand Down Expand Up @@ -2408,6 +2414,7 @@ def parse_options(options: argparse.Namespace) -> None:
Y062 = 'Y062 Duplicate "Literal[]" member "{}"'
Y063 = "Y063 Use PEP-570 syntax to indicate positional-only arguments"
Y064 = 'Y064 Use "{suggestion}" instead of "{original}"'
Y065 = 'Y065 Leave {what} unannotated rather than using "Incomplete"'
Y090 = (
'Y090 "{original}" means '
'"a tuple of length 1, in which the sole element is of type {typ!r}". '
Expand Down
39 changes: 39 additions & 0 deletions tests/incomplete.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
from _typeshed import Incomplete
from typing_extensions import TypeAlias

IncompleteAlias: TypeAlias = Incomplete # ok

att: Incomplete # ok

def ok(x: Incomplete | None) -> list[Incomplete]: ...
def aliased(x: IncompleteAlias) -> IncompleteAlias: ... # ok
def err1(
x: Incomplete, # Y065 Leave parameter "x" unannotated rather than using "Incomplete"
) -> None: ...
def err2() -> (
Incomplete # Y065 Leave return type unannotated rather than using "Incomplete"
): ...

class Foo:
att: Incomplete
def ok(self, x: Incomplete | None) -> list[Incomplete]: ...
def err1(
self,
x: Incomplete, # Y065 Leave parameter "x" unannotated rather than using "Incomplete"
) -> None: ...
def err2(
self,
) -> (
Incomplete # Y065 Leave return type unannotated rather than using "Incomplete"
): ...
def __getattr__(
self, name: str
) -> Incomplete: ... # allowed in __getattr__ return type

class Bar:
def __getattr__(
self,
name: Incomplete, # Y065 Leave parameter "name" unannotated rather than using "Incomplete"
) -> Incomplete: ...

def __getattr__(name: str) -> Incomplete: ... # allowed in __getattr__ return type
Loading