Make Interface.getTaggedValue follow the __iro__.

Previously it manually walked up __bases__, meaning the answers could be inconsistent.

Fixes #190.

Also fixes several minor issues in the documentation, mostly cross-reference related.

3 files changed, 203 insertions, 28 deletions

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..816144b 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):
@@ -148,7 +215,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 +223,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 +308,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 +389,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