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

reportOverlappingOverload when using Concatenate #8389

Closed
CGamesPlay opened this issue Jul 12, 2024 · 7 comments
Closed

reportOverlappingOverload when using Concatenate #8389

CGamesPlay opened this issue Jul 12, 2024 · 7 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@CGamesPlay
Copy link

Describe the bug
I was working on creating a fully-typed decorator that supports functions and methods in the bound and unbound formats. Getting to that point required a few different approaches to work around what is apparently unspecified behavior in ParamSpec/Concatenate, but eventually I got a working solution that works in Python and type checks in Pyright. However, I came across a few things which I am somewhat sure are Pyright bugs.

The first is an unnecessary/incorrect reportOverlappingOverload. These overloads aren't overlapping and Pyright selects the correct one, but it reports them as overlapping.

@overload
def decorator(f: Callable[Concatenate[type[S], P], R]) -> int: ...
@overload
def decorator(
    f: Callable[Concatenate[S, P], R]
) -> int: ...
@overload
def decorator(
    f: Callable[P, R]
) -> int: ...
def decorator(f: Any) -> int:
    return 1

The other issue (which is separate): the __get__ method isn't ever called by staticmethod, and is by classmethod only on Python 3.9-3.10. However, pyright seems to always treat them as being called. Notice that the revealed types are different in pyright versus at runtime: Code sample in pyright playground

@CGamesPlay CGamesPlay added the bug Something isn't working label Jul 12, 2024
erictraut added a commit that referenced this issue Jul 12, 2024
…ad` error in certain cases involving `ParamSpec` and `Concatenate`. This addresses #8389.
erictraut added a commit that referenced this issue Jul 12, 2024
…ad` error in certain cases involving `ParamSpec` and `Concatenate`. This addresses #8389. (#8393)
@erictraut
Copy link
Collaborator

Thanks for the bug report.

There are several issues that you're reporting.

First, let's look at the first overlapping overload error. This is between the first two overloads. I think pyright is correct in this case, so I don't consider this a bug. Consider the following simplified example that removes the type variables from the mix:

@overload
def func(f: Callable[[type[Any]], Any]) -> int: ...

@overload
def func(f: Callable[[object], Any]) -> int: ...

This is clearly an overlapping overload. Both pyright and mypy indicate as such. Your first two overloads for decorator are effectively the same, although you've add a P and R (which are the same in both) and replaced type[Any] with type[S] and replaced object with S.

Now, let's look at the second overlapping overload error. This is between the first and third overloads. I agree this is a bug. This will be fixed in the next release.

For the third issue, I've never seen @staticmethod or @classmethod applied to a descriptor object before and didn't realize that was even supported. I'm struggling to find documentation for this behavior. Do you have any references? None of the other major type checkers handle this either.

In any case, the static type checking behavior for descriptors and static/classmethod decorators are woefully underspecified in the typing spec currently. There should be entire chapters on these topics. These are areas where I'd like to see additional clarification added, hopefully in the near future. Until the typing spec is updated to specify these expected behaviors, I don't plan to make any changes to pyright in these areas — especially if those changes would cause pyright to deviate from the behaviors of other type checkers. Once we hammer out a specification, I'll update pyright to conform to it. If you'd like to participate in that effort, the Python typing forum is where those discussions happen.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Jul 12, 2024
@CGamesPlay
Copy link
Author

CGamesPlay commented Jul 13, 2024

I am a bit confused. I might be wrong about this, but I believe that type[S] a constructor, where as object is any object, and so the overload is valid because the earlier overload is a more specific type. I don't understand why one of these would be valid but the other invalid:

from typing import overload, Callable, Any

@overload
def func(f: Callable[[type[Any]], Any]) -> int: ...
@overload
def func(f: Callable[[object], Any]) -> int: ...
def func(f: Any) -> int:
    return 0

@overload
def val(f: type[Any]) -> int: ...
@overload
def val(f: object) -> int: ...
def val(f: Any) -> int:
    return 0

Furthermore, the pyright error is "Overload 2 for "decorator" will never be used because its parameters overlap overload 1". So, it should be safe to remove overload 2, since it's unused, right? No, this causes new errors:

demo.py:156:9 - error: Argument missing for parameter "self" (reportCallIssue)
# Class().method()
demo.py:162:9 - error: Argument missing for parameter "val" (reportCallIssue)
# Class().method_param(1)

