From 475794bf8e5f31281fca3f9ae9b6a2a1c97a7a46 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Fri, 27 Sep 2024 19:56:36 +0200 Subject: [PATCH 01/17] Improve zero argument support for `super()` in dataclasses --- Lib/dataclasses.py | 67 +++++++++++++++---- Lib/test/test_dataclasses/__init__.py | 41 ++++++++++++ ...4-09-27-19-50-30.gh-issue-90562.HeL_JA.rst | 2 + 3 files changed, 97 insertions(+), 13 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-09-27-19-50-30.gh-issue-90562.HeL_JA.rst diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 5ec91cddaa3e10..214376d42f0a79 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1289,11 +1289,6 @@ def _get_slots(cls): def _update_func_cell_for__class__(f, oldcls, newcls): - # Returns True if we update a cell, else False. - if f is None: - # f will be None in the case of a property where not all of - # fget, fset, and fdel are used. Nothing to do in that case. - return False try: idx = f.__code__.co_freevars.index("__class__") except ValueError: @@ -1317,6 +1312,31 @@ def _update_func_cell_for__class__(f, oldcls, newcls): return False +def _find_inner_functions(obj, _seen=None, _depth=0): + # inspect is imported lazily (see gh-137855). + import inspect + if _seen is None: + _seen = set() + if id(obj) in _seen: + return None + _seen.add(id(obj)) + + _depth += 1 + if _depth > 2: + return None + + obj = inspect.unwrap(obj) + + for attr in dir(obj): + value = getattr(obj, attr, None) + if value is None: + continue + if isinstance(obj, types.FunctionType): + yield obj + return + yield from _find_inner_functions(value, _seen, _depth) + + def _create_slots(defined_fields, inherited_slots, field_names, weakref_slot): # The slots for our class. Remove slots from our base classes. Add # '__weakref__' if weakref_slot was given, unless it is already present. @@ -1391,22 +1411,43 @@ def _add_slots(cls, is_frozen, weakref_slot, defined_fields): # (the newly created one, which we're returning) and not the # original class. We can break out of this loop as soon as we # make an update, since all closures for a class will share a - # given cell. + # given cell. First we try to find a pure function/properties, + # and then fallback to inspecting custom descriptors. + + # inspect is imported lazily (see gh-137855): only slots classes + # with wrapped members, properties or descriptors need it here. + import inspect + custom_descriptors_to_check = [] for member in newcls.__dict__.values(): # If this is a wrapped function, unwrap it. - if not isinstance(member, type) and hasattr(member, '__wrapped__'): - import inspect - member = inspect.unwrap(member) + member = inspect.unwrap(member) if isinstance(member, types.FunctionType): if _update_func_cell_for__class__(member, cls, newcls): break elif isinstance(member, property): - if (_update_func_cell_for__class__(member.fget, cls, newcls) - or _update_func_cell_for__class__(member.fset, cls, newcls) - or _update_func_cell_for__class__(member.fdel, cls, newcls)): - break + for f in member.fget, member.fset, member.fdel: + if f is None: + continue + # unwrap once more in case function + # was wrapped before it became property + f = inspect.unwrap(f) + if _update_func_cell_for__class__(f, cls, newcls): + break + elif hasattr(member, "__get__") and not inspect.ismemberdescriptor( + member + ): + # we don't want to inspect custom descriptors just yet + # there's still a chance we'll encounter a pure function + # or a property + custom_descriptors_to_check.append(member) + else: + # now let's ensure custom descriptors won't be left out + for descriptor in custom_descriptors_to_check: + for f in _find_inner_functions(descriptor): + if _update_func_cell_for__class__(f, cls, newcls): + break # Get new annotations to remove references to the original class # in forward references diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 2b4defe35f8a33..fd4733cca0130a 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -5533,6 +5533,47 @@ def foo(self): A().foo() + def test_wrapped_property(self): + def mydecorator(f): + @wraps(f) + def wrapper(*args, **kwargs): + return f(*args, **kwargs) + return wrapper + + class B: + @property + def foo(self): + return "bar" + + @dataclass(slots=True) + class A(B): + @property + @mydecorator + def foo(self): + return super().foo + + self.assertEqual(A().foo, "bar") + + def test_custom_descriptor(self): + class CustomDescriptor: + def __init__(self, f): + self._f = f + + def __get__(self, instance, owner): + return self._f(instance) + + class B: + def foo(self): + return "bar" + + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(cls): + return super().foo() + + self.assertEqual(A().foo, "bar") + def test_remembered_class(self): # Apply the dataclass decorator manually (not when the class # is created), so that we can keep a reference to the diff --git a/Misc/NEWS.d/next/Library/2024-09-27-19-50-30.gh-issue-90562.HeL_JA.rst b/Misc/NEWS.d/next/Library/2024-09-27-19-50-30.gh-issue-90562.HeL_JA.rst new file mode 100644 index 00000000000000..56f2e1109ce5f5 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-09-27-19-50-30.gh-issue-90562.HeL_JA.rst @@ -0,0 +1,2 @@ +Modify dataclasses to enable zero argument support for ``super()`` when ``slots=True`` is +specified and custom descriptor is used or `property` function is wrapped. From 75e769d26d6af73f0c5773f3724c4ccc4a48a3f2 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 00:41:47 +0200 Subject: [PATCH 02/17] Fix NEWS.d formatting --- .../next/Library/2024-09-27-19-50-30.gh-issue-90562.HeL_JA.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-09-27-19-50-30.gh-issue-90562.HeL_JA.rst b/Misc/NEWS.d/next/Library/2024-09-27-19-50-30.gh-issue-90562.HeL_JA.rst index 56f2e1109ce5f5..b0bbdd406f63c7 100644 --- a/Misc/NEWS.d/next/Library/2024-09-27-19-50-30.gh-issue-90562.HeL_JA.rst +++ b/Misc/NEWS.d/next/Library/2024-09-27-19-50-30.gh-issue-90562.HeL_JA.rst @@ -1,2 +1,2 @@ Modify dataclasses to enable zero argument support for ``super()`` when ``slots=True`` is -specified and custom descriptor is used or `property` function is wrapped. +specified and custom descriptor is used or ``property`` function is wrapped. From 3c09e3a035ca219a775a99ad3c7084a6ecbd8832 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 00:42:26 +0200 Subject: [PATCH 03/17] Add explanation for _depth use and tests --- Lib/dataclasses.py | 3 ++ Lib/test/test_dataclasses/__init__.py | 68 +++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 214376d42f0a79..d6bd8933a0dcf7 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1322,6 +1322,9 @@ def _find_inner_functions(obj, _seen=None, _depth=0): _seen.add(id(obj)) _depth += 1 + # we don't want to dive too deep in an object in search for a function. + # usually it will be stored on outer levels of nesting, but in just + # for sake of special cases when if _depth > 2: return None diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index fd4733cca0130a..c53cbd40cad235 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -5574,6 +5574,74 @@ def foo(cls): self.assertEqual(A().foo, "bar") + def test_custom_nested_descriptor(self): + class CustomFunctionWrapper: + def __init__(self, f): + self._f = f + + def __call__(self, *args, **kwargs): + return self._f(*args, **kwargs) + + class CustomDescriptor: + def __init__(self, f): + self._wrapper = CustomFunctionWrapper(f) + + def __get__(self, instance, owner): + return self._wrapper(instance) + + class B: + def foo(self): + return "bar" + + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(cls): + return super().foo() + + self.assertEqual(A().foo, "bar") + + def test_custom_too_nested_descriptor(self): + class UnnecessaryNestedWrapper: + def __init__(self, wrapper): + self._wrapper = wrapper + + def __call__(self, *args, **kwargs): + return self._wrapper(*args, **kwargs) + + class CustomFunctionWrapper: + def __init__(self, f): + self._f = f + + def __call__(self, *args, **kwargs): + return self._f(*args, **kwargs) + + class CustomDescriptor: + def __init__(self, f): + self._wrapper = UnnecessaryNestedWrapper(CustomFunctionWrapper(f)) + + def __get__(self, instance, owner): + return self._wrapper(instance) + + class B: + def foo(self): + return "bar" + + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(cls): + return super().foo() + + with self.assertRaises(TypeError) as context: + A().foo + + expected_error_message = ( + 'super(type, obj): obj (instance of A) is not ' + 'an instance or subtype of type (A).' + ) + self.assertEqual(context.exception.args, (expected_error_message,)) + def test_remembered_class(self): # Apply the dataclass decorator manually (not when the class # is created), so that we can keep a reference to the From d53d538bee96aa9bddc9800c3f572230246a4f21 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 00:42:42 +0200 Subject: [PATCH 04/17] Fix incorrect variable reference --- Lib/dataclasses.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index d6bd8933a0dcf7..911c588dd5ddc0 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1334,8 +1334,8 @@ def _find_inner_functions(obj, _seen=None, _depth=0): value = getattr(obj, attr, None) if value is None: continue - if isinstance(obj, types.FunctionType): - yield obj + if isinstance(value, types.FunctionType): + yield value return yield from _find_inner_functions(value, _seen, _depth) From a9c43198fa163de9c14ca228e88e19ab64407995 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 00:55:01 +0200 Subject: [PATCH 05/17] Correctly unwrap descriptor function --- Lib/dataclasses.py | 4 +--- Lib/test/test_dataclasses/__init__.py | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 911c588dd5ddc0..e0ef7ce879a882 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1328,14 +1328,12 @@ def _find_inner_functions(obj, _seen=None, _depth=0): if _depth > 2: return None - obj = inspect.unwrap(obj) - for attr in dir(obj): value = getattr(obj, attr, None) if value is None: continue if isinstance(value, types.FunctionType): - yield value + yield inspect.unwrap(value) return yield from _find_inner_functions(value, _seen, _depth) diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index c53cbd40cad235..1362a2f2882577 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -16,6 +16,7 @@ import sys import textwrap import unittest +from functools import update_wrapper from unittest.mock import Mock from typing import ClassVar, Any, List, Union, Tuple, Dict, Generic, TypeVar, Optional, Protocol, DefaultDict from typing import get_type_hints @@ -5574,6 +5575,26 @@ def foo(cls): self.assertEqual(A().foo, "bar") + def test_custom_descriptor_wrapped(self): + class CustomDescriptor: + def __init__(self, f): + self._f = update_wrapper(lambda *args, **kwargs: f(*args, **kwargs), f) + + def __get__(self, instance, owner): + return self._f(instance) + + class B: + def foo(self): + return "bar" + + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(cls): + return super().foo() + + self.assertEqual(A().foo, "bar") + def test_custom_nested_descriptor(self): class CustomFunctionWrapper: def __init__(self, f): From 26e13e7fc47ff1a4626c537c6e34f2aee68bf55b Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 00:55:48 +0200 Subject: [PATCH 06/17] Add test with using partial as custom wrapper --- Lib/test/test_dataclasses/__init__.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 1362a2f2882577..ce173bc61caf0e 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -16,6 +16,7 @@ import sys import textwrap import unittest +from functools import partial from functools import update_wrapper from unittest.mock import Mock from typing import ClassVar, Any, List, Union, Tuple, Dict, Generic, TypeVar, Optional, Protocol, DefaultDict @@ -5622,6 +5623,26 @@ def foo(cls): self.assertEqual(A().foo, "bar") + def test_custom_nested_descriptor_with_partial(self): + class CustomDescriptor: + def __init__(self, f): + self._wrapper = partial(f, value="bar") + + def __get__(self, instance, owner): + return self._wrapper(instance) + + class B: + def foo(self, value): + return value + + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(self, value): + return super().foo(value) + + self.assertEqual(A().foo, "bar") + def test_custom_too_nested_descriptor(self): class UnnecessaryNestedWrapper: def __init__(self, wrapper): From 3a08b0e7964173883d08df8f20571da1c6c13562 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 01:03:03 +0200 Subject: [PATCH 07/17] Rephrase comment explaining _depth argument --- Lib/dataclasses.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index e0ef7ce879a882..22e975abd96503 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1322,9 +1322,10 @@ def _find_inner_functions(obj, _seen=None, _depth=0): _seen.add(id(obj)) _depth += 1 - # we don't want to dive too deep in an object in search for a function. - # usually it will be stored on outer levels of nesting, but in just - # for sake of special cases when + # normally just inspection of a descriptor object itself should be enough, + # and we should encounter the function as its attribute, + # but in case function was wrapped (e.g. functools.partition was used), + # we want to dive at least one level deeper if _depth > 2: return None From 2e9588d2584021f64b2e33062d01a3c0dde3a290 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 01:18:02 +0200 Subject: [PATCH 08/17] Add clarification for function search priority --- Lib/dataclasses.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 22e975abd96503..78c7bc53c9027c 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1413,8 +1413,9 @@ def _add_slots(cls, is_frozen, weakref_slot, defined_fields): # (the newly created one, which we're returning) and not the # original class. We can break out of this loop as soon as we # make an update, since all closures for a class will share a - # given cell. First we try to find a pure function/properties, - # and then fallback to inspecting custom descriptors. + # given cell. First we try to find a pure function or a property, + # and then fallback to inspecting custom descriptors + # if no pure function or property is found. # inspect is imported lazily (see gh-137855): only slots classes # with wrapped members, properties or descriptors need it here. @@ -1440,9 +1441,9 @@ def _add_slots(cls, is_frozen, weakref_slot, defined_fields): elif hasattr(member, "__get__") and not inspect.ismemberdescriptor( member ): - # we don't want to inspect custom descriptors just yet + # We don't want to inspect custom descriptors just yet # there's still a chance we'll encounter a pure function - # or a property + # or a property and won't have to use slower recursive search. custom_descriptors_to_check.append(member) else: # now let's ensure custom descriptors won't be left out From fb4bc9fc3e63070397d7f62e3656d1c014c1c693 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 01:21:52 +0200 Subject: [PATCH 09/17] Ues consistent grammar throughout comments --- Lib/dataclasses.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 78c7bc53c9027c..901d9769a3e2cd 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1322,10 +1322,10 @@ def _find_inner_functions(obj, _seen=None, _depth=0): _seen.add(id(obj)) _depth += 1 - # normally just inspection of a descriptor object itself should be enough, + # Normally just inspection of a descriptor object itself should be enough, # and we should encounter the function as its attribute, # but in case function was wrapped (e.g. functools.partition was used), - # we want to dive at least one level deeper + # we want to dive at least one level deeper. if _depth > 2: return None @@ -1433,8 +1433,8 @@ def _add_slots(cls, is_frozen, weakref_slot, defined_fields): for f in member.fget, member.fset, member.fdel: if f is None: continue - # unwrap once more in case function - # was wrapped before it became property + # Unwrap once more in case function + # was wrapped before it became property. f = inspect.unwrap(f) if _update_func_cell_for__class__(f, cls, newcls): break @@ -1446,7 +1446,7 @@ def _add_slots(cls, is_frozen, weakref_slot, defined_fields): # or a property and won't have to use slower recursive search. custom_descriptors_to_check.append(member) else: - # now let's ensure custom descriptors won't be left out + # Now let's ensure custom descriptors won't be left out. for descriptor in custom_descriptors_to_check: for f in _find_inner_functions(descriptor): if _update_func_cell_for__class__(f, cls, newcls): From bdaeb1f486b4f4688e6787ae0c4cc5a767729e1b Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 01:35:46 +0200 Subject: [PATCH 10/17] Fix breaking out from outer loop when handling property --- Lib/dataclasses.py | 3 ++ Lib/test/test_dataclasses/__init__.py | 46 +++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 901d9769a3e2cd..b33ad9f4cfe52f 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1438,6 +1438,9 @@ def _add_slots(cls, is_frozen, weakref_slot, defined_fields): f = inspect.unwrap(f) if _update_func_cell_for__class__(f, cls, newcls): break + else: + continue + break elif hasattr(member, "__get__") and not inspect.ismemberdescriptor( member ): diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index ce173bc61caf0e..2b57d4635e7e96 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -5643,6 +5643,52 @@ def foo(self, value): self.assertEqual(A().foo, "bar") + def test_pure_functions_preferred_to_custom_descriptors(self): + class CustomDescriptor: + def __init__(self, f): + self._wrapper = partial(f, value="bar") + + def __get__(self, instance, owner): + return self._wrapper(instance) + + def __dir__(self): + raise RuntimeError("Never should be accessed") + + class B: + def foo(self, value): + return value + + with self.assertRaises(RuntimeError) as context: + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(self, value): ... + + self.assertEqual(context.exception.args, ("Never should be accessed",)) + + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(self, value): + return super().foo(value) + + @property + def bar(self): + return super() + + self.assertEqual(A().foo, "bar") + + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(self, value): + return super().foo(value) + + def bar(self): + return super() + + self.assertEqual(A().foo, "bar") + def test_custom_too_nested_descriptor(self): class UnnecessaryNestedWrapper: def __init__(self, wrapper): From 8f6140afff0c580d079836e0d0ee8ddc31de9cef Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 01:39:39 +0200 Subject: [PATCH 11/17] Use generator expression with any() instead of classic loop --- Lib/dataclasses.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index b33ad9f4cfe52f..8f710c6805d496 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1429,17 +1429,15 @@ def _add_slots(cls, is_frozen, weakref_slot, defined_fields): if isinstance(member, types.FunctionType): if _update_func_cell_for__class__(member, cls, newcls): break - elif isinstance(member, property): - for f in member.fget, member.fset, member.fdel: - if f is None: - continue + elif isinstance(member, property) and ( + any( # Unwrap once more in case function # was wrapped before it became property. - f = inspect.unwrap(f) - if _update_func_cell_for__class__(f, cls, newcls): - break - else: - continue + _update_func_cell_for__class__(inspect.unwrap(f), cls, newcls) + for f in (member.fget, member.fset, member.fdel) + if f is not None + ) + ): break elif hasattr(member, "__get__") and not inspect.ismemberdescriptor( member From 13c2dc71dc2922e6e40759b585a546b81412b9b9 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 13:03:14 +0200 Subject: [PATCH 12/17] Apply suggestions from code review Co-authored-by: sobolevn --- Lib/dataclasses.py | 4 ++-- Lib/test/test_dataclasses/__init__.py | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 8f710c6805d496..872c82fda0e722 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1322,9 +1322,9 @@ def _find_inner_functions(obj, _seen=None, _depth=0): _seen.add(id(obj)) _depth += 1 - # Normally just inspection of a descriptor object itself should be enough, + # Normally just an inspection of a descriptor object itself should be enough, # and we should encounter the function as its attribute, - # but in case function was wrapped (e.g. functools.partition was used), + # but in case function was wrapped (e.g. functools.partial was used), # we want to dive at least one level deeper. if _depth > 2: return None diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 2b57d4635e7e96..8570cfbeaf4388 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -16,8 +16,7 @@ import sys import textwrap import unittest -from functools import partial -from functools import update_wrapper +from functools import partial, update_wrapper from unittest.mock import Mock from typing import ClassVar, Any, List, Union, Tuple, Dict, Generic, TypeVar, Optional, Protocol, DefaultDict from typing import get_type_hints From 9d3d49ff0828ae94941c112020850ffc2865f760 Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 13:09:13 +0200 Subject: [PATCH 13/17] Remove leading underscores from seen and depth args --- Lib/dataclasses.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 872c82fda0e722..31f8d1e826e344 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1312,21 +1312,21 @@ def _update_func_cell_for__class__(f, oldcls, newcls): return False -def _find_inner_functions(obj, _seen=None, _depth=0): +def _find_inner_functions(obj, seen=None, depth=0): # inspect is imported lazily (see gh-137855). import inspect - if _seen is None: - _seen = set() - if id(obj) in _seen: + if seen is None: + seen = set() + if id(obj) in seen: return None - _seen.add(id(obj)) + seen.add(id(obj)) - _depth += 1 + depth += 1 # Normally just an inspection of a descriptor object itself should be enough, # and we should encounter the function as its attribute, # but in case function was wrapped (e.g. functools.partial was used), # we want to dive at least one level deeper. - if _depth > 2: + if depth > 2: return None for attr in dir(obj): @@ -1336,7 +1336,7 @@ def _find_inner_functions(obj, _seen=None, _depth=0): if isinstance(value, types.FunctionType): yield inspect.unwrap(value) return - yield from _find_inner_functions(value, _seen, _depth) + yield from _find_inner_functions(value, seen, depth) def _create_slots(defined_fields, inherited_slots, field_names, weakref_slot): From d0958641b95af18b5132d532ebfba1ff2170af4a Mon Sep 17 00:00:00 2001 From: Arseny Boykov <36469655+Bobronium@users.noreply.github.com> Date: Sat, 28 Sep 2024 22:37:06 +0200 Subject: [PATCH 14/17] Prevent user-defined code execution during attribute scanning --- Lib/dataclasses.py | 24 +++++++++++--- Lib/test/test_dataclasses/__init__.py | 46 --------------------------- 2 files changed, 20 insertions(+), 50 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 31f8d1e826e344..4b3c7dc6d1918e 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1312,6 +1312,25 @@ def _update_func_cell_for__class__(f, oldcls, newcls): return False +def _safe_get_attributes(obj): + # we should avoid triggering any user-defined code + # when inspecting attributes if possible + + # look for __slots__ descriptors + type_dict = object.__getattribute__(type(obj), "__dict__") + for value in type_dict.values(): + if isinstance(value, types.MemberDescriptorType): + yield value.__get__(obj) + + instance_dict_descriptor = type_dict.get("__dict__", None) + if not isinstance(instance_dict_descriptor, types.GetSetDescriptorType): + # __dict__ is either not present, or redefined by user + # as custom descriptor, either way, we're done here + return + + yield from instance_dict_descriptor.__get__(obj).values() + + def _find_inner_functions(obj, seen=None, depth=0): # inspect is imported lazily (see gh-137855). import inspect @@ -1329,10 +1348,7 @@ def _find_inner_functions(obj, seen=None, depth=0): if depth > 2: return None - for attr in dir(obj): - value = getattr(obj, attr, None) - if value is None: - continue + for value in _safe_get_attributes(obj): if isinstance(value, types.FunctionType): yield inspect.unwrap(value) return diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 8570cfbeaf4388..148c776274ed61 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -5642,52 +5642,6 @@ def foo(self, value): self.assertEqual(A().foo, "bar") - def test_pure_functions_preferred_to_custom_descriptors(self): - class CustomDescriptor: - def __init__(self, f): - self._wrapper = partial(f, value="bar") - - def __get__(self, instance, owner): - return self._wrapper(instance) - - def __dir__(self): - raise RuntimeError("Never should be accessed") - - class B: - def foo(self, value): - return value - - with self.assertRaises(RuntimeError) as context: - @dataclass(slots=True) - class A(B): - @CustomDescriptor - def foo(self, value): ... - - self.assertEqual(context.exception.args, ("Never should be accessed",)) - - @dataclass(slots=True) - class A(B): - @CustomDescriptor - def foo(self, value): - return super().foo(value) - - @property - def bar(self): - return super() - - self.assertEqual(A().foo, "bar") - - @dataclass(slots=True) - class A(B): - @CustomDescriptor - def foo(self, value): - return super().foo(value) - - def bar(self): - return super() - - self.assertEqual(A().foo, "bar") - def test_custom_too_nested_descriptor(self): class UnnecessaryNestedWrapper: def __init__(self, wrapper): From 6848c1b66730a3e0d061a2a0abf3646795718ac6 Mon Sep 17 00:00:00 2001 From: Arseny Boykov Date: Sat, 5 Oct 2024 14:41:17 +0200 Subject: [PATCH 15/17] Use inspect.getattr_static instead of _safe_get_attributes This uses builtins.dir, which will trigger custom __getattibute__ if any, and will trigger __get__ on __dict__ descriptor. --- Lib/dataclasses.py | 39 ++++++++++++--------------- Lib/test/test_dataclasses/__init__.py | 39 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 22 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 4b3c7dc6d1918e..3bf6b29d717f40 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1312,25 +1312,6 @@ def _update_func_cell_for__class__(f, oldcls, newcls): return False -def _safe_get_attributes(obj): - # we should avoid triggering any user-defined code - # when inspecting attributes if possible - - # look for __slots__ descriptors - type_dict = object.__getattribute__(type(obj), "__dict__") - for value in type_dict.values(): - if isinstance(value, types.MemberDescriptorType): - yield value.__get__(obj) - - instance_dict_descriptor = type_dict.get("__dict__", None) - if not isinstance(instance_dict_descriptor, types.GetSetDescriptorType): - # __dict__ is either not present, or redefined by user - # as custom descriptor, either way, we're done here - return - - yield from instance_dict_descriptor.__get__(obj).values() - - def _find_inner_functions(obj, seen=None, depth=0): # inspect is imported lazily (see gh-137855). import inspect @@ -1348,11 +1329,25 @@ def _find_inner_functions(obj, seen=None, depth=0): if depth > 2: return None - for value in _safe_get_attributes(obj): + for attribute in dir(obj): + try: + value = inspect.getattr_static(obj, attribute) + except AttributeError: + continue + builtin_value = inspect.getattr_static(object, attribute, None) + if value is builtin_value: + # don't waste time on builtin objects + continue + if ( + # isinstance() would trigger `value.__getattribute__("__class__")` + type(value) is types.MemberDescriptorType + and type not in inspect._static_getmro(type(obj)) + ): + value = value.__get__(obj) if isinstance(value, types.FunctionType): yield inspect.unwrap(value) - return - yield from _find_inner_functions(value, seen, depth) + else: + yield from _find_inner_functions(value, seen, depth) def _create_slots(defined_fields, inherited_slots, field_names, weakref_slot): diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 148c776274ed61..c8585052ff6ee5 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -5683,6 +5683,45 @@ def foo(cls): ) self.assertEqual(context.exception.args, (expected_error_message,)) + def test_user_defined_code_execution(self): + class CustomDescriptor: + def __init__(self, f): + self._wrapper = partial(f, value="bar") + + def __get__(self, instance, owner): + return object.__getattribute__(self, "_wrapper")(instance) + + def __getattribute__(self, name): + if name in { + # these are the bare minimum for the feature to work + "__class__", # accessed on `isinstance(value, Field)` + "__wrapped__", # accessed by unwrap + "__get__", # is required for the descriptor protocol + "__dict__", # is accessed by dir() to work + }: + return object.__getattribute__(self, name) + raise RuntimeError(f"Never should be accessed: {name}") + + class B: + def foo(self, value): + return value + + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(self, value): + return super().foo(value) + + self.assertEqual(A().foo, "bar") + + @dataclass(slots=True) + class A(B): + @CustomDescriptor + def foo(self, value): + return super().foo(value) + + self.assertEqual(A().foo, "bar") + def test_remembered_class(self): # Apply the dataclass decorator manually (not when the class # is created), so that we can keep a reference to the From 1d443e754f80a81fa27934b5bfc20a42fe80975a Mon Sep 17 00:00:00 2001 From: Arseny Boykov Date: Sat, 5 Oct 2024 14:51:01 +0200 Subject: [PATCH 16/17] Put back the comment in _update_func_cell_for__class__ --- Lib/dataclasses.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 3bf6b29d717f40..92f0599b60573a 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1289,6 +1289,7 @@ def _get_slots(cls): def _update_func_cell_for__class__(f, oldcls, newcls): + # Returns True if we update a cell, else False. try: idx = f.__code__.co_freevars.index("__class__") except ValueError: From 5a7037bc5d94e5c252571292922247751698bc39 Mon Sep 17 00:00:00 2001 From: Arseny Boykov Date: Sat, 5 Oct 2024 15:16:54 +0200 Subject: [PATCH 17/17] Rely on inspect.getmembers_static() --- Lib/dataclasses.py | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 92f0599b60573a..23528e6106b3d5 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1313,6 +1313,28 @@ def _update_func_cell_for__class__(f, oldcls, newcls): return False +_object_members_values = None + + +def _is_not_object_member(v): + # _object_members_values is initialized on first use, so that + # importing dataclasses does not eagerly import inspect (gh-137855). + global _object_members_values + if _object_members_values is None: + import inspect + _object_members_values = { + value for _, value in + ( + *inspect.getmembers_static(object), + *inspect.getmembers_static(object()) + ) + } + try: + return v not in _object_members_values + except TypeError: + return True + + def _find_inner_functions(obj, seen=None, depth=0): # inspect is imported lazily (see gh-137855). import inspect @@ -1330,22 +1352,14 @@ def _find_inner_functions(obj, seen=None, depth=0): if depth > 2: return None - for attribute in dir(obj): - try: - value = inspect.getattr_static(obj, attribute) - except AttributeError: - continue - builtin_value = inspect.getattr_static(object, attribute, None) - if value is builtin_value: - # don't waste time on builtin objects - continue - if ( - # isinstance() would trigger `value.__getattribute__("__class__")` - type(value) is types.MemberDescriptorType - and type not in inspect._static_getmro(type(obj)) - ): + obj_is_type_instance = type in inspect._static_getmro(type(obj)) + for _, value in inspect.getmembers_static(obj, _is_not_object_member): + value_type = type(value) + if value_type is types.MemberDescriptorType and not obj_is_type_instance: value = value.__get__(obj) - if isinstance(value, types.FunctionType): + value_type = type(value) + + if value_type is types.FunctionType: yield inspect.unwrap(value) else: yield from _find_inner_functions(value, seen, depth)