From d9b86c154467400807150a1aca7712324d3540d0 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 22 Oct 2015 23:44:28 -0400 Subject: [PATCH] Support for trait class supertype overrides Traits can now override methods of their class supertypes. Previously, in order to override a method of some class `C` by mixing in some trait `T`, both had to implement a common interface. This had two notable downsides: 1. A trait that was only compatible with details of `C` could only work with `C#M` if it implemented an interface `I` that declared `I#M`. This required the author of `C` to create interfaces where they would otherwise not be necessary. 2. As a corollary of #1---since methods of interfaces must be public, it was not possible for `T` to override any protected method of `C`; this meant that `C` would have to declare such methods public, which may break encapsulation or expose unnecessary concerns to the outside world. Until documentation is available---hopefully in the near future---the test cases provide detailed documentation of the behavior. Stackable traits work as you would expect: ```javascript var C = Class( { 'virtual foo': function() { return 'C'; }, } ); var T1 = Trait.extend( C, { 'virtual abstract override foo': function() { return 'T1' + this.__super(); }, } ); var T2 = Trait.extend( C, { 'virtual abstract override foo': function() { return 'T2' + this.__super(); }, } ); C.use( T1 ) .use( T1 ) .use( T2 ) .use( T2 ) .foo(); // result: "T2T2T1T1C" ``` If the `override` keyword is used without `abstract`, then the super method is statically bound to the supertype, rather than being resolved at runtime: ```javascript var C = Class( { 'virtual foo': function() { return 'C'; }, } ); var T1 = Trait.extend( C, { 'virtual abstract override foo': function() { return 'T1' + this.__super(); }, } ); var T2 = Trait.extend( C, { // static override 'virtual override foo': function() { return 'T2' + this.__super(); }, } ); C.use( T1 ) .use( T1 ) .use( T2 ) .use( T2 ) .foo(); // result: "T2C" ``` This latter form should be discouraged in most circumstances (as it prevents stackable traits), but the behavior is consistent with the rest of the system. Happy hacking. --- lib/ClassBuilder.js | 6 + lib/MemberBuilderValidator.js | 2 +- lib/Trait.js | 22 ++- test/Trait/ClassExtendTest.js | 278 ++++++++++++++++++++++++++++++++++ 4 files changed, 299 insertions(+), 9 deletions(-) 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/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() + ); + }, } );