About staticmethod and classmethod, I can point to the python source: staticmethod, classmethod (note: behavior was diffferent in Python 3.9-3.10); as well as the Python Descriptor Guide, which provides pure Python equivalents. I agree that the specification is very unspecified here; and the existence of descriptors makes static analysis of member accesses unsolvable in general. While I do wish there was some way to express the patterns of staticmethod and classmethod that didn't require special-casing them in the type checker, since they are special-cased, it feels like they should be special-cased correctly. Ideas for alternative implementations: statically implement their observed behavior; abort analysis in potentially ambiguous cases (e.g. resolve the member to Unknown and call it a day).

@erictraut
Copy link
Collaborator

erictraut commented Jul 13, 2024

type[S] is an instance of the type class (the base class for the metaclass hierarchy). type is also a subtype of object.

... so the overload is valid because the earlier overload is a more specific type

By "more specific", I presume you mean the first overload is a narrower type than the second overload. The converse is actually true; the second overload is a narrower type. This may be counterintuitive, but keep in mind that the type[Any] and object types describe parameters in a Callable, so they're in a contravariant position. Callable[[object], Any] is a subtype (i.e. a narrower type) of Callable[[type], Any]. Consider the following:

Code sample in pyright playground

from typing import Any, Callable

def func1(cb: Callable[[object], Any]):
    x: Callable[[type], Any] = cb

def func2(cb: Callable[[type], Any]):
    x: Callable[[object], Any] = cb  # Type error

If you're interested in all of the nuanced subtyping rules for callables, you may want to check out this section of the typing spec that I recent contributed.

So, it should be safe to remove overload 2, since it's unused, right?

One solution is to delete the first overload. Another solution is to swap the order of overload 1 and 2 so the narrower overload appears first.

Code sample in pyright playground

from typing import Any, Callable, Concatenate, overload

@overload
def decorator[S, **P, R](f: Callable[Concatenate[S, P], R]) -> int:
    ...

@overload
def decorator[S, **P, R](f: Callable[Concatenate[type[S], P], R]) -> int:
    ...

@overload
def decorator[**P, R](f: Callable[P, R]) -> int:
    ...

def decorator(f: Any) -> int:
    return 1

@CGamesPlay
Copy link
Author

I am again confused. In your sample where you swapped the order of the overloads, indeed the warning has disappeared. But the second overload is never selected in any circumstance, because for all calls to the second overload, the first overload already matched. It seems like the warning should be shown in this case, instead!

Code sample in pyright playground

from typing import Any, Callable, ParamSpec, TypeVar, overload

S = TypeVar("S")
R = TypeVar("R")
P = ParamSpec("P")


@overload
def decorator(f: Callable[[S], R]) -> int: ...
@overload
def decorator(f: Callable[[type[S]], R]) -> float: ...
def decorator(f: Any) -> Any:
    return f


def overload_1(cb: Callable[[S], R]): ...
def overload_2(cb: Callable[[type[S]], R]): ...


def takes_type(x: type) -> int: ...
def takes_int(x: int) -> int: ...


overload_1(takes_type)
overload_1(takes_int)
overload_2(takes_type)
overload_2(takes_int)

This also doesn't address the fact that with the overloads in "incorrect" order (which produces the warning), Pyright says "Overload 2 for "decorator" will never be used because its parameters overlap overload 1", however Pyright's behavior changes if the "unused" overload is removed.

Thanks for taking the time to work with me on this; I apologize for my lack of experience in helping to debug this stuff!

@CGamesPlay
Copy link
Author

CGamesPlay commented Jul 13, 2024

Simplified example of Pyright emitting a warning about an unusable overload, but simultaneously using the overload. Code sample in pyright playground

@overload
def decorator(f: Callable[[int], str]) -> int: ...
@overload
def decorator[S](f: Callable[[S], str]) -> float: ...
def decorator(f: Any) -> Any:
    return f

def takes_object(x: object) -> str: ...
def takes_str(x: str) -> str: ...

reveal_type(decorator(takes_object))  # Overload 1 selected (int)
reveal_type(decorator(takes_str))  # Overload 2 selected (float)

@erictraut
Copy link
Collaborator

This is addressed in pyright 1.1.372.

@CGamesPlay
Copy link
Author

@erictraut I can confirm that one of the reportOverlappingOverloads has been fixed, however the second one #8389 (comment) is still present in pyright 1.1.372.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants