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.

Sign up for GitHub

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

Jump to bottom

fix: type Markable is reduced to Any by mypy #8674

Closed
wants to merge 1 commit into from
Closed

fix: type Markable is reduced to Any by mypy #8674

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 7 additions & 9 deletions src/_pytest/mark/structures.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import warnings
from typing import Any
from typing import Callable
from typing import cast
from typing import Collection
from typing import Iterable
from typing import Iterator
Expand Down Expand Up @@ -264,9 +265,9 @@ def combined_with(self, other: "Mark") -> "Mark":


# A generic parameter designating an object to which a Mark may
# be applied -- a test function (callable) or class.
# be applied -- a test function (callable) or class (also callable).
# Note: a lambda is not allowed, but this can't be represented.
Markable = TypeVar("Markable", bound=Union[Callable[..., object], type])
Markable = TypeVar("Markable", bound=Callable[..., Any])
bluetech marked this conversation as resolved.
Show resolved Hide resolved


@attr.s(init=False, auto_attribs=True)
Expand Down Expand Up @@ -344,25 +345,22 @@ def with_args(self, *args: object, **kwargs: object) -> "MarkDecorator":
mark = Mark(self.name, args, kwargs, _ispytest=True)
return MarkDecorator(self.mark.combined_with(mark), _ispytest=True)

# Type ignored because the overloads overlap with an incompatible
# return type. Not much we can do about that. Thankfully mypy picks
# the first match so it works out even if we break the rules.
@overload
def __call__(self, arg: Markable) -> Markable: # type: ignore[misc]
bluetech marked this conversation as resolved.
Show resolved Hide resolved
def __call__(self, arg: Markable) -> Markable:
pass

@overload
def __call__(self, *args: object, **kwargs: object) -> "MarkDecorator":
def __call__(self, *args: Any, **kwargs: Any) -> "MarkDecorator":
bluetech marked this conversation as resolved.
Show resolved Hide resolved
pass

def __call__(self, *args: object, **kwargs: object):
def __call__(self, *args: Any, **kwargs: Any) -> Union[Markable, "MarkDecorator"]:
Copy link
Member

Choose a reason for hiding this comment

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

Markable is an unbound TypeVar here, which usually doesn't make sense. Since the typing in covered by the overloads I chose to omit the return type here.

Copy link
Author

Choose a reason for hiding this comment

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

Is it unbound? I figured mypy would use the overload, but I can adjust this so there is less overlap. I can force non keyword arguments but without knowing at least one required keyword argument I don't know how to tell mypy that at least one has to be provided. to force the MarkDecorator path.

Copy link
Author

Choose a reason for hiding this comment

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

Just to follow up on the last part of your comment... As you have it mypy (possibly any type checker) will read this as -> Any. I had thought as you are on a different codebase and I noticed that mypy wasn't checking the function at all (I had left off the argument signature too), I then added the argument signature and tested that I could return anything even outside the bounds of the @overloads. This might have been fixed in mypy, but it's best to be explicit about your return type. When I had added the return annotation it required casting the return in the Markable path since it was now checking the return was not Any.

Copy link
Member

Choose a reason for hiding this comment

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

When a function has overloads defined, the typing of the implementation function is only used for the implementation -- it is ignored in favor of the overloads everywhere else.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that's correct, but shouldn't the function return be type checked so that future editors don't accidentally make a mistake?

Copy link
Member

Choose a reason for hiding this comment

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

I think best we can do is add a comment.

"""Call the MarkDecorator."""
if args and not kwargs:
func = args[0]
is_class = inspect.isclass(func)
if len(args) == 1 and (istestfunc(func) or is_class):
store_mark(func, self.mark)
return func
return cast(Markable, func)
return self.with_args(*args, **kwargs)


Expand Down