Merge dbe4868 into 7d638c3

CHANGES.rst

@@ -5,6 +5,16 @@
 5.0.0 (unreleased)
 ==================
 
+- Adopt Python's standard `C3 resolution order
+  <https://www.python.org/download/releases/2.3/mro/>`_ for interface
+  linearization, with tweaks to support additional cases that are
+  common in interfaces but disallowed for Python classes.
+
+  In complex multiple-inheritance like scenerios, this may change the
+  interface resolution order, resulting in finding different adapters.
+  However, the results should make more sense. See `issue 21
+  <https://github.com/zopefoundation/zope.interface/issues/21>`_.
+
 - Make an internal singleton object returned by APIs like
   ``implementedBy`` and ``directlyProvidedBy`` immutable. Previously,
   it was fully mutable and allowed changing its ``__bases___``. That
@@ -152,6 +162,39 @@
 - Fix a potential interpreter crash in the low-level adapter
   registry lookup functions. See issue 11.
 
+- Use Python's standard C3 resolution order to compute the
+  ``__iro___`` and ``__sro___`` of interfaces. Previously, an ad-hoc
+  ordering that made no particular guarantees was used.
+
+  This has many beneficial properties, including the fact that base
+  interface and base classes tend to appear near the end of the
+  resolution order instead of the beginning. The resolution order in
+  general should be more predictable and consistent.
+
+  .. caution::
+     In some cases, especially with complex interface inheritance
+     trees or when manually providing or implementing interfaces, the
+     resulting IRO may be quite different. This may affect adapter
+     lookup.
+
+  The C3 order enforces some constraints in order to be able to
+  guarantee a sensible ordering. Older versions of zope.interface did
+  not impose similar constraints, so it was possible to create
+  interfaces and declarations that are inconsistent with the C3
+  constraints. In that event, zope.interface will still produce a
+  resolution order equal to the old order, but it won't be guaranteed
+  to be fully C3 compliant. In the future, strict enforcement of C3
+  order may be the default.
+
+  A set of environment variables and module constants allows
+  controlling several aspects of this new behaviour. It is possible to
+  request warnings about inconsistent resolution orders encountered,
+  and even to forbid them. Differences between the C3 resolution order
+  and the previous order can be logged, and, in extreme cases, the
+  previous order can still be used (this ability will be removed in
+  the future). For details, see the documentation for
+  ``zope.interface.ro``.
+
 4.7.2 (2020-03-10)
 ==================
 

appveyor.yml

@@ -26,10 +26,10 @@ install:
       }
   - ps: if (-not (Test-Path $env:PYTHON)) { throw "No $env:PYTHON" }
   - echo "C:\Program Files\Microsoft SDKs\Windows\v7.1\Bin\SetEnv.cmd" /x64 > "C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin\amd64\vcvars64.bat"
-  - pip install -e .
+  - python -m pip install -U pip setuptools wheel
+  - python -m pip install -U -e ".[test]"
 
 build_script:
-  - pip install wheel
   - python -W ignore setup.py -q bdist_wheel
 
 test_script:

docs/api/declarations.rst

@@ -592,7 +592,7 @@ Exmples for :meth:`Declaration.flattened`:
    >>> spec = Declaration(I4, spec)
    >>> i = spec.flattened()
    >>> [x.getName() for x in i]
-   ['I4', 'I2', 'I1', 'I3', 'Interface']
+   ['I4', 'I2', 'I3', 'I1', 'Interface']
    >>> list(i)
    []
 

docs/api/index.rst

@@ -12,3 +12,4 @@ Contents:
    adapters
    components
    common
+   ro

docs/api/ro.rst

@@ -0,0 +1,19 @@
+===========================================
+ Computing The Resolution Order (Priority)
+===========================================
+
+Just as Python classes have a method resolution order that determines
+which implementation of a method gets used when inheritance is used,
+interfaces have a resolution order that determines their ordering when
+searching for adapters.
+
+That order is computed by ``zope.interface.ro.ro``. This is an
+internal module not generally needed by a user of ``zope.interface``,
+but its documentation can be helpful to understand how orders are
+computed.
+
+``zope.interface.ro``
+=====================
+
+.. automodule:: zope.interface.ro
+   :member-order: alphabetical

setup.py

@@ -79,8 +79,11 @@ def _unavailable(self, e):
 else:
     ext_modules = codeoptimization
 tests_require = [
+    # The test dependencies should NOT have direct or transitive
+    # dependencies on zope.interface.
     'coverage >= 5.0.3',
     'zope.event',
+    'zope.testing',
 ]
 testing_extras = tests_require
 

src/zope/interface/common/__init__.py

