From 7427958ec063945972f53dd7939369458f1418b2 Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike@mikegerwitz.com>
Date: Thu, 10 Mar 2011 12:19:39 -0500
Subject: [PATCH] Returning 'this' from a method will now return the object
 instance, not the internal property object

---
 lib/member_builder.js         | 21 ++++++++++++++++++---
 test/test-class-visibility.js | 28 ++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/lib/member_builder.js b/lib/member_builder.js
index 40dcd69..f467674 100644
--- a/lib/member_builder.js
+++ b/lib/member_builder.js
@@ -314,6 +314,7 @@ function overrideMethod( super_method, new_method, instCallback )
     // __super property
     var override = null;
 
+    // are we overriding?
     if ( new_method )
     {
         override = function()
@@ -330,11 +331,25 @@ function overrideMethod( super_method, new_method, instCallback )
     }
     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..67f07e6 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,6 +61,12 @@ var common  = require( './common' ),
         {
             this[ name ] = value;
         },
+
+
+        'public getSelf': function()
+        {
+            return this;
+        },
     }),
 
     // instance of Foo
@@ -308,3 +315,24 @@ 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"
+    );
+} )();
+