From e096ce7bc75c703f0a6be897d82308126ee98f40 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sat, 1 Nov 2025 18:30:03 +0900 Subject: [PATCH] Implement property.__name__ attribute (#6230) * Implement property.__name__ attribute Add getter and setter for the __name__ attribute on property objects. The getter returns the explicitly set name if available, otherwise falls back to the getter function's __name__. Raises AttributeError if no name is available, matching CPython 3.13 behavior. The implementation handles edge cases: - Returns None when explicitly set to None - Propagates non-AttributeError exceptions from getter's __getattr__ - Raises property-specific AttributeError when getter lacks __name__ This fix enables test_property_name in test_property.py to pass. * Refactor to use get_property_name in __name__ implementation Consolidate duplicate logic by making name_getter() use the existing get_property_name() helper method. This eliminates code duplication and improves maintainability. Changes: - Update get_property_name() to return PyResult> to properly handle and propagate non-AttributeError exceptions - Simplify name_getter() to delegate to get_property_name() - Update format_property_error() to handle the new return type This addresses review feedback about the relationship between get_property_name() and __name__ implementation. * style comment --- Lib/test/test_property.py | 1 - vm/src/builtins/property.rs | 45 ++++++++++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index 65153395b..49dc2331e 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -201,7 +201,6 @@ class PropertyTests(unittest.TestCase): self.assertIsNone(prop.fdel) self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_property_name(self): def getter(self): return 42 diff --git a/vm/src/builtins/property.rs b/vm/src/builtins/property.rs index 1568cb08d..a75357f76 100644 --- a/vm/src/builtins/property.rs +++ b/vm/src/builtins/property.rs @@ -67,20 +67,30 @@ impl GetDescriptor for PyProperty { #[pyclass(with(Constructor, Initializer, GetDescriptor), flags(BASETYPE))] impl PyProperty { // Helper method to get property name - fn get_property_name(&self, vm: &VirtualMachine) -> Option { + // Returns the name if available, None if not found, or propagates errors + fn get_property_name(&self, vm: &VirtualMachine) -> PyResult> { // First check if name was set via __set_name__ if let Some(name) = self.name.read().as_ref() { - return Some(name.clone()); + return Ok(Some(name.clone())); } - // Otherwise try to get __name__ from getter - if let Some(getter) = self.getter.read().as_ref() - && let Ok(name) = getter.get_attr("__name__", vm) - { - return Some(name); - } + let getter = self.getter.read(); + let Some(getter) = getter.as_ref() else { + return Ok(None); + }; - None + match getter.get_attr("__name__", vm) { + Ok(name) => Ok(Some(name)), + Err(e) => { + // If it's an AttributeError from the getter, return None + // Otherwise, propagate the original exception (e.g., RuntimeError) + if e.class().is(vm.ctx.exceptions.attribute_error) { + Ok(None) + } else { + Err(e) + } + } + } } // Descriptor methods @@ -143,6 +153,21 @@ impl PyProperty { self.deleter.read().clone() } + #[pygetset(name = "__name__")] + fn name_getter(&self, vm: &VirtualMachine) -> PyResult { + match self.get_property_name(vm)? { + Some(name) => Ok(name), + None => Err( + vm.new_attribute_error("'property' object has no attribute '__name__'".to_owned()) + ), + } + } + + #[pygetset(name = "__name__", setter)] + fn name_setter(&self, value: PyObjectRef) { + *self.name.write() = Some(value); + } + fn doc_getter(&self) -> Option { self.doc.read().clone() } @@ -288,7 +313,7 @@ impl PyProperty { error_type: &str, vm: &VirtualMachine, ) -> PyResult { - let prop_name = self.get_property_name(vm); + let prop_name = self.get_property_name(vm)?; let obj_type = obj.class(); let qualname = obj_type.__qualname__(vm);