From 18ad2848e3bb88ef7140b2e51de36312aa5981ab Mon Sep 17 00:00:00 2001 From: Matthew Mckee Date: Wed, 6 Aug 2025 00:35:08 +0100 Subject: [PATCH] Display generic function signature properly (#19544) ## Summary Resolves https://github.com/astral-sh/ty/issues/817 ## Test Plan Update mdtest --------- Co-authored-by: Carl Meyer --- crates/ty_ide/src/completion.rs | 14 ++-- .../resources/mdtest/generics/scoping.md | 2 +- .../resources/mdtest/scopes/builtin.md | 2 +- ..._Cover_non-keyword_re…_(707b284610419a54).snap | 18 ++--- crates/ty_python_semantic/src/types.rs | 15 ++++- .../ty_python_semantic/src/types/display.rs | 66 +++++++++++++++---- .../ty_python_semantic/src/types/generics.rs | 7 +- 7 files changed, 87 insertions(+), 37 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 0c7453a892..a3bae6793f 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -1358,7 +1358,7 @@ C. __sizeof__ :: def __sizeof__(self) -> int __str__ :: def __str__(self) -> str __subclasscheck__ :: bound method .__subclasscheck__(subclass: type, /) -> bool - __subclasses__ :: bound method .__subclasses__() -> list[Self] + __subclasses__ :: bound method .__subclasses__[Self]() -> list[Self] __subclasshook__ :: bound method .__subclasshook__(subclass: type, /) -> bool __text_signature__ :: str | None __type_params__ :: tuple[TypeVar | ParamSpec | TypeVarTuple, ...] @@ -1426,7 +1426,7 @@ Meta. __sizeof__ :: def __sizeof__(self) -> int __str__ :: def __str__(self) -> str __subclasscheck__ :: def __subclasscheck__(self, subclass: type, /) -> bool - __subclasses__ :: def __subclasses__(self: Self) -> list[Self] + __subclasses__ :: def __subclasses__[Self](self: Self) -> list[Self] __subclasshook__ :: bound method .__subclasshook__(subclass: type, /) -> bool __text_signature__ :: str | None __type_params__ :: tuple[TypeVar | ParamSpec | TypeVarTuple, ...] @@ -1534,7 +1534,7 @@ Quux. __sizeof__ :: def __sizeof__(self) -> int __str__ :: def __str__(self) -> str __subclasscheck__ :: bound method .__subclasscheck__(subclass: type, /) -> bool - __subclasses__ :: bound method .__subclasses__() -> list[Self] + __subclasses__ :: bound method .__subclasses__[Self]() -> list[Self] __subclasshook__ :: bound method .__subclasshook__(subclass: type, /) -> bool __text_signature__ :: str | None __type_params__ :: tuple[TypeVar | ParamSpec | TypeVarTuple, ...] @@ -1585,14 +1585,14 @@ Answer. __flags__ :: int __format__ :: def __format__(self, format_spec: str) -> str __getattribute__ :: def __getattribute__(self, name: str, /) -> Any - __getitem__ :: bound method .__getitem__(name: str) -> _EnumMemberT@__getitem__ + __getitem__ :: bound method .__getitem__[_EnumMemberT](name: str) -> _EnumMemberT@__getitem__ __getstate__ :: def __getstate__(self) -> object __hash__ :: def __hash__(self) -> int __init__ :: def __init__(self) -> None __init_subclass__ :: def __init_subclass__(cls) -> None __instancecheck__ :: bound method .__instancecheck__(instance: Any, /) -> bool __itemsize__ :: int - __iter__ :: bound method .__iter__() -> Iterator[_EnumMemberT@__iter__] + __iter__ :: bound method .__iter__[_EnumMemberT]() -> Iterator[_EnumMemberT@__iter__] __len__ :: bound method .__len__() -> int __members__ :: MappingProxyType[str, Unknown] __module__ :: str @@ -1606,14 +1606,14 @@ Answer. __qualname__ :: str __reduce__ :: def __reduce__(self) -> str | tuple[Any, ...] __repr__ :: def __repr__(self) -> str - __reversed__ :: bound method .__reversed__() -> Iterator[_EnumMemberT@__reversed__] + __reversed__ :: bound method .__reversed__[_EnumMemberT]() -> Iterator[_EnumMemberT@__reversed__] __ror__ :: bound method .__ror__(value: Any, /) -> UnionType __setattr__ :: def __setattr__(self, name: str, value: Any, /) -> None __signature__ :: bound method .__signature__() -> str __sizeof__ :: def __sizeof__(self) -> int __str__ :: def __str__(self) -> str __subclasscheck__ :: bound method .__subclasscheck__(subclass: type, /) -> bool - __subclasses__ :: bound method .__subclasses__() -> list[Self] + __subclasses__ :: bound method .__subclasses__[Self]() -> list[Self] __subclasshook__ :: bound method .__subclasshook__(subclass: type, /) -> bool __text_signature__ :: str | None __type_params__ :: tuple[TypeVar | ParamSpec | TypeVarTuple, ...] diff --git a/crates/ty_python_semantic/resources/mdtest/generics/scoping.md b/crates/ty_python_semantic/resources/mdtest/generics/scoping.md index c625db415b..02142d006b 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/scoping.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/scoping.md @@ -152,7 +152,7 @@ already solved and specialized when the class was specialized: from ty_extensions import generic_context legacy.m("string", None) # error: [invalid-argument-type] -reveal_type(legacy.m) # revealed: bound method Legacy[int].m(x: int, y: S@m) -> S@m +reveal_type(legacy.m) # revealed: bound method Legacy[int].m[S](x: int, y: S@m) -> S@m reveal_type(generic_context(Legacy)) # revealed: tuple[T@Legacy] reveal_type(generic_context(legacy.m)) # revealed: tuple[S@m] ``` diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/builtin.md b/crates/ty_python_semantic/resources/mdtest/scopes/builtin.md index 5da5d9f357..ae684a22d9 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/builtin.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/builtin.md @@ -11,7 +11,7 @@ def _(flag: bool) -> None: abs = 1 chr: int = 1 - reveal_type(abs) # revealed: Literal[1] | (def abs(x: SupportsAbs[_T@abs], /) -> _T@abs) + reveal_type(abs) # revealed: Literal[1] | (def abs[_T](x: SupportsAbs[_T@abs], /) -> _T@abs) reveal_type(chr) # revealed: Literal[1] | (def chr(i: SupportsIndex, /) -> str) ``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/union_call.md_-_Calling_a_union_of_f…_-_Try_to_cover_all_pos…_-_Cover_non-keyword_re…_(707b284610419a54).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/union_call.md_-_Calling_a_union_of_f…_-_Try_to_cover_all_pos…_-_Cover_non-keyword_re…_(707b284610419a54).snap index 05b560e021..f3f4563dc5 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/union_call.md_-_Calling_a_union_of_f…_-_Try_to_cover_all_pos…_-_Cover_non-keyword_re…_(707b284610419a54).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/union_call.md_-_Calling_a_union_of_f…_-_Try_to_cover_all_pos…_-_Cover_non-keyword_re…_(707b284610419a54).snap @@ -86,7 +86,7 @@ error[call-non-callable]: Object of type `Literal[5]` is not callable | ^^^^ | info: Union variant `Literal[5]` is incompatible with this call site -info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4(x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` +info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4[T: str](x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` info: rule `call-non-callable` is enabled by default ``` @@ -101,7 +101,7 @@ error[call-non-callable]: Object of type `PossiblyNotCallable` is not callable ( | ^^^^ | info: Union variant `PossiblyNotCallable` is incompatible with this call site -info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4(x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` +info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4[T: str](x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` info: rule `call-non-callable` is enabled by default ``` @@ -116,7 +116,7 @@ error[missing-argument]: No argument provided for required parameter `b` of func | ^^^^ | info: Union variant `def f3(a: int, b: int) -> int` is incompatible with this call site -info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4(x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` +info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4[T: str](x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` info: rule `missing-argument` is enabled by default ``` @@ -152,7 +152,7 @@ info: Overload implementation defined here 28 | return x + y if x and y else None | info: Union variant `Overload[() -> None, (x: str, y: str) -> str]` is incompatible with this call site -info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4(x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` +info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4[T: str](x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` info: rule `no-matching-overload` is enabled by default ``` @@ -176,7 +176,7 @@ info: Function defined here 8 | return 0 | info: Union variant `def f2(name: str) -> int` is incompatible with this call site -info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4(x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` +info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4[T: str](x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` info: rule `invalid-argument-type` is enabled by default ``` @@ -199,8 +199,8 @@ info: Type variable defined here | ^^^^^^ 14 | return 0 | -info: Union variant `def f4(x: T@f4) -> int` is incompatible with this call site -info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4(x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` +info: Union variant `def f4[T: str](x: T@f4) -> int` is incompatible with this call site +info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4[T: str](x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` info: rule `invalid-argument-type` is enabled by default ``` @@ -227,7 +227,7 @@ info: Matching overload defined here info: Non-matching overloads for function `f5`: info: () -> None info: Union variant `Overload[() -> None, (x: str) -> str]` is incompatible with this call site -info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4(x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` +info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4[T: str](x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` info: rule `invalid-argument-type` is enabled by default ``` @@ -242,7 +242,7 @@ error[too-many-positional-arguments]: Too many positional arguments to function | ^ | info: Union variant `def f1() -> int` is incompatible with this call site -info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4(x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` +info: Attempted to call union type `(def f1() -> int) | (def f2(name: str) -> int) | (def f3(a: int, b: int) -> int) | (def f4[T: str](x: T@f4) -> int) | Literal[5] | (Overload[() -> None, (x: str) -> str]) | (Overload[() -> None, (x: str, y: str) -> str]) | PossiblyNotCallable` info: rule `too-many-positional-arguments` is enabled by default ``` diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 52b54ce936..70f8cf9cb8 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -5402,7 +5402,7 @@ impl<'db> Type<'db> { Some(TypeVarBoundOrConstraints::UpperBound(instance)), TypeVarVariance::Invariant, None, - TypeVarKind::Legacy, + TypeVarKind::Implicit, ))) } SpecialFormType::TypeAlias => Ok(Type::Dynamic(DynamicType::TodoTypeAlias)), @@ -5820,7 +5820,7 @@ impl<'db> Type<'db> { ) { match self { Type::TypeVar(typevar) => { - if typevar.is_legacy(db) { + if typevar.is_legacy(db) || typevar.is_implicit(db) { typevars.insert(typevar); } } @@ -6694,11 +6694,16 @@ impl<'db> FieldInstance<'db> { } } -/// Whether this typecar was created via the legacy `TypeVar` constructor, or using PEP 695 syntax. +/// Whether this typevar was created via the legacy `TypeVar` constructor, using PEP 695 syntax, +/// or an implicit typevar like `Self` was used. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub enum TypeVarKind { + /// `T = TypeVar("T")` Legacy, + /// `def foo[T](x: T) -> T: ...` Pep695, + /// `typing.Self` + Implicit, } /// Data regarding a single type variable. @@ -6803,6 +6808,10 @@ impl<'db> TypeVarInstance<'db> { matches!(self.kind(db), TypeVarKind::Legacy) } + pub(crate) fn is_implicit(self, db: &'db dyn Db) -> bool { + matches!(self.kind(db), TypeVarKind::Implicit) + } + pub(crate) fn upper_bound(self, db: &'db dyn Db) -> Option> { if let Some(TypeVarBoundOrConstraints::UpperBound(ty)) = self.bound_or_constraints(db) { Some(ty) diff --git a/crates/ty_python_semantic/src/types/display.rs b/crates/ty_python_semantic/src/types/display.rs index 3e325fcca9..b658d17baf 100644 --- a/crates/ty_python_semantic/src/types/display.rs +++ b/crates/ty_python_semantic/src/types/display.rs @@ -124,16 +124,19 @@ impl Display for DisplayRepresentation<'_> { Type::BoundMethod(bound_method) => { let function = bound_method.function(self.db); - // TODO: use the specialization from the method. Similar to the comment above - // about the function specialization, - match function.signature(self.db).overloads.as_slice() { [signature] => { + let type_parameters = DisplayOptionalGenericContext { + generic_context: signature.generic_context.as_ref(), + db: self.db, + }; + write!( f, - "bound method {instance}.{method}{signature}", + "bound method {instance}.{method}{type_parameters}{signature}", method = function.name(self.db), instance = bound_method.self_instance(self.db).display(self.db), + type_parameters = type_parameters, signature = signature.bind_self().display(self.db) ) } @@ -318,10 +321,16 @@ pub(crate) struct DisplayOverloadLiteral<'db> { impl Display for DisplayOverloadLiteral<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { let signature = self.literal.signature(self.db, None); + let type_parameters = DisplayOptionalGenericContext { + generic_context: signature.generic_context.as_ref(), + db: self.db, + }; + write!( f, - "def {name}{signature}", + "def {name}{type_parameters}{signature}", name = self.literal.name(self.db), + type_parameters = type_parameters, signature = signature.display(self.db) ) } @@ -342,15 +351,18 @@ impl Display for DisplayFunctionType<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { let signature = self.ty.signature(self.db); - // TODO: We should consider adding the type parameters to the signature of a generic - // function, i.e. `def foo[T](x: T) -> T`. - match signature.overloads.as_slice() { [signature] => { + let type_parameters = DisplayOptionalGenericContext { + generic_context: signature.generic_context.as_ref(), + db: self.db, + }; + write!( f, - "def {name}{signature}", + "def {name}{type_parameters}{signature}", name = self.ty.name(self.db), + type_parameters = type_parameters, signature = signature.display(self.db) ) } @@ -404,21 +416,51 @@ impl Display for DisplayGenericAlias<'_> { impl<'db> GenericContext<'db> { pub fn display(&'db self, db: &'db dyn Db) -> DisplayGenericContext<'db> { DisplayGenericContext { - typevars: self.variables(db), + generic_context: self, db, } } } +struct DisplayOptionalGenericContext<'db> { + generic_context: Option<&'db GenericContext<'db>>, + db: &'db dyn Db, +} + +impl Display for DisplayOptionalGenericContext<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + if let Some(generic_context) = self.generic_context { + DisplayGenericContext { + generic_context, + db: self.db, + } + .fmt(f) + } else { + Ok(()) + } + } +} + pub struct DisplayGenericContext<'db> { - typevars: &'db FxOrderSet>, + generic_context: &'db GenericContext<'db>, db: &'db dyn Db, } impl Display for DisplayGenericContext<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let variables = self.generic_context.variables(self.db); + + let non_implicit_variables: Vec<_> = variables + .iter() + .filter(|var| !var.is_implicit(self.db)) + .collect(); + + if non_implicit_variables.is_empty() { + return Ok(()); + } + f.write_char('[')?; - for (idx, var) in self.typevars.iter().enumerate() { + for (idx, var) in non_implicit_variables.iter().enumerate() { if idx > 0 { f.write_str(", ")?; } diff --git a/crates/ty_python_semantic/src/types/generics.rs b/crates/ty_python_semantic/src/types/generics.rs index 4e78c4a1cd..501a59d432 100644 --- a/crates/ty_python_semantic/src/types/generics.rs +++ b/crates/ty_python_semantic/src/types/generics.rs @@ -14,7 +14,7 @@ use crate::types::signatures::{Parameter, Parameters, Signature}; use crate::types::tuple::{TupleSpec, TupleType}; use crate::types::{ KnownInstanceType, Type, TypeMapping, TypeRelation, TypeTransformer, TypeVarBoundOrConstraints, - TypeVarInstance, TypeVarKind, TypeVarVariance, UnionType, binding_type, declaration_type, + TypeVarInstance, TypeVarVariance, UnionType, binding_type, declaration_type, }; use crate::{Db, FxOrderSet}; @@ -259,13 +259,12 @@ impl<'db> GenericContext<'db> { db: &'db dyn Db, typevar: TypeVarInstance<'db>, ) -> Option> { - assert!(typevar.kind(db) == TypeVarKind::Legacy); + assert!(typevar.is_legacy(db)); let typevar_def = typevar.definition(db); self.variables(db) .iter() .find(|self_typevar| { - self_typevar.kind(db) == TypeVarKind::Legacy - && self_typevar.definition(db) == typevar_def + self_typevar.is_legacy(db) && self_typevar.definition(db) == typevar_def }) .copied() }