Opened 6 years ago

#6526 enhancement new

FancyEqMixin's check for other.__class__ is fishy

Reported by: Wilfredo Sánchez Vega Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:
Author:

Description

I'm suspicious of FancyEqMixin's implementation of __eq__, which has the line:

        if isinstance(self, other.__class__):

This is basically saying that we'll go ahead and check compareAttributes on other because we conform to it's interface. But what we need to know, I think, is that it conforms to our interface.

Here's an example of what I think is busted:

>>> from twisted.python.util import FancyEqMixin
>>> class A(object):
...   x = 1
... 
>>> class B(A, FancyEqMixin):
...  y = 1
...  compareAttributes = ("x", "y")
... 
>>> a = A()
>>> b = B()
>>> b == a
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/twisted/python/util.py", line 556, in __eq__
    [getattr(other, name) for name in self.compareAttributes])
AttributeError: 'A' object has no attribute 'y'
>>> 

B extends A and B uses FancyEqMixin. The attribute it adds is relevant to comparisons, so it's included in compareAttributes. But this explodes because A doesn't know about the new attribute.

I believe the test should be:

        if isinstance(other, self.__class__):

Change History (0)

Note: See TracTickets for help on using tickets.