1
0
Fork 0

[bug fix] Corrected bug with implicit visibility escalation

See test case comments for details. This wasted way too much of my time.
perfodd
Mike Gerwitz 2014-02-04 23:48:40 -05:00
parent 5047d895c0
commit bcada87240
2 changed files with 44 additions and 12 deletions

View File

@ -80,7 +80,7 @@ exports.prototype.setup = function setup( dest, properties, methods )
this._doSetup( dest, this._doSetup( dest,
properties[ 'protected' ], properties[ 'protected' ],
methods[ 'protected' ], methods[ 'protected' ],
'public' true
); );
// then add the private parts // then add the private parts
@ -127,20 +127,21 @@ exports.prototype._createPrivateLayer = function( atop_of, properties )
/** /**
* Set up destination object by copying over properties and methods * Set up destination object by copying over properties and methods
* *
* @param {Object} dest destination object * The prot_priv parameter can be used to ignore both explicitly and
* @param {Object} properties properties to copy * implicitly public methods.
* @param {Object} methods methods to copy *
* @param {boolean} unless_keyword do not set if keyword is set on existing * @param {Object} dest destination object
* method * @param {Object} properties properties to copy
* @param {Object} methods methods to copy
* @param {boolean} prot_priv do not set unless protected or private
* *
* @return {undefined} * @return {undefined}
*/ */
exports.prototype._doSetup = function( exports.prototype._doSetup = function(
dest, properties, methods, unless_keyword dest, properties, methods, prot_priv
) )
{ {
var hasOwn = Array.prototype.hasOwnProperty, var hasOwn = Array.prototype.hasOwnProperty;
pre = null;
// copy over the methods // copy over the methods
if ( methods !== undefined ) if ( methods !== undefined )
@ -149,7 +150,8 @@ exports.prototype._doSetup = function(
{ {
if ( hasOwn.call( methods, method_name ) ) if ( hasOwn.call( methods, method_name ) )
{ {
pre = dest[ method_name ]; var pre = dest[ method_name ],
kw = pre && pre.___$$keywords$$;
// If requested, do not copy the method over if it already // If requested, do not copy the method over if it already
// exists in the destination object. Don't use hasOwn here; // exists in the destination object. Don't use hasOwn here;
@ -163,9 +165,9 @@ exports.prototype._doSetup = function(
// protected). This is the *last* check to ensure a performance // protected). This is the *last* check to ensure a performance
// hit is incured *only* if we're overriding protected with // hit is incured *only* if we're overriding protected with
// protected. // protected.
if ( !unless_keyword if ( !prot_priv
|| ( pre === undefined ) || ( pre === undefined )
|| !( pre.___$$keywords$$[ unless_keyword ] ) || ( kw[ 'private' ] || kw[ 'protected' ] )
) )
{ {
dest[ method_name ] = methods[ method_name ]; dest[ method_name ] = methods[ method_name ];

View File

@ -267,6 +267,36 @@ require( 'common' ).testCase(
}, },
/**
* This test addresses a particularily nasty bug that wasted hours of
* development time: When a visibility modifier keyword is omitted, then
* it should be implicitly public. In this case, however, the keyword is
* not automatically added to the keyword list (maybe one day it will
* be, but for now we'll maintain the distinction); therefore, we should
* not be checking for the `public' keyword when determining if we
* should write to the protected member object.
*/
'Public methods are not overwritten when keyword is omitted': function()
{
var f = function() {};
f.___$$keywords$$ = {};
// no keywords; should be implicitly public
var dest = { fpub: f };
// add duplicate method to protected
this.methods[ 'protected' ].fpub = function() {};
this.sut.setup( dest, this.props, this.methods );
// ensure our public method is still referenced
this.assertStrictEqual( dest.fpub, f,
"Public methods should not be overwritten by protected methods"
);
},
/** /**
* Same situation with private members as protected, with the exception that * Same situation with private members as protected, with the exception that
* we do not need to worry about the overlay problem (in regards to * we do not need to worry about the overlay problem (in regards to