Skip to content

Commit 946933e

Browse files
yangdanny97facebook-github-bot
authored andcommitted
improve contextual typing for subscripts
Summary: fixes #337 Instead of solving the ExprOrBinding before typechecking, we use the type of the TypedDict field + the param annotation for `__setitem__` as the contextual type for checking the `ExprOrBinding::Expr` case Reviewed By: stroxler Differential Revision: D75454897 fbshipit-source-id: c6ca4260bde8e6388073f4fa2c0f97e34f1b1aa6
1 parent 40108d8 commit 946933e

File tree

4 files changed

+93
-57
lines changed

4 files changed

+93
-57
lines changed

conformance/third_party/conformance.exp

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10432,22 +10432,22 @@
1043210432
"typeddicts_operations.py": [
1043310433
{
1043410434
"code": -2,
10435-
"column": 1,
10436-
"concise_description": "Expected `str`, got `Literal[1982]`",
10437-
"description": "Expected `str`, got `Literal[1982]`",
10435+
"column": 17,
10436+
"concise_description": "`Literal[1982]` is not assignable to TypedDict key `name` with type `str`",
10437+
"description": "`Literal[1982]` is not assignable to TypedDict key `name` with type `str`",
1043810438
"line": 22,
10439-
"name": "bad-assignment",
10440-
"stop_column": 14,
10439+
"name": "typed-dict-key-error",
10440+
"stop_column": 21,
1044110441
"stop_line": 22
1044210442
},
1044310443
{
1044410444
"code": -2,
10445-
"column": 1,
10446-
"concise_description": "Expected `int`, got `Literal['']`",
10447-
"description": "Expected `int`, got `Literal['']`",
10445+
"column": 17,
10446+
"concise_description": "`Literal['']` is not assignable to TypedDict key `year` with type `int`",
10447+
"description": "`Literal['']` is not assignable to TypedDict key `year` with type `int`",
1044810448
"line": 23,
10449-
"name": "bad-assignment",
10450-
"stop_column": 14,
10449+
"name": "typed-dict-key-error",
10450+
"stop_column": 19,
1045110451
"stop_line": 23
1045210452
},
1045310453
{
@@ -10728,12 +10728,12 @@
1072810728
},
1072910729
{
1073010730
"code": -2,
10731-
"column": 1,
10732-
"concise_description": "Expected `str`, got `Literal[3]`",
10733-
"description": "Expected `str`, got `Literal[3]`",
10731+
"column": 14,
10732+
"concise_description": "`Literal[3]` is not assignable to TypedDict key `ident` with type `str`",
10733+
"description": "`Literal[3]` is not assignable to TypedDict key `ident` with type `str`",
1073410734
"line": 82,
10735-
"name": "bad-assignment",
10736-
"stop_column": 11,
10735+
"name": "typed-dict-key-error",
10736+
"stop_column": 15,
1073710737
"stop_line": 82
1073810738
},
1073910739
{
@@ -10928,12 +10928,12 @@
1092810928
},
1092910929
{
1093010930
"code": -2,
10931-
"column": 1,
10932-
"concise_description": "Expected `int`, got `Literal['1982']`",
10933-
"description": "Expected `int`, got `Literal['1982']`",
10931+
"column": 17,
10932+
"concise_description": "`Literal['1982']` is not assignable to TypedDict key `year` with type `int`",
10933+
"description": "`Literal['1982']` is not assignable to TypedDict key `year` with type `int`",
1093410934
"line": 24,
10935-
"name": "bad-assignment",
10936-
"stop_column": 14,
10935+
"name": "typed-dict-key-error",
10936+
"stop_column": 23,
1093710937
"stop_line": 24
1093810938
},
1093910939
{

pyrefly/lib/alt/solve.rs

Lines changed: 56 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ use crate::error::context::ErrorContext;
7373
use crate::error::context::TypeCheckContext;
7474
use crate::error::context::TypeCheckKind;
7575
use crate::error::kind::ErrorKind;
76+
use crate::error::style::ErrorStyle;
7677
use crate::module::short_identifier::ShortIdentifier;
7778
use crate::ruff::ast::Ast;
7879
use crate::types::annotation::Annotation;
@@ -1595,19 +1596,12 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
15951596
}
15961597
}
15971598
Binding::AssignToSubscript(box (subscript, value)) => {
1598-
// TODO: Solveing `test_context_assign_subscript` will require us to push
1599-
// this down further, so that we can use contextual typing to infer the Expr case.
1600-
let value_ty = match value {
1601-
ExprOrBinding::Expr(e) => self.expr_infer(e, errors),
1602-
ExprOrBinding::Binding(b) => self.solve_binding(b, errors).arc_clone_ty(),
1603-
};
16041599
// If we can't assign to this subscript, then we don't narrow the type
1605-
let narrowed = if self.check_assign_to_subscript(subscript, &value_ty, errors)
1606-
== Type::any_error()
1607-
{
1600+
let assigned_ty = self.check_assign_to_subscript(subscript, value, errors);
1601+
let narrowed = if assigned_ty.is_any() {
16081602
None
16091603
} else {
1610-
Some(value_ty)
1604+
Some(assigned_ty)
16111605
};
16121606
if let Some((identifier, chain)) =
16131607
identifier_and_chain_for_expr(&Expr::Subscript(subscript.clone()))
@@ -1644,14 +1638,15 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
16441638
fn check_assign_to_subscript(
16451639
&self,
16461640
subscript: &ExprSubscript,
1647-
value: &Type,
1641+
value: &ExprOrBinding,
16481642
errors: &ErrorCollector,
16491643
) -> Type {
16501644
let base = self.expr_infer(&subscript.value, errors);
16511645
let slice_ty = self.expr_infer(&subscript.slice, errors);
16521646
match (&base, &slice_ty) {
16531647
(Type::TypedDict(typed_dict), Type::Literal(Lit::Str(field_name))) => {
1654-
if let Some(field) = self.typed_dict_field(typed_dict, &Name::new(field_name)) {
1648+
let field_name = Name::new(field_name);
1649+
if let Some(field) = self.typed_dict_field(typed_dict, &field_name) {
16551650
if field.read_only {
16561651
self.error(
16571652
errors,
@@ -1664,16 +1659,27 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
16641659
typed_dict.name(),
16651660
),
16661661
)
1667-
} else if !self.is_subset_eq(value, &field.ty) {
1668-
self.error(
1669-
errors,
1670-
subscript.range(),
1671-
ErrorKind::BadAssignment,
1672-
None,
1673-
format!("Expected `{}`, got `{}`", field.ty, value),
1674-
)
16751662
} else {
1676-
Type::None
1663+
let context = &|| {
1664+
TypeCheckContext::of_kind(TypeCheckKind::TypedDictKey(
1665+
field_name.clone(),
1666+
))
1667+
};
1668+
match value {
1669+
ExprOrBinding::Expr(e) => {
1670+
self.expr(e, Some((&field.ty, context)), errors)
1671+
}
1672+
ExprOrBinding::Binding(b) => {
1673+
let binding_ty = self.solve_binding(b, errors).arc_clone_ty();
1674+
self.check_and_return_type(
1675+
&field.ty,
1676+
binding_ty,
1677+
subscript.range(),
1678+
errors,
1679+
context,
1680+
)
1681+
}
1682+
}
16771683
}
16781684
} else {
16791685
self.error(
@@ -1689,19 +1695,35 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
16891695
)
16901696
}
16911697
}
1692-
(_, _) => self.call_method_or_error(
1693-
&base,
1694-
&dunder::SETITEM,
1695-
subscript.range,
1696-
&[
1697-
CallArg::Type(&slice_ty, subscript.slice.range()),
1698-
// use the subscript's location
1699-
CallArg::Type(value, subscript.range),
1700-
],
1701-
&[],
1702-
errors,
1703-
Some(&|| ErrorContext::SetItem(self.for_display(base.clone()))),
1704-
),
1698+
(_, _) => {
1699+
let call_setitem = |value_arg| {
1700+
self.call_method_or_error(
1701+
&base,
1702+
&dunder::SETITEM,
1703+
subscript.range,
1704+
&[CallArg::Type(&slice_ty, subscript.slice.range()), value_arg],
1705+
&[],
1706+
errors,
1707+
Some(&|| ErrorContext::SetItem(self.for_display(base.clone()))),
1708+
)
1709+
};
1710+
match value {
1711+
ExprOrBinding::Expr(e) => {
1712+
call_setitem(CallArg::Expr(e));
1713+
// We already emit errors for `e` during `call_method_or_error`
1714+
self.expr_infer(
1715+
e,
1716+
&ErrorCollector::new(errors.module_info().clone(), ErrorStyle::Never),
1717+
)
1718+
}
1719+
ExprOrBinding::Binding(b) => {
1720+
let binding_ty = self.solve_binding(b, errors).arc_clone_ty();
1721+
// Use the subscript's location
1722+
call_setitem(CallArg::Type(&binding_ty, subscript.range));
1723+
binding_ty
1724+
}
1725+
}
1726+
}
17051727
}
17061728
}
17071729

pyrefly/lib/test/contextual.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,14 +338,13 @@ xs: list[A] = []
338338
);
339339

340340
testcase!(
341-
bug = "TODO: No context propagated to subscript assignment target, error message is bad",
342341
test_context_assign_subscript,
343342
r#"
344343
class A: ...
345344
class B(A): ...
346345
347346
xs: list[list[A]] = [[]]
348-
xs[0] = [B()] # E: No matching overload found for function `list.__setitem__` # E: `list[B]` is not assignable to parameter with type `list[A]`
347+
xs[0] = [B()]
349348
"#,
350349
);
351350

pyrefly/lib/test/typed_dict.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,27 @@ class Coord(TypedDict):
105105
y: ReadOnly[int]
106106
def foo(c: Coord) -> None:
107107
c["x"] = 1
108-
c["x"] = "foo" # E: Expected `int`, got `Literal['foo']`
108+
c["x"] = "foo" # E: `Literal['foo']` is not assignable to TypedDict key `x` with type `int`
109109
c["y"] = 3 # E: Key `y` in TypedDict `Coord` is read-only
110110
c["z"] = 4 # E: TypedDict `Coord` does not have key `z`
111111
"#,
112112
);
113113

114+
testcase!(
115+
test_typed_dict_contextual,
116+
r#"
117+
from typing import TypedDict
118+
class MyDict(TypedDict, total=False):
119+
data: list[str | int]
120+
def test():
121+
s: MyDict = {}
122+
s['data'] = []
123+
s['data'] = [42]
124+
s['data'] = [42, 'hello']
125+
s['data'] = ['hello']
126+
"#,
127+
);
128+
114129
testcase!(
115130
test_typed_dict_metaclass,
116131
r#"

0 commit comments

Comments
 (0)