Override keyword is now required to override a virtual method (#19)

lib/member_builder.js

@@ -78,18 +78,29 @@ exports.buildMethod = function(
     // every time this method is called
     var prev_data     = scanMembers( members, name, base ),
         prev          = ( prev_data ) ? prev_data.member : null,
+        prev_keywords = ( prev && prev.___$$keywords$$ ),
         dest          = getMemberVisibility( members, keywords );
     ;
 
     // ensure that the declaration is valid (keywords make sense, argument
     // length, etc)
-    validateMethod( keywords, prev_data, value, name );
+    validateMethod( keywords, prev_data, prev_keywords, value, name );
 
     // we might be overriding an existing method
     if ( prev )
+    {
+        // by default, perform method hiding, even if the keyword was not
+        // provided (the keyword simply suppresses the warning)
+        var operation = hideMethod;
+
+        // TODO: warning if no super method when override keyword provided
+        if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] )
         {
             // override the method
-        dest[ name ] = overrideMethod( prev, value, instCallback, cid );
+            operation = overrideMethod;
+        }
+
+        dest[ name ] = operation( prev, value, instCallback, cid );
     }
     else if ( keywords[ 'abstract' ] )
     {
@@ -114,17 +125,14 @@ exports.buildMethod = function(
  *
  * @param  {Object.<string,boolean>}  keywords  parsed keywords
  *
- * @param  {Object}  prev_data  data of member being overridden, if available
+ * @param  {Object}  prev_data      data of member being overridden
+ * @param  {Object}  prev_keywords  keywords of member being overridden
  * @param  {*}       value          property value
  * @param  {string}  name           property name
  */
-function validateMethod( keywords, prev_data, value, name )
+function validateMethod( keywords, prev_data, prev_keywords, value, name )
 {
-    var prev          = ( prev_data ) ? prev_data.member : null,
+    var prev = ( prev_data ) ? prev_data.member : null;
-        prev_keywords = ( prev && prev.___$$keywords$$ )
-            ? prev.___$$keywords$$
-            : {}
-    ;
 
     if ( keywords[ 'abstract' ] )
     {
@@ -209,6 +217,8 @@ function validateMethod( keywords, prev_data, value, name )
         //
         // IMPORTANT: do this last, to ensure we throw errors before warnings
         if ( !( keywords[ 'new' ] || keywords[ 'override' ] ) )
+        {
+            if ( !( prev_keywords[ 'abstract' ] ) )
             {
                 throw Warning( Error(
                     "Hiding method '" + name + "'; " +
@@ -216,6 +226,7 @@ function validateMethod( keywords, prev_data, value, name )
                 ) );
             }
         }
+    }
 }
 
 
@@ -492,6 +503,17 @@ function scanMembers( members, name, base )
 }
 
 
+/**
+ * Hide a method with a "new" method
+ */
+function hideMethod( super_method, new_method, instCallback, cid )
+{
+    // TODO: This function is currently unimplemented. It exists at present to
+    // provide a placeholder and ensure that the override keyword is required to
+    // override a parent method.
+}
+
+
 /**
  * Generates a method override function
  *

lib/prop_parser.js

@@ -34,6 +34,7 @@ var _keywords = {
     'abstract':  true,
     'const':     true,
     'virtual':   true,
+    'override':  true,
 };
 
 

test/test-class-abstract.js

@@ -122,6 +122,8 @@ var common = require( './common' ),
 
         ConcreteFoo = Class.extend( SubAbstractFoo,
         {
+            // we should NOT need the override keyword for concrete
+            // implementations of abstract super methods
             'foo': function() {},
         } )
     ;

test/test-class-implement.js

@@ -171,8 +171,8 @@ var Type = Interface.extend( {
     // concrete implementation so that we can instantiate it
     var ConcreteFoo = Foo.extend(
         {
-            foo:  function() {},
+            'override foo':  function() {},
-            foo2: function() {},
+            'override foo2': function() {},
         }),
 
         concrete_inst = ConcreteFoo()

test/test-class-parent.js

@@ -51,12 +51,12 @@ var common = require( './common' ),
 
     SubFoo = Foo.extend(
     {
-        myMethod: function()
+        'override myMethod': function()
         {
             return this;
         },
 
-        myMethod2: function( arg )
+        'override myMethod2': function( arg )
         {
             return this.__super( arg );
         },

test/test-class-visibility.js

@@ -110,7 +110,7 @@ var common    = require( './common' ),
     SubFoo  = Foo.extend({
         'private _pfoo': 'baz',
 
-        'public getSelfOverride': function()
+        'override public getSelfOverride': function()
         {
             // return this from overridden method
             return this;
@@ -120,7 +120,7 @@ var common    = require( './common' ),
         /**
          * We have to override this so that 'this' is not bound to the supertype
          */
-        'public getProp': function( name )
+        'override public getProp': function( name )
         {
             // return property, allowing us to break encapsulation for
             // protected/private properties (for testing purposes)
@@ -519,7 +519,7 @@ var common    = require( './common' ),
             'virtual protected baz': function() {},
         } ).extend( {
             'public foo': 'bar',
-            'public baz': function() {},
+            'override public baz': function() {},
         } );
     }, Error, "Can escalate visibility of subtype members" );
 
