diff --git a/lib/class.js b/lib/class.js index e9ead69..ad830b0 100644 --- a/lib/class.js +++ b/lib/class.js @@ -329,10 +329,10 @@ function createImplement( base, ifaces, cname ) // If neither of those are available, extend from an empty class. ifaces.push( base || ext_base || extend( {} ) ); - return extend.apply( null, [ + return extend.call( null, implement.apply( this, ifaces ), def - ] ); + ); }, }; } @@ -385,8 +385,9 @@ var implement = function() len = args.length, arg = null, - abstract_list = [], - implemented = []; + implemented = [], + make_abstract = false + ; // add each of the interfaces for ( var i = 0; i < len; i++ ) @@ -397,12 +398,19 @@ var implement = function() util.propParse( arg.prototype, { method: function( name, func, is_abstract, keywords ) { - dest[ name ] = func; + dest[ name ] = func; + make_abstract = true; }, } ); implemented.push( arg ); } + // xxx: temporary + if ( make_abstract ) + { + dest.___$$abstract$$ = true; + } + // create a new class with the implemented abstract methods var class_new = module.exports.extend( base, dest ); class_builder.getMeta( class_new ).implemented = implemented; diff --git a/lib/class_abstract.js b/lib/class_abstract.js new file mode 100644 index 0000000..06cd3eb --- /dev/null +++ b/lib/class_abstract.js @@ -0,0 +1,75 @@ +/** + * Wrapper permitting the definition of abstract classes + * + * This doesn't actually introduce any new functionality. Rather, it sets a flag + * to allow abstract methods within a class, forcing users to clearly state + * that a class is abstract. + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package core + */ + +var Class = require( __dirname + '/class' ); + + +module.exports = exports = function() +{ + markAbstract( arguments ); + + // forward everything to Class + return Class.apply( this, arguments ); +}; + + +exports.extend = function() +{ + markAbstract( arguments ); + + return Class.extend.apply( this, arguments ); +}; + + +exports.implement = function() +{ + var impl = Class.implement.apply( this, arguments ), + extend = impl.extend; + + impl.extend = function() + { + markAbstract( arguments ); + return extend.apply( this, arguments ); + }; + + return impl; +}; + + +function markAbstract( args ) +{ + // the last argument _should_ be the definition + var dfn = args[ args.length - 1 ]; + + if ( typeof dfn === 'object' ) + { + // mark as abstract + dfn.___$$abstract$$ = true; + } +} + diff --git a/lib/class_builder.js b/lib/class_builder.js index 21e9732..03565f8 100644 --- a/lib/class_builder.js +++ b/lib/class_builder.js @@ -263,6 +263,8 @@ exports.build = function extend() attachFlags( new_class, props ); + validateAbstract( new_class, cname, abstract_methods ); + // We reduce the overall cost of this definition by defining it on the // prototype rather than during instantiation. While this does increase the // amount of time it takes to access the property through the prototype @@ -286,6 +288,40 @@ exports.build = function extend() }; +/** + * Validates abstract class requirements + * + * @param {function()} ctor class + * @param {string} cname class name + * @param {Object} abstract_methods + * + * @return {undefined} + */ +function validateAbstract( ctor, cname, abstract_methods ) +{ + if ( ctor.___$$abstract$$ ) + { + if ( abstract_methods.__length === 0 ) + { + throw TypeError( + "Class " + ( cname || "(anonymous)" ) + " was declared as " + + "abstract, but contains no abstract members" + ); + } + } + else + { + if ( abstract_methods.__length > 0 ) + { + throw TypeError( + "Class " + ( cname || "(anonymous)" ) + " contains abstract " + + "members and must therefore be declared abstract" + ); + } + } +} + + /** * Creates the constructor for a new class * @@ -1055,9 +1091,10 @@ function attachId( ctor, id ) function attachFlags( ctor, props ) { ctor.___$$final$$ = !!( props.___$$final$$ ); + ctor.___$$abstract$$ = !!( props.___$$abstract$$ ); // The properties are no longer needed. Set to undefined rather than delete // (v8 performance) - props.___$$final$$ = undefined; + props.___$$final$$ = props.___$$abstract$$ = undefined; } diff --git a/test/test-class-abstract.js b/test/test-class-abstract.js index 4eaf4ed..a85781d 100644 --- a/test/test-class-abstract.js +++ b/test/test-class-abstract.js @@ -24,8 +24,103 @@ var common = require( './common' ), assert = require( 'assert' ), - Class = common.require( 'class' ), - util = common.require( 'util' ); + util = common.require( 'util' ), + + Class = common.require( 'class' ), + AbstractClass = common.require( 'class_abstract' ) +; + + +/** + * In order to ensure the code documents itself, we should require that all + * classes containing abstract members must themselves be declared as abstract. + * Otherwise, you are at the mercy of the developer's documentation/comments to + * know whether or not the class is indeed abstract without looking through its + * definition. + */ +( function testMustDeclareClassesWithAbstractMembersAsAbstract() +{ + try + { + // should fail; class not declared as abstract + Class( 'Foo', + { + 'abstract foo': [], + } ); + } + catch ( e ) + { + assert.ok( + e.message.search( 'Foo' ) !== -1, + "Abstract class declaration error should contain class name" + ); + + return; + } + + assert.fail( + "Should not be able to declare abstract members unless class is also " + + "declared as abstract" + ); +} )(); + + +/** + * Abstract members should be permitted if the class itself is declared as + * abstract + */ +( function testCanDeclareClassAsAbstract() +{ + AbstractClass( + { + 'abstract foo': [], + } ); +} )(); + + +/** + * If a class is declared as abstract, it should contain at least one abstract + * method. Otherwise, the abstract definition is pointless and unnecessarily + * confusing. The whole point of the declaration is self-documenting code. + */ +( function testAbstractClassesMustContainAbstractMethods() +{ + try + { + // should fail; class not declared as abstract + AbstractClass( 'Foo', {} ); + } + catch ( e ) + { + assert.ok( + e.message.search( 'Foo' ) !== -1, + "Abstract class declaration error should contain class name" + ); + + return; + } + + assert.fail( + "Abstract classes should contain at least one abstract method" + ); +} )(); + + +( function testAbstractClassContainsExtendMethod() +{ + assert.ok( typeof AbstractClass.extend === 'function', + "AbstractClass contains extend method" + ); +} )(); + + +( function testAbstractClassContainsImplementMethod() +{ + assert.ok( typeof AbstractClass.implement === 'function', + "AbstractClass contains implement method" + ); +} )(); + // not abstract var Foo = Class.extend( {} ); @@ -33,7 +128,7 @@ var Foo = Class.extend( {} ); // abstract (ctor_called is not a class member to ensure that visibility bugs do // not impact our test) var ctor_called = false, - AbstractFoo = Class.extend( + AbstractFoo = AbstractClass.extend( { __construct: function() { @@ -48,7 +143,7 @@ var ctor_called = false, // still abstract (didn't provide a concrete implementation of both abstract // methods) -var SubAbstractFoo = AbstractFoo.extend( +var SubAbstractFoo = AbstractClass.extend( AbstractFoo, { second: function() { @@ -56,7 +151,7 @@ var SubAbstractFoo = AbstractFoo.extend( }); // concrete -var ConcreteFoo = AbstractFoo.extend( +var ConcreteFoo = Class.extend( AbstractFoo, { method: function( one, two, three ) { @@ -193,7 +288,7 @@ var ConcreteFoo = AbstractFoo.extend( assert.doesNotThrow( function() { - AbstractFoo.extend( + AbstractClass.extend( AbstractFoo, { // incorrect number of arguments 'abstract method': [ 'one', 'two', 'three', 'four' ], @@ -212,7 +307,7 @@ var ConcreteFoo = AbstractFoo.extend( assert.doesNotThrow( function() { - AbstractFoo.extend( + AbstractClass.extend( AbstractFoo, { second: function( foo ) { @@ -257,7 +352,8 @@ var ConcreteFoo = AbstractFoo.extend( { assert.doesNotThrow( function() { - SubAbstractFoo.extend( { + Class.extend( SubAbstractFoo, + { // concrete, so the result would otherwise not be abstract 'method': function( one, two, three ) {}, diff --git a/test/test-class-implement.js b/test/test-class-implement.js index 4e6e15d..dff57fb 100644 --- a/test/test-class-implement.js +++ b/test/test-class-implement.js @@ -24,8 +24,11 @@ var common = require( './common' ), assert = require( 'assert' ), - Class = common.require( 'class' ), - Interface = common.require( 'interface' ); + + Class = common.require( 'class' ), + Interface = common.require( 'interface' ), + AbstractClass = common.require( 'class_abstract' ) +; var Type = Interface.extend( { @@ -97,7 +100,7 @@ var Type = Interface.extend( { */ ( function testAbstractMethodsCopiedIntoNewClassUsingEmptyBase() { - Foo = Class.implement( Type, Type2 ).extend( {} ); + Foo = AbstractClass.implement( Type, Type2 ).extend( {} ); assert.ok( ( ( Foo.prototype.foo instanceof Function ) @@ -136,7 +139,7 @@ var Type = Interface.extend( { ( function testAbstractMethodsCopiedIntoNewClassUsingExistingBase() { - PlainFoo2 = PlainFoo.implement( Type, Type2 ).extend( {} ); + PlainFoo2 = AbstractClass.implement( Type, Type2 ).extend( PlainFoo, {} ); assert.ok( ( ( PlainFoo2.prototype.foo instanceof Function ) @@ -224,7 +227,7 @@ var Type = Interface.extend( { function() { // this /should/ work - Class.implement( Type ).extend( PlainFoo, {} ); + AbstractClass.implement( Type ).extend( PlainFoo, {} ); }, Error, "Can specify parent for exetnd() when implementing atop an " + diff --git a/test/test-class-name.js b/test/test-class-name.js index a700c3a..1f4d07f 100644 --- a/test/test-class-name.js +++ b/test/test-class-name.js @@ -25,8 +25,9 @@ var common = require( './common' ), assert = require( 'assert' ), - Class = common.require( 'class' ), - Interface = common.require( 'interface' ) + Class = common.require( 'class' ), + AbstractClass = common.require( 'class_abstract' ), + Interface = common.require( 'interface' ) ; @@ -128,7 +129,7 @@ var common = require( './common' ), // abstract assert.equal( - Class( { 'abstract foo': [] } ).toString(), + AbstractClass( { 'abstract foo': [] } ).toString(), '(AbstractClass)', "Converting abstract anonymous class to string yields class string" ); @@ -152,7 +153,7 @@ var common = require( './common' ), // abstract assert.equal( - Class( name, { 'abstract foo': [] } ).toString(), + AbstractClass( name, { 'abstract foo': [] } ).toString(), name, "Converting abstract named class to string yields string with name " + "of class" diff --git a/tools/combine b/tools/combine index 4cc3b53..011b494 100755 --- a/tools/combine +++ b/tools/combine @@ -29,7 +29,7 @@ RMTRAIL="$PATH_TOOLS/rmtrail" # order matters CAT_MODULES="prop_parser util propobj member_builder class_builder" -CAT_MODULES="$CAT_MODULES class class_final interface" +CAT_MODULES="$CAT_MODULES class class_final class_abstract interface" ## # Output template header