Skip to content

Make @implementer and classImplements not add redundant interfaces #203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 8, 2020
Merged

Make @implementer and classImplements not add redundant interfaces #203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46781f8
Make @implementer and classImplements not re-declare redundant interf…
jamadden Apr 6, 2020
4cb3e63
Special case the base Interface in @implementer/classImplements to sk…
jamadden Apr 6, 2020
97c0fbc
Fix implementerOnly when duplicating something inherited from the par…
jamadden Apr 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
5.1.0 (unreleased)
==================

- Make ``@implementer(*iface)`` and ``classImplements(cls, *iface)``
ignore redundant interfaces. If the class already implements an
interface through inheritance, it is no longer redeclared
specifically for *cls*. This solves many instances of inconsistent
resolution orders, while still allowing the interface to be declared
for readability and maintenance purposes. See `issue 199
<https://github.com/zopefoundation/zope.interface/issues/199>`_.

- Remove all bare ``except:`` statements. Previously, when accessing
special attributes such as ``__provides__``, ``__providedBy__``,
``__class__`` and ``__conform__``, this package wrapped such access
Expand Down
2 changes: 1 addition & 1 deletion docs/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ declarations. Here's a silly example:
... zope.interface.implementedBy(Foo),
... ISpecial,
... )
... class Special2(Foo):
... class Special2(object):
... reason = 'I just am'
... def brag(self):
... return "I'm special because %s" % self.reason
Expand Down
17 changes: 10 additions & 7 deletions docs/api/declarations.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,31 +208,34 @@ interfaces instances of ``A`` and ``B`` provide.

Instances of ``C`` now also provide ``I5``. Notice how ``I5`` was
added to the *beginning* of the list of things provided directly by
``C``. Unlike `classImplements`, this ignores inheritance and other
factors and does not attempt to ensure a consistent resolution order.
``C``. Unlike `classImplements`, this ignores interface inheritance
and does not attempt to ensure a consistent resolution order (except
that it continues to elide interfaces already implemented through
class inheritance)::

.. doctest::

>>> class IBA(IB, IA): pass
>>> class IBA(IB, IA):
... pass
>>> classImplementsFirst(C, IBA)
>>> classImplementsFirst(C, IA)
>>> [i.getName() for i in implementedBy(C)]
['IA', 'IBA', 'I5', 'I1', 'I2', 'IB']
['IBA', 'I5', 'I1', 'I2', 'IA', 'IB']

This cannot be used to introduce duplicates.

.. doctest::

>>> len(implementedBy(C).declared)
5
4
>>> classImplementsFirst(C, IA)
>>> classImplementsFirst(C, IBA)
>>> classImplementsFirst(C, IA)
>>> classImplementsFirst(C, IBA)
>>> [i.getName() for i in implementedBy(C)]
['IBA', 'IA', 'I5', 'I1', 'I2', 'IB']
['IBA', 'I5', 'I1', 'I2', 'IA', 'IB']
>>> len(implementedBy(C).declared)
5
4


directlyProvides
Expand Down
126 changes: 85 additions & 41 deletions src/zope/interface/declarations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class implements (that instances of the class provides).
import weakref

from zope.interface.advice import addClassAdvisor
from zope.interface.interface import Interface
from zope.interface.interface import InterfaceClass
from zope.interface.interface import SpecificationBase
from zope.interface.interface import Specification
Expand Down Expand Up @@ -418,18 +419,26 @@ def implementedBy(cls): # pylint:disable=too-many-return-statements,too-many-bra


def classImplementsOnly(cls, *interfaces):
"""Declare the only interfaces implemented by instances of a class
"""
Declare the only interfaces implemented by instances of a class

The arguments after the class are one or more interfaces or interface
specifications (`~zope.interface.interfaces.IDeclaration` objects).
The arguments after the class are one or more interfaces or interface
specifications (`~zope.interface.interfaces.IDeclaration` objects).

The interfaces given (including the interfaces in the specifications)
replace any previous declarations.
The interfaces given (including the interfaces in the specifications)
replace any previous declarations, *including* inherited definitions. If you
wish to preserve inherited declarations, you can pass ``implementedBy(cls)``
in *interfaces*. This can be used to alter the interface resolution order.
"""
spec = implementedBy(cls)
# Clear out everything inherited. It's important to
# also clear the bases right now so that we don't improperly discard
# interfaces that are already implemented by *old* bases that we're
# about to get rid of.
spec.declared = ()
spec.inherit = None
classImplements(cls, *interfaces)
spec.__bases__ = ()
_classImplements_ordered(spec, interfaces, ())


def classImplements(cls, *interfaces):
Expand All @@ -448,6 +457,14 @@ def classImplements(cls, *interfaces):
beginning or end of the list of interfaces declared for *cls*,
based on inheritance, in order to try to maintain a consistent
resolution order. Previously, all interfaces were added to the end.
.. versionchanged:: 5.1.0
If *cls* is already declared to implement an interface (or derived interface)
in *interfaces* through inheritance, the interface is ignored. Previously, it
would redundantly be made direct base of *cls*, which often produced inconsistent
interface resolution orders. Now, the order will be consistent, but may change.
Also, if the ``__bases__`` of the *cls* are later changed, the *cls* will no
longer be considered to implement such an interface (changing the ``__bases__`` of *cls*
has never been supported).
"""
spec = implementedBy(cls)
interfaces = tuple(_normalizeargs(interfaces))
Expand Down Expand Up @@ -483,13 +500,30 @@ def classImplementsFirst(cls, iface):


