[ty] Fix implementation of Top[Callable[..., object]]#22145
Merged
charliermarsh merged 12 commits intomainfrom Dec 24, 2025
Merged
[ty] Fix implementation of Top[Callable[..., object]]#22145charliermarsh merged 12 commits intomainfrom
Top[Callable[..., object]]#22145charliermarsh merged 12 commits intomainfrom
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
Member
|
|
Member
|
It's the same as the difference between |
Member
|
We implement ruff/crates/ty_python_semantic/src/types/generics.rs Lines 734 to 741 in 4a93754 CallableType and/or Parameters.
|
Member
Author
|
🫡 |
charliermarsh
force-pushed
the
charlie/x
branch
from
b8dccb9 to
b655839
Compare
AlexWaygood
changed the title
...) for parameters in Callable bottom type...) for parameters in Callable bottom type
AlexWaygood
changed the title
...) for parameters in Callable bottom type...) for parameters in Callable top materialization
Member
|
Nice, this looks like it's on the right track now!
Diagnostics like this indicate that some of the subtyping rules aren't quite right yet. All Similarly, |
Member
|
The primer report is starting to look a lot better! There's another change that we need to make... which might make the ecosystem blow up, but I do think it's the "correct" thing to do: def f(x: object):
if callable(x):
x() # this should be an error...
# we know that `x` is callable, but we do not know
# *how* it is callable (is it okay to call it with *zero arguments*?).
# `Top[Callable[..., object]]` is much stricter than `Callable[..., object]`
# for things like this
This is similar to the way we apply much stricter rules to from ty_extensions import Top
from typing import Any
def f(x: list[Any], y: Top[list[Any]]):
x.append("foo") # fine
y.append("foo") # error: expected `Never`, found `Literal["foo"] |
charliermarsh
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
2 | 36 | 14 |
unresolved-attribute |
6 | 3 | 19 |
invalid-assignment |
0 | 6 | 21 |
invalid-return-type |
0 | 4 | 13 |
possibly-missing-attribute |
0 | 7 | 8 |
unsupported-operator |
0 | 7 | 0 |
call-top-callable |
5 | 0 | 0 |
missing-argument |
5 | 0 | 0 |
not-iterable |
0 | 3 | 1 |
unused-ignore-comment |
3 | 1 | 0 |
call-non-callable |
1 | 1 | 0 |
too-many-positional-arguments |
0 | 2 | 0 |
type-assertion-failure |
2 | 0 | 0 |
invalid-parameter-default |
0 | 0 | 1 |
invalid-type-form |
0 | 0 | 1 |
no-matching-overload |
0 | 1 | 0 |
| Total | 24 | 71 | 78 |
Member
|
I wouldn't worry about the spack panic in ecosystem-analyzer — we've seen it on some other PRs; it's one of our nondeterminism issues right now |
charliermarsh
force-pushed
the
charlie/x
branch
from
c873d89 to
2c73f4f
Compare
Contributor
|
Lots of false positives going away in the ecosystem report! 🎉 I see a few new diagnostics with message "Object of type Other type checkers unsoundly use |
Member
Author
|
(I think that diagnostic is what Alex asked for here: #22145 (comment). No issue from me with using a custom diagnostic for it!) |
carljm
reviewed
Dec 24, 2025
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
Awesome!
I think it'd be good to have some tests showing the behavior of the bottom callable, too? It's always callable with any arguments.
The ways it could be created in real code are a bit complex. You'd need a gradual-form callable type in contravariant position of some type -- e.g. as an argument to another Callable -- and then you'd need to do something that would top-materialize the outer type, like return it wrapped in TypeIs. I'm not sure we need to represent this in a test; I'd just use ty_extensions.Bottom to create the type "manually".
| /// The callable type is a top materialization (e.g., `Top[Callable[..., object]]`), which | ||
| /// represents an unknown callable signature. While such types *are* callable (they pass | ||
| /// `callable()`), any specific call should fail because we don't know the actual signature. | ||
| UnknownCallableSignature(Type<'db>), |
Contributor
There was a problem hiding this comment.
Although the term "unknown" is correct here, it's confusing given our frequent usage of Unknown as a dynamic type. This isn't user-facing, it's in code: maybe CalledTopCallable?
charliermarsh
force-pushed
the
charlie/x
branch
from
2c73f4f to
9216960
Compare
dhruvmanila
reviewed
Dec 24, 2025
AlexWaygood
changed the title
...) for parameters in Callable top materializationTop[Callable[..., object]]
Member
|
The ecosystem report's looking good again! |
AlexWaygood
approved these changes
Dec 24, 2025
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
This looks great -- thank you!! 😃
crates/ty_python_semantic/resources/mdtest/type_properties/materialization.md
Outdated
Show resolved
Hide resolved
charliermarsh
force-pushed
the
charlie/x
branch
from
2a71cef to
e1bd5ad
Compare
charliermarsh
force-pushed
the
charlie/x
branch
from
e1bd5ad to
5201bc3
Compare
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Member
Author
|
Thank you @AlexWaygood! |
charliermarsh
pushed a commit
that referenced
this pull request
Dec 24, 2025
…ute on a `Callable` type (#22182) ## Summary Other type checkers allow you to access all `FunctionType` attributes on any object with a `Callable` type. ty does not, because this is demonstrably unsound, but this is often a source of confusion for users. And there were lots of diagnostics in the ecosystem report for #22145 that were complaining that "Object of type `(...) -> Unknown` has no attribute `__name__`", for example. The discrepancy between what ty does here and what other type checkers do is discussed a bit in astral-sh/ty#1495. You can see that there have been lots of issues closed as duplicates of that issue; we should probably also add an FAQ entry for it. Anyway, this PR adds a subdiagnostic to help users out when they hit this diagnostic. Unfortunately something I did meant that rustfmt increased the indentation of the whole of this huge closure, so this PR is best reviewed with the "No whitespace" option selected for viewing the diff. ## Test Plan Snapshot added
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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
Add a proper representation for the
Callabletop type, and use it to getcallable()narrowing right.Closes astral-sh/ty#1426.