Merge pull request #160 from zopefoundation/issue158

Make the singleton _empty immutable.

3 files changed, 114 insertions, 3 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 1d6923c..cfb52d4 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,6 +5,13 @@
 5.0.0 (unreleased)
 ==================
 
+- Make an internal singleton object returned by APIs like
+  ``implementedBy`` and ``directlyProvidedBy`` immutable. Previously,
+  it was fully mutable and allowed changing its ``__bases___``. That
+  could potentially lead to wrong results in pathological corner
+  cases. See `issue 158
+  <https://github.com/zopefoundation/zope.interface/issues/158>`_.
+
 - Support the ``PURE_PYTHON`` environment variable at runtime instead
   of just at wheel build time. A value of 0 forces the C extensions to
   be used (even on PyPy) failing if they aren't present. Any other
diff --git a/src/zope/interface/declarations.py b/src/zope/interface/declarations.py
index cc20f5a..83f424c 100644
--- a/src/zope/interface/declarations.py
+++ b/src/zope/interface/declarations.py
@@ -111,6 +111,59 @@ class Declaration(Specification):
     __radd__ = __add__
 
 
+class _ImmutableDeclaration(Declaration):
+    # A Declaration that is immutable. Used as a singleton to
+    # return empty answers for things like ``implementedBy``.
+    # We have to define the actual singleton after normalizeargs
+    # is defined, and that in turn is defined after InterfaceClass and
+    # Implements.
+
+    __slots__ = ()
+
+    __instance = None
+
+    def __new__(cls):
+        if _ImmutableDeclaration.__instance is None:
+            _ImmutableDeclaration.__instance = object.__new__(cls)
+        return _ImmutableDeclaration.__instance
+
+    def __reduce__(self):
+        return "_empty"
+
+    @property
+    def __bases__(self):
+        return ()
+
+    @__bases__.setter
+    def __bases__(self, new_bases):
+        # We expect the superclass constructor to set ``self.__bases__ = ()``.
+        # Rather than attempt to special case that in the constructor and allow
+        # setting __bases__ only at that time, it's easier to just allow setting
+        # the empty tuple at any time. That makes ``x.__bases__ = x.__bases__`` a nice
+        # no-op too. (Skipping the superclass constructor altogether is a recipe
+        # for maintenance headaches.)
+        if new_bases != ():
+            raise TypeError("Cannot set non-empty bases on shared empty Declaration.")
+
+    @property
+    def dependents(self):
+        return {}
+
+    def changed(self, originally_changed):
+        # Does nothing, we have no dependents or dependencies
+        return
+
+    def interfaces(self):
+        # An empty iterator
+        return iter(())
+
+    def extends(self, interface, strict=True):
+        return False
+
+    def get(self, name, default=None):
+        return default
+
+
 ##############################################################################
 #
 # Implementation specifications
@@ -914,8 +967,6 @@ def _normalizeargs(sequence, output=None):
 
     return output
 
-# XXX: Declarations are mutable, allowing adjustments to their __bases__
-# so having one as a singleton may not be a great idea.
-_empty = Declaration() # type: Declaration
+_empty = _ImmutableDeclaration()
 
 objectSpecificationDescriptor = ObjectSpecificationDescriptor()
diff --git a/src/zope/interface/tests/test_declarations.py b/src/zope/interface/tests/test_declarations.py
index 887a5cb..5256b07 100644
--- a/src/zope/interface/tests/test_declarations.py
+++ b/src/zope/interface/tests/test_declarations.py
@@ -239,6 +239,59 @@ class DeclarationTests(unittest.TestCase):
         self.assertEqual(list(after), [IFoo, IBar, IBaz])
 
 
+class TestImmutableDeclaration(unittest.TestCase):
+
+    def _getTargetClass(self):
+        from zope.interface.declarations import _ImmutableDeclaration
+        return _ImmutableDeclaration
+
+    def _getEmpty(self):
+        from zope.interface.declarations import _empty
+        return _empty
+
+    def test_pickle(self):
+        import pickle
+        copied = pickle.loads(pickle.dumps(self._getEmpty()))
+        self.assertIs(copied, self._getEmpty())
+
+    def test_singleton(self):
+        self.assertIs(
+            self._getTargetClass()(),
+            self._getEmpty()
+        )
+
+    def test__bases__(self):
+        self.assertEqual(self._getEmpty().__bases__, ())
+
+    def test_change__bases__(self):
+        empty = self._getEmpty()
+        empty.__bases__ = ()
+        self.assertEqual(self._getEmpty().__bases__, ())
+
+        with self.assertRaises(TypeError):
+            empty.__bases__ = (1,)
+
+    def test_dependents(self):
+        empty = self._getEmpty()
+        deps = empty.dependents
+        self.assertEqual({}, deps)
+        # Doesn't change the return.
+        deps[1] = 2
+        self.assertEqual({}, empty.dependents)
+
+    def test_changed(self):
+        # Does nothing, has no visible side-effects
+        self._getEmpty().changed(None)
+
+    def test_extends_always_false(self):
+        self.assertFalse(self._getEmpty().extends(self))
+        self.assertFalse(self._getEmpty().extends(self, strict=True))
+        self.assertFalse(self._getEmpty().extends(self, strict=False))
+
+    def test_get_always_default(self):
+        self.assertIsNone(self._getEmpty().get('name'))
+        self.assertEqual(self._getEmpty().get('name', 42), 42)
+
 class TestImplements(unittest.TestCase):
 
     def _getTargetClass(self):