Merge pull request #191 from zopefoundation/issue190

Make Interface.getTaggedValue follow the __iro__.

7 files changed, 292 insertions, 62 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 6f3eabc..44bb335 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,16 +5,6 @@
 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
@@ -57,6 +47,12 @@
   The changes in this release resulted in a 7% memory reduction after
   loading about 6,000 modules that define about 2,200 interfaces.
 
+  .. caution::
+
+     Details of many private attributes have changed, and external use
+     of those private attributes may break. In particular, the
+     lifetime and default value of ``_v_attrs`` has changed.
+
 - Remove support for hashing uninitialized interfaces. This could only
   be done by subclassing ``InterfaceClass``. This has generated a
   warning since it was first added in 2011 (3.6.5). Please call the
@@ -162,9 +158,12 @@
 - 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.
+- Adopt Python's standard `C3 resolution order
+  <https://www.python.org/download/releases/2.3/mro/>`_ to compute the
+  ``__iro___`` and ``__sro___`` of interfaces, with tweaks to support
+  additional cases that are common in interfaces but disallowed for
+  Python classes. 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
@@ -195,6 +194,17 @@
   the future). For details, see the documentation for
   ``zope.interface.ro``.
 
+- Make inherited tagged values in interfaces respect the resolution
+  order (``__iro__``), as method and attribute lookup does. Previously
+  tagged values could give inconsistent results. See `issue 190
+  <https://github.com/zopefoundation/zope.interface/issues/190>`_.
+
+- Add ``getDirectTaggedValue`` (and related methods) to interfaces to
+  allow accessing tagged values irrespective of inheritance. See
+  `issue 190
+  <https://github.com/zopefoundation/zope.interface/issues/190>`_.
+
+
 4.7.2 (2020-03-10)
 ==================
 
@@ -214,10 +224,13 @@
 
 - Drop support for Python 3.4.
 
-- Fix ``queryTaggedValue``, ``getTaggedValue``, ``getTaggedValueTags``
-  subclass inheritance. See `PR 144
+- Change ``queryTaggedValue``, ``getTaggedValue``,
+  ``getTaggedValueTags`` in interfaces. They now include inherited
+  values by following ``__bases__``. See `PR 144
   <https://github.com/zopefoundation/zope.interface/pull/144>`_.
 
+  .. caution:: This may be a breaking change.
+
 - Add support for Python 3.8.
 
 
diff --git a/docs/README.rst b/docs/README.rst
index d1e58cf..a8068f8 100644
--- a/docs/README.rst
+++ b/docs/README.rst
@@ -736,6 +736,28 @@ Tagged values can also be defined from within an interface definition:
   >>> IWithTaggedValues.getTaggedValue('squish')
   'squash'
 
+Tagged values are inherited in the same way that attribute and method
+descriptions are. Inheritance can be ignored by using the "direct"
+versions of functions.
+
+.. doctest::
+
+   >>> class IExtendsIWithTaggedValues(IWithTaggedValues):
+   ...     zope.interface.taggedValue('child', True)
+   >>> IExtendsIWithTaggedValues.getTaggedValue('child')
+   True
+   >>> IExtendsIWithTaggedValues.getDirectTaggedValue('child')
+   True
+   >>> IExtendsIWithTaggedValues.getTaggedValue('squish')
+   'squash'
+   >>> print(IExtendsIWithTaggedValues.queryDirectTaggedValue('squish'))
+   None
+   >>> IExtendsIWithTaggedValues.setTaggedValue('squish', 'SQUASH')
+   >>> IExtendsIWithTaggedValues.getTaggedValue('squish')
+   'SQUASH'
+   >>> IExtendsIWithTaggedValues.getDirectTaggedValue('squish')
+   'SQUASH'
+
 Invariants
 ==========
 
diff --git a/docs/api/declarations.rst b/docs/api/declarations.rst
index ce52833..26847b0 100644
--- a/docs/api/declarations.rst
+++ b/docs/api/declarations.rst
@@ -16,7 +16,7 @@ carefully at each object it documents, including providing examples.
 .. autointerface:: zope.interface.interfaces.IInterfaceDeclaration
 
 
-.. currentmodule:: zope.interface.declarations
+.. currentmodule:: zope.interface
 
 Declaring The Interfaces of Objects
 ===================================
@@ -536,7 +536,7 @@ You'll notice that an ``IDeclaration`` is a type of
 implementedBy
 -------------
 
-.. autofunction:: implementedByFallback
+.. autofunction:: implementedBy
 
 
 Consider the following example:
