From a401c319966e776c0efb1e8d1c2c39db0ac70334 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 4 Aug 2011 00:44:20 -0400 Subject: [PATCH] Can no longer override non-virtual methods (#19) --- lib/member_builder.js | 8 ++++++++ test/test-class-extend.js | 32 ++++++++++++++++++++++++++++++ test/test-class-implement.js | 4 ++-- test/test-member_builder-method.js | 17 +++++++++++----- 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/lib/member_builder.js b/lib/member_builder.js index cd3ba84..48f0048 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -183,6 +183,14 @@ function validateMethod( keywords, prev_data, prev_keywords, value, name ) ); } + // disallow overriding non-virtual methods + if ( keywords[ 'override' ] && !( prev_keywords[ 'virtual' ] ) ) + { + throw TypeError( + "Cannot override non-virtual method '" + name + "'" + ); + } + // do not allow overriding concrete methods with abstract if ( keywords[ 'abstract' ] && !( util.isAbstractMethod( prev ) ) ) { diff --git a/test/test-class-extend.js b/test/test-class-extend.js index 85f5026..4fdcf73 100644 --- a/test/test-class-extend.js +++ b/test/test-class-extend.js @@ -401,3 +401,35 @@ for ( var i = 0; i < class_count; i++ ) ); } )(); + +/** + * Only virtual methods may be overridden. + */ +( function testCannotOverrideNonVirtualMethod() +{ + try + { + var Foo = Class( + { + // non-virtual + 'public foo': function() {}, + } ), + + SubFoo = Foo.extend( + { + // should fail (cannot override non-virtual method) + 'override public foo': function() {}, + } ); + } + catch ( e ) + { + assert.ok( e.message.search( 'foo' ), + "Non-virtual override error message should contain name of method" + ); + + return; + } + + assert.fail( "Should not be permitted to override non-virtual method" ); +} )(); + diff --git a/test/test-class-implement.js b/test/test-class-implement.js index adcb744..00cf1b3 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( { - 'override foo': function() {}, - 'override foo2': function() {}, + 'foo': function() {}, + 'foo2': function() {}, }), concrete_inst = ConcreteFoo() diff --git a/test/test-member_builder-method.js b/test/test-member_builder-method.js index 5ff25c4..339fb1f 100644 --- a/test/test-member_builder-method.js +++ b/test/test-member_builder-method.js @@ -145,7 +145,8 @@ mb_common.assertCommon(); /** * Abstract members exist to be overridden. As such, they should be considered - * virtual. + * virtual. In addition, we should be able to override them WITHOUT the override + * keyword, since no concrete implementation was previously provided. */ ( function testAbstractMethodsAreConsideredVirtual() { @@ -153,10 +154,10 @@ mb_common.assertCommon(); mb_common.value = function() {}; mb_common.buildMemberQuick( { 'abstract': true } ); - // we should be able to override it + // we should be able to override it without the override keyword assert.doesNotThrow( function() { - mb_common.buildMemberQuick( { 'override': true }, true ); + mb_common.buildMemberQuick( {}, true ); }, Error, "Can overrde abstract methods" ); } )(); @@ -200,13 +201,19 @@ mb_common.assertCommon(); assert.doesNotThrow( function() { - mb_common.buildMemberQuick( { 'override': true }, true ); + mb_common.buildMemberQuick( + { 'virtual': true, 'override': true }, + true + ); }, TypeError, "Method can have equal number of parameters" ); assert.doesNotThrow( function() { mb_common.value = function( one, two, three ) {}; - mb_common.buildMemberQuick( { 'override': true }, true ); + mb_common.buildMemberQuick( + { 'virtual': true, 'override': true }, + true + ); }, TypeError, "Method can have greater number of parameters" ); assert.throws( function()