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

stubtest: fix signature construction for overload + implicit classmethod #9921

Merged
merged 1 commit into from
Jan 18, 2021
Merged

stubtest: fix signature construction for overload + implicit classmethod #9921

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
stubtest: fix signature construction for overload + implicit classmethod 
__class_getitem__ and __init_subclass__ are always classmethods, even if
they aren't decorated. We previously only handled this correctly if
these methods weren't overloaded.

This came up in python/typeshed#4937
  • Loading branch information
hauntsaninja committed Jan 17, 2021
commit 3c34f69d20c2dde1949b0bb3f7a18eab22bbcd65
22 changes: 13 additions & 9 deletions mypy/stubtest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,15 @@ def _verify_arg_default_value(
)


def maybe_strip_cls(name: str, args: List[nodes.Argument]) -> List[nodes.Argument]:
if name in ("__init_subclass__", "__class_getitem__"):
# These are implicitly classmethods. If the stub chooses not to have @classmethod, we
# should remove the cls argument
if args[0].variable.name == "cls":
Copy link
Member

Choose a reason for hiding this comment

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

What if I name it _cls instead? It sounds like we should just unconditionally remove the first arg.

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Jan 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Unconditionally removing the first argument here isn't right, because it'll do the wrong thing if you do choose to decorate with @classmethod, since those get their first argument stripped when resolving the decorator.

I don't think this is important enough to figure out how to chain enough state to keep track of that.

Especially since stubtest currently is picky about classmethod names being cls, e.g. this not-actually-needed-potentially-problematic assert when resolving the classmethod decorator:

mypy/mypy/stubtest.py

Line 815 in 92923b2

assert func.arguments[0].variable.name in ("cls", "metacls")

return args[1:]
return args


class Signature(Generic[T]):
def __init__(self) -> None:
self.pos = [] # type: List[T]
Expand Down Expand Up @@ -428,7 +437,8 @@ def get_desc(arg: Any) -> str:
@staticmethod
def from_funcitem(stub: nodes.FuncItem) -> "Signature[nodes.Argument]":
stub_sig = Signature() # type: Signature[nodes.Argument]
for stub_arg in stub.arguments:
stub_args = maybe_strip_cls(stub.name, stub.arguments)
for stub_arg in stub_args:
if stub_arg.kind in (nodes.ARG_POS, nodes.ARG_OPT):
stub_sig.pos.append(stub_arg)
elif stub_arg.kind in (nodes.ARG_NAMED, nodes.ARG_NAMED_OPT):
Expand Down Expand Up @@ -476,7 +486,8 @@ def from_overloadedfuncdef(stub: nodes.OverloadedFuncDef) -> "Signature[nodes.Ar
all_args = {} # type: Dict[str, List[Tuple[nodes.Argument, int]]]
for func in map(_resolve_funcitem_from_decorator, stub.items):
assert func is not None
for index, arg in enumerate(func.arguments):
args = maybe_strip_cls(stub.name, func.arguments)
for index, arg in enumerate(args):
# For positional-only args, we allow overloads to have different names for the same
# argument. To accomplish this, we just make up a fake index-based name.
name = (
Expand Down Expand Up @@ -658,13 +669,6 @@ def verify_funcitem(
# catch RuntimeError because of https://bugs.python.org/issue39504
return

if stub.name in ("__init_subclass__", "__class_getitem__"):
# These are implicitly classmethods. If the stub chooses not to have @classmethod, we
# should remove the cls argument
if stub.arguments[0].variable.name == "cls":
stub = copy.copy(stub)
stub.arguments = stub.arguments[1:]

stub_sig = Signature.from_funcitem(stub)
runtime_sig = Signature.from_inspect_signature(signature)

Expand Down