Use tuple[object, ...] and dict[str, object] as upper bounds for ParamSpec.args and ParamSpec.kwargs

[AlexWaygood]

Fixes #12386

Description

Mypy currently thinks that a variable annotated with P.args is not iterable, and thinks that a variable annotated with P.kwargs does not have a .pop() method:

from typing import Callable
from typing_extensions import ParamSpec

P = ParamSpec('P')

def func(callback: Callable[P, str]) -> Callable[P, str]:
    def inner(*args: P.args, **kwargs: P.kwargs) -> str:
        for a in args:  # E: "P.args" has no attribute "__iter__" (not iterable)
            pass
        b = 'foo' in args    # E: "P.args" has no attribute "__iter__" (not iterable)
        print(args.count(42))  # E: "P.args" has no attribute "count"
        print(len(args))  # E: Argument 1 to "len" has incompatible type "P.args"; expected "Sized"
        for c, d in kwargs.items():  # E: "P.kwargs" has no attribute "items"
            reveal_type(c)  # N: Revealed type is "Any"
        kwargs.pop('bar')  # E: "P.kwargs" has no attribute "pop"
        return 'baz'
    return inner

If we use tuple[Any, ...] as the upper bound of ParamSpec.args, and dict[str, Any] as the upper bound of ParamSpec.kwargs, all these false-positive errors go away.

Test Plan

Existing tests have been updated, and a test case has been added.

To avoid repetition among the ParamSpec test cases, I've added a new builtins fixture, paramspec.pyi, which basically combines tuple.pyi and dict.pyi.

[AlexWaygood]

Cc. @A5rocks as the ParamSpec expert :)

[cdce8p]

Thanks for working on this @AlexWaygood! It addresses one of the major issues left with the ParamSpec implementation, the other one being support for TypeAliases (#11855). The changes itself look simple enough.

Would be awesome if the PR could be included in 0.950, but it might be a bit too late for that 🤔

[cdce8p]

Just wondering, any particular reason not to use the new fixture for all ParamSpec tests? I noticed that a few where missing. Especially between L90 - L550.

[AlexWaygood]

I used the new fixture in all the tests that broke with the changes made in this PR ;) They mostly broke because builtins.dict is undefined in tuple.pyi, but we need it for a lot of ParamSpec-related tests if ParamSpec.kwargs is bound to dict[str, Any].

There's no particular reason not to use them in all ParamSpec tests -- but I wanted to keep the diff as small as possible, and the remaining tests don't need all the classes included in paramspec.pyi. (I think it's best to use as minimal a fixture as possible, in most situations, to avoid slowing down the test suite.)

[JukkaL]

I wonder if the fallback types should be tuple[object, ...] and Mapping[str, object] instead? This is how TypedDict fallbacks behave, and a regular TypeVar has object as the fallback, not Any. The Any types allow breaking runtime type safety without mypy complaining about it. On the other hand, maybe flexibility is more desirable than stricter type checking.

Since Pyre has the reference implementation for PEP 612, one option would be to check how it behaves and implement the same behavior (if it makes sense for mypy).

[AlexWaygood]

I wonder if the fallback types should be tuple[object, ...] and Mapping[str, object] instead?

Mapping[str, object] might be problematic, since kwargs is a mutable dict at runtime, and users evidently expect to be able to perform mutable-mapping operations on kwargs without mypy complaining (see #12386). And I think dict[str, object] might cause problems due to invariance. (Is that correct? Not sure about that, actually.)

Since Pyre has the reference implementation for PEP 612, one option would be to check how it behaves and implement the same behavior (if it makes sense for mypy).

I've no idea what Pyre does, I'll look into it!

[AlexWaygood]

I've no idea what Pyre does, I'll look into it!

Okay, it looks like Pyre does indeed use tuple[object, ...] for P.args: https://github.com/facebook/pyre-check/blob/3f7c757d05058177f00d25b982bc532b964fb4ab/source/analysis/test/integration/tupleTest.ml#L756

[AlexWaygood]

Hmm, according to Pyre playground, however, Pyre doesn't acknowledge that P.args are iterable or that .items() can be called on P.kwargs.

[AlexWaygood]

I feel like P.args and P.kwargs should probably either both use Any or both use object. I think users will find it confusing if type-checking is much more strict for P.args than it is for P.kwargs.

[AlexWaygood]

Okay, object seems to work fine, so let's go with that for now.

[JukkaL]

Looks good -- using object is consistent with how we do things elsewhere. There is a style issue that would be nice to address -- I think that it just involves moving things around a bit.

[JukkaL]

Currently we at least mostly avoid calling back to the type checker in mypy.types or mypy.nodes (even through a callback), and I'd prefer to continue to maintain this principle, as a style issue. Also, it's better if constructors of essentially data objects such as types don't perform any non-trivial logic, such as calling get_fallback.

What about defining a helper function in mypy.semanal_shared that takes some of these arguments (named_type_func etc.) and calculates the fallback? get_fallback would be removed from here and would part of this helper function. You could name it calculate_param_spec_fallback, for example, similar to calculate_tuple_fallback that is already defined there.

Also, can you add argument types for the callback?

[AlexWaygood]

I moved the code into semanal_shared -- although it could actually possibly be moved into typeanal, since it seems paramspec args and paramspec kwargs are only ever constructed in typeanal at the moment. (But perhaps that might change in the future -- what do you think?)

Moving the code into typeanal would mean that we wouldn't have to use a callback protocol at all, the functions could just call self.named_type() directly. But the functions might become more tightly coupled with the logic in typeanal.py.

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

core (https://github.com/home-assistant/core)
+ homeassistant/components/sonos/helpers.py:65: error: Unused "type: ignore" comment

[JukkaL]

Thanks for the updates! Looks good now.

[AlexWaygood]

@JukkaL thanks for the reviews!