Merge pull request #129 from timofurrer/issue/127

Do not overwrite existing class and object attributes with sure properties

CHANGELOG.md

@@ -8,7 +8,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Python 3.7-dev support (allowed to fail)
 
 ### Fixed
-- Do not overwrite existing attributes of objects with sure properties (when. should, ...). Refs #124, #128
+- Do not overwrite existing class and instance attributes with sure properties (when. should, ...). Refs #127, #129
+- Fix patched built-in `dir()` method. Refs #124, #128
 
 ## [v1.4.0]
 ### Added

sure/__init__.py

@@ -933,6 +933,7 @@ def chain(func):
     setattr(AssertionBuilder, func.__name__, func)
     return func
 
+
 def chainproperty(func):
     """Extend sure with a custom chain property."""
     func = assertionproperty(func)
@@ -949,10 +950,28 @@ def make_safe_property(method, name, should_be_property=True):
             return method(None)
 
         def deleter(method, self, *args, **kw):
-            del overwritten_object_handlers[(id(self), method.__name__)]
+            if isinstance(self, type):
+                # if the attribute has to be deleted from a class object
+                # we cannot use ``del self.__dict__[name]`` directly because we cannot
+                # modify a mappingproxy object. Thus, we have to delete it in our
+                # proxy __dict__.
+                del overwritten_object_handlers[(id(self), method.__name__)]
+            else:
+                # if the attribute has to be deleted from an instance object
+                # we are able to directly delete it from the object's __dict__.
+                del self.__dict__[name]
 
         def setter(method, self, other):
-            overwritten_object_handlers[(id(self), method.__name__)] = other
+            if isinstance(self, type):
+                # if the attribute has to be set to a class object
+                # we cannot use ``self.__dict__[name] = other`` directly because we cannot
+                # modify a mappingproxy object. Thus, we have to set it in our
+                # proxy __dict__.
+                overwritten_object_handlers[(id(self), method.__name__)] = other
+            else:
+                # if the attribute has to be set to an instance object
+                # we are able to directly set it in the object's __dict__.
+                self.__dict__[name] = other
 
         return builtins.property(
             fget=method,
@@ -962,6 +981,16 @@ def setter(method, self, other):
 
     def positive_assertion(name, prop=True):
         def method(self):
+            # check if the given object already has an attribute with the
+            # given name. If yes return it instead of patching it.
+            try:
+                if name in self.__dict__:
+                    return self.__dict__[name]
+            except AttributeError:
+                # we do not have an object with __dict__, thus
+                # it's safe to just continue and patch the `name`.
+                pass
+
             overwritten_object_handler = overwritten_object_handlers.get((id(self), name), None)
             if overwritten_object_handler:
                 return overwritten_object_handler
@@ -981,6 +1010,16 @@ def method(self):
 
     def negative_assertion(name, prop=True):
         def method(self):
+            # check if the given object already has an attribute with the
+            # given name. If yes return it instead of patching it.
+            try:
+                if name in self.__dict__:
+                    return self.__dict__[name]
+            except AttributeError:
+                # we do not have an object with __dict__, thus
+                # it's safe to just continue and patch the `name`.
+                pass
+
             overwritten_object_handler = overwritten_object_handlers.get((id(self), name), None)
             if overwritten_object_handler:
                 return overwritten_object_handler
@@ -1032,7 +1071,8 @@ def _new_dir(*obj):
         if len(obj) > 1:
             raise TypeError('dir expected at most 1 arguments, got {0}'.format(len(obj)))
 
-        return sorted(set(old_dir(obj[0])).difference(POSITIVES + NEGATIVES))
+        patched = [x for x in old_dir(obj[0]) if isinstance(getattr(obj[0], x), AssertionBuilder)]
+        return sorted(set(old_dir(obj[0])).difference(patched))
 
     builtins.dir = _new_dir
 

tests/test_cpython_patches.py

@@ -31,6 +31,51 @@ def test_it_works_with_objects():
             {}.should.be.empty
 
 
+        def test_shouldnt_overwrite_class_attributes():
+            """do not patch already existing class attributes with same name"""
+            class Foo(object):
+                when = 42
+                shouldnt = 43
+                bar = 'bar'
+
+            Foo.when.should.be.equal(42)
+            Foo.shouldnt.should.be.equal(43)
+            Foo.bar.should.be.equal('bar')
+
+            Foo.__dict__.should.contain('when')
+            Foo.__dict__.should.contain('shouldnt')
+            Foo.__dict__.should.contain('bar')
+
+            dir(Foo).should.contain('when')
+            dir(Foo).should.contain('shouldnt')
+            dir(Foo).should.contain('bar')
+            dir(Foo).shouldnt.contain('should')
+
+
+        def test_shouldnt_overwrite_instance_attributes():
+            """do not patch already existing instance attributes with same name"""
+            class Foo(object):
+                def __init__(self, when, shouldnt, bar):
+                    self.when = when
+                    self.shouldnt = shouldnt
+                    self.bar = bar
+
+            f = Foo(42, 43, 'bar')
+
+            f.when.should.be.equal(42)
+            f.shouldnt.should.be.equal(43)
+            f.bar.should.be.equal('bar')
+
+            f.__dict__.should.contain('when')
+            f.__dict__.should.contain('shouldnt')
+            f.__dict__.should.contain('bar')
+
+            dir(f).should.contain('when')
+            dir(f).should.contain('shouldnt')
+            dir(f).should.contain('bar')
+            dir(f).shouldnt.contain('should')
+
+
     def test_dir_conceals_sure_specific_attributes():
         ("dir(obj) should conceal names of methods that were grafted by sure")