diff --git a/lib/member_builder.js b/lib/member_builder.js index 40dcd69..5daf630 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -314,27 +314,52 @@ function overrideMethod( super_method, new_method, instCallback ) // __super property var override = null; + // are we overriding? if ( new_method ) { override = function() { + var context = instCallback( this ) || this, + retval = undefined + ; + // the _super property will contain the parent method this.__super = super_method; - var retval = new_method.apply( - ( instCallback( this ) || this ), arguments - ); + retval = new_method.apply( context, arguments ); + + // if the value returned from the method was the context that we + // passed in, return the actual instance (to ensure we do not break + // encapsulation) + if ( retval === context ) + { + return this; + } return retval; }; } else { + // we are defining a new method override = function() { - return super_method.apply( - ( instCallback( this ) || this ), arguments - ); + var context = instCallback( this ) || this, + retval = undefined + ; + + // invoke the method + retval = super_method.apply( context, arguments ); + + // if the value returned from the method was the context that we + // passed in, return the actual instance (to ensure we do not break + // encapsulation) + if ( retval === context ) + { + return this; + } + + return retval; }; } diff --git a/test/test-class-visibility.js b/test/test-class-visibility.js index be5df66..61bf5c9 100644 --- a/test/test-class-visibility.js +++ b/test/test-class-visibility.js @@ -45,6 +45,7 @@ var common = require( './common' ), 'protected protf': protf, 'private privf': privf, + 'public getProp': function( name ) { // return property, allowing us to break encapsulation for @@ -60,13 +61,31 @@ var common = require( './common' ), { this[ name ] = value; }, + + + 'public getSelf': function() + { + return this; + }, + + + 'public getSelfOverride': function() + { + // override me + }, }), // instance of Foo foo = Foo(), // subtype - SubFoo = Foo.extend( {} ), + SubFoo = Foo.extend({ + 'public getSelfOverride': function() + { + // return this from overridden method + return this; + }, + }), sub_foo = SubFoo() ; @@ -308,3 +327,38 @@ var common = require( './common' ), ); } )(); + +/** + * When a method is called, 'this' is bound to the property object containing + * private and protected members. Returning 'this' would therefore be a very bad + * thing. Not only would it break encapsulation, but it would likely have other + * problems down the road. + * + * Therefore, we have to check the return value of the method. If the return + * value is the property object that it was bound to, we need to replace the + * return value with the actual class instance. This allows us to transparently + * enforce encapsulation. How sweet is that? + */ +( function testReturningSelfFromMethodShouldReturnInstanceNotPropObj() +{ + assert.deepEqual( + foo.getSelf(), + foo, + "Returning 'this' from a method should return instance of self" + ); + + // what happens in the case of inheritance? + assert.deepEqual( + sub_foo.getSelf(), + sub_foo, + "Returning 'this' from a super method should return the subtype" + ); + + // finally, overridden methods should still return the instance + assert.deepEqual( + sub_foo.getSelfOverride(), + sub_foo, + "Returning 'this' from a overridden method should return the subtype" + ); +} )(); +