[ty] Include type parameters in generic callable display#22435
Merged
AlexWaygood merged 14 commits intoastral-sh:mainfrom Jan 13, 2026
Merged
[ty] Include type parameters in generic callable display#22435AlexWaygood merged 14 commits intoastral-sh:mainfrom
AlexWaygood merged 14 commits intoastral-sh:mainfrom
Conversation
Fixes astral-sh/ty#1207 ## Summary Modified `DisplaySignature` to show type parameter brackets (e.g., `[T]`) when displaying generic function and method signatures. Added a `generic_context` field and updated `fmt_detailed()` to prepend type parameters, preventing duplication when `DisplayFunctionType` already renders them. ## Test Plan Updated expected output in 6 test files. All tests pass: `test result: ok. 307 passed; 0 failed; 0 ignored` ## Example list[Unknown | ((t: T@f) -> T@f)] → list[Unknown | ([T](t: T@f) -> T@f)]
bxff
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 2026-01-13 17:26:33.731951055 +0000
+++ new-output.txt 2026-01-13 17:26:34.088951937 +0000
@@ -468,7 +468,7 @@
generics_defaults.py:131:1: error[type-assertion-failure] Type `Any` does not match asserted type `int`
generics_defaults.py:155:49: error[invalid-type-form] List literals are not allowed in this context in a type expression: Did you mean `tuple[int | float, bool]`?
generics_defaults.py:156:58: error[invalid-type-form] List literals are not allowed in this context in a type expression: Did you mean `list[bytes]`?
-generics_defaults.py:170:1: error[type-assertion-failure] Type `(Foo7[int], /) -> Foo7[int]` does not match asserted type `def meth(self, /) -> Self@meth`
+generics_defaults.py:170:1: error[type-assertion-failure] Type `(Foo7[int], /) -> Foo7[int]` does not match asserted type `def meth[Self](self, /) -> Self`
generics_defaults_referential.py:23:1: error[type-assertion-failure] Type `type[slice[int, int, int | None]]` does not match asserted type `<class 'slice'>`
generics_defaults_referential.py:36:13: error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `int`, found `Literal[""]`
generics_defaults_referential.py:37:10: error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `int`, found `Literal[""]`
|
|
AlexWaygood
reviewed
Jan 7, 2026
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
Nice, thanks! I think there's a few details here to work out, however:
crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md
Outdated
Show resolved
Hide resolved
## Summary
Refines display logic for generic callables to eliminate redundant and
confusing information in type signatures:
1. **Filter specialized type variables**: Modified
`TypeMapping::update_signature_generic_context` to exclude type
variables that have been specialized to concrete types (e.g., `T -> int`)
while preserving identity mappings. This prevents methods of specialized
generic classes (e.g., `Wrap[int]`) from incorrectly showing `[T]` prefixes.
Before: `[T](<self: Wrap[int], value: int>) -> None`
After: `(self: Wrap[int], value: int) -> None`
2. **Suppress redundant scope suffixes**: Added `active_scopes` tracking to
`DisplaySettings`. When a signature displays type parameters like `[T]`,
the `@{scope}` suffix is now omitted for type variables within that
scope, as the scope is already unambiguous.
Before: `[T](<self: ParentDataclass[T@ChildOfParentDataclass], value: T@ChildOfParentDataclass>)`
After: `[T](<self: ParentDataclass[T], value: T>)`
## Key Changes
- `crates/ty_python_semantic/src/types.rs`: Filter specialized typevars
- `crates/ty_python_semantic/src/types/display.rs`: Add `active_scopes` and
update `DisplayBoundTypeVarIdentity` to suppress suffixes when appropriate
## Test Plan
Updated expectations in 10+ mdtest files. Verified all tests pass:
`test result: ok. 308 passed; 0 failed; 0 ignored`
Contributor
Author
|
Hi @AlexWaygood, thanks for the detailed review! I've addressed all your feedback:
Let me know if you have any other suggestions! |
AlexWaygood
reviewed
Jan 7, 2026
Comment on lines
624
to
625
| reveal_type(object.__new__) # revealed: def __new__(cls) -> Self | ||
| reveal_type(object().__new__) # revealed: def __new__(cls) -> Self |
Member
There was a problem hiding this comment.
Shouldn't these be
Suggested change
| reveal_type(object.__new__) # revealed: def __new__(cls) -> Self | |
| reveal_type(object().__new__) # revealed: def __new__(cls) -> Self | |
| reveal_type(object.__new__) # revealed: def __new__[Self](cls) -> Self | |
| reveal_type(object().__new__) # revealed: def __new__[Self](cls) -> Self |
?
Contributor
Author
There was a problem hiding this comment.
I've investigated and it looks like there's a pre-existing filter that intentionally hides Self from the type parameter display to avoid noise (since it's implicitly added to most methods).
If I remove that filter, object.__new__ would correctly show as def __new__[Self](cls) -> Self. However, this would also cause Self to appear in other places where it might not be helpful:
- For methods on specialized classes like
C[int].f, we'd getdef f[Self](self, x: int) -> strwhereSelfonly exists for the implicitselfparameter and doesn't appear elsewhere in the signature - For bound methods, we'd see
[Self]even though the type has been fully resolved, which seems confusing
What's your preference here? Should I remove the filter and accept the broader display of [Self], or would you prefer a more targeted approach that only shows Self when it appears meaningfully in the signature (like in the return type or explicit parameters)?
Member
There was a problem hiding this comment.
It sounds like the easiest thing to do here (for now) would be to just restore the display for Self typevars specifically to what they are on main? I think applying this patch to your PR would do the trick:
diff --git a/crates/ty_python_semantic/resources/mdtest/call/methods.md b/crates/ty_python_semantic/resources/mdtest/call/methods.md
index d3b95d81cd..c15e14a615 100644
--- a/crates/ty_python_semantic/resources/mdtest/call/methods.md
+++ b/crates/ty_python_semantic/resources/mdtest/call/methods.md
@@ -621,17 +621,17 @@ argument:
```py
from typing_extensions import Self
-reveal_type(object.__new__) # revealed: def __new__(cls) -> Self
-reveal_type(object().__new__) # revealed: def __new__(cls) -> Self
-# revealed: Overload[(cls, x: str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc = 0, /) -> Self, (cls, x: str | bytes | bytearray, /, base: SupportsIndex) -> Self]
+reveal_type(object.__new__) # revealed: def __new__(cls) -> Self@__new__
+reveal_type(object().__new__) # revealed: def __new__(cls) -> Self@__new__
+# revealed: Overload[(cls, x: str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc = 0, /) -> Self@__new__, (cls, x: str | bytes | bytearray, /, base: SupportsIndex) -> Self@__new__]
reveal_type(int.__new__)
-# revealed: Overload[(cls, x: str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc = 0, /) -> Self, (cls, x: str | bytes | bytearray, /, base: SupportsIndex) -> Self]
+# revealed: Overload[(cls, x: str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc = 0, /) -> Self@__new__, (cls, x: str | bytes | bytearray, /, base: SupportsIndex) -> Self@__new__]
reveal_type((42).__new__)
class X:
def __init__(self, val: int): ...
def make_another(self) -> Self:
- reveal_type(self.__new__) # revealed: def __new__(cls) -> Self
+ reveal_type(self.__new__) # revealed: def __new__(cls) -> Self@__new__
return self.__new__(type(self))
```
diff --git a/crates/ty_python_semantic/src/types/display.rs b/crates/ty_python_semantic/src/types/display.rs
index 378e93c94e..ff0c4c90af 100644
--- a/crates/ty_python_semantic/src/types/display.rs
+++ b/crates/ty_python_semantic/src/types/display.rs
@@ -1092,13 +1092,19 @@ struct DisplayBoundTypeVarIdentity<'db> {
impl Display for DisplayBoundTypeVarIdentity<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
f.write_str(self.bound_typevar_identity.identity.name(self.db))?;
- if let Some(binding_context_name) = self.bound_typevar_identity.binding_context.name(self.db) {
- let mut suppressed = false;
- if let BindingContext::Definition(definition) = self.bound_typevar_identity.binding_context {
- if self.settings.active_scopes.contains(&definition) {
- suppressed = true;
- }
- }
+ if let Some(binding_context_name) =
+ self.bound_typevar_identity.binding_context.name(self.db)
+ {
+ let suppressed = if let BindingContext::Definition(definition) =
+ self.bound_typevar_identity.binding_context
+ && self.settings.active_scopes.contains(&definition)
+ && !self.bound_typevar_identity.identity.kind(self.db).is_self()
+ {
+ true
+ } else {
+ false
+ };
+
if !suppressed {
write!(f, "@{binding_context_name}")?;
}
Member
|
Could you add this test somewhere? I like the new display, but it doesn't look like we have an explicit test for the display of a non-function callable that uses both a type parameter scoped to the callable itself and a type parameter scoped to an outer scope: from typing import Generic, TypeVar
from ty_extensions import into_callable
T = TypeVar("T")
S = TypeVar("S")
class Foo(Generic[T]):
def bar(x: T, y: S) -> tuple[T, S]:
raise NotImplementedError
def f(x: type[Foo[T]]) -> T:
# revealed: [S](x: T@f, y: S) -> tuple[T@f, S]
reveal_type(into_callable(x.bar))
raise NotImplementedError |
## Summary Corrects the display logic for generic callables to properly handle the `Self` type parameter in various contexts, ensuring it appears when generic and is suppressed when bound to a concrete type. ## Changes 1. **`object.__new__` display**: Removed the `is_self()` filter to ensure `Self` appears in the generic context prefix: ```python reveal_type(object.__new__) # revealed: def __new__[Self](cls) -> Self ``` 2. **Bound method handling**: Refactored `display.rs` to use `bind_self()` before collecting type parameters. Bound methods no longer show `[Self]` when it's bound to a concrete type: ```python reveal_type(C[int]().m) # revealed: bound method C[int].m[U](x: int, y: U) -> ... ``` 3. **Specialized class methods**: Methods of specialized generic classes correctly display `[Self]` only when appropriate (unbound methods where `Self` remains generic). 4. **Mixed-scope type parameters**: Added test coverage for methods with both class-scoped and method-scoped parameters, verifying specialized types are suppressed while active ones display correctly. ## Implementation - Modified `crates/ty_python_semantic/src/types/display.rs` to use `bind_self()` and populate `active_scopes` from generic contexts - Updated test expectations in `call/methods.md`, `generics/scoping.md`, and `call/getattr_static.md` - Added new test case in `generics/scoping.md` for mixed-scope scenarios ## Test Results All 308 mdtest cases pass: `test result: ok. 308 passed; 0 failed; 0 ignored` The display logic now correctly reflects the type system's state for `Self`, generic contexts, and bound methods.
Contributor
Author
|
@AlexWaygood thanks for the patch suggestion, it would certainly restore the old behavior with minimal churn. However, after investigating, I decided to fully remove the Self filter and properly fix the underlying display logic. Here's what that entailed: Bound Methods: The key issue was that bound methods needed
Specialized Classes: For methods on specialized generic classes like Mixed-Scope Parameters: The new test case confirms proper handling: The result is modern, consistent |
AlexWaygood
reviewed
Jan 8, 2026
crates/ty_python_semantic/resources/mdtest/call/getattr_static.md
Outdated
Show resolved
Hide resolved
## Summary Only prepend `[Self]` to generic callable signatures when it appears in an explicit parameter annotation or the return type. If `Self` is only used for the implicit `self` parameter, it is now hidden from the prefix. ## Changes - Added `Type::contains_self()` helper method - Added `hide_unused_self` flag to `DisplayGenericContext` - Updated `DisplaySignature::fmt_detailed()` to conditionally hide `[Self]` - Updated test expectations in 3 files
AlexWaygood
reviewed
Jan 8, 2026
## Summary Clean up the `hide_unused_self` implementation by eliminating code duplication, investigating potential gaps in overload handling (none found), and optimizing the display logic to avoid unnecessary allocations. ## Changes ### 1. DRY Refactor: Helper Method Extraction Extracted duplicated `hide_unused_self` logic into reusable helper methods: - Added `Signature::should_hide_self_from_display(db)` in `signatures.rs` - Added `DisplaySignature::should_hide_self_from_display(db)` in `display.rs` Both helpers check if `Self` appears in explicit parameter annotations or the return type, centralizing the logic that was previously duplicated across 4 display paths. ### 2. Overload Coverage Investigation Verified that overload loops correctly delegate to `DisplaySignature`, which internally computes `hide_unused_self` for each signature. No additional fix was needed—each overload signature properly handles `Self` visibility. ### 3. Performance Optimization Refactored `fmt_normal` in `display.rs` to use a `peekable()` iterator instead of collecting into a `Vec`, avoiding an unnecessary allocation when checking if generic context variables exist.
Member
|
The changes to functionality look great to me now. I'll try to review the code tomorrow! |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
0 | 0 | 33 |
invalid-assignment |
0 | 0 | 16 |
invalid-return-type |
0 | 2 | 3 |
invalid-type-form |
0 | 0 | 4 |
possibly-missing-attribute |
0 | 0 | 1 |
unsupported-operator |
0 | 0 | 1 |
unused-ignore-comment |
1 | 0 | 0 |
| Total | 1 | 2 | 58 |
AlexWaygood
reviewed
Jan 13, 2026
| match self { | ||
| TypeMapping::ApplySpecialization(_) | ||
| | TypeMapping::UniqueSpecialization { .. } | ||
| TypeMapping::ApplySpecialization(specialization) => { |
Contributor
Author
|
Thanks a lot for the merge! Any followup issues you'd like me to look into? @AlexWaygood |
AlexWaygood
pushed a commit
that referenced
this pull request
Jan 14, 2026
Member
|
Don't think there's anything specifically related to this change, but definitely feel free to look over our issue tracker and see if any other open issues take your fancy 😃 |
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
2 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.
Fixes astral-sh/ty#1207
Summary
Modified
DisplaySignatureto show type parameter brackets (e.g.,[T]) when displaying generic function and method signatures. Added ageneric_contextfield and updatedfmt_detailed()to prepend type parameters, preventing duplication whenDisplayFunctionTypealready renders them.Test Plan
Updated expected output in 6 test files. All tests pass:
test result: ok. 307 passed; 0 failed; 0 ignoredExample