Skip to content

Commit 6f2b772

Browse files
authored
[ty] Preserve nominal type of enum.property instances (#25849)
## Summary This is a follow-up to #25681 that preserves the nominal class of synthesized property instances created by `enum.property`. Previously, `PropertyInstanceType` always fell back to `builtins.property`. That discarded the `enum.property` identity, so assignments to `enum.property` failed and enum-specific members such as `name`, `clsname`, and `member` were unavailable. The originating known class is now retained through accessor replacement, type transformations, member lookup, descriptor dispatch, relations, `super`, and display. The regression coverage also checks precise getter and setter behavior. A TODO documents the remaining limitation that constructing a subclass of `enum.property` currently collapses the result to `enum.property`. ## Test Plan Added/updated mdtests.
1 parent be4777c commit 6f2b772

8 files changed

Lines changed: 126 additions & 42 deletions

File tree

crates/ty_ide/src/completion.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4668,8 +4668,8 @@ Answer.<CURSOR>
46684668
NO :: Literal[Answer.NO]
46694669
YES :: Literal[Answer.YES]
46704670
mro :: bound method <class 'Answer'>.mro() -> list[type]
4671-
name :: property
4672-
value :: property
4671+
name :: enum.property
4672+
value :: enum.property
46734673
__annotations__ :: dict[str, Any]
46744674
__base__ :: type | None
46754675
__bases__ :: tuple[type, ...]

crates/ty_python_semantic/resources/mdtest/enums.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,14 @@ class Answer(Enum):
725725
def some_property(self) -> str:
726726
return "property value"
727727

728+
@enum_property
729+
def settable_property(self) -> int:
730+
return 1
731+
732+
@settable_property.setter
733+
def settable_property(self, value: str) -> None:
734+
pass
735+
728736
def direct_property_getter(self) -> int:
729737
return 1
730738

@@ -734,6 +742,25 @@ class Answer(Enum):
734742
reveal_type(enum_members(Answer))
735743
assert_type(Answer.YES.some_property, str)
736744
assert_type(Answer.YES.direct_property, int)
745+
Answer.YES.some_property = "new value" # error: [invalid-assignment]
746+
Answer.YES.settable_property = "new value"
747+
assert_type(Answer.YES.settable_property, int)
748+
Answer.YES.settable_property = 1 # error: [invalid-assignment]
749+
750+
def get(value: Enum) -> str:
751+
return value.name
752+
753+
descriptor = enum_property(get)
754+
reveal_type(descriptor) # revealed: enum.property
755+
# revealed: <method-wrapper '__get__' of enum.property 'get'>
756+
reveal_type(descriptor.__get__)
757+
retained: enum_property = descriptor
758+
retained_as_property: property = descriptor
759+
not_enum_property: enum_property = property(get) # error: [invalid-assignment]
760+
retained_getter: enum_property = descriptor.getter(get)
761+
assert_type(descriptor.name, str)
762+
assert_type(descriptor.clsname, str)
763+
assert_type(descriptor.member, Enum | None)
737764
```
738765

739766
Enum attributes defined using `enum.property` take precedence over generated attributes.

crates/ty_python_semantic/src/types.rs

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -594,12 +594,13 @@ pub enum PropertyAccessorRole {
594594
Deleter,
595595
}
596596

597-
/// Represents an instance of `builtins.property`.
598-
#[salsa::interned(debug, heap_size=ruff_memory_usage::heap_size)]
597+
/// Represents an instance of `builtins.property` or `enum.property`.
598+
#[salsa::interned(debug, constructor=new_internal, heap_size=ruff_memory_usage::heap_size)]
599599
pub struct PropertyInstanceType<'db> {
600600
pub getter: Option<Type<'db>>,
601601
pub setter: Option<Type<'db>>,
602602
pub deleter: Option<Type<'db>>,
603+
instance_class: KnownClass,
603604
}
604605

605606
fn walk_property_instance_type<'db, V: visitor::TypeVisitor<'db> + ?Sized>(
@@ -622,6 +623,38 @@ fn walk_property_instance_type<'db, V: visitor::TypeVisitor<'db> + ?Sized>(
622623
impl get_size2::GetSize for PropertyInstanceType<'_> {}
623624

624625
impl<'db> PropertyInstanceType<'db> {
626+
pub fn new(
627+
db: &'db dyn Db,
628+
getter: Option<Type<'db>>,
629+
setter: Option<Type<'db>>,
630+
deleter: Option<Type<'db>>,
631+
) -> Self {
632+
Self::new_internal(db, getter, setter, deleter, KnownClass::Property)
633+
}
634+
635+
fn new_enum_property(
636+
db: &'db dyn Db,
637+
getter: Option<Type<'db>>,
638+
setter: Option<Type<'db>>,
639+
deleter: Option<Type<'db>>,
640+
) -> Self {
641+
Self::new_internal(db, getter, setter, deleter, KnownClass::EnumProperty)
642+
}
643+
644+
fn with_accessors(
645+
self,
646+
db: &'db dyn Db,
647+
getter: Option<Type<'db>>,
648+
setter: Option<Type<'db>>,
649+
deleter: Option<Type<'db>>,
650+
) -> Self {
651+
Self::new_internal(db, getter, setter, deleter, self.instance_class(db))
652+
}
653+
654+
fn instance_fallback(self, db: &'db dyn Db) -> Type<'db> {
655+
self.instance_class(db).to_instance(db)
656+
}
657+
625658
/// Returns the [`PropertyAccessorRole`] that `def` plays in this property, or `None` when
626659
/// `def` is not one of this property's accessors.
627660
///
@@ -669,7 +702,7 @@ impl<'db> PropertyInstanceType<'db> {
669702
let deleter = self
670703
.deleter(db)
671704
.map(|ty| ty.apply_type_mapping_impl(db, type_mapping, tcx, visitor));
672-
Self::new(db, getter, setter, deleter)
705+
self.with_accessors(db, getter, setter, deleter)
673706
}
674707

675708
fn recursive_type_normalized_impl(
@@ -702,7 +735,7 @@ impl<'db> PropertyInstanceType<'db> {
702735
),
703736
None => None,
704737
};
705-
Some(Self::new(db, getter, setter, deleter))
738+
Some(self.with_accessors(db, getter, setter, deleter))
706739
}
707740

708741
fn find_legacy_typevars_impl(
@@ -1369,6 +1402,7 @@ impl<'db> Type<'db> {
13691402
bound.nominal_class(db)
13701403
}
13711404
Type::LiteralValue(literal) => literal.fallback_instance(db).nominal_class(db),
1405+
Type::PropertyInstance(property) => property.instance_fallback(db).nominal_class(db),
13721406
_ => None,
13731407
}
13741408
}
@@ -2538,13 +2572,13 @@ impl<'db> Type<'db> {
25382572
// Hard code this knowledge, as we look up `__set__` and `__delete__` on `FunctionType` often.
25392573
Some(Place::Undefined.into())
25402574
}
2541-
(Some(KnownClass::Property), "__get__") => Some(
2575+
(Some(KnownClass::Property | KnownClass::EnumProperty), "__get__") => Some(
25422576
Place::bound(Type::WrapperDescriptor(
25432577
WrapperDescriptorKind::PropertyDunderGet,
25442578
))
25452579
.into(),
25462580
),
2547-
(Some(KnownClass::Property), "__set__") => Some(
2581+
(Some(KnownClass::Property | KnownClass::EnumProperty), "__set__") => Some(
25482582
Place::bound(Type::WrapperDescriptor(
25492583
WrapperDescriptorKind::PropertyDunderSet,
25502584
))
@@ -2887,9 +2921,9 @@ impl<'db> Type<'db> {
28872921

28882922
Type::SpecialForm(_) | Type::KnownInstance(_) => Place::Undefined.into(),
28892923

2890-
Type::PropertyInstance(_) => KnownClass::Property
2891-
.to_instance(db)
2892-
.instance_member(db, name),
2924+
Type::PropertyInstance(property) => {
2925+
property.instance_fallback(db).instance_member(db, name)
2926+
}
28932927

28942928
// Note: `super(pivot, owner).__dict__` refers to the `__dict__` of the `builtins.super` instance,
28952929
// not that of the owner.
@@ -5804,7 +5838,7 @@ impl<'db> Type<'db> {
58045838
Type::NominalInstance(instance) => instance.to_meta_type(db),
58055839
Type::KnownInstance(known_instance) => known_instance.to_meta_type(db),
58065840
Type::SpecialForm(special_form) => special_form.to_meta_type(db),
5807-
Type::PropertyInstance(_) => KnownClass::Property.to_class_literal(db),
5841+
Type::PropertyInstance(property) => property.instance_class(db).to_class_literal(db),
58085842
Type::Union(union) => union.map(db, |ty| ty.to_meta_type(db)),
58095843
Type::TypeIs(_) | Type::TypeGuard(_) => KnownClass::Bool.to_class_literal(db),
58105844
Type::TypeForm(_) => Type::object().to_meta_type(db),

crates/ty_python_semantic/src/types/bound_super.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,7 +814,9 @@ impl<'db> BoundSuperType<'db> {
814814
return delegate_to(KnownClass::ModuleType.to_instance(db));
815815
}
816816
Type::GenericAlias(_) => return delegate_to(KnownClass::GenericAlias.to_instance(db)),
817-
Type::PropertyInstance(_) => return delegate_to(KnownClass::Property.to_instance(db)),
817+
Type::PropertyInstance(property) => {
818+
return delegate_to(property.instance_fallback(db));
819+
}
818820
Type::BoundSuper(_) => return delegate_to(KnownClass::Super.to_instance(db)),
819821
Type::TypedDict(td) => {
820822
// In general it isn't sound to upcast a `TypedDict` to a `dict`,

crates/ty_python_semantic/src/types/call/bind.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1624,7 +1624,7 @@ impl<'db> Bindings<'db> {
16241624
let mut ty_property = bound_method.self_instance(db);
16251625
if let Type::PropertyInstance(property) = ty_property {
16261626
ty_property =
1627-
Type::PropertyInstance(PropertyInstanceType::new(
1627+
Type::PropertyInstance(property.with_accessors(
16281628
db,
16291629
property.getter(db),
16301630
Some(*setter),
@@ -1639,7 +1639,7 @@ impl<'db> Bindings<'db> {
16391639
let mut ty_property = bound_method.self_instance(db);
16401640
if let Type::PropertyInstance(property) = ty_property {
16411641
ty_property =
1642-
Type::PropertyInstance(PropertyInstanceType::new(
1642+
Type::PropertyInstance(property.with_accessors(
16431643
db,
16441644
Some(*getter),
16451645
property.setter(db),
@@ -1654,7 +1654,7 @@ impl<'db> Bindings<'db> {
16541654
let mut ty_property = bound_method.self_instance(db);
16551655
if let Type::PropertyInstance(property) = ty_property {
16561656
ty_property =
1657-
Type::PropertyInstance(PropertyInstanceType::new(
1657+
Type::PropertyInstance(property.with_accessors(
16581658
db,
16591659
property.getter(db),
16601660
property.setter(db),

crates/ty_python_semantic/src/types/call/bind/enum_property.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,17 @@ impl<'db> Bindings<'db> {
1515
) {
1616
let property_instance =
1717
|getter: Option<Type<'db>>, setter: Option<Type<'db>>, deleter: Option<Type<'db>>| {
18-
Type::PropertyInstance(PropertyInstanceType::new(
18+
Type::PropertyInstance(PropertyInstanceType::new_enum_property(
1919
db,
2020
getter.filter(|ty| !ty.is_none(db)),
2121
setter.filter(|ty| !ty.is_none(db)),
2222
deleter.filter(|ty| !ty.is_none(db)),
2323
))
2424
};
2525

26+
// TODO: Preserve subclasses of `enum.property`. `PropertyInstanceType` currently records
27+
// only a known property class, so this rewrite collapses subclass instances to
28+
// `enum.property`.
2629
for constructor in self.iter_constructor_items_mut() {
2730
if !constructor
2831
.constructed_instance_type()

crates/ty_python_semantic/src/types/display.rs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ use crate::types::typevar::BoundTypeVarIdentity;
3232
use crate::types::visitor::TypeVisitor;
3333
use crate::types::{
3434
CallableType, IntersectionType, KnownBoundMethodType, KnownClass, KnownInstanceType,
35-
LiteralValueType, LiteralValueTypeKind, MaterializationKind, Protocol, ProtocolInstanceType,
36-
SpecialFormType, StringLiteralType, SubclassOfInner, SubclassOfType, Type, TypeAliasType,
37-
TypeGuardLike, TypedDictType, UnionType, WrapperDescriptorKind, visitor,
35+
LiteralValueType, LiteralValueTypeKind, MaterializationKind, PropertyInstanceType, Protocol,
36+
ProtocolInstanceType, SpecialFormType, StringLiteralType, SubclassOfInner, SubclassOfType,
37+
Type, TypeAliasType, TypeGuardLike, TypedDictType, UnionType, WrapperDescriptorKind, visitor,
3838
};
3939
use ty_python_core::definition::Definition;
4040
use ty_python_core::scope::{FileScopeId, ScopeKind};
@@ -932,6 +932,14 @@ struct DisplayRepresentation<'db> {
932932
settings: DisplaySettings<'db>,
933933
}
934934

935+
fn property_display_name(db: &dyn Db, property: PropertyInstanceType<'_>) -> &'static str {
936+
if property.instance_class(db) == KnownClass::EnumProperty {
937+
"enum.property"
938+
} else {
939+
"property"
940+
}
941+
}
942+
935943
impl Display for DisplayRepresentation<'_> {
936944
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
937945
self.fmt_detailed(&mut TypeWriter::Formatter(f))
@@ -995,7 +1003,9 @@ impl<'db> FmtDetailed<'db> for DisplayRepresentation<'db> {
9951003
f.write_char('>')
9961004
}
9971005
},
998-
Type::PropertyInstance(_) => f.with_type(self.ty).write_str("property"),
1006+
Type::PropertyInstance(property) => f
1007+
.with_type(self.ty)
1008+
.write_str(property_display_name(self.db, property)),
9991009
Type::ModuleLiteral(module) => {
10001010
f.set_invalid_type_annotation();
10011011
f.write_char('<')?;
@@ -1146,29 +1156,29 @@ impl<'db> FmtDetailed<'db> for DisplayRepresentation<'db> {
11461156
Some(&**function.name(self.db)),
11471157
),
11481158
KnownBoundMethodType::PropertyDunderGet(property) => (
1149-
KnownClass::Property,
1159+
property.instance_class(self.db),
11501160
"__get__",
1151-
"property",
1161+
property_display_name(self.db, property),
11521162
Type::PropertyInstance(property),
11531163
property
11541164
.getter(self.db)
11551165
.and_then(Type::as_function_literal)
11561166
.map(|getter| &**getter.name(self.db)),
11571167
),
11581168
KnownBoundMethodType::PropertyDunderSet(property) => (
1159-
KnownClass::Property,
1169+
property.instance_class(self.db),
11601170
"__set__",
1161-
"property",
1171+
property_display_name(self.db, property),
11621172
Type::PropertyInstance(property),
11631173
property
11641174
.setter(self.db)
11651175
.and_then(Type::as_function_literal)
11661176
.map(|setter| &**setter.name(self.db)),
11671177
),
11681178
KnownBoundMethodType::PropertyDunderDelete(property) => (
1169-
KnownClass::Property,
1179+
property.instance_class(self.db),
11701180
"__delete__",
1171-
"property",
1181+
property_display_name(self.db, property),
11721182
Type::PropertyInstance(property),
11731183
property
11741184
.deleter(self.db)

crates/ty_python_semantic/src/types/relation.rs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2174,11 +2174,11 @@ impl<'a, 'c, 'db> TypeRelationChecker<'a, 'c, 'db> {
21742174
self.check_property_instance_pair(db, source_p, target_p)
21752175
}),
21762176

2177-
(Type::PropertyInstance(_), _) => {
2178-
self.check_type_pair(db, KnownClass::Property.to_instance(db), target)
2177+
(Type::PropertyInstance(property), _) => {
2178+
self.check_type_pair(db, property.instance_fallback(db), target)
21792179
}
2180-
(_, Type::PropertyInstance(_)) => {
2181-
self.check_type_pair(db, source, KnownClass::Property.to_instance(db))
2180+
(_, Type::PropertyInstance(property)) => {
2181+
self.check_type_pair(db, source, property.instance_fallback(db))
21822182
}
21832183
// Other than the special cases enumerated above, nominal-instance types are never
21842184
// subtypes of any other variants
@@ -2198,17 +2198,24 @@ impl<'a, 'c, 'db> TypeRelationChecker<'a, 'c, 'db> {
21982198
(None | Some(_), None | Some(_)) => self.never(),
21992199
};
22002200

2201-
check_optional_methods(source.getter(db), target.getter(db)).and(
2201+
self.check_type_pair(
22022202
db,
2203-
self.constraints,
2204-
|| {
2205-
check_optional_methods(source.setter(db), target.setter(db)).and(
2206-
db,
2207-
self.constraints,
2208-
|| check_optional_methods(source.deleter(db), target.deleter(db)),
2209-
)
2210-
},
2203+
source.instance_fallback(db),
2204+
target.instance_fallback(db),
22112205
)
2206+
.and(db, self.constraints, || {
2207+
check_optional_methods(source.getter(db), target.getter(db)).and(
2208+
db,
2209+
self.constraints,
2210+
|| {
2211+
check_optional_methods(source.setter(db), target.setter(db)).and(
2212+
db,
2213+
self.constraints,
2214+
|| check_optional_methods(source.deleter(db), target.deleter(db)),
2215+
)
2216+
},
2217+
)
2218+
})
22122219
}
22132220

22142221
pub(super) fn as_equivalence_checker(&self) -> EquivalenceChecker<'_, 'c, 'db> {
@@ -3124,8 +3131,9 @@ impl<'a, 'c, 'db> DisjointnessChecker<'a, 'c, 'db> {
31243131
self.check_type_pair(db, newtype.concrete_base_type(db), other)
31253132
}
31263133

3127-
(Type::PropertyInstance(_), other) | (other, Type::PropertyInstance(_)) => {
3128-
self.check_type_pair(db, KnownClass::Property.to_instance(db), other)
3134+
(Type::PropertyInstance(property), other)
3135+
| (other, Type::PropertyInstance(property)) => {
3136+
self.check_type_pair(db, property.instance_fallback(db), other)
31293137
}
31303138

31313139
(Type::BoundSuper(left), Type::BoundSuper(right)) => self

0 commit comments

Comments
 (0)