Make Declaration.__add__ try harder to produce consistent resolution orders.
By moving things from the RHS to the front of the list if they already extend something from the LHS. Fixes #193
This commit is contained in:
parent
4a686fc8d8
commit
eb542a8a75
|
@ -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)
|
||||
==================
|
||||
|
||||
|
|
|
@ -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']
|
||||
|
||||
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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):
|
||||
|
||||
|
|
Loading…
Reference in New Issue