1
0
Fork 0

Fix for overriding protected with protected

- A better solution may be explored
closure/master
Mike Gerwitz 2011-04-13 14:48:20 -04:00
parent 71c9f6cabe
commit 63a4f95f65
3 changed files with 61 additions and 6 deletions

View File

@ -145,6 +145,9 @@ exports.buildMethod = function(
// to ensure privileged calls will work properly
dest[ name ] = overrideMethod( value, null, instCallback, cid );
}
// store keywords for later reference (needed for pre-ES5 fallback)
dest[ name ].___$$keywords$$ = keywords;
};

View File

@ -71,7 +71,11 @@ exports.setup = function( dest, properties, methods )
// public. The reason for this is because the property object is laid /atop/
// of the public members, meaning that a parent's protected members will
// take precedence over a subtype's overriding /public/ members. Uh oh.
doSetup( dest, properties[ 'protected' ], methods[ 'protected' ], true );
doSetup( dest,
properties[ 'protected' ],
methods[ 'protected' ],
'public'
);
// then add the private parts
doSetup( obj, properties[ 'private' ], methods[ 'private' ] );
@ -90,11 +94,10 @@ exports.setup = function( dest, properties, methods )
*
* @return {undefined}
*/
function doSetup( dest, properties, methods, unless_exists )
function doSetup( dest, properties, methods, unless_keyword )
{
unless_exists = !!unless_exists;
var hasOwn = Array.prototype.hasOwnProperty;
var hasOwn = Array.prototype.hasOwnProperty,
pre = null;
// copy over the methods
if ( methods !== undefined )
@ -103,13 +106,24 @@ function doSetup( dest, properties, methods, unless_exists )
{
if ( hasOwn.call( methods, method_name ) )
{
pre = dest[ method_name ];
// If requested, do not copy the method over if it already
// exists in the destination object. Don't use hasOwn here;
// unnecessary overhead and we want to traverse any prototype
// chains. We do not check the public object directly, for
// example, because we need a solution that will work if a proxy
// is unsupported by the engine.
if ( !unless_exists || ( dest[ method_name ] === undefined ) )
//
// Also note that we need to allow overriding if it exists in
// the protected object (we can override protected with
// protected). This is the *last* check to ensure a performance
// hit is incured *only* if we're overriding protected with
// protected.
if ( !unless_keyword
|| ( pre === undefined )
|| !( pre.___$$keywords$$[ unless_keyword ] )
)
{
dest[ method_name ] = methods[ method_name ];
}

View File

@ -725,3 +725,41 @@ var common = require( './common' ),
);
} )();
/**
* There was a bug introduced when we prevented protected members from
* overriding public (since in the prototype chain, protected members are laid
* atop public, and this cannot change). This bug would disallow protected
* members from being overridden by other protected members.
*
* This test is both a proof and a regression test.
*/
( function testCanProperlyOverrideProtectedWithProtected()
{
var val = 'foobar',
result = Class(
{
'protected foo': function() {},
} ).extend(
{
// provide concrete implementation
'protected foo': function()
{
return val;
},
'public doFoo': function()
{
return this.foo();
},
} )().doFoo();
;
// if everything worked as expected, the value of 'val' will have been
// returned and stored in 'result'
assert.equal( result, val,
"Protected methods can properly be overriden by another protected " +
"method"
);
} )();