From 3fc0f90e01ac4ff2a41716d6158b168406c72cfc Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 28 May 2014 23:37:36 -0400 Subject: [PATCH 1/5] Initial implementation of parameterized traits This is an important feature to permit trait reuse without excessive subtyping---composition over inheritance. For example, consider that you have a `HttpPlainAuth` trait that adds authentication support to some transport layer. Without parameterized traits, you have two options: 1. Expose setters for credentials 2. Trait closure 3. Extend the trait (not yet supported) The first option seems like a simple solution: ```javascript Transport.use( HttpPlainAuth )() .setUser( 'username', 'password' ) .send( ... ); ``` But we are now in the unfortunate situation that our initialization procedure has changed. This, for one, means that authentication logic must be added to anything that instantiates classes that mix in `HttpPlainAuth`. We'll explore that in more detail momentarily. More concerning with this first method is that, not only have we prohibited immutability, but we have also made our code reliant on *invocation order*; `setUser` must be called before `send`. What if we have other traits mixed in that have similar conventions? Normally, this is the type of problem that would be solved with a builder, but would we want every configurable trait to return a new `Transport` instance? All that on top of littering the API---what a mess! The second option is to store configuration data outside of the Trait acting as a closure: ```javascript var _user, _pass; function setCredentials( user, pass ) { _user = user; _pass = pass; } Trait( 'HttpPlainAuth', { /* use _user and _pass */ } ) ``` There are a number of problems with this; the most apparent is that, in this case, the variables `_user` and `_pass` act in place of static fields---all instances will share that data, and if the data is modified, it will affect all instances; you are therefore relying on external state, and mutability is forced upon you. You are also left with an awkward `setCredentials` call that is disjoint from `HttpPlainAuth`. The other notable issue arises if you did want to support instance-specific credentials. You would have to use ease.js' internal identifiers (which is undocumented and subject to change in future versions), and would likely accumulate garbage data as mixin instances are deallocated, since ECMAScript does not have destructor support. To recover from memory leaks, you could instead create a trait generator: ```javascript function createHttpPlainAuth( user, pass ) { return Trait( { /* ... */ } ); } ``` This uses the same closure concept, but generates new traits at runtime. This has various implications depending on your engine, and may thwart future ease.js optimization attempts. The third (which will be supported in the near future) is prohibitive: we'll add many unnecessary traits that are a nightmare to develop and maintain. Parameterized traits are similar in spirit to option three, but without creating new traits each call: traits now support being passed configuration data at the time of mixin that will be passed to every new instance: ```javascript Transport.use( HttpPlainAuth( user, pass ) )() .send( ... ); ``` Notice now how the authentication configuration is isolated to the actual mixin, *prior to* instantiation; the caller performing instantiation need not be aware of this mixin, and so the construction logic can remain wholly generic for all `Transport` types. It further allows for a convenient means of providing useful, reusable exports: ```javascript module.exports = { ServerFooAuth: HttpPlainAuth( userfoo, passfoo ), ServerBarAuth: HttpPlainAuth( userbar, passbar ), ServerFooTransport: Transport.use( module.exports.ServerFooAuth ), // ... }; var module = require( 'foo' ); // dynamic auth Transport.use( foo.ServerFooAuth )().send( ... ); // or predefined classes module.ServerFooTransport().send( ... ); ``` Note that, in all of the above cases, the initialization logic is unchanged---the caller does not need to be aware of any authentication mechanism, nor should the caller care of its existence. So how do you create parameterized traits? You need only define a `__mixin` method: Trait( 'HttpPlainAuth', { __mixin: function( user, pass ) { ... } } ); The method `__mixin` will be invoked upon instantiation of the class into which a particular configuration of `HttpPlainAuth` is mixed into; it was named differently from `__construct` to make clear that (a) traits cannot be instantiated and (b) the constructor cannot be overridden by traits. A configured parameterized trait is said to be an *argument trait*; each argument trait's configuration is discrete, as was demonstrated by `ServerFooAuth` and `ServerBarAuth` above. Once a parameterized trait is configured, its arguments are stored within the argument trait and those arguments are passed, by reference, to `__mixin`. Since any mixed in trait can have its own `__mixin` method, this permits traits to have their own initialization logic without the need for awkward overrides or explicit method calls. --- lib/ClassBuilder.js | 1 + lib/Trait.js | 124 +++++++++++++++++--- test/Trait/DefinitionTest.js | 2 + test/Trait/ParameterTest.js | 211 +++++++++++++++++++++++++++++++++++ 4 files changed, 320 insertions(+), 18 deletions(-) create mode 100644 test/Trait/ParameterTest.js diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 11bda6b..18795ec 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -72,6 +72,7 @@ var util = require( './util' ), */ public_methods = { '__construct': true, + '__mixin': true, 'toString': true, '__toString': true, }, diff --git a/lib/Trait.js b/lib/Trait.js index 4bd17c2..e140d2c 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -100,7 +100,9 @@ Trait.extend = function( dfn ) // store any provided name, since we'll be clobbering it (the definition // object will be used to define the hidden abstract class) - var name = dfn.__name || '(Trait)'; + var name = dfn.__name || '(Trait)', + type = _getTraitType( dfn ), + isparam = ( type === 'param' ); // augment the parser to handle our own oddities dfn.___$$parser$$ = { @@ -116,10 +118,41 @@ Trait.extend = function( dfn ) // give the abstract trait class a distinctive name for debugging dfn.__name = '#AbstractTrait#'; - function TraitType() - { - throw Error( "Cannot instantiate trait" ); - }; + // if __mixin was provided,in the definition, then we should crate a + // paramaterized trait + var Trait = ( isparam ) + ? function ParameterTraitType() + { + // duplicate ars in a way that v8 can optimize + var args = [], i = arguments.length; + while ( i-- ) args[ i ] = arguments[ i ]; + + var AT = function ArgumentTrait() + { + throw Error( "Cannot re-configure argument trait" ); + }; + + // TODO: mess! + AT.___$$mixinargs = args; + AT.__trait = 'arg'; + AT.__acls = Trait.__acls; + AT.__ccls = Trait.__ccls; + AT.toString = Trait.toString; + AT.__mixinImpl = Trait.__mixinImpl; + AT.__isInstanceOf = Trait.__isInstanceOf; + + // mix in the argument trait instead of the original + AT.__mixin = function( dfn, tc, base ) + { + mixin( AT, dfn, tc, base ); + }; + + return AT; + } + : function TraitType() + { + throw Error( "Cannot instantiate non-parameterized trait" ); + }; // implement interfaces if indicated var base = AbstractClass; @@ -131,33 +164,58 @@ Trait.extend = function( dfn ) // and here we can see that traits are quite literally abstract classes var tclass = base.extend( dfn ); - TraitType.__trait = true; - TraitType.__acls = tclass; - TraitType.__ccls = null; - TraitType.toString = function() + Trait.__trait = type; + Trait.__acls = tclass; + Trait.__ccls = null; + Trait.toString = function() { return ''+name; }; // invoked to trigger mixin - TraitType.__mixin = function( dfn, tc, base ) - { - mixin( TraitType, dfn, tc, base ); - }; + Trait.__mixin = ( isparam ) + ? function() + { + throw TypeError( + "Cannot mix in parameterized trait " + Trait.toString() + + "; did you forget to configure it?" + ); + } + : function( dfn, tc, base ) + { + mixin( Trait, dfn, tc, base ); + }; // mixes in implemented types - TraitType.__mixinImpl = function( dest_meta ) + Trait.__mixinImpl = function( dest_meta ) { mixinImpl( tclass, dest_meta ); }; // TODO: this and the above should use util.defineSecureProp - TraitType.__isInstanceOf = Interface.isInstanceOf; + Trait.__isInstanceOf = Interface.isInstanceOf; - return TraitType; + return Trait; }; +/** + * Retrieve a string representation of the trait type + * + * A trait is parameterized if it has a __mixin method; otherwise, it is + * standard. + * + * @param {Object} dfn trait definition object + * @return {string} trait type + */ +function _getTraitType( dfn ) +{ + return ( typeof dfn.__mixin === 'function' ) + ? 'param' + : 'std'; +} + + /** * Verifies trait member restrictions * @@ -295,8 +353,7 @@ function createImplement( ifaces, name ) /** * Determines if the provided value references a trait * - * @param {*} trait value to check - * + * @param {*} trait value to check * @return {boolean} whether the provided value references a trait */ Trait.isTrait = function( trait ) @@ -305,6 +362,32 @@ Trait.isTrait = function( trait ) }; +/** + * Determines if the provided value references a parameterized trait + * + * @param {*} trait value to check + * @return {boolean} whether the provided value references a param trait + */ +Trait.isParameterTrait = function( trait ) +{ + return !!( ( trait || {} ).__trait === 'param' ); +}; + + +/** + * Determines if the provided value references an argument trait + * + * An argument trait is a configured parameter trait. + * + * @param {*} trait value to check + * @return {boolean} whether the provided value references an arg trait + */ +Trait.isArgumentTrait = function( trait ) +{ + return !!( ( trait || {} ).__trait === 'arg' ); +}; + + /** * Create a concrete class from the abstract trait class * @@ -670,6 +753,8 @@ function addTraitInst( T, dfn, tc, base ) * resulting objects assigned to their rsepective pre-determined field * names. * + * The MIXINARGS are only useful in the case of parameterized trait. + * * This will lazily create the concrete trait class if it does not already * exist, which saves work if the trait is never used. * @@ -703,6 +788,9 @@ function tctor( tc, base, privsym ) this[ f ] = C( base, this[ privsym ].vis )[ privsym ].vis; } + // this has been previously validated to ensure that it is a function + this.__mixin && this.__mixin.apply( this, T.___$$mixinargs ); + // if we are a subtype, be sure to initialize our parent's traits this.__super && this.__super( privsym ); }; diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index 5f14802..df6ba8f 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -82,6 +82,8 @@ require( 'common' ).testCase( /** * A trait can only be used by something else---it does not make sense * to instantiate them directly, since they form an incomplete picture. + * + * Now, that said, see parameterized traits. */ '@each(ctor) Cannot instantiate trait without error': function( T ) { diff --git a/test/Trait/ParameterTest.js b/test/Trait/ParameterTest.js new file mode 100644 index 0000000..a4e87cb --- /dev/null +++ b/test/Trait/ParameterTest.js @@ -0,0 +1,211 @@ +/** + * Tests parameterized traits + * + * Copyright (C) 2014 Free Software Foundation, Inc. + * + * This file is part of GNU ease.js. + * + * ease.js is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +require( 'common' ).testCase( +{ + caseSetUp: function() + { + this.Sut = this.require( 'Trait' ); + this.Class = this.require( 'class' ); + + var _self = this; + this.createParamTrait = function( f ) + { + return _self.Sut( { __mixin: ( f || function() {} ) } ); + }; + }, + + + /** + * Since traits are reusable components mixed into classes, they + * themselves do not have a constructor. This puts the user at a + * disadvantage, because she would have to create a new trait to simply + * to provide some sort of configuration at the time the class is + * instantiated. Adding a method to do the configuration is another + * option, but that is inconvenient, especially when the state is + * intended to be immutable. + * + * This does not suffer from the issue that Scala is having in trying to + * implement a similar feature because traits cannot have non-private + * properties; the linearization process disambiguates. + * + * When a trait contains a __mixin method, it is created as a + * ParameterTraitType instead of a TraitType. Both must be recognized as + * traits so that they can both be mixed in as expected; a method is + * provided to assert whether or not a trait is a parameter trait + * programatically, since attempting to configure a non-param trait will + * throw an exception. + */ + 'Can create parameter traits': function() + { + var T = this.createParamTrait(); + + this.assertOk( this.Sut.isParameterTrait( T ) ); + this.assertOk( this.Sut.isTrait( T ) ); + }, + + + /** + * A parameter trait is in an uninitialized state---it cannot be mixed + * in until arguments have been provided; same rationale as a class + * constructor. + */ + 'Cannot mix in a parameter trait': function() + { + var _self = this; + this.assertThrows( function() + { + _self.Class.use( _self.createParamTrait() )(); + } ); + }, + + + /** + * Invoking a parameter trait will produce an argument trait which may + * be mixed in. This has the effect of appearing as though the trait is + * being instantiated (but it's not). + */ + 'Invoking parameter trait produces argument trait': function() + { + var _self = this; + this.assertDoesNotThrow( function() + { + _self.assertOk( + _self.Sut.isArgumentTrait( _self.createParamTrait()() ) + ); + } ); + }, + + + /** + * Traits cannot be instantiated; ensure that this remains true, even + * with the parameterized trait implementation. + */ + 'Invoking a standard trait throws an exception': function() + { + var Sut = this.Sut; + this.assertThrows( function() + { + // no __mixin method; not a param trait + Sut( {} )(); + } ); + }, + + + /** + * Argument traits can be mixed in just as non-parameterized traits can; + * it would be silly not to consider them to be traits through our + * reflection API. + */ + 'Recognizes argument trait as a trait': function() + { + this.assertOk( + this.Sut.isTrait( this.createParamTrait()() ) + ); + }, + + + /** + * A param trait, upon configuration, returns an immutable argument + * trait; any attempt to invoke it (e.g. to try to re-configure) is in + * error. + */ + 'Cannot re-configure argument trait': function() + { + var _self = this; + this.assertThrows( function() + { + // ParameterTrait => ArgumentTrait => Error + _self.createParamTrait()()(); + } ); + }, + + + /** + * Upon instantiating a class into which an argument trait was mixed, + * all configuration arguments should be passed to the __mixin method. + * Note that this means that __mixin *will not* be called at the point + * that the param trait is configured. + */ + '__mixin is invoked upon class instantiation': function() + { + var called = 0; + var T = this.createParamTrait( function() + { + called++; + } ); + + // ensure we only invoke __mixin a single time + this.Class( {} ).use( T() )(); + this.assertEqual( called, 1 ); + }, + + + /** + * Standard sanity check---make sure that the arguments provided during + * configuration are passed as-is, by reference, to __mixin. Note that + * this has the terrible consequence that, should one of the arguments + * be modified by __mixin (e.g. an object field), then it will be + * modified for all other __mixin calls. But that is the case with any + * function. ;) + */ + '__mixin is passed arguments by reference': function() + { + var args, + a = { a: 'a' }, + b = { b: 'b' }; + + var T = this.createParamTrait( function() + { + args = arguments; + } ); + + this.Class( {} ).use( T( a, b ) )(); + + this.assertStrictEqual( a, args[ 0 ] ); + this.assertStrictEqual( b, args[ 1 ] ); + }, + + + /** + * The __mixin method should be invoked within the context of the trait + * and should therefore have access to its private members. Indeed, + * parameterized traits would have far more limited use if __mixin did + * not have access to private members, because that would be the proper + * place to hold configuration data. + */ + '__mixin has access to trait private members': function() + { + var expected = {}; + + var T = this.Sut( + { + 'private _foo': null, + __mixin: function( arg ) { this._foo = arg; }, + getFoo: function() { return this._foo; }, + } ); + + this.assertStrictEqual( expected, + this.Class( {} ).use( T( expected ) )().getFoo() + ); + }, +} ); + From 2204ff6d28686c39b1465ac283204b6f339d2115 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 3 Jun 2014 23:10:55 -0400 Subject: [PATCH 2/5] Parameterized traits may now be mixed in without configuration Rationale is available in the test case. --- lib/Trait.js | 16 +++--------- test/Trait/ParameterTest.js | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/lib/Trait.js b/lib/Trait.js index e140d2c..41ee82f 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -173,18 +173,10 @@ Trait.extend = function( dfn ) }; // invoked to trigger mixin - Trait.__mixin = ( isparam ) - ? function() - { - throw TypeError( - "Cannot mix in parameterized trait " + Trait.toString() + - "; did you forget to configure it?" - ); - } - : function( dfn, tc, base ) - { - mixin( Trait, dfn, tc, base ); - }; + Trait.__mixin = function( dfn, tc, base ) + { + mixin( Trait, dfn, tc, base ); + }; // mixes in implemented types Trait.__mixinImpl = function( dest_meta ) diff --git a/test/Trait/ParameterTest.js b/test/Trait/ParameterTest.js index a4e87cb..4de7ce0 100644 --- a/test/Trait/ParameterTest.js +++ b/test/Trait/ParameterTest.js @@ -207,5 +207,57 @@ require( 'common' ).testCase( this.Class( {} ).use( T( expected ) )().getFoo() ); }, + + + /** + * It is still useful to be able to define a __mixin method to be called + * as an initialization method for default state; otherwise, arbitrary + * method overrides or explicit method calls are needed. + */ + '__mixin with empty parameter list is still invoked': function() + { + var expected = {}, + given; + + var T = this.createParamTrait( function() { given = expected; } ); + + // notice that we still configure T, with an empty argument list + this.Class( {} ).use( T() )(); + this.assertStrictEqual( expected, given ); + }, + + + /** + * Parameterized traits are intended to be configured. However, there + * are a number of reasons to allow them to be mixed in without + * configuration (that is---without being converted into argument + * traits): + * - Permits default behavior with no configuration, overridable with; + * - If any __mixin definition required configuration, then traits + * would break backwards-compatibility if they wished to define it, + * with no means of maintaining BC; + * - Allows trait itself to determine whether arguments are required. + */ + 'Mixing in param trait will invoke __mixin with no arguments': + function() + { + var n = 0; + + // ensure consistency at any arity; we'll test nullary and unary, + // assuming the same holds true for any n-ary __mixin method + var T0 = this.createParamTrait( function() { n |= 1; } ), + T1 = this.createParamTrait( function( a ) { n |= 2; } ); + + // ensure that param traits do not throw errors when mixed in (as + // opposed to argument traits, which have been tested thusfar) + var C = this.Class( {} ); + this.assertDoesNotThrow( function() + { + C.use( T0 )(); + C.use( T1 )(); + } ); + + this.assertEqual( n, 3 ); + }, } ); From f3cb815baa03f7775524d0dac083a884fbc42016 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 4 Jun 2014 00:37:08 -0400 Subject: [PATCH 3/5] Sibling traits will each have __mixin called distinctly --- lib/Trait.js | 17 ++++++++++++++--- test/Trait/ParameterTest.js | 26 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/lib/Trait.js b/lib/Trait.js index 41ee82f..233feb8 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -617,6 +617,14 @@ function mixMethods( src, dest, vis, iname ) continue; } + // TODO: generalize + // __mixin is exclusive to the trait (private-ish, but can be + // invoked publically internally) + if ( f === '__mixin' ) + { + continue; + } + // TODO: this is a kluge; we'll use proper reflection eventually, // but for now, this is how we determine if this is an actual method // vs. something that just happens to be on the visibility object @@ -778,10 +786,13 @@ function tctor( tc, base, privsym ) // visibility object to gain access to its protected members...quite // the intimate relationship this[ f ] = C( base, this[ privsym ].vis )[ privsym ].vis; - } - // this has been previously validated to ensure that it is a function - this.__mixin && this.__mixin.apply( this, T.___$$mixinargs ); + // this has been previously validated to ensure that it is a + // function + this[ f ].__mixin && this[ f ].__mixin.apply( + this[ f ], T.___$$mixinargs + ); + } // if we are a subtype, be sure to initialize our parent's traits this.__super && this.__super( privsym ); diff --git a/test/Trait/ParameterTest.js b/test/Trait/ParameterTest.js index 4de7ce0..31730f5 100644 --- a/test/Trait/ParameterTest.js +++ b/test/Trait/ParameterTest.js @@ -259,5 +259,31 @@ require( 'common' ).testCase( this.assertEqual( n, 3 ); }, + + + /** + * Sibling traits are an interesting case---rather than stacking, they + * are mixed in alongside each other, meaning that there may be + * multiple traits that define __mixin. Ordinarily, this is a problem; + * however, __mixin shall be treated as if it were private and shall be + * invoked once per trait, giving each a chance to initialize. + * + * Furthermore, each should retain access to their own configuration. + */ + 'Invokes __mixin of each sibling mixin': function() + { + var args = [], + vals = [ {}, [] ], + c = function() { args.push( arguments ) }; + + var Ta = this.createParamTrait( c ), + Tb = this.createParamTrait( c ); + + this.Class( {} ).use( Ta( vals[0] ), Tb( vals[1] ) )(); + + this.assertEqual( args.length, 2 ); + this.assertStrictEqual( args[0][0], vals[0] ); + this.assertStrictEqual( args[1][0], vals[1] ); + } } ); From 90a32a104ffb88a1e8df7da678a1eb699b5547ab Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 5 Jun 2014 23:35:03 -0400 Subject: [PATCH 4/5] __construct and __mixin ordering guarantees See test cases for rationale. I'm not satisfied with this implementation, but the current state of ease.js makes this kind of thing awkward. Will revisit at some point. --- lib/ClassBuilder.js | 12 ++++-- lib/Trait.js | 28 +++++++++++++ test/Trait/ParameterTest.js | 83 ++++++++++++++++++++++++++++++++++++- 3 files changed, 118 insertions(+), 5 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 18795ec..dedf9bb 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -880,12 +880,11 @@ exports.prototype.createConcreteCtor = function( cname, members ) // generate and store unique instance id attachInstanceId( this, ++_self._instanceId ); - // handle internal trait initialization logic, if provided - if ( typeof this.___$$tctor$$ === 'function' ) + if ( typeof this.___$$ctor$pre$$ === 'function' ) { // FIXME: we're exposing _priv to something that can be - // malicously set by the user; encapsulate tctor - this.___$$tctor$$.call( this, _priv ); + // malicously set by the user + this.___$$ctor$pre$$( _priv ); } // call the constructor, if one was provided @@ -897,6 +896,11 @@ exports.prototype.createConcreteCtor = function( cname, members ) this.__construct.apply( this, ( args || arguments ) ); } + if ( typeof this.___$$ctor$post$$ === 'function' ) + { + this.___$$ctor$post$$( _priv ); + } + args = null; // attach any instance properties/methods (done after diff --git a/lib/Trait.js b/lib/Trait.js index 233feb8..0cca02f 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -23,6 +23,10 @@ var AbstractClass = require( './class_abstract' ), ClassBuilder = require( './ClassBuilder' ), Interface = require( './interface' ); + +function _fvoid() {}; + + /** * Trait constructor / base object * @@ -535,6 +539,24 @@ function mixin( trait, dfn, tc, base ) // retrieve the private member name that will contain this trait object var iname = addTraitInst( trait, dfn, tc, base ); + // TODO: this should not be necessary for dfn metadata + dfn[ 'weak virtual ___$$ctor$pre$$' ] = _fvoid; + dfn[ 'weak virtual ___$$ctor$post$$' ] = _fvoid; + + // TODO: this is a kluge; generalize and move + // this ensures __construct is called before __mixin when mixing into + // the base class + if ( base === ClassBuilder.ClassBase ) + { + dfn[ 'virtual override ___$$ctor$post$$' ] = _tctorApply; + dfn[ 'virtual override ___$$ctor$pre$$' ] = _fvoid; + } + else + { + dfn[ 'virtual override ___$$ctor$post$$' ] = _fvoid; + dfn[ 'virtual override ___$$ctor$pre$$' ] = _tctorApply; + } + // recursively mix in trait's underlying abstract class (ensuring that // anything that the trait inherits from is also properly mixed in) mixinCls( acls, dfn, iname ); @@ -819,5 +841,11 @@ function createTctor( tc, base ) } +function _tctorApply() +{ + this.___$$tctor$$.apply( this, arguments ); +} + + module.exports = Trait; diff --git a/test/Trait/ParameterTest.js b/test/Trait/ParameterTest.js index 31730f5..3fb7e62 100644 --- a/test/Trait/ParameterTest.js +++ b/test/Trait/ParameterTest.js @@ -19,6 +19,9 @@ * along with this program. If not, see . */ +/*** XXX __construct or __mixin first? __mixin with no parameters should + * permit standard trait with initialization procedure ***/ + require( 'common' ).testCase( { caseSetUp: function() @@ -284,6 +287,84 @@ require( 'common' ).testCase( this.assertEqual( args.length, 2 ); this.assertStrictEqual( args[0][0], vals[0] ); this.assertStrictEqual( args[1][0], vals[1] ); - } + }, + + + /** + * This decision is not arbitrary. + * + * We shall consider two different scenarios: first, the case of mixing + * in some trait T atop of some class C. Assume that C defines a + * __construct method; it does not know whether or not a trait will be + * mixed in, nor should it care---it should proceed initializing its + * state as normal. However, what if a trait were to be mixed in, + * overriding certain behaviors? It is then imperative that T be + * initialized prior to any calls by C#__construct. It is not important + * that C be initialized prior to T#__mixin, because T can know that it + * should not invoke any methods that will fail---it should be used only + * to initialize state. (In the future, ease.js may enforce this + * restriction.) + * + * The second scenario is described in the test that follows. + */ + 'Invokes __mixin before __construct when C.use(T)': function() + { + var mixok = false; + + var T = this.createParamTrait( function() { mixok = true } ), + C = this.Class( + { + __construct: function() + { + if ( !mixok ) throw Error( + "__construct called before __mixin" + ); + } + } ); + + this.assertDoesNotThrow( function() + { + C.use( T )(); + } ); + }, + + + /** + * (Continued from above test.) + * + * In the reverse situation---whereby C effectively extends T---we want + * __construct to instead be called *after* __mixin of T (and any other + * traits in the set). This is because __construct may wish to invoke + * methods of T, but what would cause problems if T were not + * initialized. Further, T would not have knowledge of C and, if it + * expected a concrete implementation to be called from T#__mixin, then + * T would have already been initialized, or C's concrete implementation + * would know what not to do (in the case of a partial initialization). + * + * This is also more intuitive---we are invoking initialize methods as + * if they were part of a stack. + */ + 'Invokes __construct before __mixin when Class.use(T).extend()': + function() + { + var cok = false; + + var T = this.createParamTrait( function() + { + if ( !cok ) throw Error( + "__mixin called before __construct" + ); + } ); + + var C = this.Class.use( T ).extend( + { + __construct: function() { cok = true } + } ); + + this.assertDoesNotThrow( function() + { + C(); + } ); + }, } ); From e934338b41518ecb5cb898cec25e0be44b739bdb Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 27 Jul 2014 01:40:35 -0400 Subject: [PATCH 5/5] Subtype ctor guarantees with parent __mixin or __construct A solution for this problem took a disproportionally large amount of time, attempting many different approaches, and arriving still at a kluge; this is indicative of a larger issue---we've long since breached the comfort of the original design, and drastic refactoring is needed. I have ideas for this, and have already started in another branch, but I cannot but this implementation off any longer while waiting for it. Sorry for anyone waiting on the next release: this is what held it up, in combination with my attention being directed elsewhere during that time (see the sparse commit timestamps). Including this ordering guarantee is very important for a stable, well-designed [trait] system. --- lib/ClassBuilder.js | 14 +++++++++++++- test/Trait/ParameterTest.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index dedf9bb..06bac0b 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -880,11 +880,17 @@ exports.prototype.createConcreteCtor = function( cname, members ) // generate and store unique instance id attachInstanceId( this, ++_self._instanceId ); - if ( typeof this.___$$ctor$pre$$ === 'function' ) + // FIXME: this is a bit of a kluge for determining whether the ctor + // should be invoked before a child prector... + var haspre = ( typeof this.___$$ctor$pre$$ === 'function' ); + if ( haspre + && ClassInstance.prototype.hasOwnProperty( '___$$ctor$pre$$' ) + ) { // FIXME: we're exposing _priv to something that can be // malicously set by the user this.___$$ctor$pre$$( _priv ); + haspre = false; } // call the constructor, if one was provided @@ -896,6 +902,12 @@ exports.prototype.createConcreteCtor = function( cname, members ) this.__construct.apply( this, ( args || arguments ) ); } + // FIXME: see above + if ( haspre ) + { + this.___$$ctor$pre$$( _priv ); + } + if ( typeof this.___$$ctor$post$$ === 'function' ) { this.___$$ctor$post$$( _priv ); diff --git a/test/Trait/ParameterTest.js b/test/Trait/ParameterTest.js index 3fb7e62..077131a 100644 --- a/test/Trait/ParameterTest.js +++ b/test/Trait/ParameterTest.js @@ -366,5 +366,34 @@ require( 'common' ).testCase( C(); } ); }, + + + /** + * The same concept as above, extended to subtypes. In particular, we + * need to ensure that the subtype is able to properly initialize or + * alter state that __mixin of a supertype depends upon. + */ + 'Subtype invokes ctor before supertype __construct or __mixin': + function() + { + var cok = false; + + var T = this.createParamTrait( function() + { + if ( !cok ) throw Error( + "__mixin called before Sub#__construct" + ); + } ); + + var Sub = this.Class( {} ).use( T ).extend( + { + __construct: function() { cok = true } + } ); + + this.assertDoesNotThrow( function() + { + Sub(); + } ); + }, } );