[ty] Type-context aware literal promotion #20776
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ibraheemdev
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-10-09 20:41:23.943326536 +0000
+++ new-output.txt 2025-10-09 20:41:27.257340205 +0000
@@ -643,9 +643,7 @@
literals_literalstring.py:75:5: error[invalid-assignment] Object of type `Literal[b"test"]` is not assignable to `LiteralString`
literals_literalstring.py:79:21: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `bool`
literals_literalstring.py:120:22: error[invalid-argument-type] Argument to function `literal_identity` is incorrect: Argument type `str` does not satisfy upper bound `LiteralString` of type variable `TLiteral`
-literals_literalstring.py:130:35: error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `Container[T@Container]`, found `Container[str]`
literals_literalstring.py:134:51: error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Argument type `str` does not satisfy upper bound `LiteralString` of type variable `T`
-literals_literalstring.py:138:1: error[invalid-assignment] Object of type `list[Unknown | str]` is not assignable to `list[LiteralString]`
literals_literalstring.py:167:1: error[type-assertion-failure] Argument does not have asserted type `A`
literals_literalstring.py:171:5: error[invalid-assignment] Object of type `list[LiteralString]` is not assignable to `list[str]`
literals_parameterizations.py:41:15: error[invalid-type-form] Type arguments for `Literal` must be `None`, a literal value (int, bool, str, or bytes), or an enum member
@@ -900,5 +898,5 @@
typeddicts_usage.py:28:17: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `Movie` constructor
typeddicts_usage.py:28:18: error[invalid-key] Invalid key access on TypedDict `Movie`: Unknown key "title"
typeddicts_usage.py:40:24: error[invalid-type-form] The special form `typing.TypedDict` is not allowed in type expressions. Did you mean to use a concrete TypedDict or `collections.abc.Mapping[str, object]` instead?
-Found 901 diagnostics
+Found 899 diagnostics
WARN A fatal error occurred while checking some files. Not all project files were analyzed. See the diagnostics list above for details. |
ibraheemdev
force-pushed
the
ibraheem/literal-promotion
branch
from
dc1da75 to
33e0131
Compare
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
0 | 66 | 1 |
invalid-assignment |
0 | 13 | 0 |
| Total | 0 | 79 | 1 |
CodSpeed Performance ReportMerging #20776 will not alter performanceComparing Summary
Footnotes
|
ibraheemdev
force-pushed
the
ibraheem/literal-promotion
branch
from
33e0131 to
8a172d7
Compare
ibraheemdev
commented
Oct 9, 2025
These look quite odd -- any idea what's causing these diagnostics? This also doesn't look correct -- tornado (https://github.com/tornadoweb/tornado)
+ tornado/test/auth_test.py:563:69: error[invalid-argument-type] Argument to function `url_concat` is incorrect: Expected `None | dict[str, str] | list[tuple[str, str]] | tuple[tuple[str, str], ...]`, found `dict[str, str]` |
Possibly not something for this PR, but we should look into why we're creating these huge unions in (The reason why we added |
sharkdp
approved these changes
Oct 9, 2025
| .annotation | ||
| .is_none_or(|annotation| promoted.is_assignable_to(db, annotation)) | ||
| { | ||
| return promoted; |
There was a problem hiding this comment.
An interesting consequence here is that we would still perform promotion if something else in the annotation makes the promoted type assignable. For example:
from typing import Any, Literal
xs: list[Any | Literal[1]] = [1, 1]
reveal_type(xs) # list[Any | int]Probably nothing to worry about, though...
There was a problem hiding this comment.
Hrm, I think I would prefer list[Any | Literal[1]] to be the revealed type there... but I do agree that this seems unlikely to come up in the real world, so it doesn't need to block this PR
AlexWaygood
reviewed
Oct 9, 2025
Comment on lines
-528
to
-531
| # TODO: we could bidirectionally infer that the user does not want literals to be promoted here, | ||
| # and avoid this diagnostic | ||
| # | ||
| # error: [invalid-assignment] "`list[int]` is not assignable to `list[Literal[1, 2, 3]]`" |
There was a problem hiding this comment.
there are some interesting examples above this for unannotated frozensets (which don't need to be fixed in this PR, I'm just mentioning it because it might be something you'd be interested in fixing in future work). frozenset, like tuple, is covariant, so for an unannotated x = frozenset((1, 2, 3)) call, we would ideally infer the type as frozenset[Literal[1, 2, 3]], the same as we would infer (1, 2, 3) as tuple[Literal[1], Literal[2], Literal[3]] rather than tuple[int, int, int]. Unlike with invariant generics, Literal types don't really cause any ergonomic issues when they appear in specializations of covariant types.
But we need to be careful: list(frozenset((1, 2, 3))) should still be inferred as list[int] rather than list[Literal[1, 2, 3]] if there's no declared type! And [frozenset((1, 2))] should be list[frozenset[int]]
| .annotation | ||
| .is_none_or(|annotation| promoted.is_assignable_to(db, annotation)) | ||
| { | ||
| return promoted; |
There was a problem hiding this comment.
Hrm, I think I would prefer list[Any | Literal[1]] to be the revealed type there... but I do agree that this seems unlikely to come up in the real world, so it doesn't need to block this PR
| db: &'db dyn Db, | ||
| name: &str, | ||
| mut argument_types: CallArguments<'_, 'db>, | ||
| tcx: TypeContext<'db>, |
There was a problem hiding this comment.
It seems like we currently have only one use of try_call_dunder that passes a tcx which isn't TypeContext::default(), but all callsites become more verbose because of the new parameter. Is it worth maybe having two methods?
fn try_call_dunder(
self,
db: &'db dyn Db,
name: &str,
mut argument_types: CallArguments<'_, 'db>,
) -> Result<Bindings<'db>, CallDunderError<'db>> {
self.try_call_dunder_with_type_context(
self,
db: &'db dyn Db,
name: &str,
mut argument_types: CallArguments<'_, 'db>,
TypeContext::default()
)
}
fn try_call_dunder_with_type_context(
self,
db: &'db dyn Db,
name: &str,
mut argument_types: CallArguments<'_, 'db>,
tcx: TypeContext<'db>,
) -> Result<Bindings<'db>, CallDunderError<'db>> {
// Has the body of the method called `try_call_dunder` on your branch
}Then most callsites could use the try_call_dunder() method and not have to worry about the tcx argument.
Or are we anticipating that we'll want to eventually plumb the type context through many more of these try_call_dunder calls in due course?
| self, | ||
| db: &'db dyn Db, | ||
| type_mapping: &TypeMapping<'a, 'db>, | ||
| tcx: TypeContext<'db>, |
There was a problem hiding this comment.
same question here -- it seems like the vast majority of apply_type_mapping() calls currently just pass in TypeContext::default(), so I wonder if it would be more ergonomic to have separate apply_type_mapping and apply_type_mapping_with_context methods, where the former would just be a thin wrapper around the latter?
There was a problem hiding this comment.
I think passing TypeContext::default() is a useful cue that we are ignoring a potential type context. There are probably cases where we currently have type context but are not making use of it.
T-256
reviewed
Oct 9, 2025
I fixed these, we were passing incorrect The diagnostics are interesting though — we currently reinfer the RHS of annotated assignments without any type-context before displaying the diagnostics. This was originally to avoid unioning the inferred type with the type annotation, but it now also means that we always perform literal promotion (as there is no type-context to prevent it). For example: x: list[Literal[3]] = [2]The type in the diagnostic is now less precise because of the reinference. I don't think the " |
Those diagnostics don't seem too bad to me tbh — I've seen worse from mypy/pyright for this kind of thing 😆 I wouldn't spend too much time on that right now, personally |
|
The regression on |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Avoid literal promotion when a literal type annotation is provided, e.g.,
Resolves astral-sh/ty#1198. This does not fix issue astral-sh/ty#1284, but it does make it more relevant because after this change, it is possible to directly instantiate a generic type with a literal specialization.