Make the resolution order of all fields consistent.

.gitignore

@@ -1,4 +1,5 @@
 *.pyc
+*.pyo
 __pycache__
 .installed.cfg
 bin

.travis.yml

@@ -2,21 +2,17 @@ language: python
 sudo: false
 python:
  - 2.7
- - 3.4
  - 3.5
  - 3.6
+ - 3.7
+ - 3.8
  - pypy
  - pypy3
-matrix:
- include:
- - python: "3.7"
- dist: xenial
- sudo: true
 
 script:
  - coverage run -m zope.testrunner --test-path=src
- - sphinx-build  -b html -d docs/_build/doctrees docs docs/_build/html
- - coverage run -a `which sphinx-build` -b doctest -d docs/_build/doctrees docs docs/_build/doctest
+ - sphinx-build -b html -d docs/_build/doctrees docs docs/_build/html
+ - coverage run -a -m sphinx -b doctest -d docs/_build/doctrees docs docs/_build/doctest
 
 after_success:
  - coveralls

CHANGES.rst

@@ -2,11 +2,19 @@
  Changes
 =========
 
-5.0.2 (unreleased)
+6.0.0 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Require zope.interface 5.0.
 
+- Ensure the resolution orders of all fields are consistent and make
+ sense. In particular, ``Bool`` fields now correctly implement
+ ``IBool`` before ``IFromUnicode``. See `issue 80
+ <https://github.com/zopefoundation/zope.schema/issues/80>`_.
+
+- Add support for Python 3.8.
+
+- Drop support for Python 3.4.
 
 5.0.1 (2020-03-06)
 ==================

setup.py

@@ -29,7 +29,7 @@ def read(*rnames):
 
 REQUIRES = [
  'setuptools',
- 'zope.interface >= 3.6.0',
+ 'zope.interface >= 5.0.0',
  'zope.event',
 ]
 
@@ -64,10 +64,10 @@ def read(*rnames):
  "Programming Language :: Python :: 2",
  "Programming Language :: Python :: 2.7",
  "Programming Language :: Python :: 3",
- "Programming Language :: Python :: 3.4",
  "Programming Language :: Python :: 3.5",
  "Programming Language :: Python :: 3.6",
  "Programming Language :: Python :: 3.7",
+ "Programming Language :: Python :: 3.8",
  "Programming Language :: Python :: Implementation :: CPython",
  "Programming Language :: Python :: Implementation :: PyPy",
  "Framework :: Zope :: 3",

src/zope/schema/_field.py

@@ -29,7 +29,9 @@
 
 
 from zope.interface import classImplements
+from zope.interface import classImplementsFirst
 from zope.interface import implementer
+from zope.interface import implementedBy
 from zope.interface.interfaces import IInterface
 
 
@@ -135,23 +137,42 @@
 MinMaxLen.min_length = FieldProperty(IMinMaxLen['min_length'])
 MinMaxLen.max_length = FieldProperty(IMinMaxLen['max_length'])
 
-classImplements(Text, IText)
-classImplements(TextLine, ITextLine)
-classImplements(Password, IPassword)
-classImplements(Bool, IBool)
-classImplements(Iterable, IIterable)
-classImplements(Container, IContainer)
-
-classImplements(Number, INumber)
-classImplements(Complex, IComplex)
-classImplements(Real, IReal)
-classImplements(Rational, IRational)
-classImplements(Integral, IIntegral)
-classImplements(Int, IInt)
-classImplements(Decimal, IDecimal)
-
-classImplements(Object, IObject)
-
+classImplementsFirst(Text, IText)
+classImplementsFirst(TextLine, ITextLine)
+classImplementsFirst(Password, IPassword)
+classImplementsFirst(Bool, IBool)
+classImplementsFirst(Iterable, IIterable)
+classImplementsFirst(Container, IContainer)
+
+classImplementsFirst(Number, INumber)
+classImplementsFirst(Complex, IComplex)
+classImplementsFirst(Real, IReal)
+classImplementsFirst(Rational, IRational)
+classImplementsFirst(Integral, IIntegral)
+classImplementsFirst(Int, IInt)
+classImplementsFirst(Decimal, IDecimal)
+
+classImplementsFirst(Object, IObject)
+
+
+class implementer_if_needed(object):
+ # Helper to make sure we don't redundantly implement
+ # interfaces already inherited. Doing so tends to produce
+ # problems with the C3 order. This is used when we cannot
+ # statically determine if we need the interface or not, e.g,
+ # because we're picking different base classes under some circumstances.
+ def __init__(self, *ifaces):
+ self._ifaces = ifaces
+
+ def __call__(self, cls):
+ ifaces_needed = []
+ implemented = implementedBy(cls)
+ ifaces_needed = [
+ iface
+ for iface in self._ifaces
+ if not implemented.isOrExtends(iface)
+ ]
+ return implementer(*ifaces_needed)(cls)
 
 
 @implementer(ISourceText)
