Reject invalid ParamSpec locations #18278

sterliakov · 2024-12-11T01:57:14Z

Fixes #14832, fixes #13966, fixes #14622.

Still does not report error in #14777, I'll work separately on that.

Move all ParamSpec validity checking to typeanal.py. Stop treating P.args and P.kwargs as binding - only bare typevar makes it available in scope. Reject keyword arguments following P.args.

This also makes one more conformance test pass. Diff (against baseline on current master):

diff --git a/conformance/results/mypy/generics_paramspec_components.toml b/conformance/results/mypy/generics_paramspec_components.toml
index 66a386a..86ba0b0 100644
--- a/conformance/results/mypy/generics_paramspec_components.toml
+++ b/conformance/results/mypy/generics_paramspec_components.toml
@@ -8,10 +8,18 @@ Does not report error when keyword argument is specified between P.args and P.kw
 output = """
 generics_paramspec_components.py:17: error: Use "P.args" for variadic "*" parameter  [valid-type]
 generics_paramspec_components.py:17: error: Use "P.kwargs" for variadic "**" parameter  [valid-type]
+generics_paramspec_components.py:20: error: ParamSpec components are not allowed here  [valid-type]
 generics_paramspec_components.py:23: error: Use "P.kwargs" for variadic "**" parameter  [valid-type]
+generics_paramspec_components.py:26: error: ParamSpec must have "*args" typed as "P.args" and "**kwargs" typed as "P.kwargs"  [valid-type]
+generics_paramspec_components.py:30: error: ParamSpec "P" is unbound  [valid-type]
+generics_paramspec_components.py:35: error: ParamSpec components are not allowed here  [valid-type]
+generics_paramspec_components.py:36: error: ParamSpec components are not allowed here  [valid-type]
+generics_paramspec_components.py:38: error: ParamSpec must have "*args" typed as "P.args" and "**kwargs" typed as "P.kwargs"  [valid-type]
+generics_paramspec_components.py:41: error: ParamSpec must have "*args" typed as "P.args" and "**kwargs" typed as "P.kwargs"  [valid-type]
 generics_paramspec_components.py:49: error: Argument 1 has incompatible type "*P.kwargs"; expected "P.args"  [arg-type]
 generics_paramspec_components.py:49: error: Argument 2 has incompatible type "**P.args"; expected "P.kwargs"  [arg-type]
 generics_paramspec_components.py:51: error: Argument 1 has incompatible type "int"; expected "P.args"  [arg-type]
+generics_paramspec_components.py:60: error: Arguments not allowed after ParamSpec.args  [valid-type]
 generics_paramspec_components.py:70: error: Argument 1 has incompatible type "*P.args"; expected "int"  [arg-type]
 generics_paramspec_components.py:70: error: Argument 2 has incompatible type "int"; expected "P.args"  [arg-type]
 generics_paramspec_components.py:72: error: Argument 1 has incompatible type "*P.args"; expected "int"  [arg-type]
@@ -21,14 +29,6 @@ generics_paramspec_components.py:83: error: Argument 3 to "foo" has incompatible
 generics_paramspec_components.py:98: error: Argument 2 to "twice" has incompatible type "str"; expected "int"  [arg-type]
 generics_paramspec_components.py:98: error: Argument 3 to "twice" has incompatible type "int"; expected "str"  [arg-type]
 """
-conformance_automated = "Fail"
+conformance_automated = "Pass"
 errors_diff = """
-Line 20: Expected 1 errors
-Line 26: Expected 1 errors
-Line 30: Expected 1 errors
-Line 35: Expected 1 errors
-Line 36: Expected 1 errors
-Line 38: Expected 1 errors
-Line 41: Expected 1 errors
-Line 60: Expected 1 errors
 """
