Skip to content

Commit b20fa1f

Browse files
Fix false positive unexpected-keyword for named params before *args: P.args
Summary: Functions like call_with_retry(f, max_attempts=10, *args: P.args, **kwargs: P.kwargs) should allow max_attempts to be passed as a keyword argument, matching mypy and pyright behavior. Pyrefly incorrectly rejected call_with_retry(write_data, max_attempts=5) with "Unexpected keyword argument max_attempts". The issue is in callable_infer's handling of the Type::Var(var) branch — the path taken when P hasn't been solved yet (e.g. before matching f to write_data resolves P to ()). This branch constructs a param list from prefix params via ParamList::new_types, which calls into_param(). That method unconditionally converts all PrefixParam::Pos to Param::PosOnly, so max_attempts never enters the keyword params map and any keyword usage is rejected. The fix switches this branch to use to_subset_param(), which preserves the Pos kind. The PosOnly conversion is deliberately kept for the Type::Quantified branch (ParamSpec forwarding), where foo(x=1, *args, **kwargs) must be rejected — passing a prefix param as keyword while also forwarding *args positionally would cause "multiple values for argument" at runtime. The existing test_paramspec_named_arguments_concatenate test validates this distinction. Fixes #3052 Reviewed By: grievejia Differential Revision: D99916739 fbshipit-source-id: c2d92232f8ca5399f3c4d34fa2b2e140d75db07f
1 parent 5ff9350 commit b20fa1f

3 files changed

Lines changed: 54 additions & 5 deletions

File tree

crates/pyrefly_types/src/callable.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,9 @@ impl PrefixParam {
296296
}
297297
}
298298

299-
/// Convert to a positional-only `Param`. Per the typing spec, params before
300-
/// `*args: P.args` are always positional-only at the call site, regardless of
301-
/// whether they were originally `Pos` or `PosOnly` in the function definition.
299+
/// Convert to a positional-only `Param`. Per the typing spec, params in
300+
/// `Concatenate` are positional-only at the call site. This is also appropriate
301+
/// for ParamSpec forwarding where prefix params must be passed positionally.
302302
pub fn into_param(self) -> Param {
303303
match self {
304304
Self::PosOnly(name, ty, required) => Param::PosOnly(name, ty, required),
@@ -319,7 +319,8 @@ impl PrefixParam {
319319
}
320320

321321
/// Convert to a `Param` preserving the Pos vs PosOnly distinction.
322-
/// Used for subset/subtype checking where name matching matters.
322+
/// Used for subset/subtype checking where name matching matters,
323+
/// and for direct calls where prefix params should remain keyword-passable.
323324
pub fn to_subset_param(&self) -> Param {
324325
match self {
325326
Self::PosOnly(name, ty, required) => {

pyrefly/lib/alt/callable.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1345,9 +1345,11 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
13451345
),
13461346
// This can happen with a signature like `(f: Callable[P, None], *args: P.args, **kwargs: P.kwargs)`.
13471347
// Before we match an argument to `f`, we don't know what `P` is, so we don't have an answer for the Var yet.
1348+
// Use to_subset_param to preserve Pos vs PosOnly: prefix params from a
1349+
// function definition should remain keyword-passable in direct calls.
13481350
Type::Var(var) => self.callable_infer_params(
13491351
callable_name,
1350-
&ParamList::new_types(concatenate.into_vec()),
1352+
&ParamList::new(concatenate.iter().map(|p| p.to_subset_param()).collect()),
13511353
Some(var),
13521354
self_arg,
13531355
self_qs,

pyrefly/lib/test/paramspec.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -824,3 +824,49 @@ def bare_fn(a: int) -> str: ...
824824
reveal_type(bare_fn) # E: Wrapper[[a: int], str]
825825
"#,
826826
);
827+
828+
testcase!(
829+
test_paramspec_named_params_before_args,
830+
r#"
831+
from typing import ParamSpec, TypeVar, Callable
832+
833+
P = ParamSpec("P")
834+
R = TypeVar("R")
835+
836+
def call_with_retry(
837+
f: Callable[P, R],
838+
max_attempts: int = 10,
839+
*args: P.args,
840+
**kwargs: P.kwargs,
841+
) -> R:
842+
return f(*args, **kwargs)
843+
844+
def write_data() -> None:
845+
pass
846+
847+
call_with_retry(write_data, max_attempts=5)
848+
call_with_retry(write_data, 5)
849+
850+
def compute(x: int, y: str) -> bool:
851+
return True
852+
853+
call_with_retry(compute, max_attempts=3, x=1, y="hello")
854+
call_with_retry(compute, 3, 1, "hello")
855+
"#,
856+
);
857+
858+
testcase!(
859+
test_paramspec_forwarding_prefix_param_keyword,
860+
r#"
861+
from typing import ParamSpec, Callable
862+
863+
P = ParamSpec("P")
864+
Q = ParamSpec("Q")
865+
866+
def call_fn(f: Callable[P, None], x: int, *args: P.args, **kwargs: P.kwargs) -> None:
867+
f(*args, **kwargs)
868+
869+
def forward(g: Callable[Q, None], *args: Q.args, **kwargs: Q.kwargs) -> None:
870+
call_fn(g, x=1, *args, **kwargs)
871+
"#,
872+
);

0 commit comments

Comments
 (0)