Fix ParamSpec prefix params incorrectly demoted to positional-only#3675
Fix ParamSpec prefix params incorrectly demoted to positional-only#3675thatfunkymunki wants to merge 2 commits into
Conversation
…cast() When a Protocol's __call__ mixes explicit parameters (_sudo, name) with *args: P.args / **kwargs: P.kwargs, and a decorator returns cast(Protocol[P], wrapped_func), pyrefly resolves the P-bound params from the original function but drops the Protocol's own explicit params. This pattern is used by pyinfra's @operation decorator, where operations accept both operation-specific parameters (from P) and global arguments (_sudo, _env, name, etc.) defined on the Protocol. Mypy 1.17 handles this correctly: keyword args route to either the Protocol's explicit params or P.kwargs based on name matching.
prepend_types() was converting PrefixParam::Pos to Param::PosOnly via to_param(), making keyword-passable params from function definitions positional-only after ParamSpec expansion. This caused keyword arguments like _sudo=True to be rejected as unexpected when calling through a Protocol with explicit params before *args: P.args. Switch to to_subset_param() which preserves the Pos vs PosOnly distinction. PrefixParam::Pos is only created from function definitions (not Concatenate syntax, which always uses PosOnly), so this change is safe for all callers.
|
Hi @thatfunkymunki! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D107530856. |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
2 participants
Summary
When I am using the
pyinfralibrary , it fails pyrefly type check with these errors:and similar ones. MyPy correctly recognizes the types here successfully.
prepend_types() was converting PrefixParam::Pos to Param::PosOnly via to_param(), making keyword-passable params from function definitions positional-only after ParamSpec expansion. This caused keyword arguments like _sudo=True to be rejected as "unexpected keyword" when calling through a Protocol with explicit params before *args: P.args.
Switch to to_subset_param() which preserves the Pos vs PosOnly distinction. PrefixParam::Pos is only ever created from function definitions (not from Concatenate syntax, which always uses PosOnly), so this change is safe for all callers.
Repro file is here
https://gist.github.com/thatfunkymunki/c0c8a57d60f0f02217b918ef6a377189
observe additional unexpected kwarg errors in pyrefly vs mypy
pyrefly
mypy
Test Plan
Created a failing test and confirmed that it passes with my changes. I also observed that some of the existing tests were expecting incorrect result which was also resolved with the changes.