Make the RO for InterfaceClass consistent and fix handling of the STRICT_IRO env variable.issue192-issue194

Fixes #192 and fixes #194.

Also fix the IRO for OrderedDict on CPython 2

9 files changed, 94 insertions, 27 deletions

diff --git a/.travis.yml b/.travis.yml
index 4f47445..cb9713a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -2,6 +2,7 @@ language: python
 
 env:
   global:
+    ZOPE_INTERFACE_STRICT_IRO: 1
     TWINE_USERNAME: zope.wheelbuilder
     TWINE_PASSWORD:
       secure: "AyR5QxUuZKdmywiepdBG0r8hgFQfaEf2hTxOg/HiXisVNcs1sUPjephBP4MqQBbf1/qxLM95F6Mw3sKneO3gDDQSGCsmvY1MWlDc+6R5TgqVBuOoONji5zGQH7v9RR8IPOyple8BNlFePl1hQ8r0dOT1U6rDVh5FkNJgHYE9OJ4="
@@ -26,6 +27,7 @@ jobs:
         - sphinx-build -b html -d docs/_build/doctrees docs docs/_build/html
         - sphinx-build -b doctest -d docs/_build/doctrees docs docs/_build/doctest
       after_success:
+      env: ZOPE_INTERFACE_STRICT_IRO=0
 
     - name: CPython No C Extension
       env: PURE_PYTHON=1
diff --git a/CHANGES.rst b/CHANGES.rst
index 7e51290..a41b17b 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,7 +5,16 @@
 5.0.1 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Ensure the resolution order for ``InterfaceClass`` is consistent.
+  See `issue 192 <https://github.com/zopefoundation/zope.interface/issues/192>`_.
+
+- Ensure the resolution order for ``collections.OrderedDict`` is
+  consistent on CPython 2. (It was already consistent on Python 3 and PyPy).
+
+- Fix the handling of the ``ZOPE_INTERFACE_STRICT_IRO`` environment
+  variable. Previously, ``ZOPE_INTERFACE_STRICT_RO`` was read, in
+  contrast with the documentation. See `issue 194
+  <https://github.com/zopefoundation/zope.interface/issues/194>`_.
 
 
 5.0.0 (2020-03-19)
diff --git a/src/zope/interface/common/collections.py b/src/zope/interface/common/collections.py
index 6c0496e..00e2b8c 100644
--- a/src/zope/interface/common/collections.py
+++ b/src/zope/interface/common/collections.py
@@ -41,7 +41,7 @@ try:
     from collections import abc
 except ImportError:
     import collections as abc
-
+from collections import OrderedDict
 try:
     # On Python 3, all of these extend the appropriate collection ABC,
     # but on Python 2, UserDict does not (though it is registered as a
@@ -58,7 +58,6 @@ except ImportError:
     from UserDict import IterableUserDict as UserDict
     from UserString import UserString
 
-
 from zope.interface._compat import PYTHON2 as PY2
 from zope.interface._compat import PYTHON3 as PY3
 from zope.interface.common import ABCInterface
@@ -221,7 +220,12 @@ class IMutableSet(ISet):
 
 class IMapping(ICollection):
     abc = abc.Mapping
-
+    extra_classes = (dict,)
+    # OrderedDict is a subclass of dict. On CPython 2,
+    # it winds up registered as a IMutableMapping, which
+    # produces an inconsistent IRO if we also try to register it
+    # here.
+    ignored_classes = (OrderedDict,)
     if PY2:
         @optional
         def __eq__(other):
@@ -234,8 +238,8 @@ class IMapping(ICollection):
 
 class IMutableMapping(IMapping):
     abc = abc.MutableMapping
-    extra_classes = (UserDict,)
-
+    extra_classes = (dict, UserDict,)
+    ignored_classes = (OrderedDict,)
 
 class IMappingView(ISized):
     abc = abc.MappingView
diff --git a/src/zope/interface/common/tests/test_collections.py b/src/zope/interface/common/tests/test_collections.py
index f06e12e..81eea0e 100644
--- a/src/zope/interface/common/tests/test_collections.py
+++ b/src/zope/interface/common/tests/test_collections.py
@@ -120,7 +120,6 @@ class TestVerifyClass(VerifyClassMixin, unittest.TestCase):
             type({}.viewkeys()),
         })
         NON_STRICT_RO = {
-            OrderedDict
         }
 
 add_abc_interface_tests(TestVerifyClass, collections.ISet.__module__)
diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py
index e654b27..6a1ca2a 100644
--- a/src/zope/interface/interface.py
+++ b/src/zope/interface/interface.py
@@ -651,7 +651,8 @@ class _InterfaceMetaClass(type):
 
 _InterfaceClassBase = _InterfaceMetaClass(
     'InterfaceClass',
-    (InterfaceBase, Element, Specification),
+    # From least specific to most specific.
+    (InterfaceBase, Specification, Element),
     {'__slots__': ()}
 )
 
@@ -1040,7 +1041,7 @@ def fromMethod(meth, interface=None, name=None):
 # Now we can create the interesting interfaces and wire them up:
 def _wire():
     from zope.interface.declarations import classImplements
-
+    # From lest specific to most specific.
     from zope.interface.interfaces import IElement
     classImplements(Element, IElement)
 
@@ -1050,11 +1051,12 @@ def _wire():
     from zope.interface.interfaces import IMethod
     classImplements(Method, IMethod)
 
+    from zope.interface.interfaces import ISpecification
+    classImplements(Specification, ISpecification)
+
     from zope.interface.interfaces import IInterface
     classImplements(InterfaceClass, IInterface)
 
-    from zope.interface.interfaces import ISpecification
-    classImplements(Specification, ISpecification)
 
 # We import this here to deal with module dependencies.
 # pylint:disable=wrong-import-position
diff --git a/src/zope/interface/ro.py b/src/zope/interface/ro.py
index 7cb4f55..20338ed 100644
--- a/src/zope/interface/ro.py
+++ b/src/zope/interface/ro.py
@@ -163,6 +163,14 @@ class InconsistentResolutionOrderError(TypeError):
         )
 
 
