Skip to content
Open
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
13 changes: 12 additions & 1 deletion pyrefly/lib/alt/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down
35 changes: 33 additions & 2 deletions pyrefly/lib/alt/solve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions pyrefly/lib/solver/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Var>, pin_partial_types: bool) -> Vec<PinError> {
let mut seen = SmallSet::new();
let mut errors = Vec::new();
Expand Down
170 changes: 170 additions & 0 deletions pyrefly/lib/test/callable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Loading