Skip to content

Conversation

@AlexWaygood
Copy link
Member

Summary

This is a short-term fix to some of our type-equivalence logic for function-literal and bound-method types. The issue was that two Callable types will not be considered equivalent if any parameter in either type is unannotated (and it is obviously very common in methods for the self parameter to be unannotated!). Following 97058e8, equivalence of two FunctionLiteral types defers to equivalence of the CallableType supertypes of the two function-literal types.

Long-term, we should clarify this situation by splitting FunctionLiteral into two separate variants, as described by @dcreager in astral-sh/ty#459 (comment) and astral-sh/ty#462

Closes astral-sh/ty#459

Test Plan

  • cargo test -p ty_python_semantic
  • QUICKCHECK_TESTS=100000 cargo test --release -p ty_python_semantic -- --ignored types::property_tests::stable

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label May 20, 2025
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this file are not actually necessary to fix the bug, just a driveby fix for something that I noticed while fixing the bug

@github-actions
Copy link
Contributor

github-actions bot commented May 20, 2025

mypy_primer results

No ecosystem changes detected ✅

Comment on lines 7167 to 7169
fn is_equivalent_to(self, db: &'db dyn Db, other: Self) -> bool {
self.body_scope(db) == other.body_scope(db)
&& self
.into_callable_type(db)
.is_equivalent_to(db, other.into_callable_type(db))
self.normalized(db) == other.normalized(db)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed in person — I think both is_subtype_of and is_equivalent_to should follow the pattern above, using the normalized check as a fast path, but falling back on the body scope + callable check that was there previously.

(I think a lot of this will hopefully be cleaned up as part of astral-sh/ty#462)

@AlexWaygood AlexWaygood requested a review from dcreager May 20, 2025 18:01
@AlexWaygood
Copy link
Member Author

AlexWaygood commented May 20, 2025

1% slowdown on the cold benchmark: https://codspeed.io/astral-sh/ruff/branches/alex%2Fproperty-test-failure.

I think that's acceptable here. It might well be addressed when we do astral-sh/ty#462.

@AlexWaygood AlexWaygood merged commit e8d4f6d into main May 20, 2025
35 checks passed
@AlexWaygood AlexWaygood deleted the alex/property-test-failure branch May 20, 2025 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Daily property test run failed on Tue May 20 2025

3 participants