builtin_dir(): Treat classic classes like types. Use PyDict_Keys instead

of PyMapping_Keys because we know we have a real dict.  Tolerate that
objects may have an attr named "__dict__" that's not a dict (Py_None
popped up during testing).

test_descr.py, test_dir():  Test the new classic-class behavior; beef up
the new-style class test similarly.

test_pyclbr.py, checkModule():  dir(C) is no longer a synonym for
C.__dict__.keys() when C is a classic class (looks like the same thing
that burned distutils! -- should it be *made* a synoym again?  Then it
would be inconsistent with new-style class behavior.).

diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
index ee924dc0c928a41f96b9b55e3a59f739e9bd3229..dd411ac9671eb9aaaf8ae95e7a98e2f59b5a257b 100644 (file)
--- a/Lib/test/test_descr.py
+++ b/Lib/test/test_descr.py
@@ -183,8 +183,7 @@ def test_dir():
     for arg in 2, 2L, 2j, 2e0, [2], "2", u"2", (2,), {2:2}, type, test_dir:
         dir(arg)
 
-    # Check some details here because classic classes aren't working
-    # reasonably, and I want this to fail (eventually).
+    # Try classic classes.
     class C:
         Cdata = 1
         def Cmethod(self): pass
@@ -202,23 +201,45 @@ def test_dir():
     class A(C):
         Adata = 1
         def Amethod(self): pass
-    astuff = ['Adata', 'Amethod', '__doc__', '__module__']
-    # This isn't finding C's stuff at all.
+
+    astuff = ['Adata', 'Amethod'] + cstuff
     verify(dir(A) == astuff)
-    # But this is!  It's because a.__class__ exists but A.__class__ doesn't.
     a = A()
-    verify(dir(a) == astuff[:2] + cstuff)
+    verify(dir(a) == astuff)
+    a.adata = 42
+    a.amethod = lambda self: 3
+    verify(dir(a) == astuff + ['adata', 'amethod'])
+
+    # The same, but with new-style classes.  Since these have object as a
+    # base class, a lot more gets sucked in.
+    def interesting(strings):
+        return [s for s in strings if not s.startswith('_')]
 
-    # The story for new-style classes is quite different.
     class C(object):
         Cdata = 1
         def Cmethod(self): pass
+
+    cstuff = ['Cdata', 'Cmethod']
+    verify(interesting(dir(C)) == cstuff)
+
+    c = C()
+    verify(interesting(dir(c)) == cstuff)
+
+    c.cdata = 2
+    c.cmethod = lambda self: 0
+    verify(interesting(dir(c)) == cstuff + ['cdata', 'cmethod'])
+
     class A(C):
         Adata = 1
         def Amethod(self): pass
-    d = dir(A)
-    for expected in 'Cdata', 'Cmethod', 'Adata', 'Amethod':
-        verify(expected in d)
+
+    astuff = ['Adata', 'Amethod'] + cstuff
+    verify(interesting(dir(A)) == astuff)
+    a = A()
+    verify(interesting(dir(a)) == astuff)
+    a.adata = 42
+    a.amethod = lambda self: 3
+    verify(interesting(dir(a)) == astuff + ['adata', 'amethod'])
 
 binops = {
     'add': '+',

diff --git a/Lib/test/test_pyclbr.py b/Lib/test/test_pyclbr.py
index e5de6578005f364f804919d515d369c9ca9cc9c6..ce4d8acb341dfb1b51f8abc66e92117a7d86c735 100644 (file)
--- a/Lib/test/test_pyclbr.py
+++ b/Lib/test/test_pyclbr.py
@@ -76,7 +76,7 @@ class PyclbrTest(unittest.TestCase):
                 self.assertListEq(real_bases, pyclbr_bases, ignore)
 
                 actualMethods = []
-                for m in dir(py_item):
+                for m in py_item.__dict__.keys():
                     if type(getattr(py_item, m)) == MethodType:
                         actualMethods.append(m)
                 foundMethods = []

diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
index d5dc32247fdcdbb09f805dde905549252796492c..d3d32c9ff566fb8cb5a1ffd4a8eaab97fd77e8db 100644 (file)
--- a/Python/bltinmodule.c
+++ b/Python/bltinmodule.c
@@ -440,9 +440,6 @@ merge_class_dict(PyObject* dict, PyObject* aclass)
        PyObject *bases;
 
        assert(PyDict_Check(dict));
-       /* XXX Class objects fail the PyType_Check check.  Don't
-          XXX know of others. */
-       /* assert(PyType_Check(aclass)); */
        assert(aclass);
 
        /* Merge in the type's dict (if any). */
@@ -490,7 +487,7 @@ builtin_dir(PyObject *self, PyObject *args)
                PyObject *locals = PyEval_GetLocals();
                if (locals == NULL)
                        goto error;
-               result = PyMapping_Keys(locals);
+               result = PyDict_Keys(locals);
                if (result == NULL)
                        goto error;
        }
@@ -500,10 +497,13 @@ builtin_dir(PyObject *self, PyObject *args)
                masterdict = PyObject_GetAttrString(arg, "__dict__");
                if (masterdict == NULL)
                        goto error;
+               assert(PyDict_Check(masterdict));
        }
 
-       /* Elif some form of type, recurse. */
-       else if (PyType_Check(arg)) {
+       /* Elif some form of type or class, grab its dict and its bases.
+          We deliberately don't suck up its __class__, as methods belonging
+          to the metaclass would probably be more confusing than helpful. */
+       else if (PyType_Check(arg) || PyClass_Check(arg)) {
                masterdict = PyDict_New();
                if (masterdict == NULL)
                        goto error;
@@ -514,28 +514,30 @@ builtin_dir(PyObject *self, PyObject *args)
        /* Else look at its dict, and the attrs reachable from its class. */
        else {
                PyObject *itsclass;
-               /* Create a dict to start with. */
+               /* Create a dict to start with.  CAUTION:  Not everything
+                  responding to __dict__ returns a dict! */
                masterdict = PyObject_GetAttrString(arg, "__dict__");
                if (masterdict == NULL) {
                        PyErr_Clear();
                        masterdict = PyDict_New();
-                       if (masterdict == NULL)
-                               goto error;
+               }
+               else if (!PyDict_Check(masterdict)) {
+                       Py_DECREF(masterdict);
+                       masterdict = PyDict_New();
                }
                else {
                        /* The object may have returned a reference to its
                           dict, so copy it to avoid mutating it. */
                        PyObject *temp = PyDict_Copy(masterdict);
-                       if (temp == NULL)
-                               goto error;
                        Py_DECREF(masterdict);
                        masterdict = temp;
                }
-               /* Merge in attrs reachable from its class. */
+               if (masterdict == NULL)
+                       goto error;
+
+               /* Merge in attrs reachable from its class.
+                  CAUTION:  Not all objects have a __class__ attr. */
                itsclass = PyObject_GetAttrString(arg, "__class__");
-               /* XXX Sometimes this is null!  Like after "class C: pass",
-                  C.__class__ raises AttributeError.  Don't know of other
-                  cases. */
                if (itsclass == NULL)
                        PyErr_Clear();
                else {
@@ -550,7 +552,7 @@ builtin_dir(PyObject *self, PyObject *args)
        if (masterdict != NULL) {
                /* The result comes from its keys. */
                assert(result == NULL);
-               result = PyMapping_Keys(masterdict);
+               result = PyDict_Keys(masterdict);
                if (result == NULL)
                        goto error;
        }
@@ -578,7 +580,8 @@ static char dir_doc[] =
 "\n"
 "No argument:  the names in the current scope.\n"
 "Module object:  the module attributes.\n"
-"Type object:  its attributes, and recursively the attributes of its bases.\n"
+"Type or class object:  its attributes, and recursively the attributes of\n"
+"    its bases.\n"
 "Otherwise:  its attributes, its class's attributes, and recursively the\n"
 "    attributes of its class's base classes.";