@@ -774,7 +774,7 @@ Exmples for :meth:`Declaration.__add__`:
 ProvidesClass
 -------------
 
-.. autoclass:: ProvidesClass
+.. autoclass:: zope.interface.declarations.ProvidesClass
 
 
 Descriptor semantics (via ``Provides.__get__``):
@@ -851,7 +851,7 @@ collect function to help with this:
 ObjectSpecification
 -------------------
 
-.. autofunction:: ObjectSpecification
+.. autofunction:: zope.interface.declarations.ObjectSpecification
 
 
 For example:
@@ -924,7 +924,7 @@ For example:
 ObjectSpecificationDescriptor
 -----------------------------
 
-.. autoclass:: ObjectSpecificationDescriptor
+.. autoclass:: zope.interface.declarations.ObjectSpecificationDescriptor
 
 For example:
 
diff --git a/docs/api/specifications.rst b/docs/api/specifications.rst
index 2152e26..357e361 100644
--- a/docs/api/specifications.rst
+++ b/docs/api/specifications.rst
@@ -23,6 +23,7 @@ Specification objects implement the API defined by
    :member-order: bysource
 
 .. autoclass:: zope.interface.interface.Specification
+   :no-members:
 
 For example:
 
@@ -172,7 +173,22 @@ first is that of an "element", which provides us a simple way to query
 for information generically (this is important because we'll see that
 ``IInterface`` implements this interface):
 
+..
+  IElement defines __doc__ to be an Attribute, so the docstring
+  in the class isn't used._
+
 .. autointerface:: IElement
+
+   Objects that have basic documentation and tagged values.
+
+   Known derivatives include :class:`IAttribute` and its derivative
+   :class:`IMethod`; these have no notion of inheritance.
+   :class:`IInterface` is also a derivative, and it does have a
+   notion of inheritance, expressed through its ``__bases__`` and
+   ordered in its ``__iro__`` (both defined by
+   :class:`ISpecification`).
+
+
 .. autoclass:: zope.interface.interface.Element
    :no-members:
 
@@ -181,14 +197,17 @@ content, or body, of an ``Interface``.
 
 .. autointerface:: zope.interface.interfaces.IAttribute
 .. autoclass:: zope.interface.interface.Attribute
+   :no-members:
 
-.. autoclass:: IMethod
+.. autointerface:: IMethod
+.. autoclass:: zope.interface.interface.Method
+   :no-members:
 
 Finally we can look at the definition of ``IInterface``.
 
 .. autointerface:: IInterface
 
-.. autoclass:: zope.interface.Interface
+.. autointerface:: zope.interface.Interface
 
 Usage
 -----
diff --git a/src/zope/interface/interface.py b/src/zope/interface/interface.py
index 3e81b6f..f9fb333 100644
--- a/src/zope/interface/interface.py
+++ b/src/zope/interface/interface.py
@@ -87,6 +87,13 @@ class Element(object):
         """ Returns the documentation for the object. """
         return self.__doc__
 
+    ###
+    # Tagged values.
+    #
+    # Direct tagged values are set only in this instance. Others
+    # may be inherited (for those subclasses that have that concept).
+    ###
+
     def getTaggedValue(self, tag):
         """ Returns the value associated with 'tag'. """
         if not self.__tagged_values:
@@ -98,7 +105,7 @@ class Element(object):
         return self.__tagged_values.get(tag, default) if self.__tagged_values else default
 
     def getTaggedValueTags(self):
-        """ Returns a list of all tags. """
+        """ Returns a collection of all tags. """
         return self.__tagged_values.keys() if self.__tagged_values else ()
 
     def setTaggedValue(self, tag, value):
@@ -107,6 +114,10 @@ class Element(object):
             self.__tagged_values = {}
         self.__tagged_values[tag] = value
 
+    queryDirectTaggedValue = queryTaggedValue
+    getDirectTaggedValue = getTaggedValue
+    getDirectTaggedValueTags = getTaggedValueTags
+
 
 @_use_c_impl
 class SpecificationBase(object):
@@ -512,12 +523,14 @@ class InterfaceClass(Element, InterfaceBase, Specification):
             raise Invalid(errors)
 
     def queryTaggedValue(self, tag, default=None):
-        """ Returns the value associated with 'tag'. """
-        value = Element.queryTaggedValue(self, tag, default=_marker)
-        if value is not _marker:
-            return value
-        for base in self.__bases__:
-            value = base.queryTaggedValue(tag, default=_marker)
+        """
+        Queries for the value associated with *tag*, returning it from the nearest
+        interface in the ``__iro__``.
+
+        If not found, returns *default*.
+        """
+        for iface in self.__iro__:
+            value = iface.queryDirectTaggedValue(tag, _marker)
             if value is not _marker:
                 return value
         return default
