Rev 4770: Review feedback from Andrew. in http://bazaar.launchpad.net/~jameinel/bzr/2.1-st-concat

[John Arbash Meinel]

At http://bazaar.launchpad.net/~jameinel/bzr/2.1-st-concat

------------------------------------------------------------
revno: 4770
revision-id: john at arbash-meinel.com-20091021142700-rdadmjxsdi3kzn01
parent: john at arbash-meinel.com-20091021050510-ebs9d922icfovrxl
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.1-st-concat
timestamp: Wed 2009-10-21 09:27:00 -0500
message:
  Review feedback from Andrew.
  
  Disallow adding a subclass of str/unicode/int/float/long. Just in
  case those subclasses can have refcycles.
-------------- next part --------------
=== modified file 'bzrlib/_static_tuple_c.c'
--- a/bzrlib/_static_tuple_c.c	2009-10-21 05:02:35 +0000
+++ b/bzrlib/_static_tuple_c.c	2009-10-21 14:27:00 +0000
@@ -244,10 +244,10 @@
             || StaticTuple_CheckExact(obj)
             || obj == Py_None
             || PyBool_Check(obj)
-            || PyInt_Check(obj)
-            || PyLong_Check(obj)
-            || PyFloat_Check(obj)
-            || PyUnicode_Check(obj)
+            || PyInt_CheckExact(obj)
+            || PyLong_CheckExact(obj)
+            || PyFloat_CheckExact(obj)
+            || PyUnicode_CheckExact(obj)
             ) continue;
         PyErr_Format(PyExc_TypeError, "StaticTuple(...)"
             " requires that all items are one of"

=== modified file 'bzrlib/tests/test__static_tuple.py'
--- a/bzrlib/tests/test__static_tuple.py	2009-10-21 05:02:35 +0000
+++ b/bzrlib/tests/test__static_tuple.py	2009-10-21 14:27:00 +0000
@@ -135,21 +135,12 @@
     def test_concat_with_bad_tuple(self):
         st1 = self.module.StaticTuple('foo')
         t2 = (object(),)
-        try:
-            st3 = st1 + t2
-        except TypeError:
-            pass
-        else:
-            self.fail('TypeError not raised')
+        # Using st1.__add__ doesn't give the same results as doing the '+' form
+        self.assertRaises(TypeError, lambda: st1 + t2)
 
     def test_concat_with_non_tuple(self):
         st1 = self.module.StaticTuple('foo')
-        try:
-            st1 + 10
-        except TypeError:
-            pass
-        else:
-            self.fail('TypeError not raised for addition w/ an int')
+        self.assertRaises(TypeError, lambda: st1 + 10)
         
     def test_as_tuple(self):
         k = self.module.StaticTuple('foo')
@@ -224,22 +215,45 @@
 
     def test_holds_None(self):
         k1 = self.module.StaticTuple(None)
+        # You cannot subclass None anyway
 
     def test_holds_int(self):
         k1 = self.module.StaticTuple(1)
+        class subint(int):
+            pass
+        # But not a subclass, because subint could introduce refcycles
+        self.assertRaises(TypeError, self.module.StaticTuple, subint(2))
 
     def test_holds_long(self):
         k1 = self.module.StaticTuple(2L**65)
+        class sublong(long):
+            pass
+        # But not a subclass
+        self.assertRaises(TypeError, self.module.StaticTuple, sublong(1))
 
     def test_holds_float(self):
         k1 = self.module.StaticTuple(1.2)
+        class subfloat(float):
+            pass
+        self.assertRaises(TypeError, self.module.StaticTuple, subfloat(1.5))
+
+    def test_holds_str(self):
+        k1 = self.module.StaticTuple('astring')
+        class substr(str):
+            pass
+        self.assertRaises(TypeError, self.module.StaticTuple, substr('a'))
 
     def test_holds_unicode(self):
         k1 = self.module.StaticTuple(u'\xb5')
+        class subunicode(unicode):
+            pass
+        self.assertRaises(TypeError, self.module.StaticTuple,
+                          subunicode(u'\xb5'))
 
     def test_hold_bool(self):
         k1 = self.module.StaticTuple(True)
         k2 = self.module.StaticTuple(False)
+        # Cannot subclass bool
 
     def test_compare_same_obj(self):
         k1 = self.module.StaticTuple('foo', 'bar')