From 61f2f7e22d32ed92dfd16bc25d824bedcc86fed2 Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike@mikegerwitz.com>
Date: Fri, 1 Apr 2011 06:28:45 -0400
Subject: [PATCH] Methods can now be properly overridden when visibility is
 escalated

---
 lib/propobj.js                | 20 +++++++++++++++++---
 test/test-class-visibility.js | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/lib/propobj.js b/lib/propobj.js
index 5c14cda..6a78020 100644
--- a/lib/propobj.js
+++ b/lib/propobj.js
@@ -61,7 +61,12 @@ exports.setup = function( dest, properties, methods )
     // initialize each of the properties for this instance to
     // ensure we're not sharing references to prototype values
     doSetup( dest, properties[ 'public' ] );
-    doSetup( dest, properties[ 'protected' ], methods[ 'protected'] );
+
+    // Do the same for protected, but only if they do not exist already in
+    // public. The reason for this is because the property object is laid /atop/
+    // of the public members, meaning that a parent's protected members will
+    // take precedence over a subtype's overriding /public/ members. Uh oh.
+    doSetup( dest, properties[ 'protected' ], methods[ 'protected' ], true );
 
     // then add the private parts
     doSetup( obj, properties[ 'private' ], methods[ 'private' ] );
@@ -79,7 +84,7 @@ exports.setup = function( dest, properties, methods )
  *
  * @return  {undefined}
  */
-function doSetup( dest, properties, methods )
+function doSetup( dest, properties, methods, unless_exists )
 {
     var hasOwn = Array.prototype.hasOwnProperty;
 
@@ -90,7 +95,16 @@ function doSetup( dest, properties, methods )
         {
             if ( hasOwn.call( methods, method_name ) )
             {
-                dest[ method_name ] = methods[ method_name ];
+                // If requested, do not copy the method over if it already
+                // exists in the destination object. Don't use hasOwn here;
+                // unnecessary overhead and we want to traverse any prototype
+                // chains. We do not check the public object directly, for
+                // example, because we need a solution that will work if a proxy
+                // is unsupported by the engine.
+                if ( !unless_exists || ( dest[ method_name ] === undefined ) )
+                {
+                    dest[ method_name ] = methods[ method_name ];
+                }
             }
         }
     }
diff --git a/test/test-class-visibility.js b/test/test-class-visibility.js
index b8854d9..7fdc59b 100644
--- a/test/test-class-visibility.js
+++ b/test/test-class-visibility.js
@@ -657,3 +657,38 @@ var common    = require( './common' ),
     }, Error, "Cannot de-escalate visibility for interface members" );
 } )();
 
+
+/**
+ * Due to the way the property object is laid atop of the public members, we
+ * need to ensure that protected methods' functionality can /actually/ be
+ * overridden, since the protected method is higher in the prototype chain and
+ * therefore will be accessed before the public method.
+ *
+ * We don't care about private -> protected, because that's not possible through
+ * inheritance.
+ */
+( function testCanOverrideProtectedMethodFunctionalityWithPublic()
+{
+    // get the result of invoking overridden foo()
+    var result = Class(
+        {
+            'protected foo': function()
+            {
+                return false;
+            },
+        } ).extend(
+        {
+            // override and escalate visibility of method foo()
+            'public foo': function()
+            {
+                return true;
+            },
+        } )().foo();
+
+    // if the override was successful, we'll be able to invoke the overridden
+    // method
+    assert.equal( result, true,
+        "Can properly override protected methods with public"
+    );
+} )();
+