diff options
author | Jason Madden <jamadden@gmail.com> | 2021-04-15 04:42:53 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-04-15 04:42:53 -0500 |
commit | 253456fe1781bdf528b51c28b0341f840a35f2c6 (patch) | |
tree | 6f3ce310979aa71f945db961159eabff65413567 | |
parent | 4a686fc8d87d398045dc44c1b6a97a2940121800 (diff) | |
parent | eb542a8a75f93de7f1accbb448820c533bdb4ee3 (diff) | |
download | zope-interface-253456fe1781bdf528b51c28b0341f840a35f2c6.tar.gz |
Merge pull request #238 from zopefoundation/issue193
Make Declaration.__add__ try harder to produce consistent resolution orders
-rw-r--r-- | CHANGES.rst | 8 | ||||
-rw-r--r-- | docs/api/declarations.rst | 32 | ||||
-rw-r--r-- | src/zope/interface/declarations.py | 35 | ||||
-rw-r--r-- | src/zope/interface/tests/test_declarations.py | 50 |
4 files changed, 101 insertions, 24 deletions
diff --git a/CHANGES.rst b/CHANGES.rst index 600fbb8..5b57ddb 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,6 +12,14 @@ shown as ``classImplements(list, IMutableSequence, IIterable)``. See `issue 236 <https://github.com/zopefoundation/zope.interface/issues/236>`_. +- Make ``Declaration.__add__`` (as in ``implementedBy(Cls) + + ISomething``) try harder to preserve a consistent resolution order + when the two arguments share overlapping pieces of the interface + inheritance hierarchy. Previously, the right hand side was always + put at the end of the resolution order, which could easily produce + invalid orders. See `issue 193 + <https://github.com/zopefoundation/zope.interface/issues/193>`_. + 5.3.0 (2020-03-21) ================== diff --git a/docs/api/declarations.rst b/docs/api/declarations.rst index cdbf815..4327c72 100644 --- a/docs/api/declarations.rst +++ b/docs/api/declarations.rst @@ -763,34 +763,38 @@ Exmples for :meth:`Declaration.__add__`: .. doctest:: >>> from zope.interface import Interface - >>> class I1(Interface): pass + >>> class IRoot1(Interface): pass ... - >>> class I2(I1): pass + >>> class IDerived1(IRoot1): pass ... - >>> class I3(Interface): pass + >>> class IRoot2(Interface): pass ... - >>> class I4(I3): pass + >>> class IDerived2(IRoot2): pass ... >>> spec = Declaration() >>> [iface.getName() for iface in spec] [] - >>> [iface.getName() for iface in spec+I1] - ['I1'] - >>> [iface.getName() for iface in I1+spec] - ['I1'] + >>> [iface.getName() for iface in spec+IRoot1] + ['IRoot1'] + >>> [iface.getName() for iface in IRoot1+spec] + ['IRoot1'] >>> spec2 = spec - >>> spec += I1 + >>> spec += IRoot1 >>> [iface.getName() for iface in spec] - ['I1'] + ['IRoot1'] >>> [iface.getName() for iface in spec2] [] - >>> spec2 += Declaration(I3, I4) + >>> spec2 += Declaration(IRoot2, IDerived2) >>> [iface.getName() for iface in spec2] - ['I3', 'I4'] + ['IDerived2', 'IRoot2'] >>> [iface.getName() for iface in spec+spec2] - ['I1', 'I3', 'I4'] + ['IRoot1', 'IDerived2', 'IRoot2'] >>> [iface.getName() for iface in spec2+spec] - ['I3', 'I4', 'I1'] + ['IDerived2', 'IRoot2', 'IRoot1'] + >>> [iface.getName() for iface in (spec+spec2).__bases__] + ['IRoot1', 'IDerived2', 'IRoot2'] + >>> [iface.getName() for iface in (spec2+spec).__bases__] + ['IDerived2', 'IRoot2', 'IRoot1'] diff --git a/src/zope/interface/declarations.py b/src/zope/interface/declarations.py index 935b026..de90074 100644 --- a/src/zope/interface/declarations.py +++ b/src/zope/interface/declarations.py @@ -115,20 +115,35 @@ class Declaration(Specification): ]) def __add__(self, other): - """Add two specifications or a specification and an interface """ - seen = {} - result = [] - for i in self.interfaces(): - seen[i] = 1 - result.append(i) + Add two specifications or a specification and an interface + and produce a new declaration. + + .. versionchanged:: 5.4.0 + Now tries to preserve a consistent resolution order. Interfaces + being added to this object are added to the front of the resulting resolution + order if they already extend an interface in this object. Previously, + they were always added to the end of the order, which easily resulted in + invalid orders. + """ + before = [] + result = list(self.interfaces()) + seen = set(result) for i in other.interfaces(): - if i not in seen: - seen[i] = 1 + if i in seen: + continue + seen.add(i) + if any(i.extends(x) for x in result): + # It already extends us, e.g., is a subclass, + # so it needs to go at the front of the RO. + before.append(i) + else: result.append(i) + return Declaration(*(before + result)) - return Declaration(*result) - + # XXX: Is __radd__ needed? No tests break if it's removed. + # If it is needed, does it need to handle the C3 ordering differently? + # I (JAM) don't *think* it does. __radd__ = __add__ @staticmethod diff --git a/src/zope/interface/tests/test_declarations.py b/src/zope/interface/tests/test_declarations.py index 8efe2d8..04a0dca 100644 --- a/src/zope/interface/tests/test_declarations.py +++ b/src/zope/interface/tests/test_declarations.py @@ -289,6 +289,56 @@ class DeclarationTests(EmptyDeclarationTests): after = before + other self.assertEqual(list(after), [IFoo, IBar, IBaz]) + def test___add___overlapping_interface(self): + # The derived interfaces end up with higher priority, and + # don't produce a C3 resolution order violation. This + # example produced a C3 error, and the resulting legacy order + # used to be wrong ([IBase, IDerived] instead of + # the other way). + from zope.interface import Interface + from zope.interface.interface import InterfaceClass + from zope.interface.tests.test_ro import C3Setting + from zope.interface import ro + + IBase = InterfaceClass('IBase') + IDerived = InterfaceClass('IDerived', (IBase,)) + + with C3Setting(ro.C3.STRICT_IRO, True): + base = self._makeOne(IBase) + after = base + IDerived + + self.assertEqual(after.__iro__, (IDerived, IBase, Interface)) + self.assertEqual(after.__bases__, (IDerived, IBase)) + self.assertEqual(list(after), [IDerived, IBase]) + + def test___add___overlapping_interface_implementedBy(self): + # Like test___add___overlapping_interface, but pulling + # in a realistic example. This one previously produced a + # C3 error, but the resulting legacy order was (somehow) + # correct. + from zope.interface import Interface + from zope.interface import implementedBy + from zope.interface import implementer + from zope.interface.tests.test_ro import C3Setting + from zope.interface import ro + + class IBase(Interface): + pass + + class IDerived(IBase): + pass + + @implementer(IBase) + class Base(object): + pass + + with C3Setting(ro.C3.STRICT_IRO, True): + after = implementedBy(Base) + IDerived + + self.assertEqual(after.__sro__, (after, IDerived, IBase, Interface)) + self.assertEqual(after.__bases__, (IDerived, IBase)) + self.assertEqual(list(after), [IDerived, IBase]) + class TestImmutableDeclaration(EmptyDeclarationTests): |