-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Infer more precise types for collection literals #20360
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.
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
Conversation
b59e27a to
0a033e2
Compare
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-09-17 19:38:23.513997998 +0000
+++ new-output.txt 2025-09-17 19:38:26.563992249 +0000
@@ -36,7 +36,7 @@
aliases_implicit.py:70:5: error[type-assertion-failure] Argument does not have asserted type `int | str | None | list[list[int]]`
aliases_implicit.py:71:5: error[type-assertion-failure] Argument does not have asserted type `list[bool]`
aliases_implicit.py:72:5: error[type-assertion-failure] Argument does not have asserted type `(...) -> None`
-aliases_implicit.py:107:9: error[invalid-type-form] Variable of type `list[@Todo(list literal element type)]` is not allowed in a type expression
+aliases_implicit.py:107:9: error[invalid-type-form] Variable of type `list[Unknown | <class 'int'> | <class 'str'>]` is not allowed in a type expression
aliases_implicit.py:108:9: error[invalid-type-form] Variable of type `tuple[tuple[<class 'int'>, <class 'str'>]]` is not allowed in a type expression
aliases_implicit.py:109:9: error[invalid-type-form] Variable of type `list[@Todo(list comprehension element type)]` is not allowed in a type expression
aliases_implicit.py:110:9: error[invalid-type-form] Variable of type `dict[@Todo(dict literal key type), @Todo(dict literal value type)]` is not allowed in a type expression
@@ -301,7 +301,6 @@
directives_reveal_type.py:19:5: error[missing-argument] No argument provided for required parameter `obj` of function `reveal_type`
directives_reveal_type.py:20:20: error[too-many-positional-arguments] Too many positional arguments to function `reveal_type`: expected 1, got 2
directives_type_checking.py:11:5: error[invalid-assignment] Object of type `Literal[""]` is not assignable to `int`
-directives_type_checking.py:18:1: error[type-assertion-failure] Argument does not have asserted type `list[int]`
directives_type_ignore_file2.py:14:1: error[invalid-assignment] Object of type `Literal[""]` is not assignable to `int`
directives_version_platform.py:14:5: error[invalid-assignment] Object of type `Literal[""]` is not assignable to `int`
directives_version_platform.py:19:5: error[invalid-assignment] Object of type `Literal[""]` is not assignable to `int`
@@ -620,6 +619,7 @@
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:5: error[invalid-assignment] Object of type `Container[str]` is not assignable to `Container[LiteralString]`
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[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
@@ -718,7 +718,6 @@
overloads_evaluation.py:161:5: error[type-assertion-failure] Argument does not have asserted type `Literal[0, 1]`
overloads_evaluation.py:234:5: error[type-assertion-failure] Argument does not have asserted type `int`
overloads_evaluation.py:291:33: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `T@example6`
-overloads_evaluation.py:322:5: error[type-assertion-failure] Argument does not have asserted type `list[int]`
protocols_class_objects.py:58:1: error[invalid-assignment] Object of type `<class 'ConcreteA'>` is not assignable to `ProtoA1`
protocols_class_objects.py:59:1: error[invalid-assignment] Object of type `<class 'ConcreteA'>` is not assignable to `ProtoA2`
protocols_definition.py:114:1: error[invalid-assignment] Object of type `Concrete2_Bad1` is not assignable to `Template2`
@@ -862,5 +861,5 @@
typeddicts_usage.py:28:1: 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 863 diagnostics
+Found 862 diagnostics
WARN A fatal error occurred while checking some files. Not all project files were analyzed. See the diagnostics list above for details. |
|
CodSpeed WallTime Performance ReportMerging #20360 will degrade performances by 26.86%Comparing Summary
Benchmarks breakdown
|
0a033e2 to
428a6a5
Compare
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
329 | 1 | 289 |
unsupported-operator |
90 | 0 | 40 |
possibly-unbound-attribute |
98 | 0 | 27 |
invalid-assignment |
39 | 0 | 30 |
unused-ignore-comment |
0 | 53 | 0 |
invalid-return-type |
11 | 0 | 36 |
parameter-already-assigned |
30 | 0 | 0 |
non-subscriptable |
27 | 0 | 0 |
no-matching-overload |
12 | 0 | 0 |
not-iterable |
6 | 0 | 6 |
possibly-unbound-implicit-call |
9 | 0 | 2 |
unresolved-attribute |
10 | 0 | 1 |
type-assertion-failure |
0 | 8 | 0 |
call-non-callable |
6 | 0 | 0 |
index-out-of-bounds |
4 | 0 | 0 |
division-by-zero |
2 | 0 | 0 |
invalid-exception-caught |
0 | 0 | 1 |
invalid-key |
1 | 0 | 0 |
missing-argument |
1 | 0 | 0 |
too-many-positional-arguments |
1 | 0 | 0 |
| Total | 676 | 62 | 432 |
AlexWaygood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
crates/ty_python_semantic/resources/mdtest/assignment/annotations.md
Outdated
Show resolved
Hide resolved
|
Would you be able to look through the mypy_primer diff and check that it's mostly as expected? If you use the ecosystem analyzer rich diff here, it links directly to the line each new diagnostic is being emitted on |
carljm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good first step to me!
81846dd to
65ae3cd
Compare
|
Oh, I do think some analysis of the ecosystem report here would be good, if there are any common causes of false positives we should prioritize addressing next. I did check the conformance suite report -- the changes are both good. |
|
I ended up rewriting most of the code to support better inference for unannotated collection literals as well. We now infer the type of a collection literal based on the inferred type of each element, as well as the type annotation if provided, or x = [1, 2, 3] # revealed: list[Unknown | Literal[1, 2, 3]]
x: list[int] = [1, 2, 3] # revealed: list[int] |
29cd3d3 to
65f29ac
Compare
|
|
||
| h: list[list[int]] = [[], [42]] | ||
| # TODO: revealed: list[list[int]] | ||
| reveal_type(h) # revealed: list[list[int] | list[Unknown] | list[Unknown | Literal[42]]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dcreager is this something that you expect the new constraint solver to be able to simplify?
CodSpeed Instrumentation Performance ReportMerging #20360 will degrade performances by 8.1%Comparing Summary
Benchmarks breakdown
|
ec3b8c7 to
6c22ff7
Compare
ae94e6b to
228ae69
Compare
42d8895 to
a5e3d3e
Compare
carljm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you! Just a few nits inline, but I think it's ready to land.
crates/ty_python_semantic/resources/mdtest/narrow/conditionals/nested.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/narrow/conditionals/nested.md
Outdated
Show resolved
Hide resolved
| /// does not. | ||
| pub(crate) fn literal_annotation_type(self, db: &'db dyn Db) -> Option<Type<'db>> { | ||
| match self { | ||
| Type::StringLiteral(_) | Type::LiteralString => Some(KnownClass::Str.to_instance(db)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean we still error on x: list[LiteralString] = ["foo"]? Can we add a test for that (with TODO if we still error on it)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm a little hesitant about LiteralString being the default promotion type, because I think this is actually a bigger issue with unconditionally promoting the types of the RHS. For example, x: list[Literal["a"]] = ["a"] should also type-check (and mypy and pyright both handle this correctly). I think to make this work properly we would need to thread the type-context down to the literal promotions visitor, which seems non-trivial. I left a test with a TODO for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
I think to make this work properly we would need to thread the type-context down to the literal promotions visitor
I guess this is because we might want to promote some literals but not others, e.g. if we have x: list[int | Literal["a"]] = ["a", <massive list of integer literals>]?
It seems like the simpler cases could be handled via a heuristic of "if annotated element type is a literal, don't promote literals on the RHS", but that wouldn't handle the mixed union case well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is because we might want to promote some literals but not others, e.g. if we have
x: list[int | Literal["a"]] = ["a", <massive list of integer literals>]?
I think there are more straightforwards examples than the union case, e.g. x: list[tuple[Literal[1]]] = [(1,)].
One of the issues is that we promote the literals of the RHS recursively when inferring the elements of a list/set, because we currently infer (1,) to be tuple[Literal[1]], but [(1,)] to be list[tuple[int]]. That means the logic for any heuristic would need to go in the type-mapping visitor, which would then need access to the type-context.
This is actually already an issue in the other place we use PromoteLiterals:
class X[T]:
def __init__(self, x: T):
self.x = x
# ty: Object of type `tuple[X[int]]` is not assignable to `tuple[X[Literal[1]]]`
x: tuple[X[Literal[1]]] = (X(1), )So i think the bug is somewhat unrelated to the changes here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return None; | ||
| } | ||
| let specialization = class_type.into_generic_alias()?.specialization(db); | ||
| let [annotated_elts_ty] = specialization.types(db) else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need some generalization (in a subsequent PR) to handle dict literals (with two type parameters).
6599ebd to
e434d72
Compare
| # TODO: this should type-check and avoid literal promotion | ||
| # error: [invalid-assignment] "Object of type `list[int]` is not assignable to `list[Literal[1, 2, 3]]`" | ||
| n: list[typing.Literal[1, 2, 3]] = [1, 2, 3] | ||
| reveal_type(n) # revealed: list[Literal[1, 2, 3]] | ||
|
|
||
| # TODO: this should type-check and avoid literal promotion | ||
| # error: [invalid-assignment] "Object of type `list[str]` is not assignable to `list[LiteralString]`" | ||
| o: list[typing.LiteralString] = ["a", "b", "c"] | ||
| reveal_type(o) # revealed: list[LiteralString] | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you done a review of the ecosystem analyzer results? It's good to do in general just to catch bugs, but specifically it would be good to know if this is actually coming up in the ecosystem, to get a sense of how much we should prioritize fixing it.
| /// does not. | ||
| pub(crate) fn literal_annotation_type(self, db: &'db dyn Db) -> Option<Type<'db>> { | ||
| match self { | ||
| Type::StringLiteral(_) | Type::LiteralString => Some(KnownClass::Str.to_instance(db)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
I think to make this work properly we would need to thread the type-context down to the literal promotions visitor
I guess this is because we might want to promote some literals but not others, e.g. if we have x: list[int | Literal["a"]] = ["a", <massive list of integer literals>]?
It seems like the simpler cases could be handled via a heuristic of "if annotated element type is a literal, don't promote literals on the RHS", but that wouldn't handle the mixed union case well.
|
I went through the ecosystem report, and most of the changes seem to be cases where we are able to correctly catch more type errors. There were one or two cases where the literal promotion issue came up, but as discussed I think that can be addressed in a separate PR. There were also a couple instances where lists were being used as heterogenous types, e.g., def y(a: int, b: int, c: str): ...
y(*[1, 2, "3"])pyright actually doesn't error on this, because it infers |
Interesting that pyright gives up in that situation. Mypy infers |
## Summary Extends #20360 to dictionary literals. This also improves our `TypeDict` support by passing through nested type context.
Summary
Part of astral-sh/ty#168 and astral-sh/ty#543. Infer more precise types for collection literals (currently, only
listandset). For example,This could easily be extended to
dictliterals, but I am intentionally limiting scope for now.