Skip to content

fix ParamSpec forwarding doesn't work #823#2678

Open
asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
asukaminato0721:823
Open

fix ParamSpec forwarding doesn't work #823#2678
asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
asukaminato0721:823

Conversation

@asukaminato0721
Copy link
Contributor

Summary

Fixes #823

deferred ParamSpec resolution now recognizes the case where the solver has bound the callee’s ParamSpec to a quantified P, and it reuses the existing *P.args / **P.kwargs validation path instead of incorrectly forcing P into a concrete ParamList.

removes the bogus Expected 'P' to be a ParamSpec value error when one generic helper forwards another helper’s Callable[P, R], *args: P.args, **kwargs: P.kwargs.

Test Plan

added a regression test

@meta-cla meta-cla bot added the cla signed label Mar 5, 2026
@asukaminato0721 asukaminato0721 marked this pull request as ready for review March 6, 2026 00:02
Copilot AI review requested due to automatic review settings March 6, 2026 00:02
@github-actions
Copy link

github-actions bot commented Mar 6, 2026

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

pytest-robotframework (https://github.com/detachhead/pytest-robotframework)
- ERROR pytest_robotframework/__init__.py:303:30-68: Expected `P` to be a ParamSpec value in function `_KeywordDecorator.inner` [bad-argument-type]

zulip (https://github.com/zulip/zulip)
- ERROR zerver/lib/profile.py:34:30-53: Expected `ParamT` to be a ParamSpec value in function `cProfile.Profile.runcall` [bad-argument-type]

prefect (https://github.com/PrefectHQ/prefect)
- ERROR src/prefect/_internal/concurrency/api.py:32:23-46: Expected `P` to be a ParamSpec value in function `prefect._internal.concurrency.calls.Call.new` [bad-argument-type]
- ERROR src/prefect/utilities/asyncutils.py:400:44-73: Expected `P` to be a ParamSpec value in function `run_async_from_worker_thread` [bad-argument-type]
- ERROR src/prefect/utilities/asyncutils.py:404:37-66: Expected `P` to be a ParamSpec value in function `run_async_in_new_loop` [bad-argument-type]

paasta (https://github.com/yelp/paasta)
- ERROR paasta_tools/async_utils.py:184:24-51: Expected `P` to be a ParamSpec value in function `run_sync` [bad-argument-type]

async-utils (https://github.com/mikeshardmind/async-utils)
- ERROR src/async_utils/gen_transform.py:182:36-56: Expected `P` to be a ParamSpec value in function `_sync_to_async_gen` [bad-argument-type]
- ERROR src/async_utils/gen_transform.py:219:35-55: Expected `P` to be a ParamSpec value in function `_sync_to_async_gen` [bad-argument-type]

starlette (https://github.com/encode/starlette)
- ERROR starlette/applications.py:101:50-85: Expected `P` to be a ParamSpec value in function `starlette.middleware.Middleware.__init__` [bad-argument-type]
- ERROR starlette/background.py:31:30-53: Expected `P` to be a ParamSpec value in function `BackgroundTask.__init__` [bad-argument-type]

scrapy (https://github.com/scrapy/scrapy)
- ERROR scrapy/utils/asyncio.py:222:34-57: Expected `_P` to be a ParamSpec value in function `AsyncioLoopingCall.__init__` [bad-argument-type]
- ERROR scrapy/utils/defer.py:288:60-290:6: Expected `_P` to be a ParamSpec value in function `_AsyncCooperatorAdapter.__init__` [bad-argument-type]
- ERROR scrapy/utils/defer.py:430:31-54: Expected `_P` to be a ParamSpec value in function `_maybeDeferred_coro` [bad-argument-type]

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Primer Diff Classification

✅ 7 improvement(s) | 7 project(s) total

7 improvement(s) across pytest-robotframework, zulip, prefect, paasta, async-utils, starlette, scrapy.

Project Verdict Changes Error Kinds Root Cause
pytest-robotframework ✅ Improvement -1 bad-argument-type var_to_rparams()
zulip ✅ Improvement -1 bad-argument-type var_to_rparams()
prefect ✅ Improvement -3 bad-argument-type var_to_rparams()
paasta ✅ Improvement -1 bad-argument-type var_to_rparams()
async-utils ✅ Improvement -2 bad-argument-type var_to_rparams()
starlette ✅ Improvement -2 bad-argument-type is_param_spec()
scrapy ✅ Improvement -3 bad-argument-type var_to_rparams()
Detailed analysis

✅ Improvement (7)

pytest-robotframework (-1)

This is an improvement. The removed error was a false positive that incorrectly flagged valid ParamSpec forwarding. Looking at line 303, the code calls self.inner(fn, context_manager, *args, **kwargs) where inner is defined as inner(cls, fn: Callable[P, T], status_reporter: AbstractContextManager[object, bool], /, *args: P.args, **kwargs: P.kwargs) -> T. This is exactly the ParamSpec forwarding pattern that should work - the outer function receives *args: P.args, **kwargs: P.kwargs and forwards them to another function that accepts the same ParamSpec P. The PR description confirms this was a bug in pyrefly's ParamSpec resolution that has now been fixed to properly handle quantified ParamSpecs in the deferred resolution path.
Attribution: The change to var_to_rparams() in pyrefly/lib/alt/callable.rs added handling for Type::Quantified(q) if q.[is_param_spec()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/callable.rs) case, which now properly recognizes when a ParamSpec is bound to a quantified P and uses the existing *P.args / **P.kwargs validation path instead of forcing P into a concrete ParamList.

zulip (-1)

This is an improvement. The removed error was a false positive caused by pyrefly's ParamSpec resolution incorrectly handling the case where a generic helper forwards another helper's parameters. The code shows a textbook ParamSpec forwarding pattern: ParamT = ParamSpec('ParamT') defines a ParamSpec, func: Callable[ParamT, ReturnT] captures the signature, and *args: ParamT.args, **kwargs: ParamT.kwargs forwards the parameters. The PR description confirms this was a known bug (#823) where 'deferred ParamSpec resolution now recognizes the case where the solver has bound the callee's ParamSpec to a quantified P'. The fix allows proper ParamSpec forwarding between generic helpers, removing the bogus 'Expected P to be a ParamSpec value' error.
Attribution: The fix in pyrefly/lib/alt/callable.rs in the var_to_rparams() function added handling for Type::Quantified(q) if q.[is_param_spec()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/callable.rs) case, which now properly recognizes when a ParamSpec has been bound to a quantified parameter and uses the existing *P.args / **P.kwargs validation path instead of incorrectly forcing it into a concrete ParamList.

prefect (-3)

These errors were false positives that pyrefly incorrectly generated when one generic helper forwards another helper's ParamSpec arguments. The typing spec explicitly supports this pattern. Looking at the specific code:

  1. In create_call() (line 32): The function signature is __fn: _SyncOrAsyncCallable[P, T], *args: P.args, **kwargs: P.kwargs and it calls Call[T].new(__fn, *args, **kwargs). This is valid ParamSpec forwarding.

  2. In run_async_from_worker_thread() (line 400) and run_async_in_new_loop() (line 404): Both functions accept *args: P.args, **kwargs: P.kwargs and forward them to partial(__fn, *args, **kwargs). This is also valid ParamSpec forwarding.

The PR description confirms this was a bug fix: 'deferred ParamSpec resolution now recognizes the case where the solver has bound the callee's ParamSpec to a quantified P, and it reuses the existing *P.args / **P.kwargs validation path instead of incorrectly forcing P into a concrete ParamList.' The old behavior was incorrectly treating the ParamSpec as needing to be a concrete value rather than recognizing the forwarding pattern.

Attribution: The change to var_to_rparams() in pyrefly/lib/alt/callable.rs added special handling for Type::Quantified(q) if q.[is_param_spec()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/callable.rs) which now recognizes when a ParamSpec is bound to a quantified variable and uses the existing *P.args / **P.kwargs validation path via try_match_quantified_paramspec() instead of incorrectly forcing it into a concrete ParamList.

paasta (-1)

This is an improvement. The removed error was a false positive caused by pyrefly's failure to recognize valid ParamSpec forwarding between generic functions. Looking at the source code, to_blocking at line 179-186 is a generic helper that takes async_fn: Callable[P, Awaitable[T]] and forwards its parameters to run_sync using *args: P.args, **kwargs: P.kwargs. This is the exact pattern described in PEP 612 for ParamSpec forwarding. The run_sync function at line 148-152 correctly accepts Callable[P, Awaitable[T]] and *args: P.args, **kwargs: P.kwargs. The error claimed that P needed to be a ParamSpec value, but P is correctly declared as P = ParamSpec('P') on line 19 and is being used in the standard forwarding pattern. The PR description confirms this was a bug in pyrefly's ParamSpec resolution logic, and the fix now properly handles deferred ParamSpec resolution when the solver has bound the callee's ParamSpec to a quantified P.
Attribution: The fix in pyrefly/lib/alt/callable.rs in the var_to_rparams() function added handling for Type::Quantified(q) if q.[is_param_spec()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/callable.rs) case, which now calls try_match_quantified_paramspec() to properly validate ParamSpec forwarding patterns instead of erroring on quantified ParamSpecs.

async-utils (-2)

The removed errors were false positives. Looking at the code, both sync_to_async_gen (line 146) and sync_to_async_gen_noctx (line 185) are generic functions with ParamSpec **P that forward their parameters to _sync_to_async_gen (line 97), which also has the same ParamSpec signature. This is a valid ParamSpec forwarding pattern per PEP 612. The errors claimed 'Expected P to be a ParamSpec value' but P is indeed a ParamSpec - it's the quantified ParamSpec variable from the outer function being forwarded to the inner function. The PR description confirms this was a bug in pyrefly's ParamSpec resolution logic that incorrectly forced quantified ParamSpecs into concrete ParamLists instead of recognizing the forwarding pattern.
Attribution: The change to var_to_rparams() in pyrefly/lib/alt/callable.rs added handling for Type::Quantified(q) if q.[is_param_spec()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/callable.rs) case, which now recognizes when a ParamSpec is bound to another quantified ParamSpec and uses the existing validation path instead of incorrectly forcing it into a concrete ParamList.

starlette (-2)

These were false positive errors that pyrefly was incorrectly generating for valid ParamSpec forwarding patterns. The code shows proper usage where generic functions accept Callable[P, R], *args: P.args, **kwargs: P.kwargs and forward them to other callables. The PR description explicitly states this fixes 'bogus Expected P to be a ParamSpec value error when one generic helper forwards another helper's Callable[P, R], *args: P.args, **kwargs: P.kwargs' - which exactly matches these error patterns. The fix correctly recognizes when the solver has bound a ParamSpec to a quantified P and uses the existing validation path instead of incorrectly forcing P into a concrete parameter list. This brings pyrefly's behavior in line with mypy/pyright for ParamSpec forwarding.
Attribution: The fix in pyrefly/lib/alt/callable.rs in the var_to_rparams closure added handling for Type::Quantified(q) if q.[is_param_spec()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/callable.rs) case. Previously, quantified ParamSpecs were falling through to the error case that generated 'Expected P to be a ParamSpec value'. The new code path calls try_match_quantified_paramspec() which validates the *P.args, **P.kwargs pattern instead of incorrectly forcing P into a concrete ParamList.

scrapy (-3)

These errors were false positives that pyrefly has now correctly fixed. The code shows standard ParamSpec forwarding patterns:

  1. AsyncioLoopingCall.__init__ (line 143) takes func: Callable[_P, _T], *args: _P.args, **kwargs: _P.kwargs - this is the canonical ParamSpec forwarding signature
  2. _AsyncCooperatorAdapter.__init__ (line 221) has the same pattern with callable_: Callable[Concatenate[_T, _P], Deferred[Any] | None], *callable_args: _P.args, **callable_kwargs: _P.kwargs
  3. _maybeDeferred_coro (line 433) forwards f: Callable[_P, Any], *args: _P.args, **kw: _P.kwargs

In each case, the ParamSpec _P is properly quantified and the *args: _P.args, **kwargs: _P.kwargs pattern correctly captures and forwards the parameters. The PR description confirms this was a bug in pyrefly's ParamSpec resolution - it was 'incorrectly forcing P into a concrete ParamList' instead of recognizing the quantified ParamSpec forwarding pattern. Per the typing spec, this forwarding should work seamlessly.

Attribution: The change to var_to_rparams() in pyrefly/lib/alt/callable.rs added handling for Type::Quantified(q) when q.[is_param_spec()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/callable.rs), calling try_match_quantified_paramspec() instead of erroring. This new logic recognizes when a solver has bound a callee's ParamSpec to a quantified P and reuses the existing *P.args / **P.kwargs validation path.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (7 LLM)

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes ParamSpec forwarding between generic helper functions by improving deferred ParamSpec resolution when the callee’s ParamSpec is already bound to a quantified ParamSpec, avoiding incorrect forcing into a concrete ParamList and eliminating the erroneous “Expected P to be a ParamSpec value” diagnostic.

Changes:

  • Updates callable argument matching to recognize quantified ParamSpecs and validate forwarding via existing *P.args / **P.kwargs checks.
  • Adjusts argument iteration to support inspecting the “remaining args” tail when a ParamSpec is encountered.
  • Adds a regression test covering forwarding between two generic helpers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pyrefly/lib/alt/callable.rs Extends ParamSpec handling to support quantified ParamSpec forwarding and routes validation through the P.args/P.kwargs path.
pyrefly/lib/test/paramspec.rs Adds a regression testcase for forwarding Callable[P, R] with *args: P.args, **kwargs: P.kwargs between helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@yangdanny97 yangdanny97 self-assigned this Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ParamSpec forwarding doesn't work

3 participants