diff --git a/conformance/results/mypy/version.toml b/conformance/results/mypy/version.toml
index 3030c9f..42ef13b 100644
--- a/conformance/results/mypy/version.toml
+++ b/conformance/results/mypy/version.toml
@@ -1,2 +1,2 @@
-version = "mypy 1.14.0+dev.14974072c0a70f8ca29c17c740475187b800e714"
-test_duration = 8.4
+version = "mypy 1.14.0+dev.e580045ce62e3c8ccd17fb575f60ec7ede65c01b"
+test_duration = 8.2
diff --git a/conformance/results/results.html b/conformance/results/results.html
index e6ef533..1278a53 100644
--- a/conformance/results/results.html
+++ b/conformance/results/results.html
@@ -158,8 +158,8 @@
         </header>
         <div class="table_container"><table><tbody>
 <tr><th class="col1">&nbsp;</th>
-<th class='tc-header'><div class='tc-name'>mypy 1.14.0+dev.14974072c0a70f8ca29c17c740475187b800e714</div>
-<div class='tc-time'>8.4sec</div>
+<th class='tc-header'><div class='tc-name'>mypy 1.14.0+dev.e580045ce62e3c8ccd17fb575f60ec7ede65c01b</div>
+<div class='tc-time'>8.2sec</div>
 </th>
 </tr>
 <tr><th class="column" colspan="2">

…lysis - only bare paramspec binds.

JukkaL

Thanks, I'm glad to see several bugs fixed. Looks good!

JukkaL · 2024-12-20T16:33:50Z

Actually, the home-assistant error might be false positive. Can you look at it?

sterliakov · 2024-12-20T18:16:49Z

Amazing. I wasn't able to reproduce the problem on a local clone, and indeed it was apparently a glitch: another primer run did not produce an error there.

cdce8p · 2024-12-20T21:13:44Z

Actually, the home-assistant error might be false positive. Can you look at it?

Amazing. I wasn't able to reproduce the problem on a local clone, and indeed it was apparently a glitch: another primer run did not produce an error there.

No it wasn't a glitch. I just fixed it already after seeing your PR 😉
home-assistant/core#133044

_P wasn't bound by the outer function scope.

sterliakov · 2024-12-20T22:46:30Z

Ough! I was moderately certain that I reviewed primer hits and found them to be correct, but did not expect a fix in between... Please comment next time you do that!:)

hauntsaninja

This looks awesome! Makes things cleaner too.

I had one small nit regarding a test case

hauntsaninja · 2024-12-30T00:39:18Z

test-data/unit/check-parameter-specification.test

@@ -2144,7 +2165,7 @@ from typing_extensions import ParamSpec
 P= ParamSpec("P")
 T = TypeVar("T")

-def smoke_testable(*args: P.args, **kwargs: P.kwargs) -> Callable[[Callable[P, T]], Type[T]]:
+def smoke_testable(*args: P.args, **kwargs: P.kwargs) -> Callable[[Callable[P, T]], Type[T]]:  # E: ParamSpec "P" is unbound


