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

Fix crash when passing too many type arguments to generic base class accepting single ParamSpec #17770

Merged
merged 2 commits into from
Sep 21, 2024

Conversation

brianschubert
Copy link
Collaborator

Fixes #17765

The offender for this crash appears to be this snippet:

mypy/mypy/semanal.py

Lines 5905 to 5910 in 72c413d

if has_param_spec and num_args == 1 and types:
first_arg = get_proper_type(types[0])
if not (
len(types) == 1 and isinstance(first_arg, (Parameters, ParamSpecType, AnyType))
):
types = [Parameters(types, [ARG_POS] * len(types), [None] * len(types))]

This branch triggers when applying type args to a type that is generic with respect to a single ParamSpec. It allows double brackets to be omitted when providing a parameter specification by wrapping all of the provided type arguments into a single parameter specification argument (i.e. equating Foo[int, int] to Foo[[int, int]]). This wrapping occurs unless:

  • there is only a single type argument, and it resolves to Any (e.g. Foo[Any])
  • there is only a single type argument, and it's a bracketed parameter specification or a ParamSpec (e.g. Foo[[int, int]])

The problem occurs when multiple type arguments provided and at least one of them is a bracketed parameter specification, as in Foo[[int, int], str].

Per the rules above, since there is more than 1 type argument, mypy attempts to wrap the arguments into a single parameter specification. This results in the attempted creation of a Parameters instance that contains another Parameters instance, which triggers this assert inside Parameters.__init__:

assert not any(isinstance(t, Parameters) for t in arg_types)

I think a reasonable solution is to forgo wrapping the type arguments into a single Parameters if any of the provided type arguments are a Parameters/ParamSpecType. That is, don't transform Foo[A1, A2, ...] to Foo[[A1, A2, ...]] if any of A1, A2, ... are a parameter specification.

This change brings the crash case inline with mypy's current behavior for a similar case:

# Current behavior
P = ParamSpec("P")
class C(Generic[P]): ...
c: C[int, [int, str], str]  # E: Nested parameter specifications are not allowed

Before this change:

P = ParamSpec("P")
class C(Generic[P]): ...
class D(C[int, [int, str], str]): ... # !!! CRASH !!!

After this change:

P = ParamSpec("P")
class C(Generic[P]): ...
class D(C[int, [int, str], str]): ... # E: Nested parameter specifications are not allowed

Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you for the very clear writeup!

@hauntsaninja hauntsaninja merged commit 9ffb9dd into python:master Sep 21, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when passing too many type arguments to generic base class accepting single ParamSpec
2 participants