@@ -176,7 +197,7 @@ def fromBytes(self, value):
  return value
 
 
-@implementer(INativeString, IFromUnicode, IFromBytes)
+@implementer_if_needed(INativeString, IFromUnicode, IFromBytes)
 class NativeString(Text if PY3 else Bytes):
  """
  A native string is always the type `str`.
@@ -219,7 +240,7 @@ def constraint(self, value):
  return b'\n' not in value
 
 
-@implementer(INativeStringLine, IFromUnicode, IFromBytes)
+@implementer_if_needed(INativeStringLine, IFromUnicode, IFromBytes)
 class NativeStringLine(TextLine if PY3 else BytesLine):
  """
  A native string is always the type `str`; this field excludes
@@ -462,7 +483,10 @@ def _validate(self, value):
  raise ConstraintNotSatisfied(value, self.__name__).with_field_and_value(self, value)
 
 
-@implementer(IFromUnicode, IFromBytes)
+# Both of these are inherited from the parent; re-declaring them
+# here messes with the __sro__ of subclasses, causing them to be
+# inconsistent with C3.
+# @implementer(IFromUnicode, IFromBytes)
 class _StrippedNativeStringLine(NativeStringLine):
 
  _invalid_exc_type = None

src/zope/schema/accessors.py

@@ -36,25 +36,59 @@ class IMyInterface(Interface):
 
 from zope.interface import providedBy, implementedBy
 from zope.interface.interface import Method
+from zope.interface.declarations import Declaration
 
 
 class FieldReadAccessor(Method):
  """Field read accessor
  """
 
- # A read field accessor is a method and a field.
- # A read accessor is a decorator of a field, using the given
- # fields properties to provide meta data.
-
- def __provides__(self):
- return providedBy(self.field) + implementedBy(FieldReadAccessor)
- __provides__ = property(__provides__)
-
  def __init__(self, field):
  self.field = field
  Method.__init__(self, '')
  self.__doc__ = 'get %s' % field.__doc__
 
+ # A read field accessor is a method and a field.
+ # A read accessor is a decorator of a field, using the given
+ # field's properties to provide meta data.
+
+ @property
+ def __provides__(self):
+ provided = providedBy(self.field)
+ implemented = implementedBy(FieldReadAccessor)
+
+ # Declaration.__add__ is not very smart in zope.interface 5.0.0.
+ # It's very easy to produce C3 inconsistent orderings using
+ # it, because it uses itself plus any new interfaces from the
+ # second argument as the ``__bases__``, ignoring their
+ # relative order.
+ #
+ # Here, we can easily work around that. We know that ``field``
+ # will be some sub-class of Attribute, just as we are
+ # (FieldReadAccessor <- Method <- Attribute). So there will be
+ # overlap, and commonly only IMethod would be added to the end
+ # of the list of bases; but since IMethod extends IAttribute,
+ # having IAttribute earlier in the bases will be inconsistent.
+ # The fix here is to remove those duplicates from the first
+ # element so that we don't get into that situation.
+ provided_list = list(provided)
+ for iface in implemented:
+ if iface in provided_list:
+ provided_list.remove(iface)
+ provided = Declaration(*provided_list)
+ try:
+ return provided + implemented
+ except BaseException as e: # pragma: no cover pylint:disable=broad-except
+ # Sadly, zope.interface catches and silently ignores
+ # any exceptions raised in ``__providedBy__``,
+ # which is the class descriptor that invokes ``__provides__``.
+ # So, for example, if we're in strict C3 mode and fail to produce
+ # a resolution order, that gets ignored and we fallback to just what's
+ # implemented by the class.
+ # That's not good. Do our best to propagate the exception by returning it.
+ # There will be downstream errors later.
+ return e
+
  def getSignatureString(self):
  return '()'
 

src/zope/schema/tests/test__bootstrapfields.py

@@ -17,8 +17,9 @@
 import unicodedata
 
 # pylint:disable=protected-access,inherit-non-class,blacklisted-name
+# pylint:disable=attribute-defined-outside-init
 
-class EqualityTestsMixin(object):
+class InterfaceConformanceTestsMixin(object):
 
  def _getTargetClass(self):
  raise NotImplementedError
@@ -54,6 +55,26 @@ def test_instance_conforms_to_iface(self):
  verifyObject(iface, instance)
  return verifyObject
 
