From 9c8ffe0140976f00fff92c9427e022e3938df5a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 16 Aug 2024 15:31:53 +0200 Subject: [PATCH 1/3] disallow `default_factory` for dataclasses without `__init__` --- Lib/dataclasses.py | 13 ++++++ Lib/test/test_dataclasses/__init__.py | 57 +++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 141aa41c74d7ed..78298dae13b6bf 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1009,6 +1009,10 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, # Otherwise it's a field of some type. cls_fields.append(_get_field(cls, name, type, kw_only)) + # Test whether '__init__' is to be auto-generated or if + # it is provided explicitly by the user. + has_init_method = init or '__init__' in cls.__dict__ + for f in cls_fields: fields[f.name] = f @@ -1018,6 +1022,15 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, # sees a real default value, not a Field. if isinstance(getattr(cls, f.name, None), Field): if f.default is MISSING: + # https://github.com/python/cpython/issues/89529 + if f.default_factory is not MISSING and not has_init_method: + raise ValueError( + f'specifying default_factory for {f.name!r}' + f' requires the @dataclass decorator to be' + f' called with init=True or to implement' + f' an __init__ method' + ) + # If there's no default, delete the class attribute. # This happens if we specify field(repr=False), for # example (that is, we specified a field object, but diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index b93c99d8c90bf3..7d0e51c9c747e2 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -9,6 +9,7 @@ import pickle import inspect import builtins +import re import types import weakref import traceback @@ -18,6 +19,7 @@ from typing import get_type_hints from collections import deque, OrderedDict, namedtuple, defaultdict from functools import total_ordering +from itertools import product import typing # Needed for the string "typing.ClassVar[int]" to work as an annotation. import dataclasses # Needed for the string "dataclasses.InitVar[int]" to work as an annotation. @@ -1411,6 +1413,61 @@ class C: C().x self.assertEqual(factory.call_count, 2) + def test_default_factory_with_no_init_method(self): + # See https://github.com/python/cpython/issues/89529. + + @dataclass + class BaseWithInit: + x: list + + @dataclass(slots=True) + class BaseWithSlots: + x: list + + @dataclass(init=False) + class BaseWithOutInit: + x: list + + @dataclass(init=False, slots=True) + class BaseWithOutInitWithSlots: + x: list + + err = re.escape( + "specifying default_factory for 'x' requires the " + "@dataclass decorator to be called with init=True " + "or to implement an __init__ method" + ) + + for base_class, slots, field_init in product( + (object, BaseWithInit, BaseWithSlots, + BaseWithOutInit, BaseWithOutInitWithSlots), + (True, False), + (True, False), + ): + with self.subTest('generated __init__', base_class=base_class, + init=True, slots=slots, field_init=field_init): + @dataclass(init=True, slots=slots) + class C(base_class): + x: list = field(init=field_init, default_factory=list) + self.assertListEqual(C().x, []) + + with self.subTest('user-defined __init__', base_class=base_class, + init=False, slots=slots, field_init=field_init): + @dataclass(init=False, slots=slots) + class C(base_class): + x: list = field(init=field_init, default_factory=list) + def __init__(self, *a, **kw): + # deliberately use something else + self.x = 'hello' + self.assertEqual(C().x, 'hello') + + with self.subTest('no generated __init__', base_class=base_class, + init=False, slots=slots, field_init=field_init): + with self.assertRaisesRegex(ValueError, err): + @dataclass(init=False, slots=slots) + class C(base_class): + x: list = field(init=field_init, default_factory=list) + def test_default_factory_not_called_if_value_given(self): # We need a factory that we can test if it's been called. factory = Mock() From 204ec650897916d1d59eaeacf2c373316da82572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 16 Aug 2024 15:36:00 +0200 Subject: [PATCH 2/3] blurb --- .../next/Library/2024-08-16-15-35-57.gh-issue-89529.ayfZ1n.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-08-16-15-35-57.gh-issue-89529.ayfZ1n.rst diff --git a/Misc/NEWS.d/next/Library/2024-08-16-15-35-57.gh-issue-89529.ayfZ1n.rst b/Misc/NEWS.d/next/Library/2024-08-16-15-35-57.gh-issue-89529.ayfZ1n.rst new file mode 100644 index 00000000000000..e34859d1cf26f3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-08-16-15-35-57.gh-issue-89529.ayfZ1n.rst @@ -0,0 +1,2 @@ +Disallow ``default_factory`` for dataclass fields if the dataclass does not +have an ``__init__`` method. Patch by Bénédikt Tran. From c55ec4e4bc03f6f987e5b7c6c8bfd4fcefc1d3cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 17 Aug 2024 10:06:28 +0200 Subject: [PATCH 3/3] add test case with `init=True` and user-defined `__init__` --- Lib/test/test_dataclasses/__init__.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 7d0e51c9c747e2..9d23a04c8de743 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -1413,7 +1413,7 @@ class C: C().x self.assertEqual(factory.call_count, 2) - def test_default_factory_with_no_init_method(self): + def test_default_factory_and_init_method_interaction(self): # See https://github.com/python/cpython/issues/89529. @dataclass @@ -1452,8 +1452,8 @@ class C(base_class): self.assertListEqual(C().x, []) with self.subTest('user-defined __init__', base_class=base_class, - init=False, slots=slots, field_init=field_init): - @dataclass(init=False, slots=slots) + init=True, slots=slots, field_init=field_init): + @dataclass(init=True, slots=slots) class C(base_class): x: list = field(init=field_init, default_factory=list) def __init__(self, *a, **kw): @@ -1468,6 +1468,16 @@ def __init__(self, *a, **kw): class C(base_class): x: list = field(init=field_init, default_factory=list) + with self.subTest('user-defined __init__', base_class=base_class, + init=False, slots=slots, field_init=field_init): + @dataclass(init=False, slots=slots) + class C(base_class): + x: list = field(init=field_init, default_factory=list) + def __init__(self, *a, **kw): + # deliberately use something else + self.x = 'world' + self.assertEqual(C().x, 'world') + def test_default_factory_not_called_if_value_given(self): # We need a factory that we can test if it's been called. factory = Mock()