Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions crates/ty_python_semantic/resources/mdtest/call/function.md
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,53 @@ for tup in my_other_args:
f4(*tup, e=None)
```

Regression test for <https://github.com/astral-sh/ty/issues/2734>.

```py
def f5(x: int | None = None, y: str = "") -> None: ...
def f6(flag: bool) -> None:
args = () if flag else (1,)
f5(*args)

def f7(x: int | None = None, y: str = "") -> None: ...
def f8(flag: bool) -> None:
args = () if flag else ("bad",)
f7(*args) # error: [invalid-argument-type]

def f11(*args: int) -> None: ...
def f12(args: tuple[int] | int) -> None:
f11(*args) # error: [not-iterable]

def f13(a: int, b: int, c: str) -> None: ...
def f14(a: int, b: int, c: str, d: list[float], e: list[float]) -> None: ...
def f15(profile: bool, line: str) -> None:
matcher = f13
timings = []
if profile:
matcher = f14
timings = [[0.0], [1.0], [2.0], [3.0]]
matcher(1, 2, line, *timings[:2])

def f9(x: int = 0, y: str = "") -> None: ...
def f10(args: tuple[int, ...] | tuple[int, str]) -> None:
# The variable-length element `int` from `tuple[int, ...]` unions with `str`
# from `tuple[int, str]` at position 1, giving `int | str` for `y: str`.
f9(*args) # error: [invalid-argument-type]

def f18(x: int = 0, y: int = 0) -> None: ...
def f19(args: tuple[int, ...] | tuple[int, int]) -> None:
f18(*args)

# TODO: Union variadic unpacking should also work when the non-defaulted parameters
# are covered by all union elements, even if not all remaining parameters are defaulted.
# Currently we only apply per-element iteration when all remaining positional parameters
# have defaults, so this falls back to `iterate()` which produces `tuple[int, ...]` and
# greedily matches `c: str` with `int`.
def f16(a: int, b: int = 0, c: str = "") -> None: ...
def f17(x: tuple[int] | tuple[int, int]) -> None:
f16(*x) # error: [invalid-argument-type] # TODO: false positive
```

### Mixed argument and parameter containing variadic

```toml
Expand Down Expand Up @@ -1512,3 +1559,40 @@ def _(arg: int):
# error: [not-iterable] "Object of type `int` is not iterable"
foo(*arg)
```

## Union variadic unpacking with explicit keyword arguments

When a union type containing variable-length elements (like `Unknown`) is unpacked as `*args`, the
variadic expansion should not greedily consume optional positional parameters that are also provided
as explicit keyword arguments.

```py
from ty_extensions import Unknown

def f(a: int = 0, b: int = 0, c: int = 0, fmt: str | None = None) -> None: ...
def _(args: "Unknown | tuple[int, int, int]"):
f(*args, fmt="{key}") # fine
```

## Variadic unpacking should stop at max known arity

When unpacking (a union of) fixed-length tuples, variadic matching should stop once the known
positions are exhausted. Otherwise, optional positional parameters can be incorrectly treated as
already assigned, causing false positives for `**kwargs`.

(This test uses `**kwargs` unpacking of a `TypedDict` instead of the simpler `c=1` keyword argument,
because `c=1` is a known keyword argument and we always prevent unpacking `*args` over an
explicitly-provided keyword argument. The case shown here, without the explicit keyword argument,
requires instead that we use our knowledge of the tuple length to prevent over-unpacking.)

```py
from typing import TypedDict

class CKwargs(TypedDict):
c: int

