From 365f521c370413bf589f39e105f037101ccaf3e5 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 21 Aug 2025 22:36:40 +0200 Subject: [PATCH] [ty] Fix incorrect docstring in call signature completion (#20021) ## Summary This PR fixes https://github.com/astral-sh/ty/issues/1071 The core issue is that `CallableType` is a salsa interned but `Signature` (which `CallableType` stores) ignores the `Definition` in its `Eq` and `Hash` implementation. This PR tries to simplest fix by removing the custom `Eq` and `Hash` implementation. The main downside of this fix is that it can increase memory usage because `CallableType`s that are equal except for their `Definition` are now interned separately. The alternative is to remove `Definition` from `CallableType` and instead, call `bindings` directly on the callee (call_expression.func). However, this would require addressing the TODO here https://github.com/astral-sh/ruff/blob/39ee71c2a57bd6c51482430ba8bfe3728b4ca443/crates/ty_python_semantic/src/types.rs#L4582-L4586 This might probably be worth addressing anyway, but is the more involved fix. That's why I opted for removing the custom `Eq` implementation. We already "ignore" the definition during normalization, thank's to Alex's work in https://github.com/astral-sh/ruff/pull/19615 ## Test Plan https://github.com/user-attachments/assets/248d1cb1-12fd-4441-adab-b7e0866d23eb --- .../src/types/signatures.rs | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index 478b75b057..061c9b513d 100644 --- a/crates/ty_python_semantic/src/types/signatures.rs +++ b/crates/ty_python_semantic/src/types/signatures.rs @@ -247,7 +247,7 @@ impl<'db> VarianceInferable<'db> for &CallableSignature<'db> { } /// The signature of one of the overloads of a callable. -#[derive(Clone, Debug, salsa::Update, get_size2::GetSize)] +#[derive(Clone, Debug, salsa::Update, get_size2::GetSize, PartialEq, Eq, Hash)] pub struct Signature<'db> { /// The generic context for this overload, if it is generic. pub(crate) generic_context: Option>, @@ -993,28 +993,6 @@ impl<'db> Signature<'db> { } } -// Manual implementations of PartialEq, Eq, and Hash that exclude the definition field -// since the definition is not relevant for type equality/equivalence -impl PartialEq for Signature<'_> { - fn eq(&self, other: &Self) -> bool { - self.generic_context == other.generic_context - && self.inherited_generic_context == other.inherited_generic_context - && self.parameters == other.parameters - && self.return_ty == other.return_ty - } -} - -impl Eq for Signature<'_> {} - -impl std::hash::Hash for Signature<'_> { - fn hash(&self, state: &mut H) { - self.generic_context.hash(state); - self.inherited_generic_context.hash(state); - self.parameters.hash(state); - self.return_ty.hash(state); - } -} - impl<'db> VarianceInferable<'db> for &Signature<'db> { fn variance_of(self, db: &'db dyn Db, typevar: BoundTypeVarInstance<'db>) -> TypeVarVariance { tracing::debug!(