microsoft / pyright

Static Type Checker for Python
Other
13.02k stars 1.39k forks source link

reportOverlappingOverload when using Concatenate #8389

Closed CGamesPlay closed 1 month ago

CGamesPlay commented 1 month ago

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

erictraut commented 1 month ago

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.

CGamesPlay commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

This is addressed in pyright 1.1.372.

CGamesPlay commented 1 month ago

@erictraut I can confirm that one of the reportOverlappingOverloads has been fixed, however the second one https://github.com/microsoft/pyright/issues/8389#issuecomment-2226732385 is still present in pyright 1.1.372.