diff --git a/lib/member_builder.js b/lib/member_builder.js index 4c29afe..cd3ba84 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -76,20 +76,31 @@ exports.buildMethod = function( { // TODO: We can improve performance by not scanning each one individually // every time this method is called - var prev_data = scanMembers( members, name, base ), - prev = ( prev_data ) ? prev_data.member : null, - dest = getMemberVisibility( members, keywords ); + 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 ) { - // override the method - dest[ name ] = overrideMethod( prev, value, instCallback, cid ); + // 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 + operation = overrideMethod; + } + + dest[ name ] = operation( prev, value, instCallback, cid ); } else if ( keywords[ 'abstract' ] ) { @@ -114,17 +125,14 @@ exports.buildMethod = function( * * @param {Object.} keywords parsed keywords * - * @param {Object} prev_data data of member being overridden, if available - * @param {*} value property value - * @param {string} name property name + * @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, - prev_keywords = ( prev && prev.___$$keywords$$ ) - ? prev.___$$keywords$$ - : {} - ; + var prev = ( prev_data ) ? prev_data.member : null; if ( keywords[ 'abstract' ] ) { @@ -210,10 +218,13 @@ function validateMethod( keywords, prev_data, value, name ) // IMPORTANT: do this last, to ensure we throw errors before warnings if ( !( keywords[ 'new' ] || keywords[ 'override' ] ) ) { - throw Warning( Error( - "Hiding method '" + name + "'; " + - "use 'new' if intended, or 'override' to override instead" - ) ); + if ( !( prev_keywords[ 'abstract' ] ) ) + { + throw Warning( Error( + "Hiding method '" + name + "'; " + + "use 'new' if intended, or 'override' to override instead" + ) ); + } } } } @@ -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 * diff --git a/lib/prop_parser.js b/lib/prop_parser.js index 929739d..2f95c46 100644 --- a/lib/prop_parser.js +++ b/lib/prop_parser.js @@ -34,6 +34,7 @@ var _keywords = { 'abstract': true, 'const': true, 'virtual': true, + 'override': true, }; diff --git a/test/test-class-abstract.js b/test/test-class-abstract.js index cd3df92..cab0209 100644 --- a/test/test-class-abstract.js +++ b/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() {}, } ) ; diff --git a/test/test-class-implement.js b/test/test-class-implement.js index d8b10aa..adcb744 100644 --- a/test/test-class-implement.js +++ b/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() {}, - foo2: function() {}, + 'override foo': function() {}, + 'override foo2': function() {}, }), concrete_inst = ConcreteFoo() diff --git a/test/test-class-parent.js b/test/test-class-parent.js index 067a34e..a04f636 100644 --- a/test/test-class-parent.js +++ b/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 ); }, diff --git a/test/test-class-visibility.js b/test/test-class-visibility.js index f15100a..2eb8ad9 100644 --- a/test/test-class-visibility.js +++ b/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; }, diff --git a/test/test-member_builder-method.js b/test/test-member_builder-method.js index 4542485..5ff25c4 100644 --- a/test/test-member_builder-method.js +++ b/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 );