Properly implemented abstract methods list

lib/class.js

@@ -98,7 +98,18 @@ function prop_copy( props, dest, result_data )
     result_data = result_data || {};
 
     // initialize result_data
-    result_data.abstractMethods = result_data.abstractMethods || [];
+    var abstract_methods =
+        result_data.abstractMethods = result_data.abstractMethods || [];
+
+    // it's much faster to lookup a hash than it is to iterate through an entire
+    // array each time we need to find an existing abstract method
+    var abstract_map   = {},
+        abstract_regen = false;
+    for ( var i = 0, len = abstract_methods.length; i < len; i++ )
+    {
+        var method = abstract_methods[ i ];
+        abstract_map[ method ] = i;
+    }
 
     // copy each of the properties to the destination object
     for ( property in props )
@@ -112,9 +123,22 @@ function prop_copy( props, dest, result_data )
             setter = ( ( getset ) ? props.__lookupSetter__( property ) : null );
 
         // did we find an abstract method?
-        if ( ( prop instanceof Function ) && ( prop.abstractFlag === true ) )
+        if ( prop instanceof Function )
         {
-            result_data.abstractMethods.push( property );
+            if ( prop.abstractFlag === true )
+            {
+                abstract_methods.push( property );
+            }
+            else
+            {
+                // if we were given a concrete method to an abstract method,
+                // then the method should no longer be considered abstract
+                if ( abstract_map[ property ] !== undefined )
+                {
+                    delete abstract_methods[ abstract_map[ property ] ];
+                    abstract_regen = true;
+                }
+            }
         }
 
         // check for getter/setter overrides
@@ -163,6 +187,27 @@ function prop_copy( props, dest, result_data )
             dest[ property ] = prop;
         }
     }
+
+    // should we regenerate the array of abstract methods (this must be done
+    // because the length of the array remains the same after deleting elements)
+    if ( abstract_regen )
+    {
+        // copy the methods into a new array by pushing them onto it, to ensure
+        // the length property of the array will work properly
+        var methods_new = [];
+        for ( var i = 0, len = abstract_methods.length; i < len; i++ )
+        {
+            var method = abstract_methods[ i ];
+            if ( method === undefined )
+            {
+                continue;
+            }
+
+            methods_new.push( method );
+        }
+
+        result_data.abstractMethods = methods_new;
+    }
 }
 
 
@@ -195,7 +240,7 @@ function extend()
 
     // copy the given properties into the new prototype
     var result_data = {
-        abstractMethods: base.abstractMethods || [],
+        abstractMethods: ( base.abstractMethods || [] ).slice()
     };
     prop_copy( props, prototype, result_data );
 
@@ -281,8 +326,9 @@ function attach_abstract( func, methods )
         return is_abstract;
     };
 
-
+    // attach the list of abstract methods to the class (make the copy of the
-    // attach the list of abstract methods to the class
+    // methods to ensure that they won't be gc'd or later modified and screw up
+    // the value)
     define_secure_prop( func, 'abstractMethods', methods );
 }
 

test/test-class-abstract.js

@@ -81,6 +81,11 @@ assert.ok(
     "All classes should have an isAbstract() method"
 );
 
+assert.ok(
+    ( Foo.abstractMethods instanceof Array ),
+    "All classes should provide a list of their abstract methods as an array"
+);
+
 assert.equal(
     Foo.isAbstract(),
     false,
@@ -100,6 +105,28 @@ assert.equal(
         "concrete implementation for all abstract methods"
 );
 
+assert.ok(
+    ( ( AbstractFoo.abstractMethods[ 0 ] == 'method' )
+        && ( AbstractFoo.abstractMethods[ 1 ] == 'second' )
+    ),
+    "Abstract classes should provide a list of their abstract methods' names"
+);
+
+assert.ok(
+    ( ( SubAbstractFoo.abstractMethods[ 0 ] == 'method' )
+        && ( SubAbstractFoo.abstractMethods[ 1 ] == undefined )
+    ),
+    "Abstract subclasses should not have their concrete methods within the " +
+        "the abstract method list if the concrete method was abstract in " +
+        "its supertype"
+);
+
+assert.equal(
+    ConcreteFoo.abstractMethods.length,
+    0,
+    "Concrete classes should not have any abstract methods listed"
+);
+
 assert.equal(
     ConcreteFoo.isAbstract(),
     false,