From 8c959324461b6f6247c8ad764c749e655306a6d0 Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mikegerwitz@gnu.org>
Date: Fri, 2 May 2014 00:17:53 -0400
Subject: [PATCH] Trait virtual method proxies now set __length metadata

This fixes a bug that causesd virtual definitions with parameters on classes
that a trait is mixed into to fail, and prevented proper param length
validations in the reverse case.

See test case description for a less confusing description.
---
 lib/Trait.js              | 17 ++++++++++++---
 test/Trait/VirtualTest.js | 44 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/lib/Trait.js b/lib/Trait.js
index bb3cf86..8ceb5ce 100644
--- a/lib/Trait.js
+++ b/lib/Trait.js
@@ -398,11 +398,13 @@ function createVirtProxy( acls, dfn )
             ? 'public'
             : 'protected';
 
+        var plen = acls.___$$methods$$[ vis ][ f ].__length;
+
         // this is the aforementioned proxy method; see the docblock for
         // more information
         dfn[ vis + ' virtual override ' + f ] = ( function()
         {
-            return function()
+            var retf = function()
             {
                 var pmo = this.___$$pmo$$,
                     o   = pmo[ f ];
@@ -413,16 +415,22 @@ function createVirtProxy( acls, dfn )
                     ? o.apply( pmo, arguments )
                     : this.__super.apply( this, arguments );
             };
+
+            retf.__length = plen;
+            return retf;
         } )( f );
 
         // this guy bypasses the above virtual override check, which is
         // necessary in certain cases to prevent infinte recursion
         dfn[ vis + ' virtual __$$' + f ] = ( function( f )
         {
-            return function()
+            var retf = function()
             {
                 return this.___$$parent$$[ f ].apply( this, arguments );
             };
+
+            retf.__length = plen;
+            return retf;
         } )( f );
     }
 }
@@ -581,7 +589,7 @@ function mixMethods( src, dest, vis, iname )
             // beacuse we are not proxying to a method of the same name)
             dest[ pname ] = ( function( f )
             {
-                return function()
+                var retf = function()
                 {
                     var pdest = this[ iname ];
 
@@ -596,6 +604,9 @@ function mixMethods( src, dest, vis, iname )
                         ? this
                         : ret;
                 };
+
+                retf.__length = src[ f ].__length;
+                return retf;
             } )( f );
         }
     }
diff --git a/test/Trait/VirtualTest.js b/test/Trait/VirtualTest.js
index 966c259..36cecae 100644
--- a/test/Trait/VirtualTest.js
+++ b/test/Trait/VirtualTest.js
@@ -190,6 +190,50 @@ require( 'common' ).testCase(
     },
 
 
+    /**
+     * Virtual methods for traits are handled via a series of proxy methods
+     * that determine, at runtime (as opposed to when the class is created),
+     * where the call should go. (At least that was the implementation at
+     * the time this test was written.) This test relies on the proper
+     * parameter metadata being set on those proxy methods so that the
+     * necessary length requirements can be validated.
+     *
+     * This was a bug in the initial implemenation: the above tests did not
+     * catch it because the virtual methods had no arguments. The initial
+     * problem was that, since __length was not defined on the generated
+     * method that was recognized as the override, it was always zero, which
+     * always failed if there were any arguments on the virtual method. The
+     * reverse case was also a problem, but it didn't manifest as an
+     * error---rather, it did *not* error when it should have.
+     *
+     * Note the instantiation in these cases: this is because the trait
+     * implementation lazily performs the mixin on first use.
+     */
+    'Subtype must meet compatibility requirements of virtual trait method':
+    function()
+    {
+        var _self = this;
+
+        var C = this.Class.use(
+            this.Sut( { 'virtual foo': function( a, b ) {} } )
+        );
+
+        this.assertThrows( function()
+        {
+            // does not meet param requirements (note the
+            // instantiation---traits defer processing until they are used)
+            C.extend( { 'override foo': function( a ) {} } )();
+        } );
+
+        this.assertDoesNotThrow( function()
+        {
+            // does not meet param requirements (note the
+            // instantiation---traits defer processing until they are used)
+            C.extend( { 'override foo': function( a, b ) {} } )();
+        } );
+    },
+
+
     /**
      * 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