diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/zope/interface/declarations.py | 35 | ||||
| -rw-r--r-- | src/zope/interface/tests/test_declarations.py | 50 |
2 files changed, 75 insertions, 10 deletions
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): |