From c8023cb382f601ce3ecdc0afa1b23bc21a5375bb Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mikegerwitz@gnu.org>
Date: Thu, 20 Feb 2014 23:17:04 -0500
Subject: [PATCH] Trait method return value implementation correction and
 testing

---
 lib/Trait.js                 |   6 +-
 test/Trait/DefinitionTest.js |  24 ++++++++
 test/Trait/VirtualTest.js    | 113 +++++++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+), 4 deletions(-)

diff --git a/lib/Trait.js b/lib/Trait.js
index 7d1ed7e..60ff786 100644
--- a/lib/Trait.js
+++ b/lib/Trait.js
@@ -236,7 +236,7 @@ function createVirtProxy( acls, dfn )
         {
             return function()
             {
-                this.___$$parent$$[ f ].apply( this, arguments );
+                return this.___$$parent$$[ f ].apply( this, arguments );
             };
         } )( f );
     }
@@ -328,7 +328,6 @@ function mixMethods( src, dest, vis, iname )
             }
 
             // if non-virtual, a normal proxy should do
-            // TODO: test return value; see below
             if ( !( keywords['virtual'] ) )
             {
                 dest[ pname ] = iname;
@@ -350,9 +349,8 @@ function mixMethods( src, dest, vis, iname )
                     // additional overhead or confusion
                     var ret = pdest[ '__$$' + f ].apply( pdest, arguments );
 
-                    // TODO: test return value
                     // if the trait returns itself, return us instead
-                    return ( ret === iname )
+                    return ( ret === pdest )
                         ? this
                         : ret;
                 };
diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js
index f7726a8..da9eb67 100644
--- a/test/Trait/DefinitionTest.js
+++ b/test/Trait/DefinitionTest.js
@@ -280,4 +280,28 @@ require( 'common' ).testCase(
         this.Class( 'Named' ).use( T ).extend( {} )().foo();
         this.assertOk( called );
     },
+
+
+    /**
+     * When a trait is mixed into a class, it acts as though it is part of
+     * that class. Therefore, it should stand to reason that, when a mixed
+     * in method returns `this', it should actually return the instance of
+     * the class that it is mixed into (in the case of this test, its
+     * private member object, since that's our context when invoking the
+     * trait method).
+     */
+    'Trait method that returns self will return containing class':
+    function()
+    {
+        var _self = this,
+            T     = this.Sut( { foo: function() { return this; } } );
+
+        this.Class.use( T ).extend(
+        {
+            go: function()
+            {
+                _self.assertStrictEqual( this, this.foo() );
+            },
+        } )().go();
+    },
 } );
diff --git a/test/Trait/VirtualTest.js b/test/Trait/VirtualTest.js
index 82ac37f..b18feef 100644
--- a/test/Trait/VirtualTest.js
+++ b/test/Trait/VirtualTest.js
@@ -188,4 +188,117 @@ require( 'common' ).testCase(
         C().doFoo();
         this.assertOk( called );
     },
+
+
+    /**
+     * This is the same concept as the non-virtual test found in the
+     * DefinitionTest case: since a trait is mixed into a class, if it
+     * returns itself, then it should in actuality return the instance of
+     * the class it is mixed into.
+     */
+    'Virtual trait method returning self returns class instance':
+    function()
+    {
+        var _self = this;
+
+        var T = this.Sut( { 'virtual foo': function() { return this; } } );
+
+        this.Class.use( T ).extend(
+        {
+            go: function()
+            {
+                _self.assertStrictEqual( this, this.foo() );
+            },
+        } )().go();
+    },
+
+
+    /**
+     * Same concept as the above test case, but ensures that invoking the
+     * super method does not screw anything up.
+     */
+    'Overridden virtual trait method returning self returns class instance':
+    function()
+    {
+        var _self = this;
+
+        var T = this.Sut( { 'virtual foo': function() { return this; } } );
+
+        this.Class.use( T ).extend(
+        {
+            'override foo': function()
+            {
+                return this.__super();
+            },
+
+            go: function()
+            {
+                _self.assertStrictEqual( this, this.foo() );
+            },
+        } )().go();
+    },
+
+
+    /**
+     * When a trait method is overridden, ensure that the data are properly
+     * proxied back to the caller. This differs from the above tests, which
+     * just make sure that the method is actually overridden and invoked.
+     */
+    'Data are properly returned from trait override super call': function()
+    {
+        var _self    = this,
+            expected = {};
+
+        var T = this.Sut(
+        {
+            'virtual foo': function() { return expected; }
+        } );
+
+        this.Class.use( T ).extend(
+        {
+            'override foo': function()
+            {
+                _self.assertStrictEqual( expected, this.__super() );
+            },
+        } )().foo();
+    },
+
+
+    /**
+     * When a trait method is overridden by the class that it is mixed into,
+     * and the super method is called, then the trait method should execute
+     * within the private member context of the trait itself (as if it were
+     * never overridden). Some kinky stuff would have to be going on (at
+     * least in the implementation at the time this was written) for this
+     * test to fail, but let's be on the safe side.
+     */
+    'Super trait method overrided in class executed within private context':
+    function()
+    {
+        var expected = {};
+
+        var T = this.Sut(
+        {
+            'virtual foo': function()
+            {
+                // should succeed
+                return this.priv();
+            },
+
+            'private priv': function()
+            {
+                return expected;
+            },
+        } );
+
+        this.assertStrictEqual( expected,
+            this.Class.use( T ).extend(
+            {
+                'override virtual foo': function()
+                {
+                    return this.__super();
+                },
+            } )().foo()
+        );
+    },
 } );