Skip to content

[ty] pass type context to sequence literals in binary operations #24197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
oconnor663 merged 3 commits into from
Apr 2, 2026
Merged

[ty] pass type context to sequence literals in binary operations #24197

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4aff95f
[ty] pass type context to sequence literals in binary operations
oconnor663 Mar 26, 2026
def87e0
TODO test cases
oconnor663 Apr 1, 2026
8782189
pass TD context to the right operand also
oconnor663 Apr 2, 2026
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
39 changes: 35 additions & 4 deletions crates/ty_python_semantic/resources/mdtest/bidirectional.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,42 @@ def f[T](x: T, cond: bool) -> T | list[T]:

l5: int | list[int] = f(1, True)

a: list[int] = [1, 2, *(3, 4, 5)]
reveal_type(a) # revealed: list[int]
x: list[int] = [1, 2, *(3, 4, 5)]
reveal_type(x) # revealed: list[int]

b: list[list[int]] = [[1], [2], *([3], [4])]
reveal_type(b) # revealed: list[list[int]]
x: list[list[int]] = [[1], [2], *([3], [4])]
reveal_type(x) # revealed: list[list[int]]

x: list[list[int | str]] = [[1], [2]] * 3
reveal_type(x) # revealed: list[list[int | str]]

x: list[list[int | str]] = 3 * ([[1]] + [[2]])
reveal_type(x) # revealed: list[list[int | str]]

x: list[int | str] = 3 * ["x" for _ in range(3)]
reveal_type(x) # revealed: list[int | str]

# Tuple elements are inferred individually, but type context can prevent e.g. `int` widening.
x: tuple[list[Literal[1]]] = (list1(1),)
reveal_type(x) # revealed: tuple[list[Literal[1]]]

x: tuple[list[Literal[1]], ...] = (list1(1),) * 3
reveal_type(x) # revealed: tuple[list[Literal[1]], ...]

x: tuple[list[Literal[1]], ...] = 3 * ((list1(1),) + (list1(1),))
reveal_type(x) # revealed: tuple[list[Literal[1]], ...]

x: set[int | str] = {1, 2} | {3, 4}
reveal_type(x) # revealed: set[int | str]

x: set[int | str] = {42 for _ in range(3)}
reveal_type(x) # revealed: set[int | str]

x: dict[int | str, int | str] = {1: 2} | {3: 4}
reveal_type(x) # revealed: dict[int | str, int | str]

x: dict[int | str, int | str] = {str(i): i for i in range(3)}
reveal_type(x) # revealed: dict[int | str, int | str]
```

`typed_dict.py`:
Expand Down
41 changes: 33 additions & 8 deletions crates/ty_python_semantic/src/types/infer/builder/binary_expressions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
node_index: _,
} = binary;

let (left_ty, right_ty) = match self.infer_binary_expression_operand_types(left, *op, right)
{
BinaryExpressionOperandTypes::TypedDictResult(ty) => return ty,
BinaryExpressionOperandTypes::Inferred(left_ty, right_ty) => (left_ty, right_ty),
};
let (left_ty, right_ty) =
match self.infer_binary_expression_operand_types(left, *op, right, tcx) {
BinaryExpressionOperandTypes::TypedDictResult(ty) => return ty,
BinaryExpressionOperandTypes::Inferred(left_ty, right_ty) => (left_ty, right_ty),
};

self.infer_binary_expression_type(binary.into(), false, left_ty, right_ty, *op)
.unwrap_or_else(|| {
Expand Down Expand Up @@ -108,7 +108,32 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
left: &ast::Expr,
op: ast::Operator,
right: &ast::Expr,
tcx: TypeContext<'db>,
) -> BinaryExpressionOperandTypes<'db> {
// As a special case, pass `tcx` to binary operands that are collection literals/displays.
// Note that it's not correct to pass it to all binary operands, for example:
// ```
// x: list[str] = ["x"] * 3
// ```
// It doesn't make sense to pass the list type context to the `3` expression. It wouldn't
// have any effect in this case, but it could in more complicated cases.
// TODO: When we support passing `tcx` through generic method calls, we can remove this
// special case and handle the relevant dunder method instead.
let operand_tcx = |expr: &ast::Expr| -> TypeContext<'db> {
match expr {
ast::Expr::List(_)
| ast::Expr::Tuple(_)
| ast::Expr::Set(_)
| ast::Expr::Dict(_)
| ast::Expr::ListComp(_)
| ast::Expr::SetComp(_)
| ast::Expr::DictComp(_) => tcx,
// Also pass `tcx` to nested binary expressions.
ast::Expr::BinOp(_) => tcx,
_ => TypeContext::default(),
}
};

