python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.22k stars 2.78k forks source link

__init__ overloads in ABC class are not properly identified #12292

Open flisboac opened 2 years ago

flisboac commented 2 years ago

Bug Report

Mypy doesn't seem to identify all overloads of the __init__ of an ABC class. Didn't test if the same happens for concrete classes, though.

To Reproduce

Please se this mypy playground: https://mypy-play.net/?mypy=latest&python=3.10&gist=0c727c040c7b0d58aa25931dc17cd09f

This can be reproduced with the following code snippet. Please read the comments in the code for some context in the example as well:

from abc import ABCMeta, abstractmethod
from typing_extensions import overload, ParamSpec
from typing import TypeVar, Sequence, Optional, Callable

PResourceParams = ParamSpec('PResourceParams')
TResource = TypeVar('TResource', bound='Resource', covariant=True)

# Multiple implementations of Resource will exist, but
# a small set of construction-time parameters are guaranteed
# to be supported by all implementations.
class Resource(metaclass=ABCMeta):

    @overload
    def __init__(self, pathname: str):
        ...

    @overload
    def __init__(self, *, path: Sequence[str], name: str):
        ...

    @abstractmethod
    def __init__(
        self,
        pathname: Optional[str] = None,
        *,
        path: Optional[Sequence[str]] = None,
        name: Optional[str] = None,
    ):
        ...

# It's also intended for other, more specialized base ABCs to exist,
# at a conceptual level.
class AnotherResourceRoot(Resource, metaclass=ABCMeta):
    ...

# A Provider will select a suitable Resource implementation
# based on conditions internal to its implementation.
# The intention is to validate the small subset of constructor
# parameters that are supported by all resource implementations,
# and complement them with defaults, or other parameters that
# must be injected by the provider implementation.
class ResourceProvider(metaclass=ABCMeta):

    # resource_type is explicitly a callable so that PResourceParams
    # can be captured, and some type safety regarding this default
    # set of parameters can be enforced. It also allows to return the
    # exact type (or base ABC) as requested.
    def resource(
        self,
        resource_type: Callable[PResourceParams, TResource],
        *args: PResourceParams.args,
        **kwargs: PResourceParams.kwargs,
    ) -> TResource:
        ...

def main(provider: ResourceProvider):
    # Unfortunately, mypy doesn't seem to understand the __init__
    # overloads. Errors in the next line:
    #
    #  > error: Unexpected keyword argument "path" for "resource" of "ResourceProvider"
    #  > error: Unexpected keyword argument "name" for "resource" of "ResourceProvider"
    #
    # mypy does understand/recognize, however:
    #
    # 1. The @abstractmethod __init__ in Resource, if I remove all overloads; and
    # 2. The first overload only; you can swap overload positions to make it work,
    #    but it's not the desired situation.
    #
    resource = provider.resource(
        AnotherResourceRoot,
        #'abc/def/ghi.def',
        path=['abc', 'def'],
        name='ghi.def',
    )
    reveal_type(resource)  #  => Resource

Expected Behavior

Should accept all overloads.

Actual Behavior

Only accepts the very first overload of __init__. Solution so far seems to be not to use overloads at all, and rely on just the single @abstractmethod signature.

Your Environment

erictraut commented 2 years ago

I don't know how (or whether) mypy's implementation of PEP 612 handles overloads. The PEP doesn't provide much guidance about how an overloaded function should bind to a ParamSpec. I think pyre handles this case in some manner. When I implemented PEP 612 in pyright, I chose not to support this case, so pyright emits an explicit error for your code above. Mypy doesn't emit an error, but it also doesn't appear to handle the binding of an overloaded signature to a ParamSpec.

flisboac commented 2 years ago

@erictraut I know there's a PR going on regarding ParamSpec, but overall, support for it in mypy is somewhat decent. I tried it and in most places it works very well -- although things like Concatenate are entirely not implemented.

Now, it seems the ParamSpec is properly bound, but only for the first overload. You can swap overload positions, and they'll work as expected, which for me hints at the possibility of mypy not identifying all overloads in this specific scenario. I don't know if the problem is with __init__ or with the fact that the class is an ABC, or if it's something else (e.g. what if I used __new__ instead, would it be different?)

But I agree that ParamSpec could affect overload selection. Maybe the argument matching is not working, or is not implemented yet, which prevents mypy from selecting anything other the immediate/default overload (which in this case seems to be the first).

So far it's the first time I tried such an idiom (ie. __init__ to fix constructor arguments, and ParamSpec to avoid replicating them in factory functions), and it's very much possible that this is not the intended way those functionalities should be used (if that's the case, please do tell me!).