@@ -121,19 +121,20 @@ def __init__(self, name, bases, attrs):
         # go ahead and give us a name to ease debugging.
         self.__name__ = name
         extra_classes = attrs.pop('extra_classes', ())
+        ignored_classes = attrs.pop('ignored_classes', ())
 
         if 'abc' not in attrs:
             # Something like ``IList(ISequence)``: We're extending
             # abc interfaces but not an ABC interface ourself.
-            self.__class__ = InterfaceClass
             InterfaceClass.__init__(self, name, bases, attrs)
-            for cls in extra_classes:
-                classImplements(cls, self)
+            ABCInterfaceClass.__register_classes(self, extra_classes, ignored_classes)
+            self.__class__ = InterfaceClass
             return
 
         based_on = attrs.pop('abc')
         self.__abc = based_on
         self.__extra_classes = tuple(extra_classes)
+        self.__ignored_classes = tuple(ignored_classes)
 
         assert name[1:] == based_on.__name__, (name, based_on)
         methods = {
@@ -216,11 +217,14 @@ def __method_from_function(self, function, name):
         method.positional = method.positional[1:]
         return method
 
-    def __register_classes(self):
+    def __register_classes(self, conformers=None, ignored_classes=None):
         # Make the concrete classes already present in our ABC's registry
         # declare that they implement this interface.
-
-        for cls in self.getRegisteredConformers():
+        conformers = conformers if conformers is not None else self.getRegisteredConformers()
+        ignored = ignored_classes if ignored_classes is not None else self.__ignored_classes
+        for cls in conformers:
+            if cls in ignored:
+                continue
             classImplements(cls, self)
 
     def getABC(self):

src/zope/interface/common/builtins.py

@@ -37,7 +37,6 @@
 ]
 
 # pylint:disable=no-self-argument
-
 class IList(collections.IMutableSequence):
     """
     Interface for :class:`list`

src/zope/interface/common/collections.py

@@ -177,6 +177,10 @@ class ISequence(IReversible,
                 ICollection):
     abc = abc.Sequence
     extra_classes = (UserString,)
+    # On Python 2, basestring is registered as an ISequence, and
+    # its subclass str is an IByteString. If we also register str as
+    # an ISequence, that tends to lead to inconsistent resolution order.
+    ignored_classes = (basestring,) if str is bytes else () # pylint:disable=undefined-variable
 
     @optional
     def __reversed__():

src/zope/interface/common/mapping.py

@@ -43,7 +43,7 @@ def __getitem__(key):
         """
 
 
-class IReadMapping(IItemMapping, collections.IContainer):
+class IReadMapping(collections.IContainer, IItemMapping):
     """
     Basic mapping interface.
 
@@ -72,7 +72,7 @@ def __setitem__(key, value):
         """Set a new item in the mapping."""
 
 
-class IEnumerableMapping(IReadMapping, collections.ISized):
+class IEnumerableMapping(collections.ISized, IReadMapping):
     """
     Mapping objects whose items can be enumerated.
 
