Type hint improvements

[stephenfin]

These were spotted while working on adding type hints to debtcollector. I believe the simplified types are correct, as evidenced by me no longer needing type: ignore statements in debtcollector plus the reduced failures in the tests, but this should definitely receive the twice-over.

[GrahamDumpleton]

What is debtcollector and what static type checker tool are you targeting?

The type hints are the way they are because a more simplified approach doesn't work with various type checkers and results in various cases being flagged as problems when they aren't.

Would also have been helpful to see how you are using type hints in your usage of wrapt. The docs show using it in a specific way, again because of limitations in static type checkers.

[stephenfin]

I'm away without laptop this week so will have to follow up with a longer reply next week, but debtcollector is https://github.com/openstack/debtcollector and we're targeting mypy. I'm typing that as part of a larger effort to type various openstack libraries.

If you look at master of same, you'll find a TODO(stephenfin) that identifies the issue I'm attempting to fix here. I did not check with pyright etc. but in my experience, pyright is "cleverer" than mypy and generally needs last hand holding, so I'd be suprised if this fix didn't also work with that. I've no idea about ty, pyrefly, or zuban though.

[stephenfin]

Also, the fact that mypy at least is now able to correctly identify incorrect parameters and returns types rather than returning Never is a important signifier (IMO) that this is an improvement rather than a regression. I may have missed something however.

[GrahamDumpleton]

I can't find the TODO you reference.

[stephenfin]

Apologies, it hasn't merged yet.

https://review.opendev.org/c/openstack/debtcollector/+/972447

[GrahamDumpleton]

FWIW. For reference, prior type hint work on wrapt was based on testing against mypy 1.18.1. I see there is a 1.19 now. I don't know if that newer version addresses issues I see. From memory if plain Callable was used instead of complex overload I ended up using, mypy wouldn't work properly when wrapt decorators were applied to methods of a class, it just wouldn't handle self/cls argument properly and always complained was wrong number of arguments.

[GrahamDumpleton]

I could also have that round the wrong way, and it is pyright that didn't work without the mess of overloads. So making if just Callable could well work for mypy on either version, but then pylance in VS Code will not work properly. So unfortunately compromises are needed since these static type checkers tools are not fool proof and still a work in progress.

[GrahamDumpleton]

So some of the suggested changes for __get__() overloading on FunctionWrapper and BoundFunctionWrapper may be partly valid. Seems I missed at one point that mypy behaviour changed in one of the prior version updates and I incorporated the new errors in to the expected error messages for tests rather than realise that it showed an underlying issue that needed addressing.

Am not sure that:

        @overload
        def __get__(
            self, instance: None, owner: type[Any] | None = None
        ) -> "FunctionWrapper[P, R]": ...

is correct though in returning FunctionWrapper though, rather than BoundFunctionWrapper for the case of instance is None. Technically the code does still return the later for that.

I also need to understand why you use:

    _P = ParamSpec("_P")
    _R = TypeVar("_R")
    _T = TypeVar("_T")

and why existing:

    P = ParamSpec("P")
    R = TypeVar("R", covariant=True)
    T = TypeVar("T", bound=Any)

were not appropriate.

Still have big concerns over simplification of:

            callable: (
                Callable[P, R]
                | Callable[Concatenate[type[T], P], R]
                | Callable[Concatenate[Any, P], R]
                | Callable[[type[T]], R]
                | Descriptor
            ),
        ) -> FunctionWrapper[P, R]: ...

As seems to happen quite often to me, not having using VS Code for a couple of months for this, its type checking features are totally broken for pylance and don't know why, so hard to test things right now with it.

[GrahamDumpleton]

As usual, start tweaking stuff and other stuff just starts breaking. This is going to take a while to sort out as usual. 😩

[GrahamDumpleton]

Got pylance type checking working again. Simplifying from:

            callable: (
                Callable[P, R]
                | Callable[Concatenate[type[T], P], R]
                | Callable[Concatenate[Any, P], R]
                | Callable[[type[T]], R]
                | Descriptor
            ),

to:

            callable: (
                Callable[P, R]
            ),

breaks pylance.

So that is battle having to face.

So need to have a solution which can retain that yet things still work with mypy.

[GrahamDumpleton]

I may have found a solution. Perhaps can use:

class FunctionDecorator:

    @overload
    def __call__(
        self,
        callable: Callable[P, R]
                | Callable[Concatenate[type[T], P], R]
                # | Callable[Concatenate[Any, P], R] # Don't use, breaks mypy.
                | Callable[[type[T]], R]
    ) -> FunctionWrapper[P, R]: ...
    @overload
    def __call__(
        self,
        callable: Descriptor
    ) -> FunctionWrapper[P, Any]: ...

Descriptor case broken out into separate overload.

Can't have one of the argument alternatives (need to investigate implications of that on other things).

The FunctionDecorator no longer derives from Generic[P, R], with changes to where that is used.

[GrahamDumpleton]

FWIW, the changes also seem to make some things work (different to mypy issues) for ty as well which weren't working on basic test.

[GrahamDumpleton]

But all of mypy, pyrefly and ty break when decorating a method of a class. Only pypright works correctly and that relies on the more complicated overload on callable.

Only solution I can see right now is to extract a minimal self contained example of the problem and report issues against tools that cannot handle it.