zope.interface icon indicating copy to clipboard operation
zope.interface copied to clipboard

Remaining issues with strict IRO in zopetoolkit

Open jamadden opened this issue 4 years ago • 1 comments

(Taken from https://github.com/zopefoundation/zope.interface/issues/199#issuecomment-610948903)

#203 lets most of zopetoolkit's tests run in STRICT_IRO mode. There are a few exceptions:

  • ZODB's FileStorage redundantly declares multiple implemented interfaces. The fix is easy:
 @implementer(
-        IStorage,
         IStorageRestoreable,
         IStorageIteration,
         IStorageUndoable,

but…there's code in @implementer that attempts to detect things like that which should have caught and corrected it. I don't understand why it didn't work and need to investigate.

  • zope.session.http.CookieClientIdManager has a nearly identical issue.
  • zope.viewlet and zope.lifecycleevent both have the same issue in README.rst where some object is declared to @implementer(Interface), which is unnecessary and violates strict IRO. Probably the algorithm here that makes an exception for the sake of plone.app.caching.tests.test_etags should be aware of strict mode and not make that exception.
  • zope.location is being hit with https://github.com/zopefoundation/zope.proxy/issues/41, which is the same issue zope.container recently fixed (Provides(cls, providedBy(proxy)) fails when cls and proxy overlap incompatibly in their orders). It makes me wonder if Provides() should automatically do the same thing that zope.container is currently manually doing?

jamadden avatar Apr 08 '20 17:04 jamadden

zope.location is being hit with zopefoundation/zope.proxy#41, which is the same issue zope.container recently fixed (Provides(cls, providedBy(proxy)) fails when cls and proxy overlap incompatibly in their orders). It makes me wonder if Provides() should automatically do the same thing that zope.container is currently manually doing?

I think it probably should, especially since we made a similar change in #199 for @implementer and classProvides.

Doing the following lets the zope.location tests run in strict mode, and doesn't break any tests in zope.interface.

@@ -747,7 +747,13 @@ class Provides(Declaration):  # Really named ProvidesClass
     def __init__(self, cls, *interfaces):
         self.__args = (cls, ) + interfaces
         self._cls = cls
-        Declaration.__init__(self, *(interfaces + (implementedBy(cls), )))
+        implemented_by_cls = implementedBy(cls)
+        interfaces = tuple([
+            iface
+            for iface in interfaces
+            if not implemented_by_cls.isOrExtends(iface)
+        ])
+        Declaration.__init__(self, *(interfaces + (implemented_by_cls,)))

The zope.proxy tests run either way.

jamadden avatar Mar 16 '21 21:03 jamadden