From 99273dfa0a6e9eaef7b12754d0ff45b5c3dd0bbc Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 9 Oct 2019 10:35:40 -0700 Subject: [PATCH 1/5] Add @freeze_after_init decorator to make some @dataclass uses safer with V2 --- src/python/pants/util/BUILD | 5 +- src/python/pants/util/meta.py | 31 +++++++++++ tests/python/pants_test/util/BUILD | 1 + tests/python/pants_test/util/test_meta.py | 64 ++++++++++++++++++++++- 4 files changed, 99 insertions(+), 2 deletions(-) diff --git a/src/python/pants/util/BUILD b/src/python/pants/util/BUILD index 6b9cc1fb785..3febe774392 100644 --- a/src/python/pants/util/BUILD +++ b/src/python/pants/util/BUILD @@ -72,13 +72,16 @@ python_library( name = 'memo', sources = ['memo.py'], dependencies = [ - ':meta' + ':meta' ], ) python_library( name = 'meta', sources = ['meta.py'], + dependencies = [ + '3rdparty/python:dataclasses' + ], tags = {'type_checked'}, ) diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py index 98b38dbdf46..f99755c4212 100644 --- a/src/python/pants/util/meta.py +++ b/src/python/pants/util/meta.py @@ -1,6 +1,8 @@ # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +from dataclasses import FrozenInstanceError +from functools import wraps from typing import Any, Callable, Optional, Type, TypeVar, Union @@ -107,3 +109,32 @@ def staticproperty(func: Union[staticmethod, Callable]) -> ClassPropertyDescript func = staticmethod(func) return ClassPropertyDescriptor(func, doc) + + +def freeze_after_init(cls: Type[T]) -> Type[T]: + """Class decorator to freeze any modifications to the object after __init__() is done. + + The primary use case is for @dataclasses who cannot use frozen=True due to the need for a custom + __init__(), but who still want to remain as immutable as possible (e.g. for safety with the V2 + engine). When using with dataclasses, this should be the first decorator applied, i.e. be used + before @dataclass.""" + + prev_init = cls.__init__ + prev_setattr = cls.__setattr__ + + @wraps(prev_init) + def new_init(self, *args: Any, **kwargs: Any) -> None: + prev_init(self, *args, **kwargs) # type: ignore + self._is_frozen = True + + @wraps(prev_setattr) + def new_setattr(self, key: Any, value: Any) -> None: + if getattr(self, "_is_frozen", False): + raise FrozenInstanceError( + f"Attempting to modify the attribute {key} after the object {self} was created." + ) + prev_setattr(self, key, value) + + cls.__init__ = new_init # type: ignore + cls.__setattr__ = new_setattr # type: ignore + return cls diff --git a/tests/python/pants_test/util/BUILD b/tests/python/pants_test/util/BUILD index a1ed98759e4..3a107f41288 100644 --- a/tests/python/pants_test/util/BUILD +++ b/tests/python/pants_test/util/BUILD @@ -83,6 +83,7 @@ python_tests( name = 'meta', sources = ['test_meta.py'], dependencies = [ + '3rdparty/python:dataclasses', 'src/python/pants/util:meta', 'tests/python/pants_test:test_base', ], diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py index 6a7397630d1..fc031ea5bd1 100644 --- a/tests/python/pants_test/util/test_meta.py +++ b/tests/python/pants_test/util/test_meta.py @@ -1,9 +1,11 @@ # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import unittest from abc import ABC, abstractmethod +from dataclasses import FrozenInstanceError, dataclass -from pants.util.meta import SingletonMetaclass, classproperty, staticproperty +from pants.util.meta import SingletonMetaclass, classproperty, freeze_after_init, staticproperty from pants_test.test_base import TestBase @@ -242,3 +244,63 @@ class Concrete2(Abstract): def f(cls): return 'hello' self.assertEqual(Concrete2.f, 'hello') + + +class FreezeAfterInitTest(unittest.TestCase): + + def test_no_init(self) -> None: + @freeze_after_init + class Test: + pass + + with self.assertRaises(FrozenInstanceError): + test = Test() + test.x = 1 + + def test_init_still_works(self): + @freeze_after_init + class Test: + + def __init__(self, x: int) -> None: + self.x = x + self.y = "abc" + + test = Test(x=0) + self.assertEqual(test.x, 0) + self.assertEqual(test.y, "abc") + + def test_modify_preexisting_field_after_init(self) -> None: + @freeze_after_init + class Test: + + def __init__(self, x: int) -> None: + self.x = x + + with self.assertRaises(FrozenInstanceError): + test = Test(0) + test.x = 1 + + def test_add_new_field_after_init(self) -> None: + @freeze_after_init + class Test: + + def __init__(self, x: int) -> None: + self.x: x + + with self.assertRaises(FrozenInstanceError): + test = Test(0) + test.y = "abc" + + def test_works_with_dataclass(self) -> None: + @freeze_after_init + @dataclass + class Test: + x: int + + def __init__(self, x: int): + self.x = x + self.y = "abc" + + test = Test(x=0) + with self.assertRaises(FrozenInstanceError): + test.x = 1 From 8d5bc9ee2e83c5bcd2e1c8b4ca4d50309f6a503b Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 9 Oct 2019 10:51:13 -0700 Subject: [PATCH 2/5] Make dataclass test more explicit Co-Authored-By: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> --- tests/python/pants_test/util/test_meta.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py index fc031ea5bd1..290e3e56c42 100644 --- a/tests/python/pants_test/util/test_meta.py +++ b/tests/python/pants_test/util/test_meta.py @@ -293,7 +293,7 @@ def __init__(self, x: int) -> None: def test_works_with_dataclass(self) -> None: @freeze_after_init - @dataclass + @dataclass(frozen=False) class Test: x: int From 2bbbb360e640f4985a04d0747f782f5d75a4844c Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 9 Oct 2019 11:07:58 -0700 Subject: [PATCH 3/5] Make more clear in tests what should cause the exceptions --- tests/python/pants_test/util/test_meta.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py index 290e3e56c42..c33a4390357 100644 --- a/tests/python/pants_test/util/test_meta.py +++ b/tests/python/pants_test/util/test_meta.py @@ -253,8 +253,8 @@ def test_no_init(self) -> None: class Test: pass + test = Test() with self.assertRaises(FrozenInstanceError): - test = Test() test.x = 1 def test_init_still_works(self): @@ -276,8 +276,8 @@ class Test: def __init__(self, x: int) -> None: self.x = x + test = Test(x=0) with self.assertRaises(FrozenInstanceError): - test = Test(0) test.x = 1 def test_add_new_field_after_init(self) -> None: @@ -287,8 +287,8 @@ class Test: def __init__(self, x: int) -> None: self.x: x + test = Test(x=0) with self.assertRaises(FrozenInstanceError): - test = Test(0) test.y = "abc" def test_works_with_dataclass(self) -> None: From c0fee7abbd99a81d2902ee33cafe31499207c236 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 9 Oct 2019 11:23:27 -0700 Subject: [PATCH 4/5] Add one more test --- src/python/pants/util/meta.py | 2 +- tests/python/pants_test/util/test_meta.py | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py index f99755c4212..6984bd1d8aa 100644 --- a/src/python/pants/util/meta.py +++ b/src/python/pants/util/meta.py @@ -128,7 +128,7 @@ def new_init(self, *args: Any, **kwargs: Any) -> None: self._is_frozen = True @wraps(prev_setattr) - def new_setattr(self, key: Any, value: Any) -> None: + def new_setattr(self, key: str, value: Any) -> None: if getattr(self, "_is_frozen", False): raise FrozenInstanceError( f"Attempting to modify the attribute {key} after the object {self} was created." diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py index c33a4390357..94f48332ce3 100644 --- a/tests/python/pants_test/util/test_meta.py +++ b/tests/python/pants_test/util/test_meta.py @@ -291,13 +291,25 @@ def __init__(self, x: int) -> None: with self.assertRaises(FrozenInstanceError): test.y = "abc" + def test_explicitly_call_setattr_after_init(self) -> None: + @freeze_after_init + class Test: + + def __init__(self, x: int) -> None: + self.x: x + + test = Test(x=0) + with self.assertRaises(FrozenInstanceError): + setattr(test, "x", 1) + def test_works_with_dataclass(self) -> None: @freeze_after_init @dataclass(frozen=False) class Test: x: int + y: str - def __init__(self, x: int): + def __init__(self, x: int) -> None: self.x = x self.y = "abc" From 88f31cd2e5f88f4ac2fc3772b1742423ea6fd183 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 9 Oct 2019 11:32:49 -0700 Subject: [PATCH 5/5] Rename to `@frozen_after_init` --- src/python/pants/util/meta.py | 2 +- tests/python/pants_test/util/test_meta.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/python/pants/util/meta.py b/src/python/pants/util/meta.py index 6984bd1d8aa..0f3bbf8b544 100644 --- a/src/python/pants/util/meta.py +++ b/src/python/pants/util/meta.py @@ -111,7 +111,7 @@ def staticproperty(func: Union[staticmethod, Callable]) -> ClassPropertyDescript return ClassPropertyDescriptor(func, doc) -def freeze_after_init(cls: Type[T]) -> Type[T]: +def frozen_after_init(cls: Type[T]) -> Type[T]: """Class decorator to freeze any modifications to the object after __init__() is done. The primary use case is for @dataclasses who cannot use frozen=True due to the need for a custom diff --git a/tests/python/pants_test/util/test_meta.py b/tests/python/pants_test/util/test_meta.py index 94f48332ce3..ada93fa34a3 100644 --- a/tests/python/pants_test/util/test_meta.py +++ b/tests/python/pants_test/util/test_meta.py @@ -5,7 +5,7 @@ from abc import ABC, abstractmethod from dataclasses import FrozenInstanceError, dataclass -from pants.util.meta import SingletonMetaclass, classproperty, freeze_after_init, staticproperty +from pants.util.meta import SingletonMetaclass, classproperty, frozen_after_init, staticproperty from pants_test.test_base import TestBase @@ -246,10 +246,10 @@ def f(cls): self.assertEqual(Concrete2.f, 'hello') -class FreezeAfterInitTest(unittest.TestCase): +class FrozenAfterInitTest(unittest.TestCase): def test_no_init(self) -> None: - @freeze_after_init + @frozen_after_init class Test: pass @@ -258,7 +258,7 @@ class Test: test.x = 1 def test_init_still_works(self): - @freeze_after_init + @frozen_after_init class Test: def __init__(self, x: int) -> None: @@ -270,7 +270,7 @@ def __init__(self, x: int) -> None: self.assertEqual(test.y, "abc") def test_modify_preexisting_field_after_init(self) -> None: - @freeze_after_init + @frozen_after_init class Test: def __init__(self, x: int) -> None: @@ -281,7 +281,7 @@ def __init__(self, x: int) -> None: test.x = 1 def test_add_new_field_after_init(self) -> None: - @freeze_after_init + @frozen_after_init class Test: def __init__(self, x: int) -> None: @@ -292,7 +292,7 @@ def __init__(self, x: int) -> None: test.y = "abc" def test_explicitly_call_setattr_after_init(self) -> None: - @freeze_after_init + @frozen_after_init class Test: def __init__(self, x: int) -> None: @@ -303,7 +303,7 @@ def __init__(self, x: int) -> None: setattr(test, "x", 1) def test_works_with_dataclass(self) -> None: - @freeze_after_init + @frozen_after_init @dataclass(frozen=False) class Test: x: int