From ce736bea556939eca26cceb96738508118a38a9f Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 18 Mar 2011 23:42:07 -0400 Subject: [PATCH] Visibility de-escalation no longer permitted --- lib/class.js | 14 +++-- lib/member_builder.js | 95 +++++++++++++++++++++++++----- lib/util.js | 2 +- test/test-class-visibility.js | 106 ++++++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 21 deletions(-) diff --git a/lib/class.js b/lib/class.js index c020c32..d352f0a 100644 --- a/lib/class.js +++ b/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 ) @@ -539,8 +539,10 @@ var extend = ( function( extending ) attachPropInit( prototype, prop_init, members, class_id ); - new_class.prototype = prototype; - new_class.constructor = new_class; + 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 ); diff --git a/lib/member_builder.js b/lib/member_builder.js index 3df3f4e..d3c5412 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -22,7 +22,8 @@ * @package core */ -var util = require( './util' ), +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.} 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; + } +} + diff --git a/lib/util.js b/lib/util.js index 9403d05..43ddcb5 100644 --- a/lib/util.js +++ b/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 diff --git a/test/test-class-visibility.js b/test/test-class-visibility.js index 72c72a3..d313317 100644 --- a/test/test-class-visibility.js +++ b/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" ); +} )(); +