From 8b83e85c437d61597c65af5a11e1308924c49575 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 8 Jun 2011 01:11:53 -0400 Subject: [PATCH] [#19] Implemented 'virtual' keyword - Baby steps. 'override' keyword is not yet necessary. - Final not yet removed --- lib/member_builder.js | 23 +++-- lib/prop_parser.js | 1 + test/test-class-abstract.js | 22 +++++ test/test-class-implement.js | 3 +- test/test-class-parent.js | 4 +- test/test-class-visibility.js | 14 +-- test/test-member_builder-method.js | 134 +++++++++++++++++++++++++++-- 7 files changed, 177 insertions(+), 24 deletions(-) diff --git a/lib/member_builder.js b/lib/member_builder.js index f47cb4c..8f53e27 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -164,10 +164,13 @@ function validateMethod( keywords, prev_data, value, name ) ); } - // final methods cannot be overridden - if ( prev_keywords[ 'final' ] ) + // Can only override virtual methods. Abstract methods are considered to + // be virtual. + if ( !( prev_keywords[ 'virtual' ] || prev_keywords[ 'abstract' ] ) ) { - throw TypeError( "Cannot override final method '" + name + "'" ); + throw TypeError( + "Cannot override non-virtual method '" + name + "'" + ); } // do not allow overriding concrete methods with abstract @@ -391,10 +394,16 @@ function scanMembers( members, name, base ) { if ( member = members[ visibility[ i ] ][ name ] ) { - return { - member: member, - visibility: ( ( fallback ) ? 0 : i ), - }; + // We need to filter out base properties (such as + // Object.prototype.toString()), but we still need to traverse the + // prototype chain. As such, we cannot use hasOwnProperty(). + if ( member !== Object.prototype[ name ] ) + { + return { + member: member, + visibility: ( ( fallback ) ? 0 : i ), + }; + } } } diff --git a/lib/prop_parser.js b/lib/prop_parser.js index 617d472..ae49920 100644 --- a/lib/prop_parser.js +++ b/lib/prop_parser.js @@ -34,6 +34,7 @@ var _keywords = { 'final': true, 'abstract': true, 'const': true, + 'virtual': true, }; diff --git a/test/test-class-abstract.js b/test/test-class-abstract.js index 85c5734..cd3df92 100644 --- a/test/test-class-abstract.js +++ b/test/test-class-abstract.js @@ -106,6 +106,28 @@ var common = require( './common' ), } )(); +/** + * Abstract methods should remain virtual until they are overridden. That is, if + * a subtype doesn't provide a concrete implementation, it should still be + * considered virtual. + */ +( function testAbstractMethodsCanBeOverriddenBySubSubTypes() +{ + var AbstractFoo = AbstractClass( 'Foo', + { + 'abstract foo': [], + } ), + + SubAbstractFoo = AbstractClass.extend( AbstractFoo, {} ), + + ConcreteFoo = Class.extend( SubAbstractFoo, + { + 'foo': function() {}, + } ) + ; +} )(); + + /** * Just as Class contains an extend method, so should AbstractClass. */ diff --git a/test/test-class-implement.js b/test/test-class-implement.js index dff57fb..d8b10aa 100644 --- a/test/test-class-implement.js +++ b/test/test-class-implement.js @@ -175,7 +175,8 @@ var Type = Interface.extend( { foo2: function() {}, }), - concrete_inst = new ConcreteFoo(); + concrete_inst = ConcreteFoo() + ; assert.ok( ( concrete_inst.isInstanceOf( Type ) diff --git a/test/test-class-parent.js b/test/test-class-parent.js index 6ef8d64..067a34e 100644 --- a/test/test-class-parent.js +++ b/test/test-class-parent.js @@ -34,13 +34,13 @@ var common = require( './common' ), Foo = Class.extend( { - myMethod: function() + 'virtual myMethod': function() { hitMethod = true; return this; }, - myMethod2: function( arg ) + 'virtual myMethod2': function( arg ) { hitMethod2 = true; method2Arg = arg; diff --git a/test/test-class-visibility.js b/test/test-class-visibility.js index 326122b..c0d7ddf 100644 --- a/test/test-class-visibility.js +++ b/test/test-class-visibility.js @@ -47,7 +47,7 @@ var common = require( './common' ), 'private privf': privf, - 'public getProp': function( name ) + 'virtual public getProp': function( name ) { // return property, allowing us to break encapsulation for // protected/private properties (for testing purposes) @@ -79,7 +79,7 @@ var common = require( './common' ), }, - 'public getSelfOverride': function() + 'virtual public getSelfOverride': function() { // override me }, @@ -516,7 +516,7 @@ var common = require( './common' ), Class( { 'protected foo': 'bar', - 'protected baz': function() {}, + 'virtual protected baz': function() {}, } ).extend( { 'public foo': 'bar', 'public baz': function() {}, @@ -529,7 +529,7 @@ var common = require( './common' ), Class( { 'protected foo': 'bar', - 'protected baz': function() {}, + 'virtual protected baz': function() {}, } ).extend( { 'protected foo': 'bar', 'protected baz': function() {}, @@ -621,7 +621,7 @@ var common = require( './common' ), { var val = 'foobar', result = Class( { - 'protected foo': function() + 'virtual protected foo': function() { return val; }, @@ -672,7 +672,7 @@ var common = require( './common' ), // get the result of invoking overridden foo() var result = Class( { - 'protected foo': function() + 'virtual protected foo': function() { return false; }, @@ -739,7 +739,7 @@ var common = require( './common' ), var val = 'foobar', result = Class( { - 'protected foo': function() {}, + 'virtual protected foo': function() {}, } ).extend( { // provide concrete implementation diff --git a/test/test-member_builder-method.js b/test/test-member_builder-method.js index e6f7775..45567a6 100644 --- a/test/test-member_builder-method.js +++ b/test/test-member_builder-method.js @@ -65,6 +65,122 @@ mb_common.assertCommon(); } )(); +/** + * Unlike Java, PHP, Python and similar languages, methods in ease.js are *not* + * virtual by default. In order to make them override-able, the 'virtual' + * keyword must be specified for that method in the supertype. + * + * Therefore, let's ensure that non-virtual methods cannot be overridden. + */ +( function testCannotOverrideNonVirtualMethod() +{ + mb_common.value = function() {}; + mb_common.buildMemberQuick(); + + try + { + // attempt to override (should throw exception; non-virtual) + mb_common.buildMemberQuick( {}, true ); + + // should not get to this point + assert.fail( "Should not be able to override non-virtual method" ); + } + catch ( e ) + { + // ensure we have the correct error + assert.ok( + e.message.search( 'virtual' ) !== -1, + "Error message for non-virtual override should mention virtual" + ); + + assert.ok( + e.message.search( mb_common.name ) !== -1, + "Method name should be provided in non-virtual error message" + ); + } +} )(); + + +/** + * Working off of what was said in the test directly above, we *should* be able + * to override virtual methods. + */ +( function testCanOverrideVirtualMethods() +{ + // build a virtual method + mb_common.value = function() {}; + mb_common.buildMemberQuick( { 'virtual': true } ); + + // attempt to override it + assert.doesNotThrow( function() + { + mb_common.buildMemberQuick( {}, true ); + }, Error, "Should be able to override virtual methods" ); +} )(); + + +/** + * Unlike languages like C++, ease.js does not automatically mark overridden + * methods as virtual. C# and some other languages offer a 'seal' keyword or + * similar in order to make overridden methods non-virtual. In that sense, + * ease.js will "seal" overrides by default. + */ +( function testOverriddenMethodsAreNotVirtualByDefault() +{ + // build a virtual method + mb_common.value = function() {}; + mb_common.buildMemberQuick( { 'virtual': true } ); + + // override it (non-virtual) + mb_common.buildMemberQuick( {}, true ); + + // attempt to override again (should fail) + assert.throws( function() + { + mb_common.buildMemberQuick( {}, true ); + }, TypeError, "Overrides are not declared as virtual by default" ); +} )(); + + +/** + * Given the test directly above, we can therefore assume that it should be + * permitted to declare overridden methods as virtual. + */ +( function testCanDeclareOverridesAsVirtual() +{ + // build a virtual method + mb_common.value = function() {}; + mb_common.buildMemberQuick( { 'virtual': true } ); + + // override it (virtual) + mb_common.buildMemberQuick( { 'virtual': true }, true ); + + // attempt to override again + assert.doesNotThrow( function() + { + mb_common.buildMemberQuick( {}, true ); + }, Error, "Can override an override if declared virtual" ); +} )(); + + +/** + * Abstract members exist to be overridden. As such, they should be considered + * virtual. + */ +( function testAbstractMethodsAreConsideredVirtual() +{ + // build abstract method + mb_common.value = function() {}; + mb_common.buildMemberQuick( { 'abstract': true } ); + + // we should be able to override it + assert.doesNotThrow( function() + { + mb_common.buildMemberQuick( {}, true ); + }, Error, "Can overrde abstract methods" ); +} )(); + + /** * To ensure interfaces of subtypes remain compatible with that of their * supertypes, the parameter lists must match and build upon each other. @@ -72,17 +188,17 @@ mb_common.assertCommon(); ( function testMethodOverridesMustHaveEqualOrGreaterParameters() { mb_common.value = function( one, two ) {}; - mb_common.buildMemberQuick(); + mb_common.buildMemberQuick( { 'virtual': true } ); assert.doesNotThrow( function() { - mb_common.buildMemberQuick( {}, true ); + mb_common.buildMemberQuick( { 'virtual': true }, true ); }, TypeError, "Method can have equal number of parameters" ); assert.doesNotThrow( function() { mb_common.value = function( one, two, three ) {}; - mb_common.buildMemberQuick( {}, true ); + mb_common.buildMemberQuick( { 'virtual': true }, true ); }, TypeError, "Method can have greater number of parameters" ); assert.throws( function() @@ -112,7 +228,7 @@ mb_common.assertCommon(); orig_called = true; }; - mb_common.buildMemberQuick(); + mb_common.buildMemberQuick( { 'virtual': true } ); // override method mb_common.value = function() @@ -169,6 +285,10 @@ mb_common.assertCommon(); super_called = true; }; + // XXX: Bad idea. Set the keyword in another manner. This is likely to break + // in the future. + members['public'].foo.___$$keywords$$ = { 'virtual': true }; + // override builder.buildMethod( members, {}, 'foo', newfunc, {}, instCallback ); @@ -236,7 +356,7 @@ mb_common.assertCommon(); return instance; }, - members = { 'public': {}, 'protected': {}, 'private': {} } + members = builder.initMembers() ; // set instance values @@ -249,7 +369,7 @@ mb_common.assertCommon(); exports.meta, 'func', func, - [ 'public' ], + { 'virtual': true }, instCallback ); @@ -265,7 +385,7 @@ mb_common.assertCommon(); exports.meta, 'func', func2, - [ 'public' ], + {}, instCallback );