@@ -171,7 +171,7 @@ def popitem():
 
 class IFullMapping(
         collections.IMutableMapping,
-        IExtendedReadMapping, IExtendedWriteMapping, IClonableMapping, IMapping):
+        IExtendedReadMapping, IExtendedWriteMapping, IClonableMapping, IMapping,):
     """
     Full mapping interface.
 

src/zope/interface/common/tests/__init__.py

@@ -38,7 +38,8 @@ def iter_abc_interfaces(predicate=lambda iface: True):
         if not predicate(iface):
             continue
 
-        registered = list(iface.getRegisteredConformers())
+        registered = set(iface.getRegisteredConformers())
+        registered -= set(iface._ABCInterfaceClass__ignored_classes)
         if registered:
             yield iface, registered
 
@@ -50,24 +51,46 @@ def predicate(iface):
 
 
 def add_verify_tests(cls, iface_classes_iter):
+    cls.maxDiff = None
     for iface, registered_classes in iface_classes_iter:
         for stdlib_class in registered_classes:
-
             def test(self, stdlib_class=stdlib_class, iface=iface):
                 if stdlib_class in self.UNVERIFIABLE or stdlib_class.__name__ in self.UNVERIFIABLE:
                     self.skipTest("Unable to verify %s" % stdlib_class)
 
                 self.assertTrue(self.verify(iface, stdlib_class))
 
-            name = 'test_auto_' + stdlib_class.__name__ + '_' + iface.__name__
+            suffix = "%s_%s_%s" % (
+                stdlib_class.__name__,
+                iface.__module__.replace('.', '_'),
+                iface.__name__
+            )
+            name = 'test_auto_' + suffix
             test.__name__ = name
-            assert not hasattr(cls, name)
+            assert not hasattr(cls, name), (name, list(cls.__dict__))
             setattr(cls, name, test)
 
+            def test_ro(self, stdlib_class=stdlib_class, iface=iface):
+                from zope.interface import ro
+                from zope.interface import implementedBy
+                self.assertEqual(
+                    tuple(ro.ro(iface, strict=True)),
+                    iface.__sro__)
+                implements = implementedBy(stdlib_class)
+                strict = stdlib_class not in self.NON_STRICT_RO
+                self.assertEqual(
+                    tuple(ro.ro(implements, strict=strict)),
+                    implements.__sro__)
+
+            name = 'test_auto_ro_' + suffix
+            test_ro.__name__ = name
+            assert not hasattr(cls, name)
+            setattr(cls, name, test_ro)
 
 class VerifyClassMixin(unittest.TestCase):
     verifier = staticmethod(verifyClass)
     UNVERIFIABLE = ()
+    NON_STRICT_RO = ()
 
     def _adjust_object_before_verify(self, iface, x):
         return x

src/zope/interface/common/tests/test_collections.py

@@ -17,6 +17,7 @@
 except ImportError:
     import collections as abc
 from collections import deque
+from collections import OrderedDict
 
 
 try:
@@ -118,6 +119,9 @@ def test_non_iterable_UserDict(self):
             type({}.viewitems()),
             type({}.viewkeys()),
         })
+        NON_STRICT_RO = {
+            OrderedDict
+        }
 
 add_abc_interface_tests(TestVerifyClass, collections.ISet.__module__)
 

src/zope/interface/declarations.py

@@ -458,22 +458,34 @@ def classImplements(cls, *interfaces):
       are added to any interfaces previously declared.
     """
     spec = implementedBy(cls)
-    spec.declared += tuple(_normalizeargs(interfaces))
+    interfaces = tuple(_normalizeargs(interfaces))
+    append = True
+    if len(interfaces) == 1:
+        # In the common case of a single interface, take steps to try
+        # to avoid producing an invalid resolution order, while
+        # still allowing for BWC (in the past, we always appended)
+        for b in spec.declared:
+            if interfaces[0].extends(b):
+                append = False
+                break
+    if append:
+        spec.declared += interfaces
+    else:
+        spec.declared = interfaces + spec.declared
 
     # compute the bases
     bases = []
-    seen = {}
+    seen = set()
     for b in spec.declared:
         if b not in seen:
-            seen[b] = 1
+            seen.add(b)
             bases.append(b)
 
     if spec.inherit is not None:
-
         for c in spec.inherit.__bases__:
             b = implementedBy(c)
             if b not in seen:
-                seen[b] = 1
+                seen.add(b)
                 bases.append(b)
 
     spec.__bases__ = tuple(bases)
@@ -664,6 +676,13 @@ def __init__(self, cls, *interfaces):
         self._cls = cls
         Declaration.__init__(self, *(interfaces + (implementedBy(cls), )))
 
+    def __repr__(self):
+        return "<%s.%s for %s>" % (
+            self.__class__.__module__,
+            self.__class__.__name__,
+            self._cls,
+        )
+
     def __reduce__(self):
         return Provides, self.__args
 
@@ -794,6 +813,13 @@ def __init__(self, cls, metacls, *interfaces):
         self.__args = (cls, metacls, ) + interfaces
         Declaration.__init__(self, *(interfaces + (implementedBy(metacls), )))
 
+    def __repr__(self):
+        return "<%s.%s for %s>" % (
+            self.__class__.__module__,
+            self.__class__.__name__,
+            self._cls,
+        )
+
     def __reduce__(self):
         return self.__class__, self.__args
 

src/zope/interface/interface.py

@@ -283,7 +283,14 @@ def changed(self, originally_changed):
         implied = self._implied
         implied.clear()
 
-        ancestors = ro(self)
+        if len(self.__bases__) == 1:
+            # Fast path: One base makes it trivial to calculate
+            # the MRO.
+            sro = self.__bases__[0].__sro__
+            ancestors = [self]
+            ancestors.extend(sro)
+        else:
+            ancestors = ro(self)
 
         try:
             if Interface not in ancestors:

src/zope/interface/registry.py

@@ -550,7 +550,7 @@ def _getAdapterRequired(factory, required):
                 r = implementedBy(r)
             else:
                 raise TypeError("Required specification must be a "
-                                "specification or class."
+                                "specification or class, not %r" % type(r)
                                 )
         result.append(r)
     return tuple(result)