diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 1789592..d8b9d27 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -822,6 +822,12 @@ function _parseMethod( name, func, is_abstract, keywords ) { this.virtual_members[ name ] = true; } + else + { + // final (non-virtual) definitions must clear the virtual flag from + // their super method + delete this.virtual_members[ name ]; + } } diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index 4ad57f9..831e024 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -233,7 +233,7 @@ exports.prototype.validateMethod = function( // do not allow overriding concrete methods with abstract unless the // abstract method is weak - if ( keywords[ 'abstract' ] + if ( ( keywords[ 'abstract' ] && !keywords[ 'override' ] ) && !( keywords.weak ) && !( prev_keywords[ 'abstract' ] ) ) diff --git a/lib/Trait.js b/lib/Trait.js index 0075d96..7794b84 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -208,7 +208,9 @@ Trait.extend = function( /* ... */ ) } // and here we can see that traits are quite literally abstract classes - var tclass = base.extend( dfn ); + var tclass = ( ext_base ) + ? base.extend( ext_base, dfn ) + : base.extend( dfn ); Trait.__trait = type; Trait.__acls = tclass; @@ -661,12 +663,12 @@ function mixin( trait, dfn, tc, base ) * * @return {undefined} */ -function mixinCls( cls, dfn, iname ) +function mixinCls( cls, dfn, iname, inparent ) { var methods = cls.___$$methods$$; - mixMethods( methods['public'], dfn, 'public', iname ); - mixMethods( methods['protected'], dfn, 'protected', iname ); + mixMethods( methods['public'], dfn, 'public', iname, inparent ); + mixMethods( methods['protected'], dfn, 'protected', iname, inparent ); // if this class inherits from another class that is *not* the base // class, recursively process its methods; otherwise, we will have @@ -674,7 +676,7 @@ function mixinCls( cls, dfn, iname ) var parent = methods['public'].___$$parent$$; if ( parent && ( parent.constructor !== ClassBuilder.ClassBase ) ) { - mixinCls( parent.constructor, dfn, iname ); + mixinCls( parent.constructor, dfn, iname, true ); } } @@ -713,7 +715,7 @@ function mixinImpl( cls, dest_meta ) * * @return {undefined} */ -function mixMethods( src, dest, vis, iname ) +function mixMethods( src, dest, vis, iname, inparent ) { for ( var f in src ) { @@ -747,10 +749,14 @@ function mixMethods( src, dest, vis, iname ) { // copy the abstract definition (N.B. this does not copy the // param names, since that is not [yet] important); the - // visibility modified is important to prevent de-escalation + // visibility modifier is important to prevent de-escalation // errors on override dest[ vis + ' weak abstract ' + f ] = src[ f ].definition; } + else if ( inparent && !keywords[ 'abstract' ] ) + { + continue; + } else { var vk = keywords['virtual'], @@ -815,7 +821,7 @@ function mixMethods( src, dest, vis, iname ) * @param {Class} T trait * @param {Object} dfn definition object of class being mixed into * @param {Array} tc trait class object - * @param {Class} base target supertyep + * @param {Class} base target supertype * * @return {string} private member into which C instance shall be stored */ diff --git a/lib/class.js b/lib/class.js index 8096f02..1fc788c 100644 --- a/lib/class.js +++ b/lib/class.js @@ -486,9 +486,13 @@ function createImplement( base, ifaces, cname ) * with implicit extend) * * @return {Function} staging object for mixin + * + * @throws {TypeError} when object is not a trait */ function createUse( basef, traits, nonbase ) { + _validateTraits( traits ); + // invoking the partially applied class will immediately complete its // definition and instantiate it with the provided constructor arguments var partial = function() @@ -575,6 +579,30 @@ function createUse( basef, traits, nonbase ) } +/** + * Verify that each object in TRAITS will be able to be mixed in + * + * TODO: Use Trait.isTrait; we have circular dependency issues at the moment + * preventing that; refactoring is needed. + * + * @param {Array} traits objects to validate + * + * @return {undefined} + * + * @throws {TypeError} when object is not a trait + */ +function _validateTraits( traits ) +{ + for ( var t in traits ) + { + if ( typeof traits[ t ].__mixin !== 'function' ) + { + throw TypeError( "Cannot mix in non-trait " + t ); + } + } +} + + function createMixedClass( base, traits ) { // generated definition for our [abstract] class that will mix in each diff --git a/test/Trait/ClassExtendTest.js b/test/Trait/ClassExtendTest.js index 345890c..ceea701 100644 --- a/test/Trait/ClassExtendTest.js +++ b/test/Trait/ClassExtendTest.js @@ -276,4 +276,282 @@ require( 'common' ).testCase( _self.Sut.extend( _self.FinalClass( {} ), {} ); }, TypeError ); }, + + + /** + * When extending a class C with a concrete implementation for some + * method M, we should be able to override C#M as T#M and have C#M + * recognized as its super method. Just as you would expect when + * subtyping using classes. + */ + 'Traits can override public virtual super methods': function() + { + var super_val = {}; + + var C = this.Class( + { + 'virtual foo': function() + { + return super_val; + } + } ); + + var T = this.Sut.extend( C, + { + 'override foo': function() + { + return { sval: this.__super() }; + } + } ); + + this.assertStrictEqual( + C.use( T )().foo().sval, + super_val + ); + }, + + + /** + * Unlike implementing interfaces---which define only public + * APIs---class can also provide protected methods. The ability to + * override protected methods is important, since it allows modifying + * internal state. This can be used in place of a Strategy, for + * example. + * + * This otherwise does not differ at all from the public test above. + */ + 'Traits can override protected virtual super methods': function() + { + var super_val = {}; + + var C = this.Class( + { + 'virtual protected foo': function() + { + return super_val; + }, + + getFoo: function() + { + return this.foo(); + }, + } ); + + var T = this.Sut.extend( C, + { + 'override protected foo': function() + { + return { sval: this.__super() }; + }, + } ); + + this.assertStrictEqual( + C.use( T )().getFoo().sval, + super_val + ); + }, + + + /** + * When providing a concrete definition for some abstract method A on + * interface I, traits must use the `abstract override` keyword, because + * we cannot know what type of object we will be mixed into---the class + * could have a concrete implementation, or it may not. + * + * This is not the case when extending a class directly. We should + * therefore expect that we can provide a concrete definition in the + * same way we would when subclassing---without any special keywords. + * + * Note that we do _not_ have a test to define what happens when + * `abstract override` _is_ used in this scenario; this was + * intentionally left undefined, and may or may not be given proper + * attention in the future. Don't do it. + */ + 'Traits can provide concrete definition for abstract method': function() + { + var expected = {}; + + var C = this.AbstractClass( + { + foo: function() + { + return this.concrete(); + }, + + 'abstract concrete': [], + } ); + + var T = this.Sut.extend( C, + { + concrete: function() + { + return expected; + }, + } ); + + this.assertStrictEqual( + C.use( T )().foo(), + expected + ); + }, + + + /** + * The stackable property of traits should be preserved under all + * circumstances (so long as override is virtual). This is different + * than subtyping with classes, which would always invoke the + * supertype's method as the super method. + * + * Note the use of `abstract override` here---this is needed for the + * same reason that it is needed for traits that implement interfaces + * and want to override concrete methods of a class that it is being + * mixed into. The test that follows this one will demonstrate the + * behavior when a normal `override` is used. + * + * See the linearization tests for more information. + */ + 'Trait class method abstract overrides can be stacked': function() + { + var C = this.Class( + { + 'virtual foo': function() + { + return 1; + }, + } ); + + var T1 = this.Sut.extend( C, + { + 'virtual abstract override foo': function() + { + return 3 + this.__super(); + }, + } ); + + var T2 = this.Sut.extend( C, + { + 'virtual abstract override foo': function() + { + return 13 + this.__super(); + }, + } ); + + this.assertEqual( + 20, + C.use( T1 ) + .use( T1 ) + .use( T2 ) + ().foo() + ); + }, + + + /** + * This test is in the exact same format as the above in order to + * illustrate the important distinction between the two concepts. + * + * This can be confusing---and frustrating to users of an API if its + * developer does not understand the distinction---but it is important + * to note that it is consistent with the rest of the system: `override` + * on its own will always determine the super method at the time of + * definition, whereas `abstract override` will defer that determination + * until the time of mixin. + */ + 'Trait class C#M non-abstract override always uses C#M as super': + function() + { + var C = this.Class( + { + 'virtual foo': function() + { + return 1; + }, + } ); + + var T1 = this.Sut.extend( C, + { + 'virtual override foo': function() + { + return 3 + this.__super(); + }, + } ); + + var T2 = this.Sut.extend( C, + { + 'virtual override foo': function() + { + return 13 + this.__super(); + }, + } ); + + this.assertEqual( + 14, + C.use( T1 ) + .use( T1 ) + .use( T2 ) + ().foo() + ); + }, + + + /** + * The stackable property should apply when the super class's method is + * abstract as well---just as it does with interfaces. Plainly: + * abstract classes and interfaces are identical in method behavior with + * the exception that abstract classes can provide concrete + * implementations. + * + * There is one caveat: traits cannot blindly override methods, abstract + * or concrete---the `override` keyword assumes a concrete method M to + * act as the super method, which would not exist if the supertype has + * only an abstract method M. This is behavior consistent with classes. + * + * This is also consistent with Scala's stackable trait pattern: the + * abstract class C (below) is the "base", T1 acts as the "core", and T2 + * is a "stackable". This consistency was not intentional, but is a + * natural evolution for a consistent system. (It is a desirable + * consistency, though, so that others can apply their knowledge of + * Scala---and any other systems motivated by it.) + */ + 'Traits can stack concrete definitions for class abstract methods': + function() + { + var C = this.AbstractClass( + { + foo: function() + { + return this.concrete(); + }, + + 'abstract concrete': [], + } ); + + var T1 = this.Sut.extend( C, + { + // this cannot be an abstract override, because there is not yet + // a concrete definition (and we know this immediately, since + // we're explicitly extending C) + 'virtual concrete': function() + { + return 3; + }, + } ); + + var T2 = this.Sut.extend( C, + { + // T1 provides a concrete method that we can override + 'virtual abstract override concrete': function() + { + return 5 + this.__super(); + }, + } ); + + this.assertEqual( + 13, + C.use( T1 ) + .use( T2 ) + .use( T2 ) + ().foo() + ); + }, } ); diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index 0402924..e3ffe53 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -475,4 +475,30 @@ require( 'common' ).testCase( this.Class.isClass( this.Class( {} ).use( T ) ) ); }, + + + /** + * Attempts to mix in non-traits should immediately trigger an error + * during the declaration. It is important not to defer this until the + * time of actual mix in---which is lazy---since the stack will not + * provide useful information on how to correct it. + */ + 'Throws error when object to mix in is not a trait': function() + { + var _self = this; + + // one of one + this.assertThrows( function() + { + // this should error immediately; it should not wait until + // the actual mix in (which is lazy) + _self.Class( {} ).use( {} ); + }, TypeError ); + + // one of many + this.assertThrows( function() + { + _self.Class( {} ).use( _self.Trait( {} ), {} ); + }, TypeError ); + }, } );