Merge branch 'master' into doc/master

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;
         };
     }
 

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"
+    );
+} )();
+