+ def test_iface_is_first_in_sro(self):
+ from zope.interface import implementedBy
+ implemented = implementedBy(self._getTargetClass())
+ __traceback_info__ = implemented.__sro__
+ self.assertIs(implemented, implemented.__sro__[0])
+ self.assertIs(self._getTargetInterface(), implemented.__sro__[1])
+
+ def test_implements_consistent__sro__(self):
+ from zope.interface import ro
+ from zope.interface import implementedBy
+ __traceback_info__ = implementedBy(self._getTargetClass()).__sro__
+ self.assertTrue(ro.is_consistent(implementedBy(self._getTargetClass())))
+
+ def test_iface_consistent_ro(self):
+ from zope.interface import ro
+ __traceback_info__ = self._getTargetInterface().__iro__
+ self.assertTrue(ro.is_consistent(self._getTargetInterface()))
+
+class EqualityTestsMixin(InterfaceConformanceTestsMixin):
+
  def test_is_hashable(self):
  field = self._makeOne()
  hash(field) # doesn't raise

src/zope/schema/tests/test__field.py

@@ -711,7 +711,7 @@ def test__validate_w_named_vocabulary_raises_LookupError(self):
  from zope.schema.vocabulary import setVocabularyRegistry
 
  class Reg(object):
- def get(*args):
+ def get(self, *args):
  raise LookupError
 
  setVocabularyRegistry(Reg())
@@ -1369,7 +1369,7 @@ def test_mutable_sequence(self):
  from zope.schema._field import abc
 
  class MutableSequence(abc.MutableSequence):
- def insert(self, item, ix):
+ def insert(self, index, value):
  raise AssertionError("not implemented")
  def __getitem__(self, name):
  raise AssertionError("not implemented")
@@ -1754,6 +1754,21 @@ def test_fromUnicode(self):
  self.assertEqual(field.fromUnicode(u'DEADBEEF'), 'DEADBEEF')
 
 
+class StrippedNativeStringLineTests(NativeStringLineTests):
+
+ def _getTargetClass(self):
+ from zope.schema._field import _StrippedNativeStringLine
+ return _StrippedNativeStringLine
+
+ def test_strips(self):
+ field = self._makeOne()
+ self.assertEqual(field.fromBytes(b' '), '')
+ self.assertEqual(field.fromUnicode(u' '), '')
+
+ def test_iface_is_first_in_sro(self):
+ self.skipTest("Not applicable; we inherit implementation but have no interface")
+
+
 def _makeSampleVocabulary():
  from zope.interface import implementer
  from zope.schema.interfaces import IVocabulary
@@ -1784,7 +1799,7 @@ def __init__(self, vocabulary):
  VocabularyRegistry.__init__(self)
  self._vocabulary = vocabulary
 
- def get(self, object, name):
+ def get(self, context, name):
  return self._vocabulary
  return DummyRegistry(v)
 

src/zope/schema/tests/test_accessors.py

@@ -15,6 +15,7 @@
 """
 import unittest
 
+# pylint:disable=inherit-non-class
 
 class FieldReadAccessorTests(unittest.TestCase):
 
@@ -59,15 +60,41 @@ def test___provides___w_field_no_provides(self):
  def test___provides___w_field_w_provides(self):
  from zope.interface import implementedBy
  from zope.interface import providedBy
+ from zope.interface.interfaces import IAttribute
+ from zope.interface.interfaces import IMethod
  from zope.schema import Text
+
+ # When wrapping a field that provides stuff,
+ # we provide the same stuff, with the addition of
+ # IMethod at the correct spot in the IRO (just before
+ # IAttribute).
  field = Text()
  field_provides = list(providedBy(field))
  wrapped = self._makeOne(field)
  wrapped_provides = list(providedBy(wrapped))
- self.assertEqual(wrapped_provides[:len(field_provides)],
- list(providedBy(field)))
+
+ index_of_attribute = field_provides.index(IAttribute)
+ expected = list(field_provides)
+ expected.insert(index_of_attribute, IMethod)
+ self.assertEqual(expected, wrapped_provides)
  for iface in list(implementedBy(self._getTargetClass())):
- self.assertTrue(iface in wrapped_provides)
+ self.assertIn(iface, wrapped_provides)
+
+ def test___provides___w_field_w_provides_strict(self):
+ from zope.interface import ro
+ attr = 'STRICT_IRO'
+ try:
+ getattr(ro.C3, attr)
+ except AttributeError:
+ # https://github.com/zopefoundation/zope.interface/issues/194
+ # zope.interface 5.0.0 used this incorrect spelling.
+ attr = 'STRICT_RO'
+ getattr(ro.C3, attr)
+ setattr(ro.C3, attr, True)
+ try:
+ self.test___provides___w_field_w_provides()
+ finally:
+ setattr(ro.C3, attr, getattr(ro.C3, 'ORIG_' + attr))
 
  def test_getSignatureString(self):
  wrapped = self._makeOne()
@@ -180,7 +207,7 @@ class Writer(object):
  pass
 
  writer = Writer()
- writer.__name__ = 'setMe'
+ writer.__name__ = 'setMe' # pylint:disable=attribute-defined-outside-init
  getter.writer = writer
 
  class Foo(object):