From bd4e18acc6386deb9e9fc3924c0e7badc7d141f7 Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike@mikegerwitz.com>
Date: Wed, 3 Aug 2011 22:40:55 -0400
Subject: [PATCH] Implicit method hiding warning now applies to virtual methods
 as well as non-virtual (#19)

---
 lib/member_builder.js                     |  7 ++--
 test/test-member_builder-method-hiding.js | 49 ++++++++++++++++++++---
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/lib/member_builder.js b/lib/member_builder.js
index e79c001..4c29afe 100644
--- a/lib/member_builder.js
+++ b/lib/member_builder.js
@@ -203,11 +203,12 @@ function validateMethod( keywords, prev_data, value, name )
             );
         }
 
-        // if redefining non-virtual method, the new method will "hide" the
-        // parent's
+        // if redefining a method that has already been implemented in the
+        // supertype, the default behavior is to "hide" the method of the
+        // supertype, unless otherwise specified
         //
         // IMPORTANT: do this last, to ensure we throw errors before warnings
-        if ( !( prev_keywords[ 'virtual' ] || prev_keywords[ 'abstract' ] ) )
+        if ( !( keywords[ 'new' ] || keywords[ 'override' ] ) )
         {
             throw Warning( Error(
                 "Hiding method '" + name + "'; " +
diff --git a/test/test-member_builder-method-hiding.js b/test/test-member_builder-method-hiding.js
index b165447..ef596c0 100644
--- a/test/test-member_builder-method-hiding.js
+++ b/test/test-member_builder-method-hiding.js
@@ -41,10 +41,10 @@ function restoreWarningHandler()
 /**
  * If a non-virtual method is implicitly hidden (redefined without the 'new'
  * keyword), a warning should be provided. This will ensure that, should a
- * parent introduce a method that is already defined by a supertype, the
+ * parent introduce a method that is already defined by a subtype, the
  * developer of the subtype is made aware of the issue.
  */
-( function testThrowsWarningWhenHidingSuperMethod()
+( function testThrowsWarningWhenHidingNonVirtualSuperMethod()
 {
     var thrown = false;
 
@@ -55,7 +55,7 @@ function restoreWarningHandler()
 
         assert.ok(
             ( e.message.search( 'foo' ) !== -1 ),
-            "Method hiding warning should contain method name"
+            "Non-virtual method hiding warning should contain method name"
         );
     } );
 
@@ -65,13 +65,52 @@ function restoreWarningHandler()
         'public foo': function() {},
     } );
 
-    // hide the non-virtual method
+    // implicitly hide the non-virtual method
     builder.build( Foo,
     {
         'public foo': function() {},
     } );
 
-    assert.equal( thrown, true, "No warning was thrown" );
+    assert.equal( thrown, true,
+        "No warning for implicit non-virtual hiding was thrown"
+    );
+} )();
+
+
+/**
+ * Same concept as above. The API of the supertype could just as easily be
+ * changed to include a virtual method that has already been implemented by the
+ * subtype. The default behavior is to hide the method of the supertype.
+ */
+( function testThrowsWarningWhenHidingVirtualSuperMethod()
+{
+    var thrown = false;
+
+    // mock the warning handler to ensure a warning is thrown
+    warn.setHandler( function( e )
+    {
+        thrown = true;
+
+        assert.ok(
+            ( e.message.search( 'foo' ) !== -1 ),
+            "Virtual method hiding warning should contain method name"
+        );
+    } );
+
+    var Foo = builder.build(
+    {
+        'virtual public foo': function() {},
+    } );
+
+    // implicitly hide the virtual method
+    builder.build( Foo,
+    {
+        'public foo': function() {},
+    } );
+
+    assert.equal( thrown, true,
+        "No warning for implicit virtual hiding was thrown"
+    );
 } )();