def _classImplements_ordered(spec, before=(), after=()):
# Elide everything already inherited.
# Except, if it is the root, and we don't already declare anything else
# that would imply it, allow the root through. (TODO: When we disallow non-strict
# IRO, this part of the check can be removed because it's not possible to re-declare
# like that.)
before = [
x
for x in before
if not spec.isOrExtends(x) or (x is Interface and not spec.declared)
]
after = [
x
for x in after
if not spec.isOrExtends(x) or (x is Interface and not spec.declared)
]

# eliminate duplicates
new_declared = []
seen = set()
for b in before + spec.declared + after:
if b not in seen:
new_declared.append(b)
seen.add(b)
for l in before, spec.declared, after:
for b in l:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does pylint not complain about using lowercase-ell as a variable name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably would, but the check for valid names has to be disabled when working in zope repos because the false positives are overwhelming. (It's not the first one-letter temporary variable in this file.)

image

if b not in seen:
new_declared.append(b)
seen.add(b)

spec.declared = tuple(new_declared)

Expand All @@ -514,33 +548,38 @@ def _implements_advice(cls):


class implementer(object):
"""Declare the interfaces implemented by instances of a class.
"""
Declare the interfaces implemented by instances of a class.

This function is called as a class decorator.
This function is called as a class decorator.

The arguments are one or more interfaces or interface
specifications (`~zope.interface.interfaces.IDeclaration` objects).
The arguments are one or more interfaces or interface
specifications (`~zope.interface.interfaces.IDeclaration`
objects).

The interfaces given (including the interfaces in the
specifications) are added to any interfaces previously
declared.
The interfaces given (including the interfaces in the
specifications) are added to any interfaces previously declared,
unless the interface is already implemented.

Previous declarations include declarations for base classes
unless implementsOnly was used.
Previous declarations include declarations for base classes unless
implementsOnly was used.

This function is provided for convenience. It provides a more
convenient way to call `classImplements`. For example::
This function is provided for convenience. It provides a more
convenient way to call `classImplements`. For example::

@implementer(I1)
class C(object):
pass

is equivalent to calling::
is equivalent to calling::

classImplements(C, I1)

after the class has been created.
"""
after the class has been created.

.. seealso:: `classImplements`
The change history provided there applies to this function too.
"""
__slots__ = ('interfaces',)

def __init__(self, *interfaces):
Expand Down Expand Up @@ -595,10 +634,10 @@ def __call__(self, ob):
# on a method or function....
raise ValueError('The implementer_only decorator is not '
'supported for methods or functions.')
else:
# Assume it's a class:
classImplementsOnly(ob, *self.interfaces)
return ob

# Assume it's a class:
classImplementsOnly(ob, *self.interfaces)
return ob

def _implements(name, interfaces, do_classImplements):
# This entire approach is invalid under Py3K. Don't even try to fix
Expand All @@ -619,30 +658,35 @@ def _implements(name, interfaces, do_classImplements):
addClassAdvisor(_implements_advice, depth=3)

def implements(*interfaces):
"""Declare interfaces implemented by instances of a class
"""
Declare interfaces implemented by instances of a class.

This function is called in a class definition.
.. deprecated:: 5.0
This only works for Python 2. The `implementer` decorator
is preferred for all versions.

The arguments are one or more interfaces or interface
specifications (`~zope.interface.interfaces.IDeclaration` objects).
This function is called in a class definition.

The interfaces given (including the interfaces in the
specifications) are added to any interfaces previously
declared.
The arguments are one or more interfaces or interface
specifications (`~zope.interface.interfaces.IDeclaration`
objects).

Previous declarations include declarations for base classes
unless `implementsOnly` was used.
The interfaces given (including the interfaces in the
specifications) are added to any interfaces previously declared.

This function is provided for convenience. It provides a more
convenient way to call `classImplements`. For example::
Previous declarations include declarations for base classes unless
`implementsOnly` was used.

This function is provided for convenience. It provides a more
convenient way to call `classImplements`. For example::

implements(I1)

is equivalent to calling::
is equivalent to calling::

classImplements(C, I1)

after the class has been created.
after the class has been created.
"""
# This entire approach is invalid under Py3K. Don't even try to fix
# the coverage for this block there. :(
Expand Down
Loading