Skip to content

Commit 365f521

Browse files
authored
[ty] Fix incorrect docstring in call signature completion (#20021)
## Summary This PR fixes astral-sh/ty#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 #19615 ## Test Plan https://github.com/user-attachments/assets/248d1cb1-12fd-4441-adab-b7e0866d23eb
1 parent fc5321e commit 365f521

File tree

1 file changed

+1
-23
lines changed

1 file changed

+1
-23
lines changed

crates/ty_python_semantic/src/types/signatures.rs

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ impl<'db> VarianceInferable<'db> for &CallableSignature<'db> {
247247
}
248248

249249
/// The signature of one of the overloads of a callable.
250-
#[derive(Clone, Debug, salsa::Update, get_size2::GetSize)]
250+
#[derive(Clone, Debug, salsa::Update, get_size2::GetSize, PartialEq, Eq, Hash)]
251251
pub struct Signature<'db> {
252252
/// The generic context for this overload, if it is generic.
253253
pub(crate) generic_context: Option<GenericContext<'db>>,
@@ -993,28 +993,6 @@ impl<'db> Signature<'db> {
993993
}
994994
}
995995

996-
// Manual implementations of PartialEq, Eq, and Hash that exclude the definition field
997-
// since the definition is not relevant for type equality/equivalence
998-
impl PartialEq for Signature<'_> {
999-
fn eq(&self, other: &Self) -> bool {
1000-
self.generic_context == other.generic_context
1001-
&& self.inherited_generic_context == other.inherited_generic_context
1002-
&& self.parameters == other.parameters
1003-
&& self.return_ty == other.return_ty
1004-
}
1005-
}
1006-
1007-
impl Eq for Signature<'_> {}
1008-
1009-
impl std::hash::Hash for Signature<'_> {
1010-
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
1011-
self.generic_context.hash(state);
1012-
self.inherited_generic_context.hash(state);
1013-
self.parameters.hash(state);
1014-
self.return_ty.hash(state);
1015-
}
1016-
}
1017-
1018996
impl<'db> VarianceInferable<'db> for &Signature<'db> {
1019997
fn variance_of(self, db: &'db dyn Db, typevar: BoundTypeVarInstance<'db>) -> TypeVarVariance {
1020998
tracing::debug!(

0 commit comments

Comments
 (0)