Skip to content

Commit 64c4c96

Browse files
authored
[ty] make Type::BoundMethod include instances of same-named methods bound to a subclass (astral-sh#24039)
Fixes astral-sh/ty#2428.
1 parent 0cfec22 commit 64c4c96

5 files changed

Lines changed: 195 additions & 4 deletions

File tree

crates/ty_python_semantic/resources/mdtest/intersection_types.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,5 +1324,58 @@ def f(c: C):
13241324
reveal_type(c.x) # revealed: ~AlwaysFalsy
13251325
```
13261326

1327+
## Methods on intersections
1328+
1329+
### The same method from a common base
1330+
1331+
```py
1332+
from ty_extensions import Intersection
1333+
1334+
class Base:
1335+
def f(self) -> int:
1336+
return 0
1337+
1338+
class X(Base):
1339+
pass
1340+
1341+
class Y(Base):
1342+
pass
1343+
1344+
def test(x: Intersection[X, Y]) -> None:
1345+
reveal_type(x.f) # revealed: (bound method X.f() -> int) & (bound method Y.f() -> int)
1346+
reveal_type(x.f()) # revealed: int
1347+
1348+
# Example subclass that inhabits that intersection:
1349+
class Z(X, Y):
1350+
pass
1351+
1352+
test(Z())
1353+
```
1354+
1355+
### A method that could be compatible with a protocol (without violating Liskov)
1356+
1357+
```py
1358+
from typing import Protocol
1359+
from ty_extensions import Intersection
1360+
1361+
class Proto(Protocol):
1362+
def method(self) -> int: ...
1363+
1364+
class X:
1365+
# This `method` isn't compatible with `Proto`, but a subclass could refine it...
1366+
def method(self) -> int | str:
1367+
return "hello"
1368+
1369+
def test(x: Intersection[X, Proto]) -> None:
1370+
reveal_type(x.method()) # revealed: int
1371+
1372+
# ...like this. This subclass inhabits the intersection above.
1373+
class Y(X):
1374+
def method(self) -> int:
1375+
return 42
1376+
1377+
test(Y())
1378+
```
1379+
13271380
[complement laws]: https://en.wikipedia.org/wiki/Complement_(set_theory)
13281381
[de morgan's laws]: https://en.wikipedia.org/wiki/De_Morgan%27s_laws

crates/ty_python_semantic/resources/mdtest/type_properties/is_disjoint_from.md

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,88 @@ static_assert(not is_disjoint_from(TypeOf[f], FunctionType))
472472
static_assert(not is_disjoint_from(TypeOf[f], object))
473473
```
474474

475+
### Bound methods
476+
477+
```py
478+
from typing import final
479+
from ty_extensions import TypeOf, is_disjoint_from, static_assert
480+
481+
class A:
482+
def foo(self) -> None: ...
483+
def bar(self) -> None: ...
484+
485+
class B:
486+
def foo(self) -> None: ...
487+
488+
# Bound methods with different names are disjoint.
489+
static_assert(is_disjoint_from(TypeOf[A().foo], TypeOf[A().bar]))
490+
491+
# Bound methods with the same name but different self types are not disjoint, because a subclass
492+
# could inherit from both...
493+
static_assert(not is_disjoint_from(TypeOf[A().foo], TypeOf[B().foo]))
494+
495+
@final
496+
class F1:
497+
def foo(self) -> None: ...
498+
499+
@final
500+
class F2:
501+
def foo(self) -> None: ...
502+
503+
# ...unless one or both of those classes are `@final`.
504+
static_assert(is_disjoint_from(TypeOf[A().foo], TypeOf[F2().foo]))
505+
static_assert(is_disjoint_from(TypeOf[B().foo], TypeOf[F2().foo]))
506+
static_assert(is_disjoint_from(TypeOf[F1().foo], TypeOf[F2().foo]))
507+
```
508+
509+
### Bound `@final` methods
510+
511+
Two different `@final` methods are disjoint, even if they share the same name and neither class is
512+
`@final`, because no subclass could satisfy both without overriding one:
513+
514+
```py
515+
from typing import final
516+
from ty_extensions import TypeOf, is_disjoint_from, static_assert
517+
518+
class C:
519+
@final
520+
def foo(self) -> None: ...
521+
522+
class D:
523+
@final
524+
def foo(self) -> None: ...
525+
526+
static_assert(is_disjoint_from(TypeOf[C().foo], TypeOf[D().foo]))
527+
```
528+
529+
We do have to be careful not to get confused when the same `@final` method has different (but
530+
compatible) bound self types:
531+
532+
```py
533+
class E(C): ...
534+
535+
# `E.foo` and `C.foo` are the same method, so `E().foo` satisfies both `BoundMethod` types.
536+
static_assert(not is_disjoint_from(TypeOf[E().foo], TypeOf[C().foo]))
537+
static_assert(is_disjoint_from(TypeOf[E().foo], TypeOf[D().foo]))
538+
```
539+
540+
Also, we can't establish disjointness when only one of the methods is `@final`. Consider this tricky
541+
multiple inheritance case:
542+
543+
```py
544+
class F:
545+
def foo(self) -> None: ...
546+
547+
static_assert(not is_disjoint_from(TypeOf[C().foo], TypeOf[F().foo]))
548+
static_assert(not is_disjoint_from(TypeOf[D().foo], TypeOf[F().foo]))
549+
550+
class G(C, F): ...
551+
552+
static_assert(not is_disjoint_from(TypeOf[C().foo], TypeOf[G().foo]))
553+
static_assert(is_disjoint_from(TypeOf[D().foo], TypeOf[G().foo]))
554+
static_assert(not is_disjoint_from(TypeOf[F().foo], TypeOf[G().foo]))
555+
```
556+
475557
### `AlwaysTruthy` and `AlwaysFalsy`
476558

477559
```py

crates/ty_python_semantic/resources/mdtest/type_properties/is_single_valued.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ static_assert(not is_single_valued(Callable[[int, str], None]))
4747
class A:
4848
def method(self): ...
4949

50-
static_assert(is_single_valued(TypeOf[A().method]))
50+
# Binding the same method to different instances yields different objects: `[].sort != [].sort`
51+
static_assert(not is_single_valued(TypeOf[A().method]))
5152
static_assert(is_single_valued(TypeOf[types.FunctionType.__get__]))
5253
static_assert(is_single_valued(TypeOf[A.method.__get__]))
5354
```

crates/ty_python_semantic/src/types.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2085,7 +2085,6 @@ impl<'db> Type<'db> {
20852085
pub(crate) fn is_single_valued(self, db: &'db dyn Db) -> bool {
20862086
match self {
20872087
Type::FunctionLiteral(..)
2088-
| Type::BoundMethod(_)
20892088
| Type::WrapperDescriptor(_)
20902089
| Type::KnownBoundMethod(_)
20912090
| Type::ModuleLiteral(..)
@@ -2140,6 +2139,11 @@ impl<'db> Type<'db> {
21402139
false
21412140
}
21422141

2142+
Type::BoundMethod(_) => {
2143+
// Binding the same method to different instances yields different objects: `[].sort != [].sort`
2144+
false
2145+
}
2146+
21432147
Type::TypeIs(type_is) => type_is.is_bound(db),
21442148
Type::TypeGuard(type_guard) => type_guard.is_bound(db),
21452149

crates/ty_python_semantic/src/types/relation.rs

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::types::constraints::{
88
};
99
use crate::types::cyclic::PairVisitor;
1010
use crate::types::enums::is_single_member_enum;
11+
use crate::types::function::FunctionDecorators;
1112
use crate::types::set_theoretic::RecursivelyDefined;
1213
use crate::types::{
1314
CallableType, ClassBase, ClassType, CycleDetector, DynamicType, KnownBoundMethodType,
@@ -1877,15 +1878,13 @@ impl<'a, 'c, 'db> DisjointnessChecker<'a, 'c, 'db> {
18771878
(
18781879
// note `LiteralString` is not single-valued, but we handle the special case above
18791880
left @ (Type::FunctionLiteral(..)
1880-
| Type::BoundMethod(..)
18811881
| Type::KnownBoundMethod(..)
18821882
| Type::WrapperDescriptor(..)
18831883
| Type::ModuleLiteral(..)
18841884
| Type::ClassLiteral(..)
18851885
| Type::SpecialForm(..)
18861886
| Type::KnownInstance(..)),
18871887
right @ (Type::FunctionLiteral(..)
1888-
| Type::BoundMethod(..)
18891888
| Type::KnownBoundMethod(..)
18901889
| Type::WrapperDescriptor(..)
18911890
| Type::ModuleLiteral(..)
@@ -2197,6 +2196,58 @@ impl<'a, 'c, 'db> DisjointnessChecker<'a, 'c, 'db> {
21972196
.negate(db, self.constraints)
21982197
}
21992198

2199+
// A `BoundMethod` type includes instances of the same method bound to a
2200+
// subtype/subclass of the self type.
2201+
(Type::BoundMethod(a), Type::BoundMethod(b)) => {
2202+
if a.function(db).name(db) != b.function(db).name(db) {
2203+
// We typically ask about `BoundMethod` disjointness when we're looking at a
2204+
// method call on an intersection type like `A & B`. In that case, the same
2205+
// method name would show up on both sides of this check. However for
2206+
// completeness, if we're ever comparing `BoundMethod` types with different
2207+
// method names, then they're clearly disjoint.
2208+
self.always()
2209+
} else if a.function(db) != b.function(db)
2210+
&& a.function(db)
2211+
.has_known_decorator(db, FunctionDecorators::FINAL)
2212+
&& b.function(db)
2213+
.has_known_decorator(db, FunctionDecorators::FINAL)
2214+
{
2215+
// If *both* methods are `@final` (and they're not literally the same
2216+
// definition), they must be disjoint.
2217+
//
2218+
// Note that we can't establish disjointness when only one side is `@final`,
2219+
// because we have to worry about cases like this:
2220+
//
2221+
// ```
2222+
// class A:
2223+
// def f(self): ...
2224+
// class B:
2225+
// @final
2226+
// def f(self): ...
2227+
// # Valid in this order, though `C(A, B)` would be invalid.
2228+
// class C(B, A): ...
2229+
// ```
2230+
self.always()
2231+
} else {
2232+
// The names match, so `BoundMethod` disjointness depends on whether the bound
2233+
// self types are disjoint. Note that this can produce confusing results in the
2234+
// face of Liskov violations. For example:
2235+
// ```
2236+
// class A:
2237+
// def f(self) -> int: ...
2238+
// class B:
2239+
// def f(self) -> str: ...
2240+
// def _(x: Intersection[A, B]):
2241+
// x.f()
2242+
// ```
2243+
// `class C(A, B)` could inhabit that intersection, but `int` and `str` are
2244+
// disjoint, so the type of `x.f()` there is going to be inferred as `Never`.
2245+
// That's probably not correct in practice, but the right way to address it is
2246+
// to emit a diagnostic on the definition of `C.f`.
2247+
self.check_type_pair(db, a.self_instance(db), b.self_instance(db))
2248+
}
2249+
}
2250+
22002251
(Type::BoundMethod(_), other) | (other, Type::BoundMethod(_)) => {
22012252
self.check_type_pair(db, KnownClass::MethodType.to_instance(db), other)
22022253
}

0 commit comments

Comments
 (0)