// When a dict literal is `|`'d with a TypedDict, infer the non-literal side first
// so we can use bidirectional inference on the literal before calling the synthesized
// `__or__`/`__ror__` method on the TypedDict side.
Expand All @@ -128,12 +153,12 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
// If the TypedDict update path rejects the literal, fall back to ordinary inference
// even though that means re-inferring the literal without TypedDict context.
return BinaryExpressionOperandTypes::Inferred(
self.infer_expression(left, TypeContext::default()),
self.infer_expression(left, operand_tcx(left)),
right_ty,
Comment on lines 155 to 157

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove TypedDict context in | fallback literal inference

When the left operand is a dict literal and the TypedDict fast-path rejects it, this fallback now re-infers the left literal with operand_tcx(left), which may be a TypedDict target context. That reintroduces the same spurious missing-typed-dict-key/invalid-key diagnostics this branch is meant to avoid (the comment still says it should re-infer without TypedDict context). In cases like assigning a merged dict expression to a TypedDict, the left partial literal can now emit premature key errors purely because of the outer context.

Useful? React with 👍 / 👎.

@oconnor663 oconnor663 Apr 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing, and I'm not sure what we actually want to have happen here. The situation is:

x: MyTypedDict = <dict_literal> | <expression2>

If inferring expression2 with the default context yields a TypedDict type, then we speculatively check whether dict_literal matches that type. If it doesn't match, then currently on the main branch we fall back to inferring dict_literal also with the default context. But it seems like it might be more correct to infer dict_literal with the inherited MyTypedDict context? In fact it seems like it might be more correct to infer both sides that way.

On the other hand, do we care to support a wacky situation like this:

class MyTypedDict(TypedDict):
    a: int
    b: str

x: MyTypedDict = {"a": 42} | {"b": "hello"}

The problem there is that the unioned value is a valid MyTypedDict, but the two halves of it aren't, so passing MyTypedDict as type context to either side will result in errors. On the other hand, this is pretty convoluted, and we don't support it today in any case. @ibraheemdev what's your instinct here?

@ibraheemdev ibraheemdev Apr 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it seems like it might be more correct to infer dict_literal with the inherited MyTypedDict context?

If we failed to perform the speculative typed dict update, dict_literal is not even a valid subset of MyTypedDict, and so inferring it as MyTypedDict directly would likely lead to a lot of missing-key-errors that don't seem very useful.

On the other hand, do we care to support a wacky situation like this:

pyright and mypy doesn't seem to support this either, so it seems fine to ignore it. Maybe we should just ignore TypedDict context entirely outside of the speculative updates to avoid the extra errors, if there is no way to produce a valid TypedDict otherwise (unless there is a case I am missing)?

);
}

let left_ty = self.infer_expression(left, TypeContext::default());
let left_ty = self.infer_expression(left, operand_tcx(left));
if op == ast::Operator::BitOr
&& let Type::TypedDict(typed_dict) = left_ty
&& matches!(right, ast::Expr::Dict(_))
Expand All @@ -149,7 +174,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> {

BinaryExpressionOperandTypes::Inferred(
left_ty,
self.infer_expression(right, TypeContext::default()),
self.infer_expression(right, operand_tcx(right)),
)
}

Expand Down
Loading