diff --git a/pyrefly/lib/alt/expr.rs b/pyrefly/lib/alt/expr.rs index 99e82ecc77..6543736bb6 100644 --- a/pyrefly/lib/alt/expr.rs +++ b/pyrefly/lib/alt/expr.rs @@ -465,7 +465,18 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { got.with_ty(want.clone()) } } - ExprExpectation::Check { .. } => self.expr_infer_impl(x, None, options.errors), + // `want` is `Any` here, so the check above is skipped: it always succeeds. + // Succeeding trivially never solves the placeholder an empty display leaves + // behind, which then survives into an answer and is reported as an implicit + // `Any` the expectation had already absorbed. Pin it instead. Containment + // keeps this to placeholders `x` minted itself, so a name whose type is + // still open keeps its own diagnostic. + ExprExpectation::Check { .. } => { + let got = self.expr_infer_impl(x, None, options.errors); + self.solver() + .pin_contained_placeholders_within(got.ty(), x.range()); + got + } ExprExpectation::Infer(hint) => self.expr_infer_impl(x, hint, options.errors), } } diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index 75bf4cb8ad..17cebb7d44 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -5477,6 +5477,19 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ) -> (Type, Type) { let base = self.expr_infer(&subscript.value, errors); let slice_ty = self.expr_infer(&subscript.slice, errors); + // Whether an empty display assigned here can still be observed as an implicit + // `Any` is a property of the whole target. It absorbs only if its own type is + // known: not if it still holds an unpinned placeholder, and not if it holds an + // `Any` pyrefly inferred rather than the user declaring one, since a container + // pinned from `{}` absorbs nothing - it propagates the same uncertainty, and + // its contents deserve the diagnostic as much as it did. Computed before + // distributing because pinning is global, so a solved union arm would otherwise + // silence a placeholder that an unsolved arm genuinely leaks. + let target_absorbs = !base.any(|t| match t { + Type::Any(AnyStyle::Implicit) => true, + Type::Var(v) => self.solver().var_is_partial(*v), + _ => false, + }); let assigned_ty = self.distribute_over_union(&base, |base| { self.distribute_over_union(&slice_ty, |key| { match (base, key) { @@ -5527,13 +5540,31 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ExprOrBinding::Expr(e) => { call_setitem(CallArg::expr(e)); // We already emit errors for `e` during `call_method_or_error` - self.expr_infer( + let ty = self.expr_infer( e, &ErrorCollector::new( errors.module().clone(), ErrorStyle::Never, ), - ) + ); + // That call already checked `e` against `__setitem__`'s value + // parameter; this pass only produces the type the subscript + // narrows to, and re-inferring mints a *fresh* placeholder for + // every empty display in `e`. Once the target is fully solved + // it has already absorbed the value, so those placeholders are + // unobservable and are pinned rather than reported. A target + // still holding a partial var is what the diagnostic exists + // for - there the placeholder really does leak. Containment + // keeps this to placeholders `e` minted itself; one belonging + // to a name `e` merely mentions outlives the assignment and + // keeps its own diagnostic. Note the target's *shape* is never + // consulted, so `dict`, `list` and a hand-written + // `__setitem__` all absorb alike. + if target_absorbs { + self.solver() + .pin_contained_placeholders_within(&ty, e.range()); + } + ty } ExprOrBinding::Binding(b) => { let binding_ty = self diff --git a/pyrefly/lib/solver/solver.rs b/pyrefly/lib/solver/solver.rs index cbfee524ce..9cb0321e4e 100644 --- a/pyrefly/lib/solver/solver.rs +++ b/pyrefly/lib/solver/solver.rs @@ -1275,6 +1275,36 @@ impl Solver { self.sanitize_vars(ty.collect_all_vars(), pin_partial_types) } + /// Pin empty-container placeholders that were created *inside* `range`, silently. + /// + /// A `PartialContained` var records the literal that created it, so containment + /// distinguishes a placeholder the expression at `range` minted itself from one it + /// merely mentions by name. Only the former can be pinned without consequence: a + /// name's placeholder outlives the expression and keeps its own diagnostic. + pub fn pin_contained_placeholders_within(&self, ty: &Type, range: TextRange) { + let mut pending = ty.collect_all_vars(); + let mut seen = SmallSet::new(); + while let Some(var) = pending.pop() { + if !seen.insert(var) { + continue; + } + let variables = self.variables.lock(); + let mut variable = variables.get_mut(var); + match &mut *variable { + Variable::PartialContained(literal) if range.contains_range(*literal) => { + *variable = Variable::Answer(self.heap.mk_any_implicit()); + } + // Traverse only through variables that already have an answer. `force_var` + // would pin an unanswered one as a side effect, which is precisely what this + // must not do - it would silence the placeholders the containment test is + // here to protect. Missing a var this way only costs a diagnostic we would + // have suppressed, never one we should have kept. + Variable::Answer(answer) => pending.extend(answer.collect_all_vars()), + _ => {} + } + } + } + pub fn sanitize_vars(&self, mut pending: Vec, pin_partial_types: bool) -> Vec { let mut seen = SmallSet::new(); let mut errors = Vec::new(); diff --git a/pyrefly/lib/test/callable.rs b/pyrefly/lib/test/callable.rs index ac8ccc5546..673657b114 100644 --- a/pyrefly/lib/test/callable.rs +++ b/pyrefly/lib/test/callable.rs @@ -1668,6 +1668,176 @@ f: Callable[[int], None] = lambda x, y: None # E: Type of lambda parameter `y` "#, ); +// Regression test for https://github.com/facebook/pyrefly/issues/4301 +testcase!( + test_implicit_any_empty_container_absorbed_by_any, + TestEnv::new().enable_implicit_any_error(), + r#" +from typing import Any + +# An `Any` annotation absorbs the placeholder, so there is nothing to warn about. +a: Any = {"t": None} +b: dict[str, Any] = {"t": None} + +def takes_any(v: Any) -> None: ... +takes_any({"t": None}) + +def subscript(response: dict[str, Any]) -> None: + response["meta"] = {"t": None} + +# The container's own value type is still unsolved here, so the placeholder does +# leak into what `x` is inferred as, and the diagnostic is warranted. +x = {} +x["y"] = {"t": None} # E: Cannot infer type of empty container +"#, +); + +// Absorbing is a property of the expectation, not of the syntax that carries it, so +// every position that checks a display against a declared `Any` absorbs alike. +testcase!( + test_implicit_any_empty_container_absorbed_in_every_position, + TestEnv::new().enable_implicit_any_error(), + r#" +from typing import Any, TypedDict + +class C: + attr: Any + +def attribute(c: C) -> None: + c.attr = {"t": None} + +def returns_any() -> Any: + return {"t": None} + +def default_arg(x: Any = {"t": None}) -> None: ... + +class TD(TypedDict): + k: Any + +def typed_dict(t: TD) -> None: + t["k"] = {"t": None} + +G: Any = {} + +def global_assign() -> None: + global G + G = {"t": None} +"#, +); + +// Unpacking is the one position left out, and deliberately so: `bind_unpacking` +// records that "we never contextually type unpacks, we do the unpacking at type +// level for simplicity (for now)". The target's `Any` never reaches the value, so +// the placeholder is minted with no expectation to absorb it. Closing this belongs +// to contextually typing unpacks, not here. +testcase!( + bug = "unpacked subscript targets do not absorb the placeholder", + test_implicit_any_empty_container_unpacked_target, + TestEnv::new().enable_implicit_any_error(), + r#" +from typing import Any + +def unpacked(d: dict[str, Any]) -> None: + d["a"], d["b"] = {"t": None}, {"u": None} # E: Cannot infer type of empty container # E: Cannot infer type of empty container +"#, +); + +// A name keeps its own diagnostic wherever it appears on the right-hand side: its +// placeholder outlives the assignment, so an absorbing target must not silence the +// warning that the name itself has no inferrable type. Only the placeholders the +// display minted for itself are absorbed. +testcase!( + test_implicit_any_empty_container_name_rhs_still_reported, + TestEnv::new().enable_implicit_any_error(), + r#" +from typing import Any + +def name_rhs(d: dict[str, Any]) -> None: + y = [] # E: Cannot infer type of empty container + d["k"] = y + +def name_nested_in_display(d: dict[str, Any]) -> None: + y = [] # E: Cannot infer type of empty container + d["k"] = {"a": y} + +def name_nested_deeply(d: dict[str, Any]) -> None: + y = [] # E: Cannot infer type of empty container + d["k"] = {"a": [y]} + +def display_rhs(d: dict[str, Any]) -> None: + d["k"] = {"a": []} +"#, +); + +// A target only absorbs if its own type is known. One pinned from `{}` holds an +// `Any` pyrefly inferred rather than one the user declared, so it propagates the +// same uncertainty and its contents stay reportable. The union case is why this is +// decided for the whole target rather than per arm: pinning is global, so a solved +// arm must not silence what an unsolved arm leaks. +testcase!( + test_implicit_any_empty_container_unknown_target_still_reported, + TestEnv::new().enable_implicit_any_error(), + r#" +from typing import Any + +def inferred_target() -> None: + x = {} # E: Cannot infer type of empty container + y = x + y["k"] = {"t": None} # E: Cannot infer type of empty container + +def chained() -> None: + x = {} + x["a"] = {} # E: Cannot infer type of empty container + x["a"]["b"] = {"t": None} # E: Cannot infer type of empty container + +def union_with_unsolved_arm(flag: bool, declared: dict[str, Any]) -> None: + x = {} # E: Cannot infer type of empty container + target = declared if flag else x + target["k"] = {"t": None} # E: Cannot infer type of empty container +"#, +); + +// A fully-known target that cannot hold the value at all: `__setitem__` already +// rejects it, and nothing can observe the placeholder inside a value being +// rejected, so the second diagnostic is dropped as redundant. +testcase!( + test_implicit_any_empty_container_narrow_target_reports_once, + TestEnv::new().enable_implicit_any_error(), + r#" +def narrow(d: dict[str, int]) -> None: + d["k"] = {"t": None} # E: Cannot set item in `dict[str, int]` +"#, +); + +// Whether the placeholder can escape depends on the target, not on which container +// the target happens to be, so every fully-solved target absorbs it alike. +testcase!( + test_implicit_any_empty_container_absorbed_by_any_containers, + TestEnv::new().enable_implicit_any_error(), + r#" +from collections import defaultdict +from typing import Any, MutableMapping + +def sequence(xs: list[Any]) -> None: + xs[0] = {"t": None} + +def non_str_key(d: dict[int, Any]) -> None: + d[0] = {"t": None} + +def protocol(d: MutableMapping[str, Any]) -> None: + d["k"] = {"t": None} + +def subclass(d: defaultdict[str, Any]) -> None: + d["k"] = {"t": None} + +class Custom: + def __setitem__(self, key: str, value: Any) -> None: ... + +def custom(c: Custom) -> None: + c["k"] = {"t": None} +"#, +); + testcase!( test_implicit_any_lambda_explicit_any_context, TestEnv::new().enable_implicit_any_lambda_error(),