From 101e0746d36caa56b93d5c61d8e54938c6edf93b Mon Sep 17 00:00:00 2001 From: Ethan Furman Date: Sun, 15 Sep 2013 12:34:36 -0700 Subject: [PATCH] Close #18989: enum members will no longer overwrite other attributes, nor be overwritten by them. --- Doc/library/enum.rst | 6 ++++++ Lib/enum.py | 36 +++++++++++++++++++++--------------- Lib/test/test_enum.py | 37 ++++++++++++++++++++++++++----------- 3 files changed, 53 insertions(+), 26 deletions(-) diff --git a/Doc/library/enum.rst b/Doc/library/enum.rst index 236275dc34b..c5076b297ec 100644 --- a/Doc/library/enum.rst +++ b/Doc/library/enum.rst @@ -154,6 +154,12 @@ return A:: >>> Shape(2) +.. note:: + + Attempting to create a member with the same name as an already + defined attribute (another member, a method, etc.) or attempting to create + an attribute with the same name as a member is not allowed. + Ensuring unique enumeration values ---------------------------------- diff --git a/Lib/enum.py b/Lib/enum.py index 0d72f3e2d20..40546da761c 100644 --- a/Lib/enum.py +++ b/Lib/enum.py @@ -29,6 +29,14 @@ class _RouteClassAttributeToGetattr: raise AttributeError("can't delete attribute") +def _is_descriptor(obj): + """Returns True if obj is a descriptor, False otherwise.""" + return ( + hasattr(obj, '__get__') or + hasattr(obj, '__set__') or + hasattr(obj, '__delete__')) + + def _is_dunder(name): """Returns True if a __dunder__ name, False otherwise.""" return (name[:2] == name[-2:] == '__' and @@ -50,8 +58,9 @@ def _make_class_unpicklable(cls): cls.__reduce__ = _break_on_call_reduce cls.__module__ = '' + class _EnumDict(dict): - """Keeps track of definition order of the enum items. + """Track enum member order and ensure member names are not reused. EnumMeta will use the names found in self._member_names as the enumeration member names. @@ -62,11 +71,7 @@ class _EnumDict(dict): self._member_names = [] def __setitem__(self, key, value): - """Changes anything not dundered or that doesn't have __get__. - - If a descriptor is added with the same name as an enum member, the name - is removed from _member_names (this may leave a hole in the numerical - sequence of values). + """Changes anything not dundered or not a descriptor. If an enum member name is used twice, an error is raised; duplicate values are not checked for. @@ -76,19 +81,20 @@ class _EnumDict(dict): """ if _is_sunder(key): raise ValueError('_names_ are reserved for future Enum use') - elif _is_dunder(key) or hasattr(value, '__get__'): - if key in self._member_names: - # overwriting an enum with a method? then remove the name from - # _member_names or it will become an enum anyway when the class - # is created - self._member_names.remove(key) - else: - if key in self._member_names: - raise TypeError('Attempted to reuse key: %r' % key) + elif _is_dunder(key): + pass + elif key in self._member_names: + # descriptor overwriting an enum? + raise TypeError('Attempted to reuse key: %r' % key) + elif not _is_descriptor(value): + if key in self: + # enum overwriting a descriptor? + raise TypeError('Key already defined as: %r' % self[key]) self._member_names.append(key) super().__setitem__(key, value) + # Dummy value for Enum as EnumMeta explicitly checks for it, but of course # until EnumMeta finishes running the first time the Enum class doesn't exist. # This is also why there are checks in EnumMeta like `if Enum is not None` diff --git a/Lib/test/test_enum.py b/Lib/test/test_enum.py index 130800338da..5d96d6d36ae 100644 --- a/Lib/test/test_enum.py +++ b/Lib/test/test_enum.py @@ -228,6 +228,32 @@ class TestEnum(unittest.TestCase): ['FALL', 'ANOTHER_SPRING'], ) + def test_duplicate_name(self): + with self.assertRaises(TypeError): + class Color(Enum): + red = 1 + green = 2 + blue = 3 + red = 4 + + with self.assertRaises(TypeError): + class Color(Enum): + red = 1 + green = 2 + blue = 3 + def red(self): + return 'red' + + with self.assertRaises(TypeError): + class Color(Enum): + @property + def red(self): + return 'redder' + red = 1 + green = 2 + blue = 3 + + def test_enum_with_value_name(self): class Huh(Enum): name = 1 @@ -618,17 +644,6 @@ class TestEnum(unittest.TestCase): self.assertIsNot(type(whatever.really), whatever) self.assertEqual(whatever.this.really(), 'no, not that') - def test_overwrite_enums(self): - class Why(Enum): - question = 1 - answer = 2 - propisition = 3 - def question(self): - print(42) - self.assertIsNot(type(Why.question), Why) - self.assertNotIn(Why.question, Why._member_names_) - self.assertNotIn(Why.question, Why) - def test_wrong_inheritance_order(self): with self.assertRaises(TypeError): class Wrong(Enum, str):