def f(a: int = 0, b: int = 0, c: int = 0) -> None: ...
def _(args_tuple: tuple[int, int], args_union: tuple[int] | tuple[int, int], kwargs: CKwargs) -> None:
f(*args_tuple, **kwargs) # fine
f(*args_union, **kwargs) # fine
```
119 changes: 105 additions & 14 deletions crates/ty_python_semantic/src/types/call/bind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use super::{Argument, CallArguments, CallError, CallErrorKind, InferContext, Sig
use crate::db::Db;
use crate::dunder_all::dunder_all_names;
use crate::place::{DefinedPlace, Definedness, Place, known_module_symbol};
use crate::subscript::PyIndex;
use crate::types::call::arguments::{Expansion, is_expandable_type};
use crate::types::constraints::{ConstraintSet, ConstraintSetBuilder};
use crate::types::diagnostic::{
Expand Down Expand Up @@ -3313,6 +3314,14 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> {
) -> Result<(), ()> {
enum VariadicArgumentType<'db> {
ParamSpec(Type<'db>),
/// A union type where each element has been individually iterated into a tuple spec.
/// We pre-compute the per-position union types, length bounds, and variable element
/// so the rest of the matching logic can handle unions without special-casing.
Union {
argument_types: Vec<Type<'db>>,
length: TupleLength,
variable_element: Option<Type<'db>>,
},
Other(Cow<'db, TupleSpec<'db>>),
None,
}
Expand All @@ -3328,13 +3337,86 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> {
// `*args: P.args` (another ParamSpec).
Some(argument_type) => match argument_type.as_paramspec_typevar(db) {
Some(paramspec) => VariadicArgumentType::ParamSpec(paramspec),
// TODO: `Type::iterate` internally handles unions, but in a lossy way.
// It might be superior here to manually map over the union and call `try_iterate`
// on each element, similar to the way that `unpacker.rs` does in the `unpack_inner` method.
// It might be a bit of a refactor, though.
// See <https://github.com/astral-sh/ruff/pull/20377#issuecomment-3401380305>
// for more details. --Alex
None => VariadicArgumentType::Other(argument_type.iterate(db)),
None => match argument_type {
// `Type::iterate` unions tuple specs in a way that can invent additional
// arities. Iterate each union element individually and compute per-position
// union types, length bounds, and variable element so that the rest of the
// matching logic handles unions correctly.
//
// We restrict this to cases where all remaining positional parameters are
// defaulted and there is no variadic parameter, because the per-position
// union loses the correlation between element lengths and per-position types.
// For example, given overloads `f(x: int, y: int)` and `f(x: int, y: str, z: int)`
// with `t: tuple[int, str] | tuple[int, str, int]`, the per-position union
// would collapse the two arities, preventing the expansion step from correctly
// splitting the union into separate argument lists per overload.
//
// TODO: This is overly conservative. We could also apply this when all
// non-defaulted parameters are covered by the shortest union element,
// e.g. `f(a: int, b: int = 0)` with `*x` where `x: tuple[int] | tuple[int, int]`.
Type::Union(union)
if self.parameters.variadic().is_none()
&& self
.parameters
.positional()
.skip(self.next_positional)
.all(|parameter| parameter.default_type().is_some()) =>
{
let tuple_specs: Vec<_> =
union.elements(db).iter().map(|ty| ty.iterate(db)).collect();

let min_len = tuple_specs
.iter()
.map(|s| s.len().minimum())
.min()
.unwrap_or(0);
let any_variable = tuple_specs.iter().any(|s| s.len().is_variable());
let max_elements = tuple_specs
.iter()
.map(|s| s.all_elements().len())
.max()
.unwrap_or(0);

let variable_element = {
let var_types: Vec<_> = tuple_specs
.iter()
.filter_map(|s| s.variable_element().copied())
.collect();
if var_types.is_empty() {
None
} else {
Some(UnionType::from_elements_leave_aliases(db, var_types))
}
};

let max_elements = i32::try_from(max_elements).unwrap_or(i32::MAX);
let mut argument_types_vec = Vec::new();
for index in 0..max_elements {
let positional_types: Vec<_> = tuple_specs
.iter()
.filter_map(|s| s.py_index(db, index).ok())
.collect();
if positional_types.is_empty() {
break;
}
argument_types_vec
.push(UnionType::from_elements_leave_aliases(db, positional_types));
}

let length = if any_variable || argument_types_vec.len() > min_len {
TupleLength::Variable(min_len, 0)
} else {
TupleLength::Fixed(min_len)
};

VariadicArgumentType::Union {
argument_types: argument_types_vec,
length,
variable_element,
}
}
_ => VariadicArgumentType::Other(argument_type.iterate(db)),
},
},
None => VariadicArgumentType::None,
};
Expand All @@ -3343,6 +3425,11 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> {
VariadicArgumentType::ParamSpec(paramspec) => {
([].as_slice(), TupleLength::unknown(), Some(*paramspec))
}
VariadicArgumentType::Union {
argument_types,
length,
variable_element,
} => (argument_types.as_slice(), *length, *variable_element),
VariadicArgumentType::Other(tuple) => (
tuple.all_elements(),
tuple.len(),
Expand All @@ -3352,6 +3439,9 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> {
};

let mut argument_types = argument_types.iter().copied();
// This can be true either if we have a true variable-length tuple (in which case
// `variable_element.is_some()`) or if we have a union of different fixed-length tuples (in
// which case `variable_element.is_none()`).
let is_variable = length.is_variable();

// We must be able to match up the fixed-length portion of the argument with positional
Expand All @@ -3367,7 +3457,9 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> {

// If the tuple is variable-length, we assume that it will soak up all remaining positional
// parameters, stopping only when we reach a parameter that has an explicit keyword argument
// or a parameter that can only be provided via keyword argument.
// or a parameter that can only be provided via keyword argument, or if we run out of
// `argument_types` and have no `variable_element`. (The combination of `is_variable` with
// no `variable_element` can only happen with a union of different-fixed-length tuples.)
if is_variable {
while self
.parameters
Expand All @@ -3380,12 +3472,11 @@ impl<'a, 'db> ArgumentMatcher<'a, 'db> {
{
break;
}
self.match_positional(
argument_index,
argument,
argument_types.next().or(variable_element),
is_variable,
)?;
let arg_type = argument_types.next().or(variable_element);
if arg_type.is_none() {
break;
}
self.match_positional(argument_index, argument, arg_type, is_variable)?;
}
}

Expand Down