zopefoundation/zope.interface

Can `@implementer` only add an interface if not implemented already?

jamadden opened this issue · 4 comments

Currently it always does. There's some small bits of logic to re-order interfaces given at the same time to avoid the more egregious ways it would produce invalid resolution orders, but it doesn't pay attention to inheritance. This makes it easy to produce invalid orders if a sub-class redeclares something implemented by a superclass.

Alternately, what about a new decorator that does that?

See zopefoundation/zope.formlib#31 (comment) for more context.

IMO a consistent resolution order has highest priority. Re-ordering feels wrong. So, if implemented, then explicit as an own decorator with many exclamation marks in the documentation. At the end this sounds a bit like supporting an anti-pattern and I would just not support it at all.

After reading the discussion in zopefoundation/zope.formlib#31 I support changing @implementer to add only the interface if needed. Even if this might be a breaking change, zope.interface 5 is still young and it already broke some things, so adding another breaking change now would hurt less than doing in in half a year after consumers have updated to zope.interface 5.0.x.

I personally do not like to have @implementer_if_needed decorators in some random packages as they will be needed in consumer code, too, and it is the only sane way to use the @implementer decorator to prevent unpleasant surprises.

Fixed in #203.

This 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 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?