+class _NamedBool(int): # cannot actually inherit bool
+
+    def __new__(cls, val, name):
+        inst = super(cls, _NamedBool).__new__(cls, val)
+        inst.__name__ = name
+        return inst
+
+
 class _ClassBoolFromEnv(object):
     """
     Non-data descriptor that reads a transformed environment variable
@@ -184,6 +192,7 @@ class _ClassBoolFromEnv(object):
 
         env_name = 'ZOPE_INTERFACE_' + my_name
         val = os.environ.get(env_name, '') == '1'
+        val = _NamedBool(val, my_name)
         setattr(klass, my_name, val)
         setattr(klass, 'ORIG_' + my_name, self)
         return val
@@ -208,7 +217,7 @@ class C3(object):
 
     @staticmethod
     def resolver(C, strict, base_mros):
-        strict = strict if strict is not None else C3.STRICT_RO
+        strict = strict if strict is not None else C3.STRICT_IRO
         factory = C3
         if strict:
             factory = _StrictC3
@@ -263,7 +272,7 @@ class C3(object):
         return list(self.__legacy_ro)
 
     TRACK_BAD_IRO = _ClassBoolFromEnv()
-    STRICT_RO = _ClassBoolFromEnv()
+    STRICT_IRO = _ClassBoolFromEnv()
     WARN_BAD_IRO = _ClassBoolFromEnv()
     LOG_CHANGED_IRO = _ClassBoolFromEnv()
     USE_LEGACY_IRO = _ClassBoolFromEnv()
diff --git a/src/zope/interface/tests/test_interfaces.py b/src/zope/interface/tests/test_interfaces.py
index 285d857..3f9a504 100644
--- a/src/zope/interface/tests/test_interfaces.py
+++ b/src/zope/interface/tests/test_interfaces.py
@@ -93,3 +93,36 @@ class UnregisteredTests(unittest.TestCase,
         from zope.interface.interfaces import IUnregistered
         from zope.interface.verify import verifyObject
         verifyObject(IUnregistered, self._makeOne())
+
+
+class InterfaceClassTests(unittest.TestCase):
+
+    def _getTargetClass(self):
+        from zope.interface.interface import InterfaceClass
+        return InterfaceClass
+
+    def _getTargetInterface(self):
+        from zope.interface.interfaces import IInterface
+        return IInterface
+
+    def _makeOne(self):
+        from zope.interface.interface import Interface
+        return Interface
+
+    def test_class_conforms(self):
+        from zope.interface.verify import verifyClass
+        verifyClass(self._getTargetInterface(), self._getTargetClass())
+
+    def test_instance_conforms(self):
+        from zope.interface.verify import verifyObject
+        verifyObject(self._getTargetInterface(), self._makeOne())
+
+    def test_instance_consistent__iro__(self):
+        from zope.interface import ro
+        self.assertTrue(ro.is_consistent(self._getTargetInterface()))
+
+    def test_class_consistent__iro__(self):
+        from zope.interface import ro
+        from zope.interface import implementedBy
+
+        self.assertTrue(ro.is_consistent(implementedBy(self._getTargetClass())))
diff --git a/src/zope/interface/tests/test_ro.py b/src/zope/interface/tests/test_ro.py
index ce0a6f4..61f92b6 100644
--- a/src/zope/interface/tests/test_ro.py
+++ b/src/zope/interface/tests/test_ro.py
@@ -176,6 +176,20 @@ class Test_ro(unittest.TestCase):
              implementedBy(object)])
 
 
+class C3Setting(object):
+
+    def __init__(self, setting, value):
+        self._setting = setting
+        self._value = value
+
+    def __enter__(self):
+        from zope.interface import ro
+        setattr(ro.C3, self._setting.__name__, self._value)
+
+    def __exit__(self, t, v, tb):
+        from zope.interface import ro
+        setattr(ro.C3, self._setting.__name__, self._setting)
+
 class Test_c3_ro(Test_ro):
 
     def setUp(self):
@@ -302,7 +316,7 @@ Object <InterfaceClass zope.interface.tests.test_ro.A> has different legacy and
 
         # We were able to resolve it, and in exactly the same way as
         # the legacy RO did, even though it is inconsistent.
-        result = self._callFUT(ExtendedPathIndex, log_changed_ro=True)
+        result = self._callFUT(ExtendedPathIndex, log_changed_ro=True, strict=False)
         self.assertEqual(result, [
             ExtendedPathIndex,
             ILimitedResultIndex,
@@ -348,30 +362,22 @@ Object <InterfaceClass zope.interface.tests.test_ro.A> has different legacy and
 
         with warnings.catch_warnings():
             warnings.simplefilter('error')
-            orig_val = ro.C3.WARN_BAD_IRO
-            ro.C3.WARN_BAD_IRO = True
-            try:
+            with C3Setting(ro.C3.WARN_BAD_IRO, True), C3Setting(ro.C3.STRICT_IRO, False):
                 with self.assertRaises(ro.InconsistentResolutionOrderWarning):
                     super(Test_c3_ro, self).test_non_orderable()
-            finally:
-                ro.C3.WARN_BAD_IRO = orig_val
 
         IOErr, _ = self._make_IOErr()
         with self.assertRaises(ro.InconsistentResolutionOrderError):
             self._callFUT(IOErr, strict=True)
 
-        old_val = ro.C3.TRACK_BAD_IRO
-        try:
-            ro.C3.TRACK_BAD_IRO = True
+        with C3Setting(ro.C3.TRACK_BAD_IRO, True), C3Setting(ro.C3.STRICT_IRO, False):
             with warnings.catch_warnings():
                 warnings.simplefilter('ignore')
                 self._callFUT(IOErr)
             self.assertIn(IOErr, ro.C3.BAD_IROS)
-        finally:
-            ro.C3.TRACK_BAD_IRO = old_val
 
-        iro = self._callFUT(IOErr)
-        legacy_iro = self._callFUT(IOErr, use_legacy_ro=True)
+        iro = self._callFUT(IOErr, strict=False)
+        legacy_iro = self._callFUT(IOErr, use_legacy_ro=True, strict=False)
         self.assertEqual(iro, legacy_iro)
 
 
diff --git a/tox.ini b/tox.ini
index be90fe3..0b366cb 100644
--- a/tox.ini
+++ b/tox.ini
@@ -14,7 +14,8 @@ usedevelop = true
 commands =
     coverage run -p -m unittest discover -s src
 extras = test
-
+setenv =
+    ZOPE_INTERFACE_STRICT_IRO=1
 
 [testenv:py27-pure]
 setenv =
@@ -55,3 +56,5 @@ commands =
 deps =
     Sphinx
     repoze.sphinx.autointerface
+setenv =
+    ZOPE_INTERFACE_STRICT_IRO=0