From 84872a8d2ab6b980aebc1c3c1ffb6aee9cf97ea5 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 2 Apr 2020 07:53:05 -0500 Subject: [PATCH 1/3] Remove the bare except: statements. Only catch AttributeError instead of everything. Fixes #200 Note that this does break a doctest in five.intid (it's expecting a TypeError but it now gets Acquisition's RuntimeError). --- CHANGES.rst | 28 ++- .../_zope_interface_coptimizations.c | 132 ++++++++--- src/zope/interface/declarations.py | 10 +- src/zope/interface/interface.py | 12 +- src/zope/interface/tests/__init__.py | 60 +++++ src/zope/interface/tests/test_adapter.py | 135 ++++++----- src/zope/interface/tests/test_declarations.py | 218 +++++++++++------- src/zope/interface/tests/test_interface.py | 10 +- 8 files changed, 408 insertions(+), 197 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 7075008a..bf8ce262 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,10 +2,30 @@ Changes ========= -5.0.3 (unreleased) -================== - -- Nothing changed yet. +5.1.0 (unreleased) +================== + +- Remove all bare ``except:`` statements. Previously, when accessing + special attributes such as ``__provides__``, ``__providedBy__``, + ``__class__`` and ``__conform__``, this package wrapped such access + in a bare ``except:`` statement, meaning that many errors could pass + silently; typically this would result in a fallback path being taken + and sometimes (like with ``providedBy()``) the result would be + non-sensical. This is especially true when those attributes are + implemented with descriptors. Now, only ``AttributeError`` is + caught. This makes errors more obvious. + + Obviously, this means that some exceptions will be propagated + differently than before. In particular, ``RuntimeError`` raised by + Acquisition in the case of circular containment will now be + propagated. Previously, when adapting such a broken object, a + ``TypeError`` would be the common result, but now it will be a more + informative ``RuntimeError``. + + In addition, ZODB errors like ``POSKeyError`` could now be + propagated where previously they would ignored by this package. + + See `issue 200 `_. 5.0.2 (2020-03-30) diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c index 4e09614c..9e891697 100644 --- a/src/zope/interface/_zope_interface_coptimizations.c +++ b/src/zope/interface/_zope_interface_coptimizations.c @@ -178,23 +178,46 @@ getObjectSpecification(PyObject *ignored, PyObject *ob) PyObject *cls, *result; result = PyObject_GetAttr(ob, str__provides__); - if (result != NULL && PyObject_TypeCheck(result, &SpecificationBaseType)) - return result; - - - PyErr_Clear(); + if (!result) + { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) + { + /* Propagate non AttributeError exceptions. */ + return NULL; + } + PyErr_Clear(); + } + else + { + int is_instance = -1; + is_instance = PyObject_IsInstance(result, (PyObject*)&SpecificationBaseType); + if (is_instance < 0) + { + /* Propagate all errors */ + return NULL; + } + if (is_instance) + { + return result; + } + } /* We do a getattr here so as not to be defeated by proxies */ cls = PyObject_GetAttr(ob, str__class__); if (cls == NULL) - { + { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) + { + /* Propagate non-AttributeErrors */ + return NULL; + } PyErr_Clear(); if (imported_declarations == 0 && import_declarations() < 0) return NULL; Py_INCREF(empty); return empty; - } + } result = implementedBy(NULL, cls); Py_DECREF(cls); @@ -205,10 +228,20 @@ static PyObject * providedBy(PyObject *ignored, PyObject *ob) { PyObject *result, *cls, *cp; - + int is_instance = -1; result = NULL; - if (PyObject_TypeCheck(ob, &PySuper_Type)) + is_instance = PyObject_IsInstance(ob, (PyObject*)&PySuper_Type); + if (is_instance < 0) + { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) + { + /* Propagate non-AttributeErrors */ + return NULL; + } + PyErr_Clear(); + } + if (is_instance) { return implementedBy(NULL, ob); } @@ -216,10 +249,15 @@ providedBy(PyObject *ignored, PyObject *ob) result = PyObject_GetAttr(ob, str__providedBy__); if (result == NULL) - { + { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) + { + return NULL; + } + PyErr_Clear(); return getObjectSpecification(NULL, ob); - } + } /* We want to make sure we have a spec. We can't do a type check @@ -737,26 +775,31 @@ static struct PyMethodDef ib_methods[] = { }; /* - def __call__(self, obj, alternate=_marker): - conform = getattr(obj, '__conform__', None) - if conform is not None: - adapter = self._call_conform(conform) - if adapter is not None: - return adapter - - adapter = self.__adapt__(obj) - + def __call__(self, obj, alternate=_marker): + try: + conform = obj.__conform__ + except AttributeError: # pylint:disable=bare-except + conform = None + + if conform is not None: + adapter = self._call_conform(conform) if adapter is not None: return adapter - elif alternate is not _marker: - return alternate - else: - raise TypeError("Could not adapt", obj, self) + + adapter = self.__adapt__(obj) + + if adapter is not None: + return adapter + if alternate is not _marker: + return alternate + raise TypeError("Could not adapt", obj, self) + */ static PyObject * ib_call(PyObject *self, PyObject *args, PyObject *kwargs) { - PyObject *conform, *obj, *alternate=NULL, *adapter; + PyObject *conform, *obj, *alternate, *adapter; + conform = obj = alternate = adapter = NULL; static char *kwlist[] = {"obj", "alternate", NULL}; @@ -765,35 +808,52 @@ ib_call(PyObject *self, PyObject *args, PyObject *kwargs) return NULL; conform = PyObject_GetAttr(obj, str__conform__); - if (conform != NULL) - { + if (conform == NULL) + { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) + { + /* Propagate non-AttributeErrors */ + return NULL; + } + PyErr_Clear(); + + Py_INCREF(Py_None); + conform = Py_None; + } + + if (conform != Py_None) + { adapter = PyObject_CallMethodObjArgs(self, str_call_conform, conform, NULL); Py_DECREF(conform); if (adapter == NULL || adapter != Py_None) - return adapter; + return adapter; Py_DECREF(adapter); - } + } else - PyErr_Clear(); + { + Py_DECREF(conform); + } - adapter = __adapt__(self, obj); + adapter = __adapt__(self, obj); // XXX: should be self.__adapt__. if (adapter == NULL || adapter != Py_None) - return adapter; + { + return adapter; + } Py_DECREF(adapter); if (alternate != NULL) - { + { Py_INCREF(alternate); return alternate; - } + } adapter = Py_BuildValue("sOO", "Could not adapt", obj, self); if (adapter != NULL) - { + { PyErr_SetObject(PyExc_TypeError, adapter); Py_DECREF(adapter); - } + } return NULL; } diff --git a/src/zope/interface/declarations.py b/src/zope/interface/declarations.py index e84a728e..b42c906e 100644 --- a/src/zope/interface/declarations.py +++ b/src/zope/interface/declarations.py @@ -984,6 +984,7 @@ def moduleProvides(*interfaces): # XXX: is this a fossil? Nobody calls it, no unit tests exercise it, no # doctests import it, and the package __init__ doesn't import it. +# (Answer: Versions of zope.container prior to 4.4.0 called this.) def ObjectSpecification(direct, cls): """Provide object specifications @@ -994,16 +995,17 @@ def ObjectSpecification(direct, cls): @_use_c_impl def getObjectSpecification(ob): try: - provides = getattr(ob, '__provides__', None) - except: + provides = ob.__provides__ + except AttributeError: provides = None + if provides is not None: if isinstance(provides, SpecificationBase): return provides try: cls = ob.__class__ - except: + except AttributeError: # We can't get the class, so just consider provides return _empty return implementedBy(cls) @@ -1028,7 +1030,7 @@ def providedBy(ob): return implementedBy(ob) r = ob.__providedBy__ - except: + except AttributeError: # Not set yet. Fall back to lower-level thing that computes it return getObjectSpecification(ob) diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py index ac556339..d035ade0 100644 --- a/src/zope/interface/interface.py +++ b/src/zope/interface/interface.py @@ -275,16 +275,10 @@ def __call__(self, obj, alternate=_marker): """Adapt an object to the interface """ try: - conform = getattr(obj, '__conform__', None) - # XXX: Ideally, we don't want to catch BaseException. Things - # like MemoryError, KeyboardInterrupt, and other BaseExceptions should - # get through. Here, and everywhere we have bare ``except:`` clauses. - # The bare ``except:`` clauses exist for compatibility with the C code in - # ``_zope_interface_coptimizations.c`` (``ib_call()`` in this case). Both - # implementations would need to change. But beware of things like proxies and - # Acquisition wrappers. See https://github.com/zopefoundation/zope.interface/issues/163 - except: # pylint:disable=bare-except + conform = obj.__conform__ + except AttributeError: conform = None + if conform is not None: adapter = self._call_conform(conform) if adapter is not None: diff --git a/src/zope/interface/tests/__init__.py b/src/zope/interface/tests/__init__.py index 8b9ef258..6a11218b 100644 --- a/src/zope/interface/tests/__init__.py +++ b/src/zope/interface/tests/__init__.py @@ -32,6 +32,66 @@ def test_optimizations(self): else: self.assertIs(used, fallback) + +class MissingSomeAttrs(object): + """ + Helper for tests that raises a specific exception + for attributes that are missing. This is usually not + an AttributeError, and this object is used to test that + those errors are not improperly caught and treated like + an AttributeError. + """ + + def __init__(self, exc_kind, **other_attrs): + self.__exc_kind = exc_kind + d = object.__getattribute__(self, '__dict__') + d.update(other_attrs) + + def __getattribute__(self, name): + # Note that we ignore objects found in the class dictionary. + d = object.__getattribute__(self, '__dict__') + try: + return d[name] + except KeyError: + raise d['_MissingSomeAttrs__exc_kind'](name) + + EXCEPTION_CLASSES = ( + TypeError, + RuntimeError, + BaseException, + ValueError, + ) + + @classmethod + def test_raises(cls, unittest, test_func, expected_missing, **other_attrs): + """ + Loop through various exceptions, calling *test_func* inside a ``assertRaises`` block. + + :param test_func: A callable of one argument, the instance of this + class. + :param str expected_missing: The attribute that should fail with the exception. + This is used to ensure that we're testing the path we think we are. + :param other_attrs: Attributes that should be provided on the test object. + Must not contain *expected_missing*. + """ + assert isinstance(expected_missing, str) + assert expected_missing not in other_attrs + for exc in cls.EXCEPTION_CLASSES: + ob = cls(exc, **other_attrs) + with unittest.assertRaises(exc) as ex: + test_func(ob) + + unittest.assertEqual(ex.exception.args[0], expected_missing) + + # Now test that the AttributeError for that expected_missing is *not* raised. + ob = cls(AttributeError, **other_attrs) + try: + test_func(ob) + except AttributeError as e: + unittest.assertNotIn(expected_missing, str(e)) + except Exception: # pylint:disable=broad-except + pass + # Be sure cleanup functionality is available; classes that use the adapter hook # need to be sure to subclass ``CleanUp``. # diff --git a/src/zope/interface/tests/test_adapter.py b/src/zope/interface/tests/test_adapter.py index 0456b1c8..869df663 100644 --- a/src/zope/interface/tests/test_adapter.py +++ b/src/zope/interface/tests/test_adapter.py @@ -17,21 +17,32 @@ from zope.interface.tests import OptimizationTestMixin +# pylint:disable=inherit-non-class,protected-access,too-many-lines +# pylint:disable=attribute-defined-outside-init,blacklisted-name def _makeInterfaces(): from zope.interface import Interface - class IB0(Interface): pass - class IB1(IB0): pass - class IB2(IB0): pass - class IB3(IB2, IB1): pass - class IB4(IB1, IB2): pass - - class IF0(Interface): pass - class IF1(IF0): pass - - class IR0(Interface): pass - class IR1(IR0): pass + class IB0(Interface): + pass + class IB1(IB0): + pass + class IB2(IB0): + pass + class IB3(IB2, IB1): + pass + class IB4(IB1, IB2): + pass + + class IF0(Interface): + pass + class IF1(IF0): + pass + + class IR0(Interface): + pass + class IR1(IR0): + pass return IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 @@ -51,7 +62,7 @@ def add_extendor(self, provided): self._extendors += (provided,) def remove_extendor(self, provided): self._extendors = tuple([x for x in self._extendors - if x != provided]) + if x != provided]) for name in BaseAdapterRegistry._delegated: setattr(_CUT.LookupClass, name, object()) return _CUT @@ -80,13 +91,14 @@ def test__generation_after_calling_changed(self): self.assertEqual(registry._v_lookup._changed, (registry, orig,)) def test__generation_after_changing___bases__(self): - class _Base(object): pass + class _Base(object): + pass registry = self._makeOne() registry.__bases__ = (_Base,) self.assertEqual(registry._generation, 2) def test_register(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() registry.register([IB0], IR0, '', 'A1') self.assertEqual(registry.registered([IB0], IR0, ''), 'A1') @@ -94,20 +106,20 @@ def test_register(self): self.assertEqual(registry._generation, 2) def test_register_with_invalid_name(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() with self.assertRaises(ValueError): registry.register([IB0], IR0, object(), 'A1') def test_register_with_value_None_unregisters(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() registry.register([None], IR0, '', 'A1') registry.register([None], IR0, '', None) self.assertEqual(len(registry._adapters), 0) def test_register_with_same_value(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() _value = object() registry.register([None], IR0, '', _value) @@ -120,7 +132,7 @@ def test_registered_empty(self): self.assertEqual(registry.registered([None], None, ''), None) def test_registered_non_empty_miss(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() registry.register([IB1], None, '', 'A1') self.assertEqual(registry.registered([IB2], None, ''), None) @@ -136,21 +148,21 @@ def test_unregister_empty(self): self.assertEqual(registry.registered([None], None, ''), None) def test_unregister_non_empty_miss_on_required(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() registry.register([IB1], None, '', 'A1') registry.unregister([IB2], None, '') #doesn't raise self.assertEqual(registry.registered([IB1], None, ''), 'A1') def test_unregister_non_empty_miss_on_name(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() registry.register([IB1], None, '', 'A1') registry.unregister([IB1], None, 'nonesuch') #doesn't raise self.assertEqual(registry.registered([IB1], None, ''), 'A1') def test_unregister_with_value_not_None_miss(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() orig = object() nomatch = object() @@ -159,7 +171,7 @@ def test_unregister_with_value_not_None_miss(self): self.assertTrue(registry.registered([IB1], None, '') is orig) def test_unregister_hit_clears_empty_subcomponents(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() one = object() another = object() @@ -177,7 +189,7 @@ def test_unsubscribe_empty(self): self.assertEqual(registry.registered([None], None, ''), None) def test_unsubscribe_hit(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() orig = object() registry.subscribe([IB1], None, orig) @@ -185,7 +197,7 @@ def test_unsubscribe_hit(self): self.assertEqual(len(registry._subscribers), 0) def test_unsubscribe_after_multiple(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() first = object() second = object() @@ -202,18 +214,18 @@ def test_unsubscribe_after_multiple(self): self.assertEqual(len(registry._subscribers), 0) def test_unsubscribe_w_None_after_multiple(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() first = object() second = object() - third = object() + registry.subscribe([IB1], None, first) registry.subscribe([IB1], None, second) registry.unsubscribe([IB1], None) self.assertEqual(len(registry._subscribers), 0) def test_unsubscribe_non_empty_miss_on_required(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() registry.subscribe([IB1], None, 'A1') self.assertEqual(len(registry._subscribers), 2) @@ -221,7 +233,7 @@ def test_unsubscribe_non_empty_miss_on_required(self): self.assertEqual(len(registry._subscribers), 2) def test_unsubscribe_non_empty_miss_on_value(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() registry.subscribe([IB1], None, 'A1') self.assertEqual(len(registry._subscribers), 2) @@ -229,7 +241,7 @@ def test_unsubscribe_non_empty_miss_on_value(self): self.assertEqual(len(registry._subscribers), 2) def test_unsubscribe_with_value_not_None_miss(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() orig = object() nomatch = object() @@ -241,7 +253,7 @@ def _instance_method_notify_target(self): self.fail("Example method, not intended to be called.") def test_unsubscribe_instance_method(self): - IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() + IB0, IB1, IB2, IB3, IB4, IF0, IF1, IR0, IR1 = _makeInterfaces() # pylint:disable=unused-variable registry = self._makeOne() self.assertEqual(len(registry._subscribers), 0) registry.subscribe([IB1], None, self._instance_method_notify_target) @@ -252,13 +264,14 @@ def test_unsubscribe_instance_method(self): class LookupBaseFallbackTests(unittest.TestCase): def _getFallbackClass(self): - from zope.interface.adapter import LookupBaseFallback + from zope.interface.adapter import LookupBaseFallback # pylint:disable=no-name-in-module return LookupBaseFallback _getTargetClass = _getFallbackClass def _makeOne(self, uc_lookup=None, uc_lookupAll=None, uc_subscriptions=None): + # pylint:disable=function-redefined if uc_lookup is None: def uc_lookup(self, required, provided, name): pass @@ -285,7 +298,7 @@ def test_lookup_miss_no_default(self): _called_with = [] def _lookup(self, required, provided, name): _called_with.append((required, provided, name)) - return None + lb = self._makeOne(uc_lookup=_lookup) found = lb.lookup(('A',), 'B', 'C') self.assertTrue(found is None) @@ -296,7 +309,7 @@ def test_lookup_miss_w_default(self): _default = object() def _lookup(self, required, provided, name): _called_with.append((required, provided, name)) - return None + lb = self._makeOne(uc_lookup=_lookup) found = lb.lookup(('A',), 'B', 'C', _default) self.assertTrue(found is _default) @@ -384,7 +397,7 @@ def test_lookup1_miss_no_default(self): _called_with = [] def _lookup(self, required, provided, name): _called_with.append((required, provided, name)) - return None + lb = self._makeOne(uc_lookup=_lookup) found = lb.lookup1('A', 'B', 'C') self.assertTrue(found is None) @@ -395,7 +408,7 @@ def test_lookup1_miss_w_default(self): _default = object() def _lookup(self, required, provided, name): _called_with.append((required, provided, name)) - return None + lb = self._makeOne(uc_lookup=_lookup) found = lb.lookup1('A', 'B', 'C', _default) self.assertTrue(found is _default) @@ -406,7 +419,7 @@ def test_lookup1_miss_w_default_negative_cache(self): _default = object() def _lookup(self, required, provided, name): _called_with.append((required, provided, name)) - return None + lb = self._makeOne(uc_lookup=_lookup) found = lb.lookup1('A', 'B', 'C', _default) self.assertTrue(found is _default) @@ -479,7 +492,7 @@ def test_adapter_hook_hit_factory_returns_None(self): _f_called_with = [] def _factory(context): _f_called_with.append(context) - return None + def _lookup(self, required, provided, name): return _factory req, prv, _default = object(), object(), object() @@ -588,13 +601,14 @@ def _getTargetClass(self): class VerifyingBaseFallbackTests(unittest.TestCase): def _getFallbackClass(self): - from zope.interface.adapter import VerifyingBaseFallback + from zope.interface.adapter import VerifyingBaseFallback # pylint:disable=no-name-in-module return VerifyingBaseFallback _getTargetClass = _getFallbackClass def _makeOne(self, registry, uc_lookup=None, uc_lookupAll=None, uc_subscriptions=None): + # pylint:disable=function-redefined if uc_lookup is None: def uc_lookup(self, required, provided, name): raise NotImplementedError() @@ -641,7 +655,7 @@ def _lookup(self, required, provided, name): found = lb.lookup(('A',), 'B', 'C') self.assertTrue(found is b) self.assertEqual(_called_with, - [(('A',), 'B', 'C'), (('A',), 'B', 'C')]) + [(('A',), 'B', 'C'), (('A',), 'B', 'C')]) self.assertEqual(_results, [c]) def test_lookup1(self): @@ -662,7 +676,7 @@ def _lookup(self, required, provided, name): found = lb.lookup1('A', 'B', 'C') self.assertTrue(found is b) self.assertEqual(_called_with, - [(('A',), 'B', 'C'), (('A',), 'B', 'C')]) + [(('A',), 'B', 'C'), (('A',), 'B', 'C')]) self.assertEqual(_results, [c]) def test_adapter_hook(self): @@ -815,8 +829,7 @@ class FauxWeakref(object): def __init__(self, here): self._here = here def __call__(self): - if self._here: - return self + return self if self._here else None def unsubscribe(self, target): self._unsub = target gone = FauxWeakref(False) @@ -939,7 +952,7 @@ def test__uncached_lookup_components_miss_wrong_name(self): IBar = InterfaceClass('IBar', IFoo) registry = self._makeRegistry(IFoo, IBar) subr = self._makeSubregistry() - irrelevant = object() + wrongname = object() subr._adapters = [ #utilities, single adapters {}, @@ -1012,22 +1025,27 @@ class Foo(object): self.assertTrue(result is _default) def test_queryMultiAdapter_errors_on_attribute_access(self): - # Which leads to using the _empty singleton as "requires" - # argument. See https://github.com/zopefoundation/zope.interface/issues/162 + # Any error on attribute access previously lead to using the _empty singleton as "requires" + # argument (See https://github.com/zopefoundation/zope.interface/issues/162) + # but after https://github.com/zopefoundation/zope.interface/issues/200 + # they get propagated. from zope.interface.interface import InterfaceClass + from zope.interface.tests import MissingSomeAttrs + IFoo = InterfaceClass('IFoo') registry = self._makeRegistry() alb = self._makeOne(registry) alb.lookup = alb._uncached_lookup - class UnexpectedErrorsLikeAcquisition(object): - def __getattribute__(self, name): - raise RuntimeError("Acquisition does this. Ha-ha!") + def test(ob): + return alb.queryMultiAdapter( + (ob,), + IFoo, + ) - result = alb.queryMultiAdapter( - (UnexpectedErrorsLikeAcquisition(),), - IFoo, - ) + PY3 = str is not bytes + MissingSomeAttrs.test_raises(self, test, + expected_missing='__class__' if PY3 else '__providedBy__') def test_queryMultiAdaptor_factory_miss(self): from zope.interface.declarations import implementer @@ -1044,7 +1062,7 @@ class Foo(object): _called_with = [] def _factory(context): _called_with.append(context) - return None + subr._adapters = [ #utilities, single adapters {}, {IFoo: {IBar: {'': _factory}}}, @@ -1343,7 +1361,7 @@ def _factory2(context): return _exp2 def _side_effect_only(context): _called.setdefault('_side_effect_only', []).append(context) - return None + subr._subscribers = [ #utilities, single adapters {}, {IFoo: {IBar: {'': (_factory1, _factory2, _side_effect_only)}}}, @@ -1441,13 +1459,12 @@ def test__convert_None_to_Interface_w_other(self): self.assertTrue(_convert_None_to_Interface(other) is other) def test__normalize_name_str(self): - import sys from zope.interface.adapter import _normalize_name STR = b'str' - if sys.version_info[0] < 3: - self.assertEqual(_normalize_name(STR), unicode(STR)) - else: - self.assertEqual(_normalize_name(STR), str(STR, 'ascii')) + UNICODE = u'str' + norm = _normalize_name(STR) + self.assertEqual(norm, UNICODE) + self.assertIsInstance(norm, type(UNICODE)) def test__normalize_name_unicode(self): from zope.interface.adapter import _normalize_name diff --git a/src/zope/interface/tests/test_declarations.py b/src/zope/interface/tests/test_declarations.py index 9344968b..5d7272a9 100644 --- a/src/zope/interface/tests/test_declarations.py +++ b/src/zope/interface/tests/test_declarations.py @@ -16,31 +16,36 @@ import unittest from zope.interface._compat import _skip_under_py3k +from zope.interface._compat import PYTHON3 from zope.interface.tests import OptimizationTestMixin +from zope.interface.tests import MissingSomeAttrs from zope.interface.tests.test_interface import NameAndModuleComparisonTestsMixin +# pylint:disable=inherit-non-class,too-many-lines,protected-access +# pylint:disable=blacklisted-name,attribute-defined-outside-init class _Py3ClassAdvice(object): def _run_generated_code(self, code, globs, locs, fails_under_py3k=True, ): + # pylint:disable=exec-used,no-member import warnings - from zope.interface._compat import PYTHON3 with warnings.catch_warnings(record=True) as log: warnings.resetwarnings() if not PYTHON3: exec(code, globs, locs) self.assertEqual(len(log), 0) # no longer warn return True + + try: + exec(code, globs, locs) + except TypeError: + return False else: - try: - exec(code, globs, locs) - except TypeError: - return False - else: - if fails_under_py3k: - self.fail("Didn't raise TypeError") + if fails_under_py3k: + self.fail("Didn't raise TypeError") + return None class NamedTests(unittest.TestCase): @@ -52,7 +57,7 @@ def test_class(self): class Foo(object): pass - self.assertEqual(Foo.__component_name__, u'foo') + self.assertEqual(Foo.__component_name__, u'foo') # pylint:disable=no-member def test_function(self): from zope.interface.declarations import named @@ -71,7 +76,7 @@ class Foo(object): foo = Foo() named(u'foo')(foo) - self.assertEqual(foo.__component_name__, u'foo') + self.assertEqual(foo.__component_name__, u'foo') # pylint:disable=no-member class EmptyDeclarationTests(unittest.TestCase): @@ -155,8 +160,6 @@ def test_changed_w_existing__v_attrs(self): self.assertIsNone(decl._v_attrs) def test___contains__w_self(self): - from zope.interface.interface import InterfaceClass - IFoo = InterfaceClass('IFoo') decl = self._makeOne() self.assertNotIn(decl, decl) @@ -241,7 +244,7 @@ def test___sub___unrelated_interface(self): IBar = InterfaceClass('IBar') before = self._makeOne(IFoo) after = before - IBar - self.assertIsInstance(after, self._getTargetClass()) + self.assertIsInstance(after, self._getTargetClass()) self.assertEqual(list(after), [IFoo]) def test___sub___related_interface(self): @@ -265,7 +268,7 @@ def test___add___unrelated_interface(self): IBar = InterfaceClass('IBar') before = self._makeOne(IFoo) after = before + IBar - self.assertIsInstance(after, self._getTargetClass()) + self.assertIsInstance(after, self._getTargetClass()) self.assertEqual(list(after), [IFoo, IBar]) def test___add___related_interface(self): @@ -379,7 +382,7 @@ class B(object): self.assertEqual(implementedBy(A), implementedBy(A)) self.assertEqual(hash(implementedBy(A)), hash(implementedBy(A))) self.assertTrue(implementedBy(A) < None) - self.assertTrue(None > implementedBy(A)) + self.assertTrue(None > implementedBy(A)) # pylint:disable=misplaced-comparison-constant self.assertTrue(implementedBy(A) < implementedBy(B)) self.assertTrue(implementedBy(A) > IFoo) self.assertTrue(implementedBy(A) <= implementedBy(B)) @@ -414,7 +417,7 @@ class B(object): # The order of arguments to the operators matters, # test both - self.assertTrue(implementedByA == implementedByA) + self.assertTrue(implementedByA == implementedByA) # pylint:disable=comparison-with-itself self.assertTrue(implementedByA != implementedByB) self.assertTrue(implementedByB != implementedByA) @@ -451,6 +454,7 @@ def test_changed_does_not_add_super_cache(self): class Test_implementedByFallback(unittest.TestCase): def _getTargetClass(self): + # pylint:disable=no-name-in-module from zope.interface.declarations import implementedByFallback return implementedByFallback @@ -527,10 +531,10 @@ def test_builtins_added_to_cache(self): self.assertEqual(list(self._callFUT(dict)), []) for typ in (tuple, list, dict): spec = specs[typ] - self.assertIsInstance(spec, Implements) + self.assertIsInstance(spec, Implements) self.assertEqual(repr(spec), - '' - % (_BUILTINS, typ.__name__)) + '' + % (_BUILTINS, typ.__name__)) def test_builtins_w_existing_cache(self): from zope.interface import declarations @@ -574,9 +578,9 @@ def __call__(self): spec = self._callFUT(foo) self.assertEqual(spec.__name__, 'zope.interface.tests.test_declarations.foo') - self.assertTrue(spec.inherit is foo) - self.assertTrue(foo.__implemented__ is spec) - self.assertTrue(foo.__providedBy__ is objectSpecificationDescriptor) + self.assertIs(spec.inherit, foo) + self.assertIs(foo.__implemented__, spec) + self.assertIs(foo.__providedBy__, objectSpecificationDescriptor) # pylint:disable=no-member self.assertNotIn('__provides__', foo.__dict__) def test_w_None_no_bases_w_class(self): @@ -586,11 +590,11 @@ class Foo(object): spec = self._callFUT(Foo) self.assertEqual(spec.__name__, 'zope.interface.tests.test_declarations.Foo') - self.assertTrue(spec.inherit is Foo) - self.assertTrue(Foo.__implemented__ is spec) - self.assertIsInstance(Foo.__providedBy__, ClassProvides) - self.assertIsInstance(Foo.__provides__, ClassProvides) - self.assertEqual(Foo.__provides__, Foo.__providedBy__) + self.assertIs(spec.inherit, Foo) + self.assertIs(Foo.__implemented__, spec) + self.assertIsInstance(Foo.__providedBy__, ClassProvides) # pylint:disable=no-member + self.assertIsInstance(Foo.__provides__, ClassProvides) # pylint:disable=no-member + self.assertEqual(Foo.__provides__, Foo.__providedBy__) # pylint:disable=no-member def test_w_existing_Implements(self): from zope.interface.declarations import Implements @@ -793,14 +797,14 @@ class Foo(object): pass ifoo = InterfaceClass('IFoo') self._callFUT(Foo, ifoo) - spec = Foo.__implemented__ + spec = Foo.__implemented__ # pylint:disable=no-member self.assertEqual(spec.__name__, 'zope.interface.tests.test_declarations.Foo') - self.assertTrue(spec.inherit is None) - self.assertTrue(Foo.__implemented__ is spec) - self.assertIsInstance(Foo.__providedBy__, ClassProvides) - self.assertIsInstance(Foo.__provides__, ClassProvides) - self.assertEqual(Foo.__provides__, Foo.__providedBy__) + self.assertIsNone(spec.inherit) + self.assertIs(Foo.__implemented__, spec) # pylint:disable=no-member + self.assertIsInstance(Foo.__providedBy__, ClassProvides) # pylint:disable=no-member + self.assertIsInstance(Foo.__provides__, ClassProvides) # pylint:disable=no-member + self.assertEqual(Foo.__provides__, Foo.__providedBy__) # pylint:disable=no-member def test_w_existing_Implements(self): from zope.interface.declarations import Implements @@ -832,14 +836,14 @@ class Foo(object): pass IFoo = InterfaceClass('IFoo') self._callFUT(Foo, IFoo) - spec = Foo.__implemented__ + spec = Foo.__implemented__ # pylint:disable=no-member self.assertEqual(spec.__name__, 'zope.interface.tests.test_declarations.Foo') - self.assertTrue(spec.inherit is Foo) - self.assertTrue(Foo.__implemented__ is spec) - self.assertIsInstance(Foo.__providedBy__, ClassProvides) - self.assertIsInstance(Foo.__provides__, ClassProvides) - self.assertEqual(Foo.__provides__, Foo.__providedBy__) + self.assertIs(spec.inherit, Foo) + self.assertIs(Foo.__implemented__, spec) # pylint:disable=no-member + self.assertIsInstance(Foo.__providedBy__, ClassProvides) # pylint:disable=no-member + self.assertIsInstance(Foo.__provides__, ClassProvides) # pylint:disable=no-member + self.assertEqual(Foo.__provides__, Foo.__providedBy__) # pylint:disable=no-member def test_w_existing_Implements(self): from zope.interface.declarations import Implements @@ -896,8 +900,8 @@ class Foo(object): __implements_advice_data__ = ((IFoo,), classImplements) self._callFUT(Foo) self.assertNotIn('__implements_advice_data__', Foo.__dict__) - self.assertIsInstance(Foo.__implemented__, Implements) - self.assertEqual(list(Foo.__implemented__), [IFoo]) + self.assertIsInstance(Foo.__implemented__, Implements) # pylint:disable=no-member + self.assertEqual(list(Foo.__implemented__), [IFoo]) # pylint:disable=no-member class Test_implementer(unittest.TestCase): @@ -919,14 +923,14 @@ class Foo: decorator = self._makeOne(IFoo) returned = decorator(Foo) self.assertTrue(returned is Foo) - spec = Foo.__implemented__ + spec = Foo.__implemented__ # pylint:disable=no-member self.assertEqual(spec.__name__, 'zope.interface.tests.test_declarations.Foo') - self.assertTrue(spec.inherit is Foo) - self.assertTrue(Foo.__implemented__ is spec) - self.assertIsInstance(Foo.__providedBy__, ClassProvides) - self.assertIsInstance(Foo.__provides__, ClassProvides) - self.assertEqual(Foo.__provides__, Foo.__providedBy__) + self.assertIs(spec.inherit, Foo) + self.assertIs(Foo.__implemented__, spec) # pylint:disable=no-member + self.assertIsInstance(Foo.__providedBy__, ClassProvides) # pylint:disable=no-member + self.assertIsInstance(Foo.__provides__, ClassProvides) # pylint:disable=no-member + self.assertEqual(Foo.__provides__, Foo.__providedBy__) # pylint:disable=no-member def test_newstyle_class(self): from zope.interface.declarations import ClassProvides @@ -937,14 +941,14 @@ class Foo(object): decorator = self._makeOne(IFoo) returned = decorator(Foo) self.assertTrue(returned is Foo) - spec = Foo.__implemented__ + spec = Foo.__implemented__ # pylint:disable=no-member self.assertEqual(spec.__name__, 'zope.interface.tests.test_declarations.Foo') - self.assertTrue(spec.inherit is Foo) - self.assertTrue(Foo.__implemented__ is spec) - self.assertIsInstance(Foo.__providedBy__, ClassProvides) - self.assertIsInstance(Foo.__provides__, ClassProvides) - self.assertEqual(Foo.__provides__, Foo.__providedBy__) + self.assertIs(spec.inherit, Foo) + self.assertIs(Foo.__implemented__, spec) # pylint:disable=no-member + self.assertIsInstance(Foo.__providedBy__, ClassProvides) # pylint:disable=no-member + self.assertIsInstance(Foo.__provides__, ClassProvides) # pylint:disable=no-member + self.assertEqual(Foo.__provides__, Foo.__providedBy__) # pylint:disable=no-member def test_nonclass_cannot_assign_attr(self): from zope.interface.interface import InterfaceClass @@ -961,10 +965,10 @@ class Foo(object): decorator = self._makeOne(IFoo) returned = decorator(foo) self.assertTrue(returned is foo) - spec = foo.__implemented__ + spec = foo.__implemented__ # pylint:disable=no-member self.assertEqual(spec.__name__, 'zope.interface.tests.test_declarations.?') - self.assertTrue(spec.inherit is None) - self.assertTrue(foo.__implemented__ is spec) + self.assertIsNone(spec.inherit,) + self.assertIs(foo.__implemented__, spec) # pylint:disable=no-member class Test_implementer_only(unittest.TestCase): @@ -989,7 +993,7 @@ def test_method(self): IFoo = InterfaceClass('IFoo') decorator = self._makeOne(IFoo) class Bar: - def _method(): + def _method(self): raise NotImplementedError() self.assertRaises(ValueError, decorator, Bar._method) @@ -1034,7 +1038,6 @@ class Test_implementsOnly(unittest.TestCase, _Py3ClassAdvice): def test_simple(self): import warnings from zope.interface.declarations import implementsOnly - from zope.interface._compat import PYTHON3 from zope.interface.interface import InterfaceClass IFoo = InterfaceClass("IFoo") globs = {'implementsOnly': implementsOnly, @@ -1048,7 +1051,7 @@ def test_simple(self): with warnings.catch_warnings(record=True) as log: warnings.resetwarnings() try: - exec(CODE, globs, locs) + exec(CODE, globs, locs) # pylint:disable=exec-used except TypeError: self.assertTrue(PYTHON3, "Must be Python 3") else: @@ -1107,7 +1110,6 @@ def test_called_twice_from_class(self): import warnings from zope.interface.declarations import implements from zope.interface.interface import InterfaceClass - from zope.interface._compat import PYTHON3 IFoo = InterfaceClass("IFoo") IBar = InterfaceClass("IBar") globs = {'implements': implements, 'IFoo': IFoo, 'IBar': IBar} @@ -1120,7 +1122,7 @@ def test_called_twice_from_class(self): with warnings.catch_warnings(record=True) as log: warnings.resetwarnings() try: - exec(CODE, globs, locs) + exec(CODE, globs, locs) # pylint:disable=exec-used except TypeError: if not PYTHON3: self.assertEqual(len(log), 0) # no longer warn @@ -1245,8 +1247,8 @@ class Foo(object): pass obj = Foo() self._callFUT(obj, IFoo) - self.assertIsInstance(obj.__provides__, ProvidesClass) - self.assertEqual(list(obj.__provides__), [IFoo]) + self.assertIsInstance(obj.__provides__, ProvidesClass) # pylint:disable=no-member + self.assertEqual(list(obj.__provides__), [IFoo]) # pylint:disable=no-member def test_w_class(self): from zope.interface.declarations import ClassProvides @@ -1255,8 +1257,8 @@ def test_w_class(self): class Foo(object): pass self._callFUT(Foo, IFoo) - self.assertIsInstance(Foo.__provides__, ClassProvides) - self.assertEqual(list(Foo.__provides__), [IFoo]) + self.assertIsInstance(Foo.__provides__, ClassProvides) # pylint:disable=no-member + self.assertEqual(list(Foo.__provides__), [IFoo]) # pylint:disable=no-member @_skip_under_py3k def test_w_non_descriptor_aware_metaclass(self): @@ -1292,7 +1294,7 @@ def __setattr__(self, name, value): the_dict[name] = value obj = Foo() self._callFUT(obj, IFoo) - self.assertIsInstance(the_dict['__provides__'], ProvidesClass) + self.assertIsInstance(the_dict['__provides__'], ProvidesClass) self.assertEqual(list(the_dict['__provides__']), [IFoo]) @@ -1310,8 +1312,8 @@ class Foo(object): pass obj = Foo() self._callFUT(obj, IFoo) - self.assertIsInstance(obj.__provides__, ProvidesClass) - self.assertEqual(list(obj.__provides__), [IFoo]) + self.assertIsInstance(obj.__provides__, ProvidesClass) # pylint:disable=no-member + self.assertEqual(list(obj.__provides__), [IFoo]) # pylint:disable=no-member def test_w_existing_provides(self): from zope.interface.declarations import directlyProvides @@ -1324,8 +1326,8 @@ class Foo(object): obj = Foo() directlyProvides(obj, IFoo) self._callFUT(obj, IBar) - self.assertIsInstance(obj.__provides__, ProvidesClass) - self.assertEqual(list(obj.__provides__), [IFoo, IBar]) + self.assertIsInstance(obj.__provides__, ProvidesClass) # pylint:disable=no-member + self.assertEqual(list(obj.__provides__), [IFoo, IBar]) # pylint:disable=no-member class Test_noLongerProvides(unittest.TestCase): @@ -1341,7 +1343,7 @@ class Foo(object): pass obj = Foo() self._callFUT(obj, IFoo) - self.assertEqual(list(obj.__provides__), []) + self.assertEqual(list(obj.__provides__), []) # pylint:disable=no-member def test_w_existing_provides_hit(self): from zope.interface.declarations import directlyProvides @@ -1352,7 +1354,7 @@ class Foo(object): obj = Foo() directlyProvides(obj, IFoo) self._callFUT(obj, IFoo) - self.assertEqual(list(obj.__provides__), []) + self.assertEqual(list(obj.__provides__), []) # pylint:disable=no-member def test_w_existing_provides_miss(self): from zope.interface.declarations import directlyProvides @@ -1364,7 +1366,7 @@ class Foo(object): obj = Foo() directlyProvides(obj, IFoo) self._callFUT(obj, IBar) - self.assertEqual(list(obj.__provides__), [IFoo]) + self.assertEqual(list(obj.__provides__), [IFoo]) # pylint:disable=no-member def test_w_iface_implemented_by_class(self): from zope.interface.declarations import implementer @@ -1380,6 +1382,7 @@ class Foo(object): class ClassProvidesBaseFallbackTests(unittest.TestCase): def _getTargetClass(self): + # pylint:disable=no-name-in-module from zope.interface.declarations import ClassProvidesBaseFallback return ClassProvidesBaseFallback @@ -1406,8 +1409,8 @@ def test_w_same_class_via_instance(self): class Foo(object): pass foo = Foo() - cpbp = Foo.__provides__ = self._makeOne(Foo, IFoo) - self.assertTrue(foo.__provides__ is IFoo) + Foo.__provides__ = self._makeOne(Foo, IFoo) + self.assertIs(foo.__provides__, IFoo) def test_w_different_class(self): from zope.interface.interface import InterfaceClass @@ -1417,7 +1420,7 @@ class Foo(object): class Bar(Foo): pass bar = Bar() - cpbp = Foo.__provides__ = self._makeOne(Foo, IFoo) + Foo.__provides__ = self._makeOne(Foo, IFoo) self.assertRaises(AttributeError, getattr, Bar, '__provides__') self.assertRaises(AttributeError, getattr, bar, '__provides__') @@ -1431,6 +1434,7 @@ def _getTargetClass(self): return ClassProvidesBase def _getFallbackClass(self): + # pylint:disable=no-name-in-module from zope.interface.declarations import ClassProvidesBaseFallback return ClassProvidesBaseFallback @@ -1522,12 +1526,12 @@ class Foo(object): class Test_classProvides(unittest.TestCase, _Py3ClassAdvice): + # pylint:disable=exec-used def test_called_from_function(self): import warnings from zope.interface.declarations import classProvides from zope.interface.interface import InterfaceClass - from zope.interface._compat import PYTHON3 IFoo = InterfaceClass("IFoo") globs = {'classProvides': classProvides, 'IFoo': IFoo} locs = {} @@ -1547,7 +1551,6 @@ def test_called_twice_from_class(self): import warnings from zope.interface.declarations import classProvides from zope.interface.interface import InterfaceClass - from zope.interface._compat import PYTHON3 IFoo = InterfaceClass("IFoo") IBar = InterfaceClass("IBar") globs = {'classProvides': classProvides, 'IFoo': IFoo, 'IBar': IBar} @@ -1601,11 +1604,12 @@ def test_w_class(self): @self._makeOne(IFoo) class Foo(object): pass - self.assertIsInstance(Foo.__provides__, ClassProvides) - self.assertEqual(list(Foo.__provides__), [IFoo]) + self.assertIsInstance(Foo.__provides__, ClassProvides) # pylint:disable=no-member + self.assertEqual(list(Foo.__provides__), [IFoo]) # pylint:disable=no-member class Test_moduleProvides(unittest.TestCase): + # pylint:disable=exec-used def test_called_from_function(self): from zope.interface.declarations import moduleProvides @@ -1655,7 +1659,7 @@ def test_called_twice_from_module_scope(self): IFoo = InterfaceClass("IFoo") globs = {'__name__': 'zope.interface.tests.foo', 'moduleProvides': moduleProvides, 'IFoo': IFoo} - locs = {} + CODE = "\n".join([ 'moduleProvides(IFoo)', 'moduleProvides(IFoo)', @@ -1667,6 +1671,7 @@ def test_called_twice_from_module_scope(self): class Test_getObjectSpecificationFallback(unittest.TestCase): def _getFallbackClass(self): + # pylint:disable=no-name-in-module from zope.interface.declarations import getObjectSpecificationFallback return getObjectSpecificationFallback @@ -1700,7 +1705,7 @@ def foo(): raise NotImplementedError() directlyProvides(foo, IFoo) spec = self._callFUT(foo) - self.assertTrue(spec is foo.__provides__) + self.assertIs(spec, foo.__provides__) # pylint:disable=no-member def test_existing_provides_is_not_spec(self): def foo(): @@ -1738,6 +1743,38 @@ class Foo(object): spec = self._callFUT(foo) self.assertEqual(list(spec), []) + def test_catches_only_AttributeError_on_provides(self): + MissingSomeAttrs.test_raises(self, self._callFUT, expected_missing='__provides__') + + def test_catches_only_AttributeError_on_class(self): + MissingSomeAttrs.test_raises(self, self._callFUT, expected_missing='__class__', + __provides__=None) + + def test_raises_AttributeError_when_provides_fails_type_check_AttributeError(self): + # isinstance(ob.__provides__, SpecificationBase) is not + # protected inside any kind of block. + + class Foo(object): + __provides__ = MissingSomeAttrs(AttributeError) + + # isinstance() ignores AttributeError on __class__ + self._callFUT(Foo()) + + def test_raises_AttributeError_when_provides_fails_type_check_RuntimeError(self): + # isinstance(ob.__provides__, SpecificationBase) is not + # protected inside any kind of block. + class Foo(object): + __provides__ = MissingSomeAttrs(RuntimeError) + + if PYTHON3: + with self.assertRaises(RuntimeError) as exc: + self._callFUT(Foo()) + + self.assertEqual('__class__', exc.exception.args[0]) + else: + # Python 2 catches everything. + self._callFUT(Foo()) + class Test_getObjectSpecification(Test_getObjectSpecificationFallback, OptimizationTestMixin): @@ -1751,6 +1788,7 @@ def _getTargetClass(self): class Test_providedByFallback(unittest.TestCase): def _getFallbackClass(self): + # pylint:disable=no-name-in-module from zope.interface.declarations import providedByFallback return providedByFallback @@ -1978,6 +2016,19 @@ class Derived(M1, M2): self.assertEqual(list(self._callFUT(sm2)), [IBase]) + def test_catches_only_AttributeError_on_providedBy(self): + MissingSomeAttrs.test_raises(self, self._callFUT, + expected_missing='__providedBy__', + __class__=object) + + def test_catches_only_AttributeError_on_class(self): + # isinstance() tries to get the __class__, which is non-obvious, + # so it must be protected too. + PY3 = str is not bytes + MissingSomeAttrs.test_raises(self, self._callFUT, + expected_missing='__class__' if PY3 else '__providedBy__') + + class Test_providedBy(Test_providedByFallback, OptimizationTestMixin): @@ -1991,6 +2042,7 @@ def _getTargetClass(self): class ObjectSpecificationDescriptorFallbackTests(unittest.TestCase): def _getFallbackClass(self): + # pylint:disable=no-name-in-module from zope.interface.declarations \ import ObjectSpecificationDescriptorFallback return ObjectSpecificationDescriptorFallback @@ -2059,7 +2111,7 @@ class _Monkey(object): # context-manager for replacing module names in the scope of a test. def __init__(self, module, **kw): self.module = module - self.to_restore = dict([(key, getattr(module, key)) for key in kw]) + self.to_restore = {key: getattr(module, key) for key in kw} for key, value in kw.items(): setattr(module, key, value) diff --git a/src/zope/interface/tests/test_interface.py b/src/zope/interface/tests/test_interface.py index 7b7b7e8d..21003407 100644 --- a/src/zope/interface/tests/test_interface.py +++ b/src/zope/interface/tests/test_interface.py @@ -24,6 +24,7 @@ import unittest from zope.interface._compat import _skip_under_py3k +from zope.interface.tests import MissingSomeAttrs from zope.interface.tests import OptimizationTestMixin from zope.interface.tests import CleanUp @@ -396,8 +397,13 @@ def test___call___wo___conform___ob_no_provides_w_alternate(self): def test___call___w___conform___ob_no_provides_wo_alternate(self): ib = self._makeOne(False) - adapted = object() - self.assertRaises(TypeError, ib, adapted) + with self.assertRaises(TypeError) as exc: + ib(object()) + + self.assertIn('Could not adapt', str(exc.exception)) + + def test___call___w_no_conform_catches_only_AttributeError(self): + MissingSomeAttrs.test_raises(self, self._makeOne(), expected_missing='__conform__') class InterfaceBaseTests(InterfaceBaseTestsMixin, From a4ed0566f9dd9cd65af39576f6c8fdeb252d1ea9 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 2 Apr 2020 08:10:23 -0500 Subject: [PATCH 2/3] MS VS stuck on c89 strikes again. --- src/zope/interface/_zope_interface_coptimizations.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c index 9e891697..69f5bbcf 100644 --- a/src/zope/interface/_zope_interface_coptimizations.c +++ b/src/zope/interface/_zope_interface_coptimizations.c @@ -799,9 +799,9 @@ static PyObject * ib_call(PyObject *self, PyObject *args, PyObject *kwargs) { PyObject *conform, *obj, *alternate, *adapter; + static char *kwlist[] = {"obj", "alternate", NULL}; conform = obj = alternate = adapter = NULL; - static char *kwlist[] = {"obj", "alternate", NULL}; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|O", kwlist, &obj, &alternate)) From 719851072c41ff0914def1368716a049859ed32a Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 2 Apr 2020 10:35:51 -0500 Subject: [PATCH 3/3] More micro benchmarks. Comparing to current master, there is no substantial difference. +-------------------------------------------------------------+----------------+------------------------------+ | Benchmark | bench_master38 | bench_issue200 | +=============================================================+================+==============================+ | call interface (alternate, no conform, not provided) | 395 ns | 414 ns: 1.05x slower (+5%) | +-------------------------------------------------------------+----------------+------------------------------+ | call interface (no alternate, valid conform, not provided) | 250 ns | 240 ns: 1.04x faster (-4%) | +-------------------------------------------------------------+----------------+------------------------------+ | read __module__ | 45.3 ns | 43.4 ns: 1.04x faster (-4%) | +-------------------------------------------------------------+----------------+------------------------------+ | query adapter (no registrations) | 3.23 ms | 3.31 ms: 1.02x slower (+2%) | +-------------------------------------------------------------+----------------+------------------------------+ | query adapter (all trivial registrations) | 3.93 ms | 4.40 ms: 1.12x slower (+12%) | +-------------------------------------------------------------+----------------+------------------------------+ | query adapter (all trivial registrations, wide inheritance) | 43.3 us | 45.5 us: 1.05x slower (+5%) | +-------------------------------------------------------------+----------------+------------------------------+ | query adapter (all trivial registrations, deep inheritance) | 43.2 us | 46.9 us: 1.09x slower (+9%) | +-------------------------------------------------------------+----------------+------------------------------+ | sort mixed | 361 us | 354 us: 1.02x faster (-2%) | +-------------------------------------------------------------+----------------+------------------------------+ | contains (populated dict: interfaces) | 61.3 ns | 59.7 ns: 1.03x faster (-3%) | +-------------------------------------------------------------+----------------+------------------------------+ Not significant (13): call interface (provides; deep); call interface (provides; wide); call interface (no alternate, no conform, not provided); call interface (alternate, invalid conform, not provided); read __name__; read __doc__; read providedBy; sort interfaces; sort implementedBy; contains (empty dict); contains (populated list: interfaces); contains (populated dict: implementedBy); contains (populated list: implementedBy) --- benchmarks/micro.py | 118 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 2 deletions(-) diff --git a/benchmarks/micro.py b/benchmarks/micro.py index a9527dba..dce40cee 100644 --- a/benchmarks/micro.py +++ b/benchmarks/micro.py @@ -37,6 +37,21 @@ class DeepestInheritance(object): pass classImplements(DeepestInheritance, deep_ifaces[-1]) + +class ImplementsNothing(object): + pass + + +class HasConformReturnNone(object): + def __conform__(self, iface): + return None + + +class HasConformReturnObject(object): + def __conform__(self, iface): + return self + + def make_implementer(iface): c = type('Implementer' + iface.__name__, (object,), {}) classImplements(c, iface) @@ -77,16 +92,17 @@ def bench_sort(loops, objs): return pyperf.perf_counter() - t0 def bench_query_adapter(loops, components, objs=providers): + components_queryAdapter = components.queryAdapter # One time through to prime the caches for iface in ifaces: for provider in providers: - components.queryAdapter(provider, iface) + components_queryAdapter(provider, iface) t0 = pyperf.perf_counter() for _ in range(loops): for iface in ifaces: for provider in objs: - components.queryAdapter(provider, iface) + components_queryAdapter(provider, iface) return pyperf.perf_counter() - t0 @@ -106,8 +122,106 @@ def bench_getattr(loops, name, get=getattr): get(Interface, name) # 10 return pyperf.perf_counter() - t0 + +def bench_iface_call_no_conform_no_alternate_not_provided(loops): + inst = ImplementsNothing() + t0 = pyperf.perf_counter() + for _ in range(loops): + for _ in range(INNER): + for iface in ifaces: + try: + iface(inst) + except TypeError: + pass + else: + raise TypeError("Should have failed") + return pyperf.perf_counter() - t0 + + +def bench_iface_call_no_conform_w_alternate_not_provided(loops): + inst = ImplementsNothing() + t0 = pyperf.perf_counter() + for _ in range(loops): + for _ in range(INNER): + for iface in ifaces: + iface(inst, 42) + return pyperf.perf_counter() - t0 + + +def bench_iface_call_w_conform_return_none_not_provided(loops): + inst = HasConformReturnNone() + t0 = pyperf.perf_counter() + for _ in range(loops): + for _ in range(INNER): + for iface in ifaces: + iface(inst, 42) + return pyperf.perf_counter() - t0 + + +def bench_iface_call_w_conform_return_non_none_not_provided(loops): + inst = HasConformReturnObject() + t0 = pyperf.perf_counter() + for _ in range(loops): + for _ in range(INNER): + for iface in ifaces: + iface(inst) + return pyperf.perf_counter() - t0 + +def _bench_iface_call_simple(loops, inst): + t0 = pyperf.perf_counter() + for _ in range(loops): + for _ in range(INNER): + for iface in ifaces: + iface(inst) + return pyperf.perf_counter() - t0 + + +def bench_iface_call_no_conform_provided_wide(loops): + return _bench_iface_call_simple(loops, WideInheritance()) + + +def bench_iface_call_no_conform_provided_deep(loops): + return _bench_iface_call_simple(loops, DeepestInheritance()) + + runner = pyperf.Runner() +runner.bench_time_func( + 'call interface (provides; deep)', + bench_iface_call_no_conform_provided_deep, + inner_loops=INNER * len(ifaces) +) + +runner.bench_time_func( + 'call interface (provides; wide)', + bench_iface_call_no_conform_provided_wide, + inner_loops=INNER * len(ifaces) +) + +runner.bench_time_func( + 'call interface (no alternate, no conform, not provided)', + bench_iface_call_no_conform_no_alternate_not_provided, + inner_loops=INNER * len(ifaces) +) + +runner.bench_time_func( + 'call interface (alternate, no conform, not provided)', + bench_iface_call_no_conform_w_alternate_not_provided, + inner_loops=INNER * len(ifaces) +) + +runner.bench_time_func( + 'call interface (no alternate, valid conform, not provided)', + bench_iface_call_w_conform_return_non_none_not_provided, + inner_loops=INNER * len(ifaces) +) + +runner.bench_time_func( + 'call interface (alternate, invalid conform, not provided)', + bench_iface_call_w_conform_return_none_not_provided, + inner_loops=INNER * len(ifaces) +) + runner.bench_time_func( 'read __module__', # stored in C, accessed through __getattribute__ bench_getattr,