@@ -532,7 +532,7 @@ var common    = require( './common' ),
             'virtual protected baz': function() {},
         } ).extend( {
             'protected foo': 'bar',
-            'protected baz': function() {},
+            'override protected baz': function() {},
         } );
     }, Error, "Can retain level of visibility for subtype members" );
 } )();
@@ -628,7 +628,7 @@ var common    = require( './common' ),
         } ).extend(
         {
             // we override to public just so we can call it externally
-            'public foo': function()
+            'override public foo': function()
             {
                 return this.__super();
             },
@@ -679,7 +679,7 @@ var common    = require( './common' ),
         } ).extend(
         {
             // override and escalate visibility of method foo()
-            'public foo': function()
+            'override public foo': function()
             {
                 return true;
             },
@@ -743,7 +743,7 @@ var common    = require( './common' ),
         } ).extend(
         {
             // provide concrete implementation
-            'protected foo': function()
+            'override protected foo': function()
             {
                 return val;
             },

test/test-member_builder-method.js

@@ -88,7 +88,7 @@ mb_common.assertCommon();
     // attempt to override it
     assert.doesNotThrow( function()
     {
-        mb_common.buildMemberQuick( {}, true );
+        mb_common.buildMemberQuick( { 'override': true }, true );
     }, Error, "Should be able to override virtual methods" );
 } )();
 
@@ -106,7 +106,7 @@ mb_common.assertCommon();
     mb_common.buildMemberQuick( { 'virtual': true } );
 
     // override it (non-virtual)
-    mb_common.buildMemberQuick( {}, true );
+    mb_common.buildMemberQuick( { 'override': true }, true );
 
     // attempt to override again (should fail)
     try
@@ -133,12 +133,12 @@ mb_common.assertCommon();
     mb_common.buildMemberQuick( { 'virtual': true } );
 
     // override it (virtual)
-    mb_common.buildMemberQuick( { 'virtual': true }, true );
+    mb_common.buildMemberQuick( { 'virtual': true, 'override': true }, true );
 
     // attempt to override again
     assert.doesNotThrow( function()
     {
-        mb_common.buildMemberQuick( {}, true );
+        mb_common.buildMemberQuick( { 'override': true }, true );
     }, Error, "Can override an override if declared virtual" );
 } )();
 
@@ -156,7 +156,7 @@ mb_common.assertCommon();
     // we should be able to override it
     assert.doesNotThrow( function()
     {
-        mb_common.buildMemberQuick( {}, true );
+        mb_common.buildMemberQuick( { 'override': true }, true );
     }, Error, "Can overrde abstract methods" );
 } )();
 
@@ -200,19 +200,19 @@ mb_common.assertCommon();
 
     assert.doesNotThrow( function()
     {
-        mb_common.buildMemberQuick( { 'virtual': true }, true );
+        mb_common.buildMemberQuick( { 'override': true }, true );
     }, TypeError, "Method can have equal number of parameters" );
 
     assert.doesNotThrow( function()
     {
         mb_common.value = function( one, two, three ) {};
-        mb_common.buildMemberQuick( { 'virtual': true }, true );
+        mb_common.buildMemberQuick( { 'override': true }, true );
     }, TypeError, "Method can have greater number of parameters" );
 
     assert.throws( function()
     {
         mb_common.value = function( one ) {};
-        mb_common.buildMemberQuick( {}, true );
+        mb_common.buildMemberQuick( { 'override': true }, true );
     }, TypeError, "Method cannot have lesser number of parameters" );
 } )();
 
@@ -255,7 +255,7 @@ mb_common.assertCommon();
         );
     };
 
-    mb_common.buildMemberQuick( {}, true );
+    mb_common.buildMemberQuick( { 'override': true }, true );
 
     // invoke the method
     mb_common.members[ 'public' ][ mb_common.name ]();
@@ -298,7 +298,14 @@ mb_common.assertCommon();
     members['public'].foo.___$$keywords$$ = { 'virtual': true };
 
     // override
-    builder.buildMethod( members, {}, 'foo', newfunc, {}, instCallback );
+    builder.buildMethod(
+        members,
+        {},
+        'foo',
+        newfunc,
+        { 'override': true },
+        instCallback
+    );
 
     // call the overriding method
     members[ 'public' ].foo();
@@ -393,7 +400,7 @@ mb_common.assertCommon();
         exports.meta,
         'func',
         func2,
-        {},
+        { 'override': true },
         instCallback
     );