Skip to content

Commit fa56c15

Browse files
authored
[ty] Fix bound method access on None (#23246)
## Summary Just a small quick win that should fix the last remaining conformance test in [`specialtypes_none.py`](https://shark.fish/typing-conformance-report/specialtypes_none.html). `types.FunctionType` has the following overloads for `__get__` to model accessing functions on a class vs. on an instance: ```py class FunctionType: @overload def __get__(self, instance: None, owner: type, /) -> FunctionType: """Return an attribute of instance, which is of type owner.""" @overload def __get__(self, instance: object, owner: type | None = None, /) -> MethodType: ... ``` The `instance: None` sentinel of the descriptor protocol is problematic when we're actually accessing something on an instance of type `None`. Here, we simply add a special case to fix this. Also, instead of using `Type::none(db)` as a sentinel in the Rust code, we now use `Optional<Type<…>>` such that we can use the Rust `None` if we want to call `__get__` on a non-instance. Fixes astral-sh/ty#737 ## Ecosystem Looks good. Some strange-looking new diagnostics because symbols were inferred as `None` where users did not intend that (but no wrong behavior of ty). ## Test Plan Adapted existing tests.
1 parent 4fd07d0 commit fa56c15

5 files changed

Lines changed: 35 additions & 40 deletions

File tree

crates/ty_python_semantic/resources/mdtest/call/methods.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,8 @@ def _(a: object, b: SupportsStr, c: Falsy, d: AlwaysFalsy, e: None, f: Foo | Non
229229
b.__str__()
230230
c.__str__()
231231
d.__str__()
232-
# TODO: these should not error
233-
e.__str__() # error: [missing-argument]
234-
f.__str__() # error: [missing-argument]
232+
e.__str__()
233+
f.__str__()
235234
```
236235

237236
## Error cases: Calling `__get__` for methods

crates/ty_python_semantic/src/place.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ impl<'db> Place<'db> {
267267

268268
Place::Defined(defined) => {
269269
if let Some((dunder_get_return_ty, _)) =
270-
defined.ty.try_call_dunder_get(db, Type::none(db), owner)
270+
defined.ty.try_call_dunder_get(db, None, owner)
271271
{
272272
Place::Defined(DefinedPlace {
273273
ty: dunder_get_return_ty,

crates/ty_python_semantic/src/types.rs

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2750,13 +2750,13 @@ impl<'db> Type<'db> {
27502750
pub(crate) fn try_call_dunder_get(
27512751
self,
27522752
db: &'db dyn Db,
2753-
instance: Type<'db>,
2753+
instance: Option<Type<'db>>,
27542754
owner: Type<'db>,
27552755
) -> Option<(Type<'db>, AttributeKind)> {
27562756
tracing::trace!(
27572757
"try_call_dunder_get: {}, {}, {}",
27582758
self.display(db),
2759-
instance.display(db),
2759+
instance.unwrap_or_else(|| Type::none(db)).display(db),
27602760
owner.display(db)
27612761
);
27622762
match self {
@@ -2776,22 +2776,36 @@ impl<'db> Type<'db> {
27762776
// method of these synthesized functions. The method-wrapper would then be returned from
27772777
// `find_name_in_mro` when called on function-like `Callable`s. This would allow us to
27782778
// correctly model the behavior of *explicit* `SomeDataclass.__init__.__get__` calls.
2779-
return if instance.is_none(db) && callable.is_function_like(db) {
2779+
return if instance.is_none() && callable.is_function_like(db) {
27802780
Some((self, AttributeKind::NormalOrNonDataDescriptor))
27812781
} else {
2782-
// For classmethod-like callables, bind to the owner class. For function-like callables, bind to the instance.
2783-
let self_type = if callable.is_classmethod_like(db) && instance.is_none(db) {
2782+
let self_type = instance.unwrap_or_else(|| {
2783+
// For classmethod-like callables, bind to the owner class.
27842784
owner.to_instance(db).unwrap_or(owner)
2785-
} else {
2786-
instance
2787-
};
2785+
});
27882786

27892787
Some((
27902788
Type::Callable(callable.bind_self(db, Some(self_type))),
27912789
AttributeKind::NormalOrNonDataDescriptor,
27922790
))
27932791
};
27942792
}
2793+
Type::FunctionLiteral(function)
2794+
if instance.is_some_and(|ty| ty.is_none(db))
2795+
&& !function.is_staticmethod(db)
2796+
&& !function.is_classmethod(db) =>
2797+
{
2798+
// When the instance is of type `None` (`NoneType`), we must handle
2799+
// `FunctionType.__get__` here rather than falling through to the generic
2800+
// `__get__` path. The stubs for `FunctionType.__get__` use an overload
2801+
// with `instance: None` to indicate class-level access (returning the
2802+
// unbound function). This incorrectly matches when the instance is actually
2803+
// an instance of `None`
2804+
return Some((
2805+
Type::BoundMethod(BoundMethodType::new(db, function, instance.unwrap())),
2806+
AttributeKind::NormalOrNonDataDescriptor,
2807+
));
2808+
}
27952809
_ => {}
27962810
}
27972811

@@ -2803,8 +2817,9 @@ impl<'db> Type<'db> {
28032817
..
28042818
}) = descr_get
28052819
{
2820+
let instance_ty = instance.unwrap_or_else(|| Type::none(db));
28062821
let return_ty = descr_get
2807-
.try_call(db, &CallArguments::positional([self, instance, owner]))
2822+
.try_call(db, &CallArguments::positional([self, instance_ty, owner]))
28082823
.map(|bindings| {
28092824
if descr_get_boundness == Definedness::AlwaysDefined {
28102825
bindings.return_type(db)
@@ -2834,7 +2849,7 @@ impl<'db> Type<'db> {
28342849
fn try_call_dunder_get_on_attribute(
28352850
db: &'db dyn Db,
28362851
attribute: PlaceAndQualifiers<'db>,
2837-
instance: Type<'db>,
2852+
instance: Option<Type<'db>>,
28382853
owner: Type<'db>,
28392854
) -> (PlaceAndQualifiers<'db>, AttributeKind) {
28402855
match attribute {
@@ -3023,7 +3038,7 @@ impl<'db> Type<'db> {
30233038
) = Self::try_call_dunder_get_on_attribute(
30243039
db,
30253040
self.class_member_with_policy(db, name.into(), member_policy),
3026-
self,
3041+
Some(self),
30273042
self.to_meta_type(db),
30283043
);
30293044

@@ -3498,13 +3513,8 @@ impl<'db> Type<'db> {
34983513
let class_attr_plain =
34993514
class_attr_plain.map_type(|ty| ty.bind_self_typevars(db, self_instance));
35003515

3501-
let class_attr_fallback = Self::try_call_dunder_get_on_attribute(
3502-
db,
3503-
class_attr_plain,
3504-
Type::none(db),
3505-
self,
3506-
)
3507-
.0;
3516+
let class_attr_fallback =
3517+
Self::try_call_dunder_get_on_attribute(db, class_attr_plain, None, self).0;
35083518

35093519
let result = self.invoke_descriptor_protocol(
35103520
db,

crates/ty_python_semantic/src/types/bound_super.rs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -691,37 +691,23 @@ impl<'db> BoundSuperType<'db> {
691691
// Also, invoking a descriptor on a dynamic attribute is meaningless, so we don't handle this.
692692
SuperOwnerKind::Dynamic(_) => None,
693693
SuperOwnerKind::Class(_) => Some(
694-
Type::try_call_dunder_get_on_attribute(
695-
db,
696-
attribute,
697-
Type::none(db),
698-
owner.owner_type(db),
699-
)
700-
.0,
694+
Type::try_call_dunder_get_on_attribute(db, attribute, None, owner.owner_type(db)).0,
701695
),
702696
SuperOwnerKind::Instance(_) | SuperOwnerKind::InstanceTypeVar(..) => {
703697
let owner_type = owner.owner_type(db);
704698
Some(
705699
Type::try_call_dunder_get_on_attribute(
706700
db,
707701
attribute,
708-
owner_type,
702+
Some(owner_type),
709703
owner_type.to_meta_type(db),
710704
)
711705
.0,
712706
)
713707
}
714708
SuperOwnerKind::ClassTypeVar(..) => {
715709
let owner_type = owner.owner_type(db);
716-
Some(
717-
Type::try_call_dunder_get_on_attribute(
718-
db,
719-
attribute,
720-
Type::none(db),
721-
owner_type,
722-
)
723-
.0,
724-
)
710+
Some(Type::try_call_dunder_get_on_attribute(db, attribute, None, owner_type).0)
725711
}
726712
}
727713
}

crates/ty_python_semantic/src/types/class.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3266,7 +3266,7 @@ impl<'db> StaticClassLiteral<'db> {
32663266

32673267
if let Some(ref mut default_ty) = default_ty {
32683268
*default_ty = default_ty
3269-
.try_call_dunder_get(db, Type::none(db), Type::from(self))
3269+
.try_call_dunder_get(db, None, Type::from(self))
32703270
.map(|(return_ty, _)| return_ty)
32713271
.unwrap_or_else(Type::unknown);
32723272
}

0 commit comments

Comments
 (0)