this is functionality that mypy is losing, right? (you can see what it's going for in the interesting OtherClass example)

i think that's fine (and seems fine looking at the new primer warnings), but maybe we should clarify this test case? e.g. at least the comments are out of date

Thanks! I've removed this test. This definition is not allowed by PEP612:

These “properties” can only be used as the annotated types for *args and **kwargs, accessed from a ParamSpec already in scope.

*args and **kwargs do not add a ParamSpec to the scope, so the test has never been conformant. IMO this is an extremely rare case: "given a set of arguments, produce a callable depending on those". I think dropping this implementation-defined extension to prevent silently accepting and misinterpreting invalid typing constructs is sensible.

… testParamSpecLocations

github-actions · 2025-01-02T17:58:23Z

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

altair (https://github.com/vega/altair)
+ altair/theme.py:251: error: ParamSpec "P" is unbound  [valid-type]

prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/flows.py:1260: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/flows.py:1269: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/flows.py:1277: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/tasks.py:957: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/tasks.py:969: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/tasks.py:979: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/tasks.py:989: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/tasks.py:998: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/tasks.py:1043: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/tasks.py:1053: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/tasks.py:1063: error: Arguments not allowed after ParamSpec.args  [valid-type]
+ src/prefect/tasks.py:1073: error: Arguments not allowed after ParamSpec.args  [valid-type]

pwndbg (https://github.com/pwndbg/pwndbg)
+ pwndbg/gdblib/events.py: note: In function "connect":
+ pwndbg/gdblib/events.py:277: error: ParamSpec "P" is unbound  [valid-type]

sterliakov · 2025-01-02T18:03:29Z

In the meantime prefect fixed one occurrence of arg between *args and **kwargs and added a few more invalid functions of the same kind:)

hauntsaninja · 2025-01-02T20:38:53Z

Thank you!

Fixes #14832, fixes #13966, fixes #14622. Still does not report error in #14777, I'll work separately on that. Move all `ParamSpec` validity checking to `typeanal.py`. Stop treating `P.args` and `P.kwargs` as binding - only bare typevar makes it available in scope. Reject keyword arguments following `P.args`. This also makes one more conformance test pass.

sterliakov added 4 commits December 11, 2024 01:20

Add compliance test, stop ignoring some weird paramspec positions

ab6f17e

Move ParamSpec validity checks to typeanal entirely

6ef6d89

Remove "special" treatment for ParamSpec parts encountered during ana…

5d9d703

…lysis - only bare paramspec binds.

Fix broken Callable[[Unpack[SomeTypedDict]], None]

d461ba2

This comment has been minimized.

Sign in to view

sterliakov added 2 commits December 11, 2024 04:30

Recover Unpack sanity checks

dfa0887

Fix runtime crash in prefect, clean up naming

643a0db

This comment has been minimized.

Sign in to view

sterliakov marked this pull request as ready for review December 11, 2024 05:06

JukkaL approved these changes Dec 20, 2024

View reviewed changes

Merge branch 'master' into bugfix/st-paramspec-locations

caeb13e

This comment has been minimized.

Sign in to view

hauntsaninja mentioned this pull request Dec 28, 2024

TYP: use mypy_primer to surface type checking regressions numpy/numpy#28073

Merged

hauntsaninja added the upnext label Dec 28, 2024

hauntsaninja approved these changes Dec 30, 2024

View reviewed changes

Remove an non-conformant old test, move the problematic part of it to…

c8204ed

… testParamSpecLocations

hauntsaninja merged commit 025642b into python:master Jan 2, 2025
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reject invalid ParamSpec locations #18278

Reject invalid ParamSpec locations #18278

sterliakov commented Dec 11, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

JukkaL left a comment

JukkaL commented Dec 20, 2024

sterliakov commented Dec 20, 2024

This comment has been minimized.

cdce8p commented Dec 20, 2024 •

edited

Loading

sterliakov commented Dec 20, 2024

hauntsaninja left a comment

hauntsaninja Dec 30, 2024 •

edited

Loading

sterliakov Jan 2, 2025 •

edited

Loading

github-actions bot commented Jan 2, 2025

sterliakov commented Jan 2, 2025

hauntsaninja commented Jan 2, 2025

Reject invalid ParamSpec locations #18278

Reject invalid ParamSpec locations #18278

Conversation

sterliakov commented Dec 11, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

JukkaL left a comment

Choose a reason for hiding this comment

JukkaL commented Dec 20, 2024

sterliakov commented Dec 20, 2024

This comment has been minimized.

cdce8p commented Dec 20, 2024 • edited Loading

sterliakov commented Dec 20, 2024

hauntsaninja left a comment

Choose a reason for hiding this comment

hauntsaninja Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

sterliakov Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 2, 2025

sterliakov commented Jan 2, 2025

hauntsaninja commented Jan 2, 2025

sterliakov commented Dec 11, 2024 •

edited

Loading

cdce8p commented Dec 20, 2024 •

edited

Loading

hauntsaninja Dec 30, 2024 •

edited

Loading

sterliakov Jan 2, 2025 •

edited

Loading