@@ -531,11 +544,9 @@ class InterfaceClass(Element, InterfaceBase, Specification):
 
     def getTaggedValueTags(self):
         """ Returns a list of all tags. """
-        keys = list(Element.getTaggedValueTags(self))
-        for base in self.__bases__:
-            for key in base.getTaggedValueTags():
-                if key not in keys:
-                    keys.append(key)
+        keys = set()
+        for base in self.__iro__:
+            keys.update(base.getDirectTaggedValueTags())
         return keys
 
     def __repr__(self):  # pragma: no cover
@@ -783,6 +794,9 @@ def fromMethod(meth, interface=None, name=None):
 def _wire():
     from zope.interface.declarations import classImplements
 
+    from zope.interface.interfaces import IElement
+    classImplements(Element, IElement)
+
     from zope.interface.interfaces import IAttribute
     classImplements(Attribute, IAttribute)
 
diff --git a/src/zope/interface/interfaces.py b/src/zope/interface/interfaces.py
index bf0d6c7..9ddf8f0 100644
--- a/src/zope/interface/interfaces.py
+++ b/src/zope/interface/interfaces.py
@@ -44,29 +44,96 @@ __all__ = [
 
 
 class IElement(Interface):
-    """Objects that have basic documentation and tagged values.
     """
+    Objects that have basic documentation and tagged values.
+
+    Known derivatives include :class:`IAttribute` and its derivative
+    :class:`IMethod`; these have no notion of inheritance.
+    :class:`IInterface` is also a derivative, and it does have a
+    notion of inheritance, expressed through its ``__bases__`` and
+    ordered in its ``__iro__`` (both defined by
+    :class:`ISpecification`).
+    """
+
+    # Note that defining __doc__ as an Attribute hides the docstring
+    # from introspection. When changing it, also change it in the Sphinx
+    # ReST files.
 
     __name__ = Attribute('__name__', 'The object name')
     __doc__  = Attribute('__doc__', 'The object doc string')
 
+    ###
+    # Tagged values.
+    #
+    # Direct values are established in this instance. Others may be
+    # inherited. Although ``IElement`` itself doesn't have a notion of
+    # inheritance, ``IInterface`` *does*. It might have been better to
+    # make ``IInterface`` define new methods
+    # ``getIndirectTaggedValue``, etc, to include inheritance instead
+    # of overriding ``getTaggedValue`` to do that, but that ship has sailed.
+    # So to keep things nice and symmetric, we define the ``Direct`` methods here.
+    ###
+
     def getTaggedValue(tag):
-        """Returns the value associated with `tag`.
+        """Returns the value associated with *tag*.
+
+        Raise a `KeyError` if the tag isn't set.
 
-        Raise a `KeyError` of the tag isn't set.
+        If the object has a notion of inheritance, this searches
+        through the inheritance hierarchy and returns the nearest result.
+        If there is no such notion, this looks only at this object.
+
+        .. versionchanged:: 4.7.0
+           This method should respect inheritance if present.
         """
 
     def queryTaggedValue(tag, default=None):
-        """Returns the value associated with `tag`.
+        """
+        As for `getTaggedValue`, but instead of raising a `KeyError`, returns *default*.
 
-        Return the default value of the tag isn't set.
+
+        .. versionchanged:: 4.7.0
+           This method should respect inheritance if present.
         """
 
     def getTaggedValueTags():
-        """Returns a list of all tags."""
+        """
+        Returns a collection of all tags in no particular order.
+
+        If the object has a notion of inheritance, this
+        includes all the inherited tagged values. If there is
+        no such notion, this looks only at this object.
+
+        .. versionchanged:: 4.7.0
+           This method should respect inheritance if present.
+        """
 
     def setTaggedValue(tag, value):
-        """Associates `value` with `key`."""
+        """
+        Associates *value* with *key* directly in this object.
+        """
+
+    def getDirectTaggedValue(tag):
+        """
+        As for `getTaggedValue`, but never includes inheritance.
+
+        .. versionadded:: 5.0.0
+        """
+
+    def queryDirectTaggedValue(tag, default=None):
+        """
+        As for `queryTaggedValue`, but never includes inheritance.
+
+        .. versionadded:: 5.0.0
+        """
+
+    def getDirectTaggedValueTags():
+        """
+        As for `getTaggedValueTags`, but includes only tags directly
+        set on this object.
+
+        .. versionadded:: 5.0.0
+        """
 
 
 class IAttribute(IElement):
@@ -83,25 +150,26 @@ class IMethod(IAttribute):
     def getSignatureInfo():
         """Returns the signature information.
 
-        This method returns a dictionary with the following keys:
-
-        o `positional` - All positional arguments.
-
-        o `required` - A list of all required arguments.
+        This method returns a dictionary with the following string keys:
 
-        o `optional` - A list of all optional arguments.
-
-        o `varargs` - The name of the varargs argument.
-
-        o `kwargs` - The name of the kwargs argument.
+        - positional
+            A sequence of the names of positional arguments.
+        - required
+            A sequence of the names of required arguments.
+        - optional
+            A dictionary mapping argument names to their default values.
+        - varargs
+            The name of the varargs argument (or None).
+        - kwargs
+            The name of the kwargs argument (or None).
         """
 
     def getSignatureString():
         """Return a signature string suitable for inclusion in documentation.
 
         This method returns the function signature string. For example, if you
-        have `func(a, b, c=1, d='f')`, then the signature string is `(a, b,
-        c=1, d='f')`.
+        have ``def func(a, b, c=1, d='f')``, then the signature string is ``"(a, b,
+        c=1, d='f')"``.
         """
 
 class ISpecification(Interface):
@@ -148,7 +216,7 @@ class ISpecification(Interface):
 
     __bases__ = Attribute("""Base specifications
 
-    A tuple if specifications from which this specification is
+    A tuple of specifications from which this specification is
     directly derived.
 
     """)
@@ -156,14 +224,15 @@ class ISpecification(Interface):
     __sro__ = Attribute("""Specification-resolution order
 
     A tuple of the specification and all of it's ancestor
-    specifications from most specific to least specific.
+    specifications from most specific to least specific. The specification
+    itself is the first element.
 
     (This is similar to the method-resolution order for new-style classes.)
     """)
 
     __iro__ = Attribute("""Interface-resolution order
 
-    A tuple of the of the specification's ancestor interfaces from
+    A tuple of the specification's ancestor interfaces from
     most specific to least specific.  The specification itself is
     included if it is an interface.
 
@@ -240,14 +309,14 @@ class IInterface(ISpecification, IElement):
 
     - You assert that your object implement the interfaces.
 
-      There are several ways that you can assert that an object
-      implements an interface:
+      There are several ways that you can declare that an object
+      provides an interface:
 
-      1. Call `zope.interface.implements` in your class definition.
+      1. Call `zope.interface.implementer` on your class definition.
 
-      2. Call `zope.interfaces.directlyProvides` on your object.
+      2. Call `zope.interface.directlyProvides` on your object.
 
-      3. Call `zope.interface.classImplements` to assert that instances
+      3. Call `zope.interface.classImplements` to declare that instances
          of a class implement an interface.
 
          For example::
@@ -321,6 +390,7 @@ class IInterface(ISpecification, IElement):
 
     __module__ = Attribute("""The name of the module defining the interface""")
 
+
 class IDeclaration(ISpecification):
     """Interface declaration
 
diff --git a/src/zope/interface/tests/test_interface.py b/src/zope/interface/tests/test_interface.py
index df7f84b..70e5d64 100644
--- a/src/zope/interface/tests/test_interface.py
+++ b/src/zope/interface/tests/test_interface.py
@@ -121,6 +121,13 @@ class ElementTests(unittest.TestCase):
         element = self._makeOne()
         self.assertRaises(KeyError, element.getTaggedValue, 'nonesuch')
 
+    def test_getDirectTaggedValueTags(self):
+        element = self._makeOne()
+        self.assertEqual([], list(element.getDirectTaggedValueTags()))
+
+        element.setTaggedValue('foo', 'bar')
+        self.assertEqual(['foo'], list(element.getDirectTaggedValueTags()))
+
     def test_queryTaggedValue_miss(self):
         element = self._makeOne()
         self.assertEqual(element.queryTaggedValue('nonesuch'), None)
@@ -129,6 +136,18 @@ class ElementTests(unittest.TestCase):
         element = self._makeOne()
         self.assertEqual(element.queryTaggedValue('nonesuch', 'bar'), 'bar')
 
+    def test_getDirectTaggedValue_miss(self):
+        element = self._makeOne()
+        self.assertRaises(KeyError, element.getDirectTaggedValue, 'nonesuch')
+
+    def test_queryDirectTaggedValue_miss(self):
+        element = self._makeOne()
+        self.assertEqual(element.queryDirectTaggedValue('nonesuch'), None)
+
+    def test_queryDirectTaggedValue_miss_w_default(self):
+        element = self._makeOne()
+        self.assertEqual(element.queryDirectTaggedValue('nonesuch', 'bar'), 'bar')
+
     def test_setTaggedValue(self):
         element = self._makeOne()
         element.setTaggedValue('foo', 'bar')
@@ -136,6 +155,13 @@ class ElementTests(unittest.TestCase):
         self.assertEqual(element.getTaggedValue('foo'), 'bar')
         self.assertEqual(element.queryTaggedValue('foo'), 'bar')
 
+    def test_verifies(self):
+        from zope.interface.interfaces import IElement
+        from zope.interface.verify import verifyObject
+
+        element = self._makeOne()
+        verifyObject(IElement, element)
+
 
 class GenericSpecificationBaseTests(unittest.TestCase):
     # Tests that work with both implementations
@@ -1792,12 +1818,78 @@ class InterfaceTests(unittest.TestCase):
 
         self.assertEqual(ITagged.getTaggedValue('qux'), 'Spam')
         self.assertRaises(KeyError, ITagged.getTaggedValue, 'foo')
-        self.assertEqual(ITagged.getTaggedValueTags(), ['qux'])
+        self.assertEqual(list(ITagged.getTaggedValueTags()), ['qux'])
 
         self.assertEqual(IDerived2.getTaggedValue('qux'), 'Spam Spam')
         self.assertEqual(IDerived2.getTaggedValue('foo'), 'bar')
         self.assertEqual(set(IDerived2.getTaggedValueTags()), set(['qux', 'foo']))
 
+    def _make_taggedValue_tree(self, base):
+        from zope.interface import taggedValue
+        from zope.interface import Attribute
+        O = base
+        class F(O):
+            taggedValue('tag', 'F')
+            tag = Attribute('F')
+        class E(O):
+            taggedValue('tag', 'E')
+            tag = Attribute('E')
+        class D(O):
+            taggedValue('tag', 'D')
+            tag = Attribute('D')
+        class C(D, F):
+            taggedValue('tag', 'C')
+            tag = Attribute('C')
+        class B(D, E):
+            pass
+        class A(B, C):
+            pass
+
+        return A
+
+    def test_getTaggedValue_follows__iro__(self):
+        # And not just looks at __bases__.
+        # https://github.com/zopefoundation/zope.interface/issues/190
+        from zope.interface import Interface
+
+        # First, confirm that looking at a true class
+        # hierarchy follows the __mro__.
+        class_A = self._make_taggedValue_tree(object)
+        self.assertEqual(class_A.tag.__name__, 'C')
+
+        # Now check that Interface does, both for attributes...
+        iface_A = self._make_taggedValue_tree(Interface)
+        self.assertEqual(iface_A['tag'].__name__, 'C')
+        # ... and for tagged values.
+        self.assertEqual(iface_A.getTaggedValue('tag'), 'C')
+        self.assertEqual(iface_A.queryTaggedValue('tag'), 'C')
+        # Of course setting something lower overrides it.
+        assert iface_A.__bases__[0].__name__ == 'B'
+        iface_A.__bases__[0].setTaggedValue('tag', 'B')
+        self.assertEqual(iface_A.getTaggedValue('tag'), 'B')
+
+    def test_getDirectTaggedValue_ignores__iro__(self):
+        # https://github.com/zopefoundation/zope.interface/issues/190
+        from zope.interface import Interface
+
+        A = self._make_taggedValue_tree(Interface)
+        self.assertIsNone(A.queryDirectTaggedValue('tag'))
+        self.assertEqual([], list(A.getDirectTaggedValueTags()))
+
+        with self.assertRaises(KeyError):
+            A.getDirectTaggedValue('tag')
+
+        A.setTaggedValue('tag', 'A')
+        self.assertEqual(A.queryDirectTaggedValue('tag'), 'A')
+        self.assertEqual(A.getDirectTaggedValue('tag'), 'A')
+        self.assertEqual(['tag'], list(A.getDirectTaggedValueTags()))
+
+        assert A.__bases__[1].__name__ == 'C'
+        C = A.__bases__[1]
+        self.assertEqual(C.queryDirectTaggedValue('tag'), 'C')
+        self.assertEqual(C.getDirectTaggedValue('tag'), 'C')
+        self.assertEqual(['tag'], list(C.getDirectTaggedValueTags()))
+
     def test_description_cache_management(self):
         # See https://bugs.launchpad.net/zope.interface/+bug/185974
         # There was a bug where the cache used by Specification.get() was not