fix bad-argument-count false positive with unpacking #3138

[asukaminato0721]

Summary

Fixes #3138

Starred slice arguments like *xs[-4:-2] now carry a finite upper bound when the slice bounds are statically known, and the positional matcher reserves slots for later concrete positional arguments instead of greedily consuming everything.

just in case:

>>> xs = [1,2,3]
>>> xs[-4:-2]
[1]

Test Plan

add test

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[Copilot]

Pull request overview

Fixes a bad-argument-count false positive when calling functions with *-unpacked, statically-bounded slices (e.g. *xs[-4:-2]) followed by additional positional arguments.

Changes:

• Introduce a bounded star-arg pre-evaluation mode for simple slice subscripts with statically-known bounds.
• Update positional-parameter matching so bounded * args reserve capacity for later positional arguments instead of greedily consuming parameters.
• Add regression tests for bounded-slice splat calls with trailing positional arguments.

Reviewed changes

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

File	Description
pyrefly/lib/alt/callable.rs	Adds bounded slice length inference for starred subscripts and adjusts argument/parameter matching to avoid false positives.
pyrefly/lib/test/callable.rs	Adds a regression test covering *xs[-4:-2] with trailing positional args.

---

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

[Copilot]

min_remaining_positional_args is intended to compute a minimum number of positional arguments contributed by remaining args, but for CallArg::Star with list/set/tuple literals it uses elts.len() unconditionally. If the literal contains starred elements (e.g. *[1, *xs]), the minimum contribution is smaller (at least the count of non-starred elements, possibly 0), and overestimating here can cause the bounded-star reservation logic to stop consuming params too early and potentially introduce false-positive missing-argument errors. Consider computing the minimum as the number of non-starred elements (or 0 if you want to stay conservative) and mirroring the has_starred check used in CallArg::pre_eval.

Suggested change

	Expr::List(list_expr) => list_expr.elts.len(),
	Expr::Set(set_expr) => set_expr.elts.len(),
	Expr::Tuple(tuple_expr) => tuple_expr.elts.len(),
	Expr::List(list_expr) => list_expr
	.elts
	.iter()
	.filter(|elt| !matches!(elt, Expr::Starred(_)))
	.count(),
	Expr::Set(set_expr) => set_expr
	.elts
	.iter()
	.filter(|elt| !matches!(elt, Expr::Starred(_)))
	.count(),
	Expr::Tuple(tuple_expr) => tuple_expr
	.elts
	.iter()
	.filter(|elt| !matches!(elt, Expr::Starred(_)))
	.count(),

[Copilot]

remaining_positional_args is recomputed for every argument by scanning the suffix of positional_args, making this loop O(n^2) in the number of positional arguments. Since callable_infer_params is on a hot path, consider precomputing a suffix-min array once (single reverse pass) and indexing into it for each arg_idx to keep this O(n).

Suggested change

	let min_remaining_positional_args = |args: &[&CallArg<'_>]| {
	args.iter()
	.map(|arg| match arg {
	CallArg::Arg(_) => 1,
	CallArg::Star(TypeOrExpr::Expr(expr), _) => match expr {
	Expr::List(list_expr) => list_expr.elts.len(),
	Expr::Set(set_expr) => set_expr.elts.len(),
	Expr::Tuple(tuple_expr) => tuple_expr.elts.len(),
	_ => 0,
	},
	CallArg::Star(..) => 0,
	})
	.sum::<usize>()
	};
	for (arg_idx, arg) in positional_args.iter().enumerate() {
	let arg = *arg;
	let mut arg_pre = arg.pre_eval(self, arg_errors);
	let remaining_positional_args =
	min_remaining_positional_args(&positional_args[arg_idx + 1..]);
	let positional_arg_count = |arg: &CallArg<'_>| match arg {
	CallArg::Arg(_) => 1,
	CallArg::Star(TypeOrExpr::Expr(expr), _) => match expr {
	Expr::List(list_expr) => list_expr.elts.len(),
	Expr::Set(set_expr) => set_expr.elts.len(),
	Expr::Tuple(tuple_expr) => tuple_expr.elts.len(),
	_ => 0,
	},
	CallArg::Star(..) => 0,
	};
	let mut remaining_positional_args_by_index = vec![0; positional_args.len() + 1];
	for arg_idx in (0..positional_args.len()).rev() {
	remaining_positional_args_by_index[arg_idx] =
	remaining_positional_args_by_index[arg_idx + 1]
	+ positional_arg_count(positional_args[arg_idx]);
	}
	for (arg_idx, arg) in positional_args.iter().enumerate() {
	let arg = *arg;
	let mut arg_pre = arg.pre_eval(self, arg_errors);
	let remaining_positional_args = remaining_positional_args_by_index[arg_idx + 1];

[Copilot]

bounded_star_slice_len derives a max length purely from the slice bounds syntax and applies it to any Expr::Subscript slice. For user-defined __getitem__ implementations, there is no guarantee that obj[a:b] produces an iterable whose length is bounded by b-a, so this can suppress real bad-argument-count errors. Consider gating this optimization to known built-in sequence types (e.g., list/tuple/str/bytes/range) or to cases where the inferred type of the subscript expression is a built-in sliceable sequence with standard semantics.

Suggested change

	fn bounded_star_slice_len<Ans: LookupAnswer>(
	subscript: &ExprSubscript,
	solver: &AnswersSolver<Ans>,
	arg_errors: &ErrorCollector,
	) -> Option<usize> {
	let Expr::Slice(slice) = &*subscript.slice else {
	return None;
	};
	if slice.step.is_some() {
	return None;
	}
	let parse_literal = |expr: &Option<Box<Expr>>| -> Option<i64> {
	let expr = expr.as_ref()?;
	match solver.expr_infer(expr, arg_errors) {
	Type::Literal(lit) => lit.value.as_index_i64(),
	_ => None,
	}
	};
	let lower = parse_literal(&slice.lower)?;
	let upper = parse_literal(&slice.upper)?;
	if (lower < 0 && upper >= 0) || (lower >= 0 && upper < 0) {
	return None;
	}
	usize::try_from((upper - lower).max(0)).ok()
	///
	/// However, deriving a bound from slice syntax alone is unsound for arbitrary
	/// `Expr::Subscript` values, because user-defined `__getitem__` implementations
	/// are not required to return iterables whose length is bounded by `upper - lower`.
	/// Be conservative here unless we can prove built-in slice semantics.
	fn bounded_star_slice_len<Ans: LookupAnswer>(
	_subscript: &ExprSubscript,
	_solver: &AnswersSolver<Ans>,
	_arg_errors: &ErrorCollector,
	) -> Option<usize> {
	None

[rchen152]

This is an interesting one. None of pyright, mypy, or ty supports the pattern that #3138 is asking for. From what I recall, it was intentional that we matched the behavior of other type checkers when deciding how to handle unpacking. If we want to track concrete slice indices, why only the upper bound? Don't we know the exact size? On the other hand, if we track the exact size, does that mean we should error if we now think a slice is contributing the wrong number of arguments?

Implementation-wise, I'd rather we convert *xs[-4:-2] to a fixed-length tuple, after which I think the existing logic would handle everything correctly. But this may need some more discussion first about desired behavior.