Visibility de-escalation no longer permitted

lib/class.js

@@ -439,9 +439,9 @@ var extend = ( function( extending )
             prototype = new base(),
             cname     = '',
 
-            hasOwn = Array.prototype.hasOwnProperty;
+            hasOwn = Array.prototype.hasOwnProperty,
 
-        var properties = {},
+            properties = {},
             prop_init  = member_builder.initMembers(),
             members    = member_builder.initMembers( prototype ),
 
@@ -489,7 +489,7 @@ var extend = ( function( extending )
                 // build a new property, passing in the other members to compare
                 // against for preventing nonsensical overrides
                 member_builder.buildProp(
-                    prop_init, null, name, value, keywords, members
+                    prop_init, null, name, value, keywords, base
                 );
             },
 
@@ -511,7 +511,7 @@ var extend = ( function( extending )
             {
                 member_builder.buildMethod(
                     members, null, name, func, keywords, getMethodInstance,
-                    class_id
+                    class_id, base
                 );
 
                 if ( is_abstract )
@@ -541,6 +541,8 @@ var extend = ( function( extending )
 
         new_class.prototype      = prototype;
         new_class.constructor    = new_class;
+        new_class.___$$props$$   = prop_init;
+        new_class.___$$methods$$ = members;
 
         // important: call after setting prototype
         setupProps( new_class, abstract_methods, class_id );

lib/member_builder.js

@@ -23,6 +23,7 @@
  */
 
 var util       = require( './util' ),
+    fallback   = util.definePropertyFallback(),
     visibility = [ 'public', 'protected', 'private' ];
 
 
@@ -67,13 +68,16 @@ exports.initMembers = function( mpublic, mprotected, mprivate )
  * @return  {undefined}
  */
 exports.buildMethod = function(
-    members, meta, name, value, keywords, instCallback, cid
+    members, meta, name, value, keywords, instCallback, cid, base
 )
 {
-    var prev;
+    // TODO: We can improve performance by not scanning each one individually
+    // every time this method is called
+    var prev_data = scanMembers( members, name, base ),
+        prev      = ( prev_data ) ? prev_data.member : null;
 
     // search for any previous instances of this member
-    if ( prev = scanMembers( members, name ) )
+    if ( prev )
     {
         // disallow overriding properties with methods
         if ( !( prev instanceof Function ) )
@@ -102,6 +106,14 @@ exports.buildMethod = function(
                     "with that of its supertype"
             );
         }
+
+        // do not permit visibility de-escalation
+        if ( prev_data.visibility < getVisibilityValue( keywords ) )
+        {
+            throw TypeError(
+                "Cannot de-escalate visibility of method '" + name + "'"
+            );
+        }
     }
 
     var dest = getMemberVisibility( members, keywords );
@@ -138,20 +150,33 @@ exports.buildMethod = function(
  *
  * @param  {Object.<string,boolean>}  keywords  parsed keywords
 
- * @param  {Object=}  cmp  optional second member object to scan
+ * @param  {Object=}  base  optional base object to scan
  *
  * @return  {undefined}
  */
-exports.buildProp = function( members, meta, name, value, keywords, cmp )
+exports.buildProp = function( members, meta, name, value, keywords, base )
 {
+    // TODO: We can improve performance by not scanning each one individually
+    // every time this method is called
+    var prev_data = scanMembers( members, name, base ),
+        prev      = ( prev_data ) ? prev_data.member : null;
+
     // disallow overriding methods with properties
-    if ( scanMembers( members, name, cmp ) instanceof Function )
+    if ( prev instanceof Function )
     {
         throw new TypeError(
             "Cannot override method '" + name + "' with property"
         );
     }
 
+    // do not permit visibility de-escalation
+    if ( prev && ( prev_data.visibility < getVisibilityValue( keywords ) ) )
+    {
+        throw TypeError(
+            "Cannot de-escalate visibility of property '" + name + "'"
+        );
+    }
+
     getMemberVisibility( members, keywords )[ name ] = value;
 };
 
@@ -262,11 +287,12 @@ function getMemberVisibility( members, keywords )
  * @param  {{public: Object, protected: Object, private: Object}} members
  *
  * @param  {string}   name  member to locate
- * @param  {Object=}  cmp   optional second member object to scan
+ * @param  {Object=}  base  optional base object to scan
  *
- * @return  {*}  member, if located, otherwise undefined
+ * @return  {Object}  Array of member and number corresponding to visibility,
+ *                    level if located, otherwise an empty object
  */
-function scanMembers( members, name, cmp )
+function scanMembers( members, name, base )
 {
     var i      = visibility.length,
         member = null;
@@ -276,19 +302,29 @@ function scanMembers( members, name, cmp )
     {
         if ( member = members[ visibility[ i ] ][ name ] )
         {
-            return member;
+            return {
+                member:     member,
+                visibility: ( ( fallback ) ? 0 : i ),
+            };
         }
     }
 
     // if a second comparison object was given, try again using it instead of
     // the original members object
-    if ( cmp !== undefined )
+    if ( base !== undefined )
     {
-        return scanMembers( cmp, name );
+        var base_methods = base.___$$methods$$,
+            base_props   = base.___$$props$$;
+
+        // scan the base's methods and properties, if they are available
+        return ( base_methods && scanMembers( base_methods, name ) )
+            || ( base_props && scanMembers( base_props, name ) )
+            || null
+        ;
     }
 
     // nothing was found
-    return undefined;
+    return null;
 }
 
 
@@ -378,3 +414,34 @@ function overrideMethod( super_method, new_method, instCallback, cid )
     return override;
 }
 
+
+/**
+ * Return the visibility level as a numeric value, where 0 is public and 2 is
+ * private
+ *
+ * @param  {Object}  keywords  keywords to scan for visibility level
+ *
+ * @return  {number}  visibility level as a numeric value
+ */
+function getVisibilityValue( keywords )
+{
+    if ( fallback )
+    {
+        // if we have to fall back, we don't support levels of visibility
+        return 0;
+    }
+    else if ( keywords[ 'protected' ] )
+    {
+        return 1;
+    }
+    else if ( keywords[ 'private' ] )
+    {
+        return 2;
+    }
+    else
+    {
+        // default is public
+        return 0;
+    }
+}
+

lib/util.js

@@ -240,7 +240,7 @@ exports.propParse = function( data, options )
         // if an 'each' callback was provided, pass the data before parsing it
         if ( callbackEach )
         {
-            callbackEach.call( callbackEach, name, value );
+            callbackEach.call( callbackEach, name, value, keywords );
         }
 
         // getter/setter

test/test-class-visibility.js

@@ -503,3 +503,109 @@ var common  = require( './common' ),
     );
 } )();
 
+
+/**
+ * Visibility escalation (protected -> private) should be permitted
+ */
+( function testCanEscalateMemberVisibility()
+{
+    // escalate
+    assert.doesNotThrow( function()
+    {
+        Class(
+        {
+            'protected foo': 'bar',
+            'protected baz': function() {},
+        } ).extend( {
+            'public foo': 'bar',
+            'public baz': function() {},
+        } );
+    }, Error, "Can escalate visibility of subtype members" );
+
+    // same level of visibility
+    assert.doesNotThrow( function()
+    {
+        Class(
+        {
+            'protected foo': 'bar',
+            'protected baz': function() {},
+        } ).extend( {
+            'protected foo': 'bar',
+            'protected baz': function() {},
+        } );
+    }, Error, "Can retain level of visibility for subtype members" );
+} )();
+
+
+/**
+ * We should /not/ be able to de-escalate member visibility
+ * (public -> {protected,private}
+ */
+( function testCannotDeescalateMemberVisibility()
+{
+    // public -> protected
+    assert.throws( function()
+    {
+        Class(
+        {
+            'public foo': 'bar',
+        } ).extend( {
+            'protected foo': 'bar',
+        } );
+    }, Error, "Cannot de-escalate visibility of subtype props to protected" );
+
+    assert.throws( function()
+    {
+        Class(
+        {
+            'public baz': function() {},
+        } ).extend( {
+            'protected baz': function() {},
+        } );
+    }, Error, "Cannot de-escalate visibility of subtype methods to protected" );
+
+
+    // public -> private
+    assert.throws( function()
+    {
+        Class(
+        {
+            'public foo': 'bar',
+        } ).extend( {
+            'private foo': 'bar',
+        } );
+    }, Error, "Cannot de-escalate visibility of subtype props to private" );
+
+    assert.throws( function()
+    {
+        Class(
+        {
+            'public baz': function() {},
+        } ).extend( {
+            'private baz': function() {},
+        } );
+    }, Error, "Cannot de-escalate visibility of subtype methods to private" );
+
+
+    // protected -> private
+    assert.throws( function()
+    {
+        Class(
+        {
+            'protected foo': 'bar',
+        } ).extend( {
+            'private foo': 'bar',
+        } );
+    }, Error, "Cannot de-escalate visibility of subtype props to private2" );
+
+    assert.throws( function()
+    {
+        Class(
+        {
+            'protected baz': function() {},
+        } ).extend( {
+            'private baz': function() {},
+        } );
+    }, Error, "Cannot de-escalate visibility of subtype methods to private2" );
+} )();
+