From 62035a0b4c37a47b429b35e837a646ebd44a2e33 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 21 Jan 2014 22:57:04 -0500 Subject: [PATCH 01/52] Beginning of Trait and Class.use This is a rough concept showing how traits will be used at definition time by classes (note that this does not yet address how they will be ``mixed in'' at the time of instantiation). --- lib/Trait.js | 76 +++++++++++++++++ lib/class.js | 33 ++++++++ test/Trait/DefinitionTest.js | 156 +++++++++++++++++++++++++++++++++++ 3 files changed, 265 insertions(+) create mode 100644 lib/Trait.js create mode 100644 test/Trait/DefinitionTest.js diff --git a/lib/Trait.js b/lib/Trait.js new file mode 100644 index 0000000..ee9f088 --- /dev/null +++ b/lib/Trait.js @@ -0,0 +1,76 @@ +/** + * Provides system for code reuse via traits + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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 . + */ + + +function Trait() +{ + switch ( arguments.length ) + { + case 1: + return Trait.extend.apply( this, arguments ); + break; + } +}; + + +Trait.extend = function( dfn ) +{ + function TraitType() + { + throw Error( "Cannot instantiate trait" ); + }; + + TraitType.__trait = true; + TraitType.__dfn = dfn; + + return TraitType; +}; + + +Trait.isTrait = function( trait ) +{ + return !!( trait || {} ).__trait; +}; + + +/** + * Mix trait into the given definition + * + * The original object DFN is modified; it is not cloned. + * + * @param {Trait} trait trait to mix in + * @param {Object} dfn definition object to merge into + * + * @return {Object} dfn + */ +Trait.mixin = function( trait, dfn ) +{ + var tdfn = trait.__dfn || {}; + for ( var f in tdfn ) + { + dfn[ f ] = tdfn[ f ]; + } + + return dfn; +}; + + +module.exports = Trait; diff --git a/lib/class.js b/lib/class.js index 8e1bca4..1d676ef 100644 --- a/lib/class.js +++ b/lib/class.js @@ -28,6 +28,8 @@ var util = require( __dirname + '/util' ), MethodWrapperFactory = require( __dirname + '/MethodWrapperFactory' ), wrappers = require( __dirname + '/MethodWrappers' ).standard, + Trait = require( __dirname + '/Trait' ), + class_builder = ClassBuilder( require( __dirname + '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), @@ -120,6 +122,16 @@ module.exports.implement = function( interfaces ) }; +module.exports.use = function( traits ) +{ + // consume traits onto an empty base + return createUse( + null, + Array.prototype.slice.call( arguments ) + ); +}; + + /** * Determines whether the provided object is a class created through ease.js * @@ -359,6 +371,27 @@ function createImplement( base, ifaces, cname ) } +function createUse( base, traits ) +{ + return { + extend: function() + { + var args = Array.prototype.slice.call( arguments ), + dfn = args.pop(), + base = args.pop(); + + // "mix" each trait into the provided definition object + for ( var i = 0, n = traits.length; i < n; i++ ) + { + Trait.mixin( traits[ i ], dfn ); + } + + return extend.call( null, base, dfn ); + }, + }; +} + + /** * Mimics class inheritance * diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js new file mode 100644 index 0000000..1fb992e --- /dev/null +++ b/test/Trait/DefinitionTest.js @@ -0,0 +1,156 @@ +/** + * Tests basic trait definition + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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' ); + + // means of creating anonymous traits + this.ctor = [ + this.Sut.extend, + this.Sut, + ]; + }, + + + /** + * We continue with the same concept used for class + * definitions---extending the Trait module itself will create an + * anonymous trait. + */ + '@each(ctor) Can extend Trait to create anonymous trait': function( T ) + { + this.assertOk( this.Sut.isTrait( T( {} ) ) ); + }, + + + /** + * A trait can only be used by something else---it does not make sense + * to instantiate them directly, since they form an incomplete picture. + */ + '@each(ctor) Cannot instantiate trait without error': function( T ) + { + this.assertThrows( function() + { + T( {} )(); + }, Error ); + }, + + + /** + * One way that traits acquire meaning is by their use in creating + * classes. This also allows us to observe whether traits are actually + * working as intended without testing too closely to their + * implementation. This test simply ensures that the Class module will + * accept our traits. + * + * Classes consume traits as part of their definition using the `use' + * method. We should be able to then invoke the `extend' method to + * provide our own definition, without having to inherit from another + * class. + */ + '@each(ctor) Base class definition is applied when using traits': + function( T ) + { + var expected = 'bar'; + + var C = this.Class.use( T( {} ) ).extend( + { + foo: expected, + } ); + + this.assertOk( this.Class.isClass( C ) ); + this.assertEqual( C().foo, expected ); + }, + + + /** + * Traits contribute to the definition of the class that `use's them; + * therefore, it would stand to reason that we should still be able to + * inherit from a supertype while using traits. + */ + '@each(ctor) Supertype definition is applied when using traits': + function( T ) + { + var expected = 'bar'; + expected2 = 'baz'; + Foo = this.Class( { foo: expected } ), + SubFoo = this.Class.use( T( {} ) ) + .extend( Foo, { bar: expected2 } ); + + var inst = SubFoo(); + + this.assertOk( this.Class.isA( Foo, inst ) ); + this.assertEqual( inst.foo, expected, "Supertype failure" ); + this.assertEqual( inst.bar, expected2, "Subtype failure" ); + }, + + + /** + * The above tests have ensured that classes are still operable with + * traits; we can now test that traits are mixed into the class + * definition via `use' by asserting on the trait definitions. + */ + '@each(ctor) Trait definition is mixed into base class definition': + function( T ) + { + var called = false; + + var Trait = T( { foo: function() { called = true; } } ), + inst = this.Class.use( Trait ).extend( {} )(); + + // if mixin was successful, then we should have the `foo' method. + this.assertDoesNotThrow( function() + { + inst.foo(); + }, Error, "Should have access to mixed in fields" ); + + // if our variable was not set, then it was a bs copy + this.assertOk( called, "Mixed in field copy error" ); + }, + + + /** + * The above test should apply just the same to subtypes. + */ + '@each(ctor) Trait definition is mixed into subtype definition': + function( T ) + { + var called = false; + + var Trait = T( { foo: function() { called = true; } } ), + Foo = this.Class( {} ), + inst = this.Class.use( Trait ).extend( Foo, {} )(); + + inst.foo(); + this.assertOk( called ); + }, + + + // + // At this point, we assume that each ctor method is working as expected + // (that is---the same); we will proceed to test only a single method of + // construction under that assumption. + // +} ); From dfc83032d76e3bf3a8e09db3e03e7ba10838aa3c Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 22 Jan 2014 01:11:17 -0500 Subject: [PATCH 02/52] Basic, incomplete, but workable traits Note the incomplete tests. These are very important, but the current state at least demonstrates conceptually how this will work (and is even useful in its current state, albeit dangerous and far from ready for production). --- lib/Trait.js | 17 ++++ test/Trait/DefinitionTest.js | 152 +++++++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) diff --git a/lib/Trait.js b/lib/Trait.js index ee9f088..292065f 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -56,6 +56,9 @@ Trait.isTrait = function( trait ) * * The original object DFN is modified; it is not cloned. * + * TODO: we could benefit from processing the keywords now (since we need + * the name anyway) and not re-processing them later for the class. + * * @param {Trait} trait trait to mix in * @param {Object} dfn definition object to merge into * @@ -66,6 +69,20 @@ Trait.mixin = function( trait, dfn ) var tdfn = trait.__dfn || {}; for ( var f in tdfn ) { + // this is a simple check that will match only when all keywords, + // etc are the same; we expect that---at least for the time + // being---class validations will ensures that redefinitions do not + // occur when the field strings vary + if ( dfn[ f ] ) + { + // TODO: conflcit resolution + throw Error( "Trait field `" + f + "' conflits" ); + } + else if ( f.match( /\b__construct\b/ ) ) + { + throw Error( "Traits may not define __construct" ); + } + dfn[ f ] = tdfn[ f ]; } diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index 1fb992e..bc9facf 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -31,6 +31,38 @@ require( 'common' ).testCase( this.Sut.extend, this.Sut, ]; + + // trait field name conflicts + this.fconflict = [ + // same name; property + [ 'foo', + { foo: 'a' }, + { foo: 'b' }, + ], + + // same name; prop and method + [ 'foo', + { foo: 'a' }, + { foo: function() {} }, + ], + + // same keywords + [ 'foo', + { 'public foo': 'a' }, + { 'public foo': 'b' }, + ], + + // different keywords should be (for the time being) picked up + // by existing class error checks + [ 'foo', + { 'public foo': 'a' }, + { 'protected foo': 'b' }, + ], + [ 'bar', + { 'virtual bar': function() {} }, + { 'override bar': function() {} }, + ], + ]; }, @@ -153,4 +185,124 @@ require( 'common' ).testCase( // (that is---the same); we will proceed to test only a single method of // construction under that assumption. // + + + /** + * Traits cannot be instantiated, so they need not define __construct + * for themselves; however, they may wish to influence the construction + * of anything that uses them. This is poor practice, since that + * introduces a war between traits to take over the constructor; + * instead, the class using the traits should handle calling the methods + * on the traits and we should disallow traits from attempting to set + * the constructor. + */ + 'Traits cannot define __construct': function() + { + try + { + this.Class + .use( this.Sut( { __construct: function() {} } ) ) + .extend( {} ); + } + catch ( e ) + { + this.assertOk( e.message.match( /\b__construct\b/ ) ); + return; + } + + this.fail( "Traits should not be able to define __construct" ); + }, + + + /** + * If two traits attempt to define the same field (by name, regardless + * of its type), then an error should be thrown to warn the developer of + * a problem; automatic resolution would be a fertile source of nasty + * and confusing bugs. + * + * TODO: conflict resolution through aliasing + */ + '@each(fconflict) Cannot use multiple traits definining same field': + function( dfns ) + { + var fname = dfns[ 0 ]; + + // both traits define `foo' + var A = this.Sut( dfns[ 1 ] ), + B = this.Sut( dfns[ 2 ] ); + + // this, therefore, should error + try + { + this.Class.use( A, B ).extend( {} ); + } + catch ( e ) + { + // the assertion should contain the name of the field that + // caused the error + this.assertOk( + e.message.match( '\\b' + fname + '\\b' ), + "Missing field name" + ); + + // TODO: we can also make less people hate us if we include the + // names of the conflicting traits; in the case of an anonymous + // trait, maybe include its index in the use list + + return; + } + + this.fail( "Mixin must fail on conflict" ); + }, + + + /** + * Once a trait is mixed in, its methods should execute with `this' + * bound to the instance of the class that it was mixed into, not the + * trait itself. In particular, this means that the trait can access + * members of the class in which it mixes into (but see tests that + * follow). + */ + 'Trait methods execute within context of the containing class': + function() + { + var expected = 'bar'; + + var T = this.Sut( + { + // attempts to invoke protected method of containing class + 'public setFoo': function( val ) { this.doSet( val ); }, + } ); + + var C = this.Class.use( T ).extend( + { + 'private _foo': null, + 'protected doSet': function( val ) { this._foo = val; }, + 'public getFoo': function() { return this._foo; }, + } ); + + // we do not use method chaining for this test just to ensure that + // any hiccups with returning `this' from setFoo will not compromise + // the assertion + var inst = C(); + inst.setFoo( expected ); + this.assertEqual( inst.getFoo(), expected ); + }, + + + 'Private class members are not accessible to useed traits': function() + { + // TODO: this is not yet the case + }, + + + /** + * Traits will need to be able to keep and manipulate their own internal + * state. + */ + 'Private trait members are not accessible to containing class': + function() + { + // TODO: this is not yet the case + }, } ); From 71358eab5940564ea3009ae7df95e411277bc4f2 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 23 Jan 2014 00:34:15 -0500 Subject: [PATCH 03/52] Began implementing composition-based traits As described in . The benefit of this approach over definition object merging is primarily simplicitly---we're re-using much of the existing system. We may provide more tight integration eventually for performance reasons (this is a proof-of-concept), but this is an interesting start. This also allows us to study and reason about traits by building off of existing knowledge of composition; the documentation will make mention of this to explain design considerations and issues of tight coupling introduced by mixing in of traits. --- lib/ClassBuilder.js | 9 +- lib/Trait.js | 154 ++++++++++++++++++++++++++++++----- lib/class.js | 4 +- test/Trait/DefinitionTest.js | 47 ++--------- 4 files changed, 151 insertions(+), 63 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 8b4db46..8947f11 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -659,6 +659,12 @@ 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' ) + { + this.___$$tctor$$.call( this ); + } + // call the constructor, if one was provided if ( typeof this.__construct === 'function' ) { @@ -666,9 +672,10 @@ exports.prototype.createConcreteCtor = function( cname, members ) // subtypes), and since we're using apply with 'this', the // constructor will be applied to subtypes without a problem this.__construct.apply( this, ( args || arguments ) ); - args = null; } + args = null; + // attach any instance properties/methods (done after // constructor to ensure they are not overridden) attachInstanceOf( this ); diff --git a/lib/Trait.js b/lib/Trait.js index 292065f..29604d3 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -19,6 +19,8 @@ * along with this program. If not, see . */ +var AbstractClass = require( __dirname + '/class_abstract' ); + function Trait() { @@ -33,13 +35,34 @@ function Trait() Trait.extend = function( dfn ) { + // we need at least one abstract member in order to declare a class as + // abstract (in this case, our trait class), so let's create a dummy one + // just in case DFN does not contain any abstract members itself + dfn[ 'abstract protected __$$trait$$' ] = []; + function TraitType() { throw Error( "Cannot instantiate trait" ); }; + // and here we can see that traits are quite literally abstract classes + var tclass = AbstractClass( dfn ); + TraitType.__trait = true; - TraitType.__dfn = dfn; + TraitType.__acls = tclass; + TraitType.__ccls = createConcrete( tclass ); + + // traits are not permitted to define constructors + if ( tclass.___$$methods$$['public'].__construct !== undefined ) + { + throw Error( "Traits may not define __construct" ); + } + + // invoked to trigger mixin + TraitType.__mixin = function( dfn ) + { + mixin( TraitType, dfn ); + }; return TraitType; }; @@ -51,42 +74,135 @@ Trait.isTrait = function( trait ) }; +/** + * Create a concrete class from the abstract trait class + * + * This class is the one that will be instantiated by classes that mix in + * the trait. + * + * @param {AbstractClass} acls abstract trait class + * + * @return {Class} concrete trait class for instantiation + */ +function createConcrete( acls ) +{ + // start by providing a concrete implementation for our dummy method + var dfn = { + 'protected __$$trait$$': function() {}, + }; + + // TODO: everything else + + return acls.extend( dfn ); +} + + /** * Mix trait into the given definition * * The original object DFN is modified; it is not cloned. * - * TODO: we could benefit from processing the keywords now (since we need - * the name anyway) and not re-processing them later for the class. - * * @param {Trait} trait trait to mix in * @param {Object} dfn definition object to merge into * * @return {Object} dfn */ -Trait.mixin = function( trait, dfn ) +function mixin( trait, dfn ) { - var tdfn = trait.__dfn || {}; - for ( var f in tdfn ) + // the abstract class hidden within the trait + var acls = trait.__acls, + methods = acls.___$$methods$$, + pub = methods['public']; + + // retrieve the private member name that will contain this trait object + var iname = addTraitInst( trait.__ccls, dfn ); + + for ( var f in pub ) { - // this is a simple check that will match only when all keywords, - // etc are the same; we expect that---at least for the time - // being---class validations will ensures that redefinitions do not - // occur when the field strings vary - if ( dfn[ f ] ) + if ( !( Object.hasOwnProperty.call( pub, f ) ) ) { - // TODO: conflcit resolution - throw Error( "Trait field `" + f + "' conflits" ); - } - else if ( f.match( /\b__construct\b/ ) ) - { - throw Error( "Traits may not define __construct" ); + continue; } - dfn[ f ] = tdfn[ f ]; + // TODO: this is a kluge; we'll use proper reflection eventually, + // but for now, this is how we determine if this is an actual public + // method vs. something that just happens to be on the public + // visibility object + if ( !( pub[ f ].___$$keywords$$ ) ) + { + continue; + } + + // proxy this method to what will be the encapsulated trait object + dfn[ 'public proxy ' + f ] = iname; } return dfn; +} + + +/** + * Add concrete trait class to a class instantion list + * + * This list---which will be created if it does not already exist---will be + * used upon instantiation of the class consuming DFN to instantiate the + * concrete trait classes. + * + * Here, `tc' and `to' are understood to be, respectively, ``trait class'' + * and ``trait object''. + * + * @param {Class} C concrete trait class + * @param {Object} dfn definition object of class being mixed into + * + * @return {string} private member into which C instance shall be stored + */ +function addTraitInst( C, dfn ) +{ + var tc = ( dfn.___$$tc$$ = ( dfn.___$$tc$$ || [] ) ), + iname = '___$to$' + tc.length; + + // the trait object array will contain two values: the destination field + // and the class to instantiate + tc.push( [ iname, C ] ); + + // we must also add the private field to the definition object to + // support the object assignment indicated by TC + dfn[ 'private ' + iname ] = null; + + // create internal trait ctor if not available + if ( dfn.___$$tctor$$ === undefined ) + { + dfn.___$$tctor$$ = tctor; + } + + return iname; +} + + +/** + * Trait initialization constructor + * + * May be used to initialize all traits mixed into the class that invokes + * this function. All concrete trait classes are instantiated and their + * resulting objects assigned to their rsepective pre-determined field + * names. + * + * @return {undefined} + */ +function tctor() +{ + // instantiate all traits and assign the object to their + // respective fields + var tc = this.___$$tc$$; + for ( var t in tc ) + { + var f = tc[ t ][ 0 ], + C = tc[ t ][ 1 ]; + + // TODO: pass protected visibility object once we create + // trait class ctors + this[ f ] = C(); + } }; diff --git a/lib/class.js b/lib/class.js index 1d676ef..0a0cdfa 100644 --- a/lib/class.js +++ b/lib/class.js @@ -28,8 +28,6 @@ var util = require( __dirname + '/util' ), MethodWrapperFactory = require( __dirname + '/MethodWrapperFactory' ), wrappers = require( __dirname + '/MethodWrappers' ).standard, - Trait = require( __dirname + '/Trait' ), - class_builder = ClassBuilder( require( __dirname + '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), @@ -383,7 +381,7 @@ function createUse( base, traits ) // "mix" each trait into the provided definition object for ( var i = 0, n = traits.length; i < n; i++ ) { - Trait.mixin( traits[ i ], dfn ); + traits[ i ].__mixin( dfn ); } return extend.call( null, base, dfn ); diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index bc9facf..8c1dcc9 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -60,7 +60,7 @@ require( 'common' ).testCase( ], [ 'bar', { 'virtual bar': function() {} }, - { 'override bar': function() {} }, + { 'public bar': function() {} }, ], ]; }, @@ -200,9 +200,7 @@ require( 'common' ).testCase( { try { - this.Class - .use( this.Sut( { __construct: function() {} } ) ) - .extend( {} ); + this.Sut( { __construct: function() {} } ); } catch ( e ) { @@ -222,9 +220,12 @@ require( 'common' ).testCase( * * TODO: conflict resolution through aliasing */ - '@each(fconflict) Cannot use multiple traits definining same field': + '@each(fconflict) Cannot mix in multiple concrete methods of same name': function( dfns ) { + // TODO: not yet working with composition approach + this.skip(); + var fname = dfns[ 0 ]; // both traits define `foo' @@ -256,41 +257,7 @@ require( 'common' ).testCase( }, - /** - * Once a trait is mixed in, its methods should execute with `this' - * bound to the instance of the class that it was mixed into, not the - * trait itself. In particular, this means that the trait can access - * members of the class in which it mixes into (but see tests that - * follow). - */ - 'Trait methods execute within context of the containing class': - function() - { - var expected = 'bar'; - - var T = this.Sut( - { - // attempts to invoke protected method of containing class - 'public setFoo': function( val ) { this.doSet( val ); }, - } ); - - var C = this.Class.use( T ).extend( - { - 'private _foo': null, - 'protected doSet': function( val ) { this._foo = val; }, - 'public getFoo': function() { return this._foo; }, - } ); - - // we do not use method chaining for this test just to ensure that - // any hiccups with returning `this' from setFoo will not compromise - // the assertion - var inst = C(); - inst.setFoo( expected ); - this.assertEqual( inst.getFoo(), expected ); - }, - - - 'Private class members are not accessible to useed traits': function() + 'Private class members are not accessible to used traits': function() { // TODO: this is not yet the case }, From 3724b1bc0de5c72e678492c5fdbe557673021702 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 23 Jan 2014 01:00:16 -0500 Subject: [PATCH 04/52] Re-implemented mixin error for member name conflicts --- lib/Trait.js | 13 ++++++++- test/Trait/DefinitionTest.js | 53 +++++++++++++++--------------------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/lib/Trait.js b/lib/Trait.js index 29604d3..aa60424 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -117,6 +117,7 @@ function mixin( trait, dfn ) // retrieve the private member name that will contain this trait object var iname = addTraitInst( trait.__ccls, dfn ); + // TODO: protected; ignore abstract for ( var f in pub ) { if ( !( Object.hasOwnProperty.call( pub, f ) ) ) @@ -133,8 +134,18 @@ function mixin( trait, dfn ) continue; } + var pname = 'public proxy ' + f; + + // if we have already set up a proxy for a field of this name, then + // multiple traits have defined the same concrete member + if ( dfn[ pname ] !== undefined ) + { + // TODO: between what traits? + throw Error( "Trait member conflict: `" + f + "'" ); + } + // proxy this method to what will be the encapsulated trait object - dfn[ 'public proxy ' + f ] = iname; + dfn[ pname ] = iname; } return dfn; diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index 8c1dcc9..40ba6f0 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -32,36 +32,31 @@ require( 'common' ).testCase( this.Sut, ]; - // trait field name conflicts + // trait field name conflicts (methods) this.fconflict = [ - // same name; property - [ 'foo', - { foo: 'a' }, - { foo: 'b' }, - ], - - // same name; prop and method - [ 'foo', - { foo: 'a' }, + [ 'foo', "same name; no keywords", + { foo: function() {} }, { foo: function() {} }, ], - // same keywords - [ 'foo', - { 'public foo': 'a' }, - { 'public foo': 'b' }, + [ 'foo', "same keywords; same visibility", + { 'public foo': function() {} }, + { 'public foo': function() {} }, ], - // different keywords should be (for the time being) picked up - // by existing class error checks - [ 'foo', - { 'public foo': 'a' }, - { 'protected foo': 'b' }, + // should (at least for the time being) be picked up by existing + // class error checks + [ 'foo', "varying keywords; same visibility", + { 'virtual public foo': function() {} }, + { 'public virtual foo': function() {} }, ], - [ 'bar', - { 'virtual bar': function() {} }, - { 'public bar': function() {} }, + + /* TODO + [ 'foo', "different visibility", + { 'public foo': function() {} }, + { 'protected foo': function() {} }, ], + */ ]; }, @@ -223,14 +218,10 @@ require( 'common' ).testCase( '@each(fconflict) Cannot mix in multiple concrete methods of same name': function( dfns ) { - // TODO: not yet working with composition approach - this.skip(); - - var fname = dfns[ 0 ]; - - // both traits define `foo' - var A = this.Sut( dfns[ 1 ] ), - B = this.Sut( dfns[ 2 ] ); + var fname = dfns[ 0 ], + desc = dfns[ 1 ], + A = this.Sut( dfns[ 2 ] ), + B = this.Sut( dfns[ 3 ] ); // this, therefore, should error try @@ -253,7 +244,7 @@ require( 'common' ).testCase( return; } - this.fail( "Mixin must fail on conflict" ); + this.fail( false, true, "Mixin must fail on conflict: " + desc ); }, From e44ac3190b67fe5dd8992dd739e487d9295f60f2 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 23 Jan 2014 01:15:53 -0500 Subject: [PATCH 05/52] Protected trait methods are now mixed in --- lib/Trait.js | 47 ++++++++++++++++++++++++------------ test/Trait/DefinitionTest.js | 7 +++--- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/lib/Trait.js b/lib/Trait.js index aa60424..209decf 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -38,7 +38,7 @@ Trait.extend = function( dfn ) // we need at least one abstract member in order to declare a class as // abstract (in this case, our trait class), so let's create a dummy one // just in case DFN does not contain any abstract members itself - dfn[ 'abstract protected __$$trait$$' ] = []; + dfn[ 'abstract protected ___$$trait$$' ] = []; function TraitType() { @@ -88,7 +88,7 @@ function createConcrete( acls ) { // start by providing a concrete implementation for our dummy method var dfn = { - 'protected __$$trait$$': function() {}, + 'protected ___$$trait$$': function() {}, }; // TODO: everything else @@ -111,44 +111,59 @@ function mixin( trait, dfn ) { // the abstract class hidden within the trait var acls = trait.__acls, - methods = acls.___$$methods$$, - pub = methods['public']; + methods = acls.___$$methods$$; // retrieve the private member name that will contain this trait object var iname = addTraitInst( trait.__ccls, dfn ); - // TODO: protected; ignore abstract - for ( var f in pub ) + mixMethods( methods['public'], dfn, 'public', iname ); + mixMethods( methods['protected'], dfn, 'protected', iname ); + + return dfn; +} + + +/** + * Mix methods from SRC into DEST using proxies + * + * @param {Object} src visibility object to scavenge from + * @param {Object} dest destination definition object + * @param {string} vis visibility modifier + * @param {string} ianem proxy destination (trait instance) + * + * @return {undefined} + */ +function mixMethods( src, dest, vis, iname ) +{ + // TODO: ignore abstract + for ( var f in src ) { - if ( !( Object.hasOwnProperty.call( pub, f ) ) ) + if ( !( Object.hasOwnProperty.call( src, f ) ) ) { 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 public - // method vs. something that just happens to be on the public - // visibility object - if ( !( pub[ f ].___$$keywords$$ ) ) + // 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 + if ( !( src[ f ].___$$keywords$$ ) || f === '___$$trait$$' ) { continue; } - var pname = 'public proxy ' + f; + var pname = vis + ' proxy ' + f; // if we have already set up a proxy for a field of this name, then // multiple traits have defined the same concrete member - if ( dfn[ pname ] !== undefined ) + if ( dest[ pname ] !== undefined ) { // TODO: between what traits? throw Error( "Trait member conflict: `" + f + "'" ); } // proxy this method to what will be the encapsulated trait object - dfn[ pname ] = iname; + dest[ pname ] = iname; } - - return dfn; } diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index 40ba6f0..ad2897d 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -45,18 +45,17 @@ require( 'common' ).testCase( ], // should (at least for the time being) be picked up by existing - // class error checks + // class error checks; TODO: but let's provide trait-specific + // error messages to avoid frustration and infuriation [ 'foo', "varying keywords; same visibility", { 'virtual public foo': function() {} }, { 'public virtual foo': function() {} }, ], - /* TODO [ 'foo', "different visibility", { 'public foo': function() {} }, { 'protected foo': function() {} }, ], - */ ]; }, @@ -234,7 +233,7 @@ require( 'common' ).testCase( // caused the error this.assertOk( e.message.match( '\\b' + fname + '\\b' ), - "Missing field name" + "Error message missing field name: " + e.message ); // TODO: we can also make less people hate us if we include the From 93eda3c14bdf830fcef44fe71af4a2a43251d258 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 23 Jan 2014 01:52:15 -0500 Subject: [PATCH 06/52] Added tests proving traits' scopes are disjoint from classes' and each others' This was the original motiviation behind, and is a beautiful consequence of, the composition-based trait implementation (see ). --- test/Trait/DefinitionTest.js | 17 ----- test/Trait/ScopeTest.js | 129 +++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 17 deletions(-) create mode 100644 test/Trait/ScopeTest.js diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index ad2897d..cbaff31 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -245,21 +245,4 @@ require( 'common' ).testCase( this.fail( false, true, "Mixin must fail on conflict: " + desc ); }, - - - 'Private class members are not accessible to used traits': function() - { - // TODO: this is not yet the case - }, - - - /** - * Traits will need to be able to keep and manipulate their own internal - * state. - */ - 'Private trait members are not accessible to containing class': - function() - { - // TODO: this is not yet the case - }, } ); diff --git a/test/Trait/ScopeTest.js b/test/Trait/ScopeTest.js new file mode 100644 index 0000000..b39ada2 --- /dev/null +++ b/test/Trait/ScopeTest.js @@ -0,0 +1,129 @@ +/** + * Tests trait scoping + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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' ); + }, + + + /** + * Since the private scope of classes and the traits that they use are + * disjoint, traits should never be able to access any private member of + * a class that uses it. + * + * The beauty of this is that we get this ``feature'' for free with + * our composition-based trait implementation. + */ + 'Private class members are not accessible to used traits': function() + { + var T = this.Sut( + { + // attempts to access C._priv + 'public getPriv': function() { return this._priv; }, + + // attempts to invoke C._privMethod + 'public invokePriv': function() { this._privMethod(); }, + } ); + + var inst = this.Class.use( T ).extend( + { + 'private _priv': 'foo', + 'private _privMethod': function() {}, + } )(); + + this.assertEqual( inst.getPriv(), undefined ); + this.assertThrows( function() + { + inst.invokePriv(); + }, Error ); + }, + + + /** + * Similar concept to the above---class and trait scopes are disjoint. + * This is particularily important, since traits will have no idea what + * other traits they will be mixed in with and therefore must be immune + * from nasty state clashes. + */ + 'Private trait members are not accessible to containing class': + function() + { + var T = this.Sut( + { + 'private _priv': 'bar', + 'private _privMethod': function() {}, + } ); + + // reverse of the previous test case + var inst = this.Class.use( T ).extend( + { + // attempts to access T._priv + 'public getPriv': function() { return this._priv; }, + + // attempts to invoke T._privMethod + 'public invokePriv': function() { this._privMethod(); }, + } )(); + + + this.assertEqual( inst.getPriv(), undefined ); + this.assertThrows( function() + { + inst.invokePriv(); + }, Error ); + }, + + + /** + * Since all scopes are disjoint, it would stand to reason that all + * traits should also have their own private scope independent of other + * traits that are mixed into the same class. This is also very + * important for the same reasons as the previous test---we cannot have + * state clashes between traits. + */ + 'Traits do not have access to each others\' private members': function() + { + var T1 = this.Sut( + { + 'private _priv1': 'foo', + 'private _privMethod1': function() {}, + } ), + T2 = this.Sut( + { + // attempts to access T1._priv1 + 'public getPriv': function() { return this._priv1; }, + + // attempts to invoke T1._privMethod1 + 'public invokePriv': function() { this._privMethod1(); }, + } ); + + var inst = this.Class.use( T1, T2 ).extend( {} )(); + + this.assertEqual( inst.getPriv(), undefined ); + this.assertThrows( function() + { + inst.invokePriv(); + }, Error ); + }, +} ); From 00c76c69dfc63c01afdede9d383974c88cc782de Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 23 Jan 2014 22:24:27 -0500 Subject: [PATCH 07/52] Trait concrete class will now be lazily created on first use This just saves some time and memory in the event that the trait is never actually used, which may be the case in libraries or dynamically loaded modules. --- lib/Trait.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/Trait.js b/lib/Trait.js index 209decf..cff6c07 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -50,7 +50,7 @@ Trait.extend = function( dfn ) TraitType.__trait = true; TraitType.__acls = tclass; - TraitType.__ccls = createConcrete( tclass ); + TraitType.__ccls = null; // traits are not permitted to define constructors if ( tclass.___$$methods$$['public'].__construct !== undefined ) @@ -114,7 +114,7 @@ function mixin( trait, dfn ) methods = acls.___$$methods$$; // retrieve the private member name that will contain this trait object - var iname = addTraitInst( trait.__ccls, dfn ); + var iname = addTraitInst( trait, dfn ); mixMethods( methods['public'], dfn, 'public', iname ); mixMethods( methods['protected'], dfn, 'protected', iname ); @@ -177,19 +177,19 @@ function mixMethods( src, dest, vis, iname ) * Here, `tc' and `to' are understood to be, respectively, ``trait class'' * and ``trait object''. * - * @param {Class} C concrete trait class + * @param {Class} T trait * @param {Object} dfn definition object of class being mixed into * * @return {string} private member into which C instance shall be stored */ -function addTraitInst( C, dfn ) +function addTraitInst( T, dfn ) { var tc = ( dfn.___$$tc$$ = ( dfn.___$$tc$$ || [] ) ), iname = '___$to$' + tc.length; // the trait object array will contain two values: the destination field - // and the class to instantiate - tc.push( [ iname, C ] ); + // and the trait to instantiate + tc.push( [ iname, T ] ); // we must also add the private field to the definition object to // support the object assignment indicated by TC @@ -213,6 +213,9 @@ function addTraitInst( C, dfn ) * resulting objects assigned to their rsepective pre-determined field * names. * + * This will lazily create the concrete trait class if it does not already + * exist, which saves work if the trait is never used. + * * @return {undefined} */ function tctor() @@ -223,7 +226,8 @@ function tctor() for ( var t in tc ) { var f = tc[ t ][ 0 ], - C = tc[ t ][ 1 ]; + T = tc[ t ][ 1 ], + C = T.__ccls || ( T.__ccls = createConcrete( T.__acls ) ); // TODO: pass protected visibility object once we create // trait class ctors From 18ac37c87110f9e039a3b74f94d8379347c0bc8e Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 23 Jan 2014 23:43:34 -0500 Subject: [PATCH 08/52] Added support for `weak' keyword Note that, even though it's permitted, the validator still needs to be modified to permit useful cases. In particular, I need weak abstract and strong concrete methods for use in traits. --- lib/ClassBuilder.js | 11 ++-- lib/prop_parser.js | 1 + test/ClassBuilder/MemberRestrictionTest.js | 65 +++++++++++++++++++++- test/PropParserKeywordsTest.js | 2 +- 4 files changed, 72 insertions(+), 7 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 8947f11..c736808 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -338,7 +338,7 @@ exports.prototype.build = function extend( _, __ ) // increment class identifier this._classId++; - // build the various class components (xxx: this is temporary; needs + // build the various class components (XXX: this is temporary; needs // refactoring) try { @@ -464,8 +464,11 @@ exports.prototype.buildMembers = function buildMembers( } // if a member was defined multiple times in the same class - // declaration, throw an error - if ( hasOwn.call( defs, name ) ) + // declaration, throw an error (unless the `weak' keyword is + // provided, which exists to accomodate this situation) + if ( hasOwn.call( defs, name ) + && !( keywords['weak'] || defs[ name ].weak ) + ) { throw Error( "Cannot redefine method '" + name + "' in same declaration" @@ -474,7 +477,7 @@ exports.prototype.buildMembers = function buildMembers( // keep track of the definitions (only during class declaration) // to catch duplicates - defs[ name ] = 1; + defs[ name ] = keywords; }, property: function( name, value, keywords ) diff --git a/lib/prop_parser.js b/lib/prop_parser.js index 3c36b0a..c3f3bbb 100644 --- a/lib/prop_parser.js +++ b/lib/prop_parser.js @@ -33,6 +33,7 @@ var _keywords = { 'virtual': true, 'override': true, 'proxy': true, + 'weak': true, }; diff --git a/test/ClassBuilder/MemberRestrictionTest.js b/test/ClassBuilder/MemberRestrictionTest.js index 7643249..5cf7020 100644 --- a/test/ClassBuilder/MemberRestrictionTest.js +++ b/test/ClassBuilder/MemberRestrictionTest.js @@ -23,8 +23,18 @@ require( 'common' ).testCase( { caseSetUp: function() { - this.Class = this.require( 'class' ); - this.Sut = this.require( 'ClassBuilder' ); + // XXX: the Sut is not directly tested; get rid of these! + this.Class = this.require( 'class' ); + this.AbstractClass = this.require( 'class_abstract' ); + + this.Sut = this.require( 'ClassBuilder' ); + + // weak flag test data + this.weak = [ + [ 'weak foo', 'foo' ], // former weak + [ 'foo', 'weak foo' ], // latter weak + [ 'weak foo', 'weak foo' ], // both weak + ]; }, @@ -227,4 +237,55 @@ require( 'common' ).testCase( }, Error, "Forced-public methods must be declared as public" ); } }, + + + /** + * If different keywords are used, then a definition object could + * contain two members of the same name. This is probably a bug in the + * user's implementation, so we should flip our shit. + * + * But, see the next test. + */ + 'Cannot define two members of the same name': function() + { + var _self = this; + this.assertThrows( function() + { + // duplicate foos + _self.Class( + { + 'public foo': function() {}, + 'protected foo': function() {}, + } ); + } ); + }, + + + /** + * Code generation tools may find it convenient to declare a duplicate + * member without knowing whether or not a duplicate will exist; this + * may save time and complexity when ease.js has been designed to handle + * certain situations. If at least one of the conflicting members has + * been flagged as `weak', then we should ignore the error. + * + * As an example, this is used interally with ease.js to inherit + * abstract members from traits while still permitting concrete + * definitions. + */ + '@each(weak) Can define members of the same name if one is weak': + function( weak ) + { + // TODO: this makes assumptions about how the code works; the code + // needs to be refactored to permit more sane testing (since right + // now it'd be a clusterfuck) + var dfn = {}; + dfn[ 'abstract ' + weak[ 0 ] ] = []; + dfn[ 'abstract ' + weak[ 1 ] ] = []; + + var _self = this; + this.assertDoesNotThrow( function() + { + _self.AbstractClass( dfn ); + } ); + }, } ); diff --git a/test/PropParserKeywordsTest.js b/test/PropParserKeywordsTest.js index 0225d21..0680660 100644 --- a/test/PropParserKeywordsTest.js +++ b/test/PropParserKeywordsTest.js @@ -100,7 +100,7 @@ require( 'common' ).testCase( parse( 'public protected private ' + 'virtual abstract override ' + - 'static const proxy ' + + 'static const proxy weak ' + 'var' ); }, Error ); From 548c38503fba11c7a3d8c429eeb97ef0359303a7 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 26 Jan 2014 00:08:42 -0500 Subject: [PATCH 09/52] Added support for weak abstract methods This adds the `weak' keyword and permits abstract method definitions to appear in the same definition object as the concrete implementation. This should never be used with hand-written code---it is intended for code generators (e.g. traits) that do not know if a concrete implementation will be provided, and would waste cycles duplicating the property parsing that ease.js will already be doing. It also allows for more concise code generator code. --- lib/ClassBuilder.js | 12 ++++- lib/MemberBuilder.js | 8 ++- lib/MemberBuilderValidator.js | 33 +++++++++--- lib/class_abstract.js | 26 +++++++--- test/MemberBuilder/MethodTest.js | 62 ++++++++++++++++++++--- test/MemberBuilderValidator/MethodTest.js | 56 ++++++++++++++++++++ test/MemberBuilderValidator/inc-common.js | 4 +- 7 files changed, 176 insertions(+), 25 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index c736808..65ce8aa 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -525,11 +525,21 @@ exports.prototype.buildMembers = function buildMembers( } } - _self._memberBuilder.buildMethod( + var used = _self._memberBuilder.buildMethod( dest, null, name, func, keywords, instLookup, class_id, base ); + // do nothing more if we didn't end up using this definition + // (this may be the case, for example, with weak members) + if ( !used ) + { + return; + } + + // note the concrete method check; this ensures that weak + // abstract methods will not count if a concrete method of the + // smae name has already been seen if ( is_abstract ) { abstract_methods[ name ] = true; diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index 51f8bb3..0f12c51 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -140,7 +140,6 @@ exports.buildMethod = function( } else if ( prev ) { - if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] ) { // override the method @@ -148,6 +147,12 @@ exports.buildMethod = function( prev, value, instCallback, cid ); } + else if ( keywords.weak && keywords[ 'abstract' ] ) + { + // another member of the same name has been found; discard the + // weak declaration + return false; + } else { // by default, perform method hiding, even if the keyword was not @@ -170,6 +175,7 @@ exports.buildMethod = function( // store keywords for later reference (needed for pre-ES5 fallback) dest[ name ].___$$keywords$$ = keywords; + return true; }; diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index 0730043..707ff88 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -138,8 +138,12 @@ exports.prototype.validateMethod = function( ); } - // do not allow overriding concrete methods with abstract - if ( keywords[ 'abstract' ] && !( prev_keywords[ 'abstract' ] ) ) + // do not allow overriding concrete methods with abstract unless the + // abstract method is weak + if ( keywords[ 'abstract' ] + && !( keywords.weak ) + && !( prev_keywords[ 'abstract' ] ) + ) { throw TypeError( "Cannot override concrete method '" + name + "' with " + @@ -147,9 +151,19 @@ exports.prototype.validateMethod = function( ); } + var lenprev = prev, + lennow = value; + if ( keywords.weak && !( prev_keywords[ 'abstract' ] ) ) + { + // weak abstract declaration found after its concrete + // definition; check in reverse order + lenprev = value; + lennow = prev; + } + // ensure parameter list is at least the length of its supertype - if ( ( value.__length || value.length ) - < ( prev.__length || prev.length ) + if ( ( lennow.__length || lennow.length ) + < ( lenprev.__length || lenprev.length ) ) { throw TypeError( @@ -168,10 +182,13 @@ exports.prototype.validateMethod = function( ); } - // Disallow overriding method without override keyword (unless parent - // method is abstract). In the future, this will provide a warning to - // default to method hiding. - if ( !( keywords[ 'override' ] || prev_keywords[ 'abstract' ] ) ) + // Disallow overriding method without override keyword (unless + // parent method is abstract). In the future, this will provide a + // warning to default to method hiding. Note the check for a + if ( !( keywords[ 'override' ] + || prev_keywords[ 'abstract' ] + || ( keywords[ 'abstract' ] && keywords.weak ) + ) ) { throw TypeError( "Attempting to override method '" + name + diff --git a/lib/class_abstract.js b/lib/class_abstract.js index 42a0327..c52df72 100644 --- a/lib/class_abstract.js +++ b/lib/class_abstract.js @@ -61,19 +61,31 @@ exports.extend = function() }; +/** + * Mixes in a trait + * + * @return {Object} staged abstract class + */ +exports.use = function() +{ + return abstractOverride( + Class.use.apply( this, arguments ) + ); +}; + + /** * Creates an abstract class implementing the given members * * Simply wraps the class module's implement() method. * - * @return {Object} abstract class + * @return {Object} staged abstract class */ exports.implement = function() { - var impl = Class.implement.apply( this, arguments ); - - abstractOverride( impl ); - return impl; + return abstractOverride( + Class.implement.apply( this, arguments ) + ); }; @@ -112,8 +124,8 @@ function abstractOverride( obj ) var extend = obj.extend, impl = obj.implement; - // wrap and apply the abstract flag, only if the method is defined (it may - // not be under all circumstances, e.g. after an implement()) + // wrap and apply the abstract flag, only if the method is defined (it + // may not be under all circumstances, e.g. after an implement()) impl && ( obj.implement = function() { return abstractOverride( impl.apply( this, arguments ) ); diff --git a/test/MemberBuilder/MethodTest.js b/test/MemberBuilder/MethodTest.js index 72b86c8..4ca7200 100644 --- a/test/MemberBuilder/MethodTest.js +++ b/test/MemberBuilder/MethodTest.js @@ -52,6 +52,10 @@ require( 'common' ).testCase( }; } ); }; + + // simply intended to execute test two two perspectives + this.weakab = [ + ]; }, @@ -105,9 +109,9 @@ require( 'common' ).testCase( _self.testArgs( arguments, name, value, keywords ); }; - this.sut.buildMethod( + this.assertOk( this.sut.buildMethod( this.members, {}, name, value, keywords, function() {}, 1, {} - ); + ) ); this.assertEqual( true, called, 'validateMethod() was not called' ); }, @@ -133,9 +137,9 @@ require( 'common' ).testCase( _self.testArgs( arguments, name, value, keywords ); }; - this.sut.buildMethod( + this.assertOk( this.sut.buildMethod( this.members, {}, name, value, keywords, function() {}, 1, {} - ); + ) ); this.assertEqual( true, called, 'validateMethod() was not called' ); }, @@ -159,9 +163,9 @@ require( 'common' ).testCase( ; // build the proxy - this.sut.buildMethod( + this.assertOk( this.sut.buildMethod( this.members, {}, name, value, keywords, instCallback, cid, {} - ); + ) ); this.assertNotEqual( null, this.proxyFactoryCall, "Proxy factory should be used when `proxy' keyword is provided" @@ -181,4 +185,50 @@ require( 'common' ).testCase( "Generated proxy method should be properly assigned to members" ); }, + + + /** + * A weak abstract method may exist in a situation where a code + * generator is not certain whether a concrete implementation may be + * provided. In this case, we would not want to actually create an + * abstract method if a concrete one already exists. + */ + 'Weak abstract methods are not processed if concrete is available': + function() + { + var _self = this, + called = false, + + cid = 1, + name = 'foo', + cval = function() { called = true; }, + aval = [], + + ckeywords = {}, + akeywords = { weak: true, 'abstract': true, }, + + instCallback = function() {} + ; + + // first define abstract + this.assertOk( this.sut.buildMethod( + this.members, {}, name, aval, akeywords, instCallback, cid, {} + ) ); + + // concrete should take precedence + this.assertOk( this.sut.buildMethod( + this.members, {}, name, cval, ckeywords, instCallback, cid, {} + ) ); + + this.members[ 'public' ].foo(); + this.assertOk( called, "Concrete method did not take precedence" ); + + // now try abstract again to ensure this works from both directions + this.assertOk( this.sut.buildMethod( + this.members, {}, name, aval, akeywords, instCallback, cid, {} + ) === false ); + + this.members[ 'public' ].foo(); + this.assertOk( called, "Concrete method unkept" ); + }, } ); diff --git a/test/MemberBuilderValidator/MethodTest.js b/test/MemberBuilderValidator/MethodTest.js index 637cc46..b454dd3 100644 --- a/test/MemberBuilderValidator/MethodTest.js +++ b/test/MemberBuilderValidator/MethodTest.js @@ -27,6 +27,7 @@ require( 'common' ).testCase( caseSetUp: function() { var _self = this; + this.util = this.require( 'util' ); this.quickKeywordMethodTest = function( keywords, identifier, prev ) { @@ -208,6 +209,21 @@ require( 'common' ).testCase( }, + /** + * Contrary to the above test, an abstract method may appear after its + * concrete implementation if the `weak' keyword is provided; this + * exists to allow code generation tools to fall back to abstract + * without having to invoke the property parser directly, complicating + * their logic and duplicating work that ease.js will already do. + */ + 'Concrete method may appear with weak abstract method': function() + { + this.quickKeywordMethodTest( + [ 'weak', 'abstract' ], null, [] + ); + }, + + /** * The parameter list is part of the class interface. Changing the length * will make the interface incompatible with that of its parent and make @@ -267,6 +283,46 @@ require( 'common' ).testCase( }, + /** + * Same concept as the above test, but ensure that the logic for weak + * abstract members does not skip the valiation. Furthermore, if a weak + * abstract member is found *after* the concrete definition, the same + * restrictions should apply retroacively. + */ + 'Weak abstract overrides must meet compatibility requirements': + function() + { + var _self = this, + name = 'foo', + amethod = _self.util.createAbstractMethod( [ 'one' ] ); + + + // abstract appears before + this.quickFailureTest( name, 'compatible', function() + { + _self.sut.validateMethod( + name, + function() {}, + {}, + { member: amethod }, + { 'weak': true, 'abstract': true } + ); + } ); + + // abstract appears after + this.quickFailureTest( name, 'compatible', function() + { + _self.sut.validateMethod( + name, + amethod, + { 'weak': true, 'abstract': true }, + { member: function() {} }, + {} + ); + } ); + }, + + /** * One should not be able to, for example, declare a private method it had * previously been declared protected, or declare it as protected if it has diff --git a/test/MemberBuilderValidator/inc-common.js b/test/MemberBuilderValidator/inc-common.js index e717ea9..e6e7233 100644 --- a/test/MemberBuilderValidator/inc-common.js +++ b/test/MemberBuilderValidator/inc-common.js @@ -68,7 +68,7 @@ exports.quickFailureTest = function( name, identifier, action ) return; } - _self.fail( "Expected failure" ); + _self.fail( false, true, "Expected failure" ); }; @@ -124,7 +124,7 @@ exports.quickKeywordTest = function( } else { - this.assertDoesNotThrow( testfunc, Error ); + this.assertDoesNotThrow( testfunc ); } }; From c10fe5e248dc4f52531c4fd8977745205cd280af Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 26 Jan 2014 03:26:15 -0500 Subject: [PATCH 10/52] Proxy methods may now override abstract methods The method for doing so is a kluge; see the test case for more info. --- lib/MethodWrappers.js | 9 +- lib/Trait.js | 65 +++++++++++--- test/MemberBuilderValidator/MethodTest.js | 1 - test/MethodWrappersTest.js | 22 +++++ test/Trait/AbstractTest.js | 103 ++++++++++++++++++++++ 5 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 test/Trait/AbstractTest.js diff --git a/lib/MethodWrappers.js b/lib/MethodWrappers.js index c568b43..7677d13 100644 --- a/lib/MethodWrappers.js +++ b/lib/MethodWrappers.js @@ -89,7 +89,7 @@ exports.standard = { // not to keep an unnecessary reference to the keywords object var is_static = keywords && keywords[ 'static' ]; - return function() + var ret = function() { var context = getInst( this, cid ) || this, retval = undefined, @@ -122,6 +122,13 @@ exports.standard = { ? this : retval; }; + + // ensures that proxies can be used to provide concrete + // implementations of abstract methods with param requirements (we + // have no idea what we'll be proxying to at runtime, so we need to + // just power through it; see test case for more info) + ret.__length = Infinity; + return ret; }, }; diff --git a/lib/Trait.js b/lib/Trait.js index cff6c07..b4e2fe4 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -19,7 +19,8 @@ * along with this program. If not, see . */ -var AbstractClass = require( __dirname + '/class_abstract' ); +var AbstractClass = require( __dirname + '/class_abstract' ), + ClassBuilder = require( __dirname + '/ClassBuilder' ); function Trait() @@ -86,12 +87,39 @@ Trait.isTrait = function( trait ) */ function createConcrete( acls ) { - // start by providing a concrete implementation for our dummy method + // start by providing a concrete implementation for our dummy method and + // a constructor that accepts the protected member object of the + // containing class var dfn = { 'protected ___$$trait$$': function() {}, + + // protected member object + 'private ___$$pmo$$': null, + __construct: function( pmo ) + { + this.___$$pmo$$ = pmo; + }, + + // mainly for debugging; should really never see this. + __name: '#ConcreteTrait#', }; - // TODO: everything else + // every abstract method should be overridden with a proxy to the + // protected member object that will be passed in via the ctor + var amethods = ClassBuilder.getMeta( acls ).abstractMethods; + for ( var f in amethods ) + { + // TODO: would be nice if this check could be for '___'; need to + // replace amethods.__length with something else, then + if ( !( Object.hasOwnProperty.call( amethods, f ) ) + || ( f.substr( 0, 2 ) === '__' ) + ) + { + continue; + } + + dfn[ 'public proxy ' + f ] = '___$$pmo$$'; + } return acls.extend( dfn ); } @@ -135,7 +163,6 @@ function mixin( trait, dfn ) */ function mixMethods( src, dest, vis, iname ) { - // TODO: ignore abstract for ( var f in src ) { if ( !( Object.hasOwnProperty.call( src, f ) ) ) @@ -151,18 +178,30 @@ function mixMethods( src, dest, vis, iname ) continue; } - var pname = vis + ' proxy ' + f; - - // if we have already set up a proxy for a field of this name, then - // multiple traits have defined the same concrete member - if ( dest[ pname ] !== undefined ) + // if abstract, then we are expected to provide the implementation; + // otherwise, we proxy to the trait's implementation + if ( src[ f ].___$$keywords$$['abstract'] ) { - // TODO: between what traits? - throw Error( "Trait member conflict: `" + f + "'" ); + // copy the abstract definition (N.B. this does not copy the + // param names, since that is not [yet] important) + dest[ 'weak abstract ' + f ] = src[ f ].definition; } + else + { + var pname = vis + ' proxy ' + f; - // proxy this method to what will be the encapsulated trait object - dest[ pname ] = iname; + // if we have already set up a proxy for a field of this name, + // then multiple traits have defined the same concrete member + if ( dest[ pname ] !== undefined ) + { + // TODO: between what traits? + throw Error( "Trait member conflict: `" + f + "'" ); + } + + // proxy this method to what will be the encapsulated trait + // object + dest[ pname ] = iname; + } } } diff --git a/test/MemberBuilderValidator/MethodTest.js b/test/MemberBuilderValidator/MethodTest.js index b454dd3..9fe6a00 100644 --- a/test/MemberBuilderValidator/MethodTest.js +++ b/test/MemberBuilderValidator/MethodTest.js @@ -296,7 +296,6 @@ require( 'common' ).testCase( name = 'foo', amethod = _self.util.createAbstractMethod( [ 'one' ] ); - // abstract appears before this.quickFailureTest( name, 'compatible', function() { diff --git a/test/MethodWrappersTest.js b/test/MethodWrappersTest.js index 849da43..28fc6ec 100644 --- a/test/MethodWrappersTest.js +++ b/test/MethodWrappersTest.js @@ -389,5 +389,27 @@ require( 'common' ).testCase( "Should properly proxy to static membesr via static accessor method" ); }, + + + /** + * A proxy method should be able to be used as a concrete implementation + * for an abstract method; this means that it must properly expose the + * number of arguments of the method that it is proxying to. The problem + * is---it can't, because we do not have a type system and so we cannot + * know what we will be proxying to at runtime! + * + * As such, we have no choice (since validations are not at proxy time) + * but to set the length to something ridiculous so that it will never + * fail. + */ + 'Proxy methods are able to satisfy abstract method param requirements': + function() + { + var f = this._sut.standard.wrapProxy( + {}, null, 0, function() {}, '', {} + ); + + this.assertEqual( f.__length, Infinity ); + }, } ); diff --git a/test/Trait/AbstractTest.js b/test/Trait/AbstractTest.js new file mode 100644 index 0000000..0859673 --- /dev/null +++ b/test/Trait/AbstractTest.js @@ -0,0 +1,103 @@ +/** + * Tests basic trait definition + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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' ); + this.AbstractClass = this.require( 'class_abstract' ); + }, + + + /** + * If a trait contains an abstract member, then any class that uses it + * should too be considered abstract if no concrete implementation is + * provided. + */ + 'Abstract traits create abstract classes when used': function() + { + var T = this.Sut( { 'abstract foo': [] } ); + + var _self = this; + this.assertDoesNotThrow( function() + { + // no concrete `foo; should be abstract (this test is sufficient + // because AbstractClass will throw an error if there are no + // abstract members) + _self.AbstractClass.use( T ).extend( {} ); + }, Error ); + }, + + + /** + * A class may still be concrete even if it uses abstract traits so long + * as it provides concrete implementations for each of the trait's + * abstract members. + */ + 'Concrete classes may use abstract traits by definining members': + function() + { + var T = this.Sut( { 'abstract traitfoo': [ 'foo' ] } ), + C = null, + called = false; + + var _self = this; + this.assertDoesNotThrow( function() + { + C = _self.Class.use( T ).extend( + { + traitfoo: function( foo ) { called = true; }, + } ); + } ); + + // sanity check + C().traitfoo(); + this.assertOk( called ); + }, + + + /** + * The concrete methods provided by a class must be compatible with the + * abstract definitions of any used traits. This test ensures not only + * that the check is being performed, but that the abstract declaration + * is properly inherited from the trait. + * + * TODO: The error mentions "supertype" compatibility, which (although + * true) may be confusing; perhaps reference the trait that declared the + * method as abstract. + */ + 'Concrete classes must be compatible with abstract traits': function() + { + var T = this.Sut( { 'abstract traitfoo': [ 'foo' ] } ); + + var _self = this; + this.assertThrows( function() + { + C = _self.Class.use( T ).extend( + { + // missing param in definition + traitfoo: function() { called = true; }, + } ); + } ); + }, +} ); From 987a2b88ec98cbbc77b472473610a5e0f2b5ec9a Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 26 Jan 2014 03:28:36 -0500 Subject: [PATCH 11/52] Classes can now access trait protected members Slight oversight in the original commit. --- lib/Trait.js | 9 ++++++--- test/Trait/ScopeTest.js | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/lib/Trait.js b/lib/Trait.js index b4e2fe4..eeb340b 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -268,9 +268,12 @@ function tctor() T = tc[ t ][ 1 ], C = T.__ccls || ( T.__ccls = createConcrete( T.__acls ) ); - // TODO: pass protected visibility object once we create - // trait class ctors - this[ f ] = C(); + // instantiate the trait, providing it with our protected visibility + // object so that it has access to our public and protected members + // (but not private); in return, we will use its own protected + // visibility object to gain access to its protected members...quite + // the intimate relationship + this[ f ] = C( this.___$$vis$$ ).___$$vis$$; } }; diff --git a/test/Trait/ScopeTest.js b/test/Trait/ScopeTest.js index b39ada2..e5ba614 100644 --- a/test/Trait/ScopeTest.js +++ b/test/Trait/ScopeTest.js @@ -126,4 +126,27 @@ require( 'common' ).testCase( inst.invokePriv(); }, Error ); }, + + + /** + * If this seems odd at first, consider this: traits provide + * copy/paste-style functionality, meaning they need to be able to + * provide public methods. However, we may not always want to mix trait + * features into a public API; therefore, we need the ability to mix in + * protected members. + */ + 'Classes can access protected trait members': function() + { + var T = this.Sut( { 'protected foo': function() {} } ); + + var _self = this; + this.assertDoesNotThrow( function() + { + _self.Class.use( T ).extend( + { + // invokes protected trait method + 'public callFoo': function() { this.foo(); } + } )().callFoo(); + } ); + }, } ); From b04a8473b809d3da5de0e63800a1c8ddd3411d29 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 26 Jan 2014 03:30:52 -0500 Subject: [PATCH 12/52] Implemented abstract traits This is just an initial implementation, so there may be some quirks; there are more tests to come. --- lib/Trait.js | 24 ++++++++-- test/Trait/AbstractTest.js | 92 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 109 insertions(+), 7 deletions(-) diff --git a/lib/Trait.js b/lib/Trait.js index eeb340b..eb48ad7 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -41,6 +41,9 @@ Trait.extend = function( dfn ) // just in case DFN does not contain any abstract members itself dfn[ 'abstract protected ___$$trait$$' ] = []; + // give the abstract trait class a distinctive name for debugging + dfn.__name = '#AbstractTrait#'; + function TraitType() { throw Error( "Cannot instantiate trait" ); @@ -118,7 +121,15 @@ function createConcrete( acls ) continue; } - dfn[ 'public proxy ' + f ] = '___$$pmo$$'; + // we know that if it's not public, then it must be protected + var vis = ( acls.___$$methods$$['public'][ f ] !== undefined ) + ? 'public' + : 'protected'; + + // setting the correct visibility modified is important to prevent + // visibility de-escalation errors if a protected concrete method is + // provided + dfn[ vis + ' proxy ' + f ] = '___$$pmo$$'; } return acls.extend( dfn ); @@ -178,13 +189,18 @@ function mixMethods( src, dest, vis, iname ) continue; } + var keywords = src[ f ].___$$keywords$$; + // if abstract, then we are expected to provide the implementation; // otherwise, we proxy to the trait's implementation - if ( src[ f ].___$$keywords$$['abstract'] ) + if ( keywords['abstract'] ) { // copy the abstract definition (N.B. this does not copy the - // param names, since that is not [yet] important) - dest[ 'weak abstract ' + f ] = src[ f ].definition; + // param names, since that is not [yet] important); the + // visibility modified is important to prevent de-escalation + // errors on override + var vis = keywords['protected'] ? 'protected' : 'public'; + dest[ vis + ' weak abstract ' + f ] = src[ f ].definition; } else { diff --git a/test/Trait/AbstractTest.js b/test/Trait/AbstractTest.js index 0859673..130d971 100644 --- a/test/Trait/AbstractTest.js +++ b/test/Trait/AbstractTest.js @@ -1,5 +1,5 @@ /** - * Tests basic trait definition + * Tests abstract trait definition and use * * Copyright (C) 2014 Mike Gerwitz * @@ -88,7 +88,7 @@ require( 'common' ).testCase( */ 'Concrete classes must be compatible with abstract traits': function() { - var T = this.Sut( { 'abstract traitfoo': [ 'foo' ] } ); + var T = this.Sut( { 'abstract traitfoo': [ 'foo' ] } ); var _self = this; this.assertThrows( function() @@ -96,8 +96,94 @@ require( 'common' ).testCase( C = _self.Class.use( T ).extend( { // missing param in definition - traitfoo: function() { called = true; }, + traitfoo: function() {}, } ); } ); }, + + + /** + * If a trait defines an abstract method, then it should be able to + * invoke a concrete method of the same name defined by a class. + */ + 'Traits can invoke concrete class implementation of abstract method': + function() + { + var expected = 'foobar'; + + var T = this.Sut( + { + 'public getFoo': function() + { + return this.echo( expected ); + }, + + 'abstract protected echo': [ 'value' ], + } ); + + var result = this.Class.use( T ).extend( + { + // concrete implementation of abstract trait method + 'protected echo': function( value ) + { + return value; + }, + } )().getFoo(); + + this.assertEqual( result, expected ); + }, + + + /** + * Even more kinky is when a trait provides a concrete implementation + * for an abstract method that is defined in another trait that is mixed + * into the same class. This makes sense, because that class acts as + * though the trait's abstract method is its own. This allows for + * message passing between two traits with the class as the mediator. + * + * This is otherwise pretty much the same as the above test. Note that + * we use a public `echo' method; this is to ensure that we do not break + * in the event that protected trait members break (that is: are not + * exposed to the class). + */ + 'Traits can invoke concrete trait implementation of abstract method': + function() + { + var expected = 'traitbar'; + + // same as the previous test + var Ta = this.Sut( + { + 'public getFoo': function() + { + return this.echo( expected ); + }, + + 'abstract public echo': [ 'value' ], + } ); + + // but this is new + var Tc = this.Sut( + { + // concrete implementation of abstract trait method + 'public echo': function( value ) + { + return value; + }, + } ); + + this.assertEqual( + this.Class.use( Ta, Tc ).extend( {} )().getFoo(), + expected + ); + + // order shouldn't matter (because that'd be confusing and + // frustrating to users, depending on how the traits are named), so + // let's do this again in reverse order + this.assertEqual( + this.Class.use( Tc, Ta ).extend( {} )().getFoo(), + expected, + "Crap; order matters?!" + ); + }, } ); From dac4b9b3a112872aaaa7c939fbd675449e30584c Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 29 Jan 2014 00:20:08 -0500 Subject: [PATCH 13/52] The `weak' keyword can now apply to overrides Well, technically anything, but we're leaving that behavior as undefined for now (the documentation will say the same thing). --- lib/MemberBuilder.js | 14 +++++----- lib/MemberBuilderValidator.js | 2 +- test/MemberBuilder/MethodTest.js | 47 ++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index 0f12c51..ce7883e 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -140,19 +140,19 @@ exports.buildMethod = function( } else if ( prev ) { - if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] ) + if ( keywords.weak ) + { + // another member of the same name has been found; discard the + // weak declaration + return false; + } + else if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] ) { // override the method dest[ name ] = this._overrideMethod( prev, value, instCallback, cid ); } - else if ( keywords.weak && keywords[ 'abstract' ] ) - { - // another member of the same name has been found; discard the - // weak declaration - return false; - } else { // by default, perform method hiding, even if the keyword was not diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index 707ff88..ffe657c 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -187,7 +187,7 @@ exports.prototype.validateMethod = function( // warning to default to method hiding. Note the check for a if ( !( keywords[ 'override' ] || prev_keywords[ 'abstract' ] - || ( keywords[ 'abstract' ] && keywords.weak ) + || keywords.weak ) ) { throw TypeError( diff --git a/test/MemberBuilder/MethodTest.js b/test/MemberBuilder/MethodTest.js index 4ca7200..ca2dcb7 100644 --- a/test/MemberBuilder/MethodTest.js +++ b/test/MemberBuilder/MethodTest.js @@ -231,4 +231,51 @@ require( 'common' ).testCase( this.members[ 'public' ].foo(); this.assertOk( called, "Concrete method unkept" ); }, + + + /** + * Same concept as the above, but with virtual methods (which have a + * concrete implementation available by default). + */ + 'Weak virtual methods are not processed if override is available': + function() + { + var _self = this, + called = false, + + cid = 1, + name = 'foo', + oval = function() { called = true; }, + vval = function() + { + _self.fail( true, false, "Method not overridden." ); + }, + + okeywords = { 'override': true }, + vkeywords = { weak: true, 'virtual': true }, + + instCallback = function() {} + ; + + // define the virtual method + this.assertOk( this.sut.buildMethod( + this.members, {}, name, vval, vkeywords, instCallback, cid, {} + ) ); + + // override should take precedence + this.assertOk( this.sut.buildMethod( + this.members, {}, name, oval, okeywords, instCallback, cid, {} + ) ); + + this.members[ 'public' ].foo(); + this.assertOk( called, "Override did not take precedence" ); + + // now try virtual again to ensure this works from both directions + this.assertOk( this.sut.buildMethod( + this.members, {}, name, vval, vkeywords, instCallback, cid, {} + ) === false ); + + this.members[ 'public' ].foo(); + this.assertOk( called, "Override unkept" ); + }, } ); From 1b323ed80bee53ef4328ab75ff8edd15567dfd75 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 30 Jan 2014 23:13:52 -0500 Subject: [PATCH 14/52] Validation warnings now stored in state object This will allow for additional processing before actually triggering the warnings. For the sake of this commit, though, we just keep with existing functionality. --- lib/ClassBuilder.js | 8 ++- lib/MemberBuilder.js | 26 +++++++- lib/MemberBuilderValidator.js | 75 ++++++++++++++++++++-- lib/interface.js | 6 +- test/ClassBuilder/MemberRestrictionTest.js | 38 ++++++++--- test/MemberBuilder/GetterSetterTest.js | 44 +++++++------ test/MemberBuilder/MethodTest.js | 50 ++++++++------- test/MemberBuilder/PropTest.js | 44 +++++++------ test/MemberBuilder/inc-common.js | 25 +++++--- test/MemberBuilderValidator/MethodTest.js | 31 +++++---- test/MemberBuilderValidator/inc-common.js | 5 +- 11 files changed, 247 insertions(+), 105 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 65ce8aa..f72c3b1 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -451,6 +451,9 @@ exports.prototype.buildMembers = function buildMembers( smethods = static_members.methods, sprops = static_members.props, + // holds member builder state + state = {}, + _self = this ; @@ -527,7 +530,7 @@ exports.prototype.buildMembers = function buildMembers( var used = _self._memberBuilder.buildMethod( dest, null, name, func, keywords, instLookup, - class_id, base + class_id, base, state ); // do nothing more if we didn't end up using this definition @@ -556,6 +559,9 @@ exports.prototype.buildMembers = function buildMembers( } }, } ); + + // process accumulated member state + this._memberBuilder.end( state ); } diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index ce7883e..6d80a71 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -95,6 +95,10 @@ exports.initMembers = function( mpublic, mprotected, mprivate ) * Copies a method to the appropriate member prototype, depending on * visibility, and assigns necessary metadata from keywords * + * The provided ``member run'' state object is required and will be + * initialized automatically if it has not been already. For the first + * member of a run, the object should be empty. + * * @param {__visobj} members * @param {!Object} meta metadata container * @param {string} name property name @@ -108,10 +112,12 @@ exports.initMembers = function( mpublic, mprotected, mprivate ) * @param {number} cid class id * @param {Object=} base optional base object to scan * + * @param {Object} state member run state object + * * @return {undefined} */ exports.buildMethod = function( - members, meta, name, value, keywords, instCallback, cid, base + members, meta, name, value, keywords, instCallback, cid, base, state ) { // TODO: We can improve performance by not scanning each one individually @@ -125,7 +131,7 @@ exports.buildMethod = function( // ensure that the declaration is valid (keywords make sense, argument // length, etc) this._validate.validateMethod( - name, value, keywords, prev_data, prev_keywords + name, value, keywords, prev_data, prev_keywords, state ); // we might be overriding an existing method @@ -492,3 +498,19 @@ exports._getVisibilityValue = function( keywords ) } } + +/** + * End member run and perform post-processing on state data + * + * A ``member run'' should consist of the members required for a particular + * object (class/interface/etc). This action will perform validation + * post-processing if a validator is available. + * + * @param {Object} state member run state + * + * @return {undefined} + */ +exports.end = function( state ) +{ + this._validate && this._validate.end( state ); +}; diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index ffe657c..d0a0af9 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -32,10 +32,71 @@ module.exports = exports = function MemberBuilderValidator( warn_handler ) /** - * Validates a method declaration, ensuring that keywords are valid, overrides - * make sense, etc. + * Initialize validation state if not already done * - * Throws exception on validation failure + * @param {Object} state validation state + * + * @return {Object} provided state object STATE + */ +exports.prototype._initState = function( state ) +{ + if ( state.__vready ) return state; + + state.warn = {}; + state.__vready = true; + return state; +}; + + +/** + * Perform post-processing on and invalidate validation state + * + * All queued warnings will be triggered. + * + * @param {Object} state validation state + * + * @return {undefined} + */ +exports.prototype.end = function( state ) +{ + // trigger warnings + for ( var f in state.warn ) + { + var warns = state.warn[ f ]; + for ( var id in warns ) + { + this._warningHandler( warns[ id ] ); + } + } + + state.__vready = false; +}; + + +/** + * Enqueue warning within validation state + * + * @param {Object} state validation state + * @param {string} member member name + * @param {string} id warning identifier + * @param {Warning} warn warning + * + * @return {undefined} + */ +function _addWarn( state, member, id, warn ) +{ + ( state.warn[ member ] = state.warn[ member ] || {} )[ id ] = warn; +} + + +/** + * Validates a method declaration, ensuring that keywords are valid, + * overrides make sense, etc. + * + * Throws exception on validation failure. Warnings are stored in the state + * object for later processing. The state object will be initialized if it + * has not been already; for the initial validation, the state object should + * be empty. * * @param {string} name method name * @param {*} value method value @@ -45,12 +106,16 @@ module.exports = exports = function MemberBuilderValidator( warn_handler ) * @param {Object} prev_data data of member being overridden * @param {Object} prev_keywords keywords of member being overridden * + * @param {Object} state pre-initialized state object + * * @return {undefined} */ exports.prototype.validateMethod = function( - name, value, keywords, prev_data, prev_keywords + name, value, keywords, prev_data, prev_keywords, state ) { + this._initState( state ); + var prev = ( prev_data ) ? prev_data.member : null; if ( keywords[ 'abstract' ] ) @@ -202,7 +267,7 @@ exports.prototype.validateMethod = function( // but it shouldn't stop the class definition (it doesn't adversely // affect the functionality of the class, unless of course the method // attempts to reference a supertype) - this._warningHandler( Error( + _addWarn( state, name, 'no', Error( "Method '" + name + "' using 'override' keyword without super method" ) ); diff --git a/lib/interface.js b/lib/interface.js index dcb6beb..b2661f2 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -190,6 +190,9 @@ var extend = ( function( extending ) prototype = new base(), iname = '', + // holds validation state + vstate = {}, + members = member_builder.initMembers( prototype, prototype, prototype ) @@ -235,7 +238,8 @@ var extend = ( function( extending ) } member_builder.buildMethod( - members, null, name, value, keywords + members, null, name, value, keywords, + null, 0, {}, vstate ); }, } ); diff --git a/test/ClassBuilder/MemberRestrictionTest.js b/test/ClassBuilder/MemberRestrictionTest.js index 5cf7020..dd79fd1 100644 --- a/test/ClassBuilder/MemberRestrictionTest.js +++ b/test/ClassBuilder/MemberRestrictionTest.js @@ -38,16 +38,6 @@ require( 'common' ).testCase( }, - setUp: function() - { - this.builder = this.Sut( - this.require( 'MemberBuilder' )(), - this.require( 'VisibilityObjectFactoryFactory' ) - .fromEnvironment() - ); - }, - - /** * It's always useful to be able to quickly reference a list of reserved * members so that an implementer can programatically handle runtime @@ -288,4 +278,32 @@ require( 'common' ).testCase( _self.AbstractClass( dfn ); } ); }, + + + /** + * During the course of processing, certain data are accumulated into + * the member builder state; this state must be post-processed to + * complete anything that may be pending. + */ + 'Member builder state is ended after processing': function() + { + var _self = this, + build = this.require( 'MemberBuilder' )(); + + var sut = this.Sut( + build, + this.require( 'VisibilityObjectFactoryFactory' ) + .fromEnvironment() + ); + + // TODO: test that we're passed the right state + var called = false; + build.end = function( state ) + { + called = true; + }; + + sut.build( {} ); + this.assertOk( called ); + }, } ); diff --git a/test/MemberBuilder/GetterSetterTest.js b/test/MemberBuilder/GetterSetterTest.js index e09e89f..297016a 100644 --- a/test/MemberBuilder/GetterSetterTest.js +++ b/test/MemberBuilder/GetterSetterTest.js @@ -30,30 +30,32 @@ require( 'common' ).testCase( { var _self = this; - this.testArgs = function( args, name, value, keywords ) + this.testArgs = function( args, name, value, keywords, state ) { - shared.testArgs( _self, args, name, value, keywords, function( - prev_default, pval_given, pkey_given - ) - { - var expected = _self.members[ 'public' ][ name ]; - - if ( !expected ) + shared.testArgs( _self, args, name, value, keywords, state, + function( + prev_default, pval_given, pkey_given + ) { - return prev_default; - } + var expected = _self.members[ 'public' ][ name ]; - return { - value: { - expected: expected, - given: pval_given.member, - }, - keywords: { - expected: null, // XXX - given: pkey_given, - }, - }; - } ); + if ( !expected ) + { + return prev_default; + } + + return { + value: { + expected: expected, + given: pval_given.member, + }, + keywords: { + expected: null, // XXX + given: pkey_given, + }, + }; + } + ); }; }, diff --git a/test/MemberBuilder/MethodTest.js b/test/MemberBuilder/MethodTest.js index ca2dcb7..b88fc0b 100644 --- a/test/MemberBuilder/MethodTest.js +++ b/test/MemberBuilder/MethodTest.js @@ -27,30 +27,32 @@ require( 'common' ).testCase( { var _self = this; - this.testArgs = function( args, name, value, keywords ) + this.testArgs = function( args, name, value, keywords, state ) { - shared.testArgs( _self, args, name, value, keywords, function( - prev_default, pval_given, pkey_given - ) - { - var expected = _self.members[ 'public' ][ name ]; - - if ( !expected ) + shared.testArgs( _self, args, name, value, keywords, state, + function( + prev_default, pval_given, pkey_given + ) { - return prev_default; - } + var expected = _self.members[ 'public' ][ name ]; - return { - value: { - expected: expected, - given: pval_given.member, - }, - keywords: { - expected: expected.___$$keywords$$, // XXX - given: pkey_given, - }, - }; - } ); + if ( !expected ) + { + return prev_default; + } + + return { + value: { + expected: expected, + given: pval_given.member, + }, + keywords: { + expected: expected.___$$keywords$$, // XXX + given: pkey_given, + }, + }; + } + ); }; // simply intended to execute test two two perspectives @@ -100,17 +102,19 @@ require( 'common' ).testCase( name = 'foo', value = function() {}, + state = {}, keywords = {} ; this.mockValidate.validateMethod = function() { called = true; - _self.testArgs( arguments, name, value, keywords ); + _self.testArgs( arguments, name, value, keywords, state ); }; this.assertOk( this.sut.buildMethod( - this.members, {}, name, value, keywords, function() {}, 1, {} + this.members, {}, name, value, keywords, function() {}, 1, {}, + state ) ); this.assertEqual( true, called, 'validateMethod() was not called' ); diff --git a/test/MemberBuilder/PropTest.js b/test/MemberBuilder/PropTest.js index fbb530b..8b8ffd2 100644 --- a/test/MemberBuilder/PropTest.js +++ b/test/MemberBuilder/PropTest.js @@ -27,30 +27,32 @@ require( 'common' ).testCase( { var _self = this; - this.testArgs = function( args, name, value, keywords ) + this.testArgs = function( args, name, value, keywords, state ) { - shared.testArgs( _self, args, name, value, keywords, function( - prev_default, pval_given, pkey_given - ) - { - var expected = _self.members[ 'public' ][ name ]; - - if ( !expected ) + shared.testArgs( _self, args, name, value, keywords, state, + function( + prev_default, pval_given, pkey_given + ) { - return prev_default; - } + var expected = _self.members[ 'public' ][ name ]; - return { - value: { - expected: expected[ 0 ], - given: pval_given.member[ 0 ], - }, - keywords: { - expected: expected[ 1 ], - given: pkey_given, - }, - }; - } ); + if ( !expected ) + { + return prev_default; + } + + return { + value: { + expected: expected[ 0 ], + given: pval_given.member[ 0 ], + }, + keywords: { + expected: expected[ 1 ], + given: pkey_given, + }, + }; + } + ); }; }, diff --git a/test/MemberBuilder/inc-common.js b/test/MemberBuilder/inc-common.js index 7a92e19..1dd23be 100644 --- a/test/MemberBuilder/inc-common.js +++ b/test/MemberBuilder/inc-common.js @@ -27,11 +27,14 @@ * @param {string} name member name * @param {*} value expected value * @param {Object} keywords expected keywords - * @param {function()} prevLookup function to use to look up prev member data + * @param {Object} state validation state + * @param {function()} prevLookup function to look up prev member data * * @return {undefined} */ -exports.testArgs = function( testcase, args, name, value, keywords, prevLookup ) +exports.testArgs = function( + testcase, args, name, value, keywords, state, prevLookup +) { var prev = { value: { expected: null, given: args[ 3 ] }, @@ -41,24 +44,28 @@ exports.testArgs = function( testcase, args, name, value, keywords, prevLookup ) prev = prevLookup( prev, prev.value.given, prev.keywords.given ); testcase.assertEqual( name, args[ 0 ], - 'Incorrect name passed to validator' + "Incorrect name passed to validator" ); testcase.assertDeepEqual( value, args[ 1 ], - 'Incorrect value passed to validator' + "Incorrect value passed to validator" ); testcase.assertStrictEqual( keywords, args[ 2 ], - 'Incorrect keywords passed to validator' + "Incorrect keywords passed to validator" ); testcase.assertStrictEqual( prev.value.expected, prev.value.given, - 'Previous data should contain prev value if overriding, ' + - 'otherwise null' + "Previous data should contain prev value if overriding, " + + "otherwise null" ); testcase.assertDeepEqual( prev.keywords.expected, prev.keywords.given, - 'Previous keywords should contain prev keyword if ' + - 'overriding, otherwise null' + "Previous keywords should contain prev keyword if " + + "overriding, otherwise null" + ); + + testcase.assertStrictEqual( state, args[ 5 ], + "State object was not passed to validator" ); }; diff --git a/test/MemberBuilderValidator/MethodTest.js b/test/MemberBuilderValidator/MethodTest.js index 9fe6a00..58c71bd 100644 --- a/test/MemberBuilderValidator/MethodTest.js +++ b/test/MemberBuilderValidator/MethodTest.js @@ -51,13 +51,18 @@ require( 'common' ).testCase( startobj.virtual = true; overrideobj.override = true; + var state = {}; + _self.sut.validateMethod( name, function() {}, overrideobj, { member: function() {} }, - startobj + startobj, + state ); + + _self.sut.end( state ); }, failstr ); @@ -128,7 +133,7 @@ require( 'common' ).testCase( _self.sut.validateMethod( name, function() {}, {}, { get: function() {} }, - {} + {}, {} ); } ); @@ -138,7 +143,7 @@ require( 'common' ).testCase( _self.sut.validateMethod( name, function() {}, {}, { set: function() {} }, - {} + {}, {} ); } ); }, @@ -161,7 +166,7 @@ require( 'common' ).testCase( _self.sut.validateMethod( name, function() {}, {}, { member: 'immaprop' }, - {} + {}, {} ); } ); }, @@ -246,7 +251,8 @@ require( 'common' ).testCase( // this function returns each of its arguments, otherwise // they'll be optimized away by Closure Compiler. { member: function( a, b, c ) { return [a,b,c]; } }, - { 'virtual': true } + { 'virtual': true }, + {} ); } ); @@ -262,7 +268,8 @@ require( 'common' ).testCase( function() {}, { 'override': true }, { member: parent_method }, - { 'virtual': true } + { 'virtual': true }, + {} ); } ); @@ -277,7 +284,8 @@ require( 'common' ).testCase( method, { 'override': true }, { member: function( a, b, c ) {} }, - { 'virtual': true } + { 'virtual': true }, + {} ); }, Error ); }, @@ -304,7 +312,8 @@ require( 'common' ).testCase( function() {}, {}, { member: amethod }, - { 'weak': true, 'abstract': true } + { 'weak': true, 'abstract': true }, + {} ); } ); @@ -316,7 +325,7 @@ require( 'common' ).testCase( amethod, { 'weak': true, 'abstract': true }, { member: function() {} }, - {} + {}, {} ); } ); }, @@ -448,7 +457,7 @@ require( 'common' ).testCase( { // provide function instead of string _self.sut.validateMethod( - name, function() {}, { 'proxy': true }, {}, {} + name, function() {}, { 'proxy': true }, {}, {}, {} ); } ); }, @@ -464,7 +473,7 @@ require( 'common' ).testCase( this.assertDoesNotThrow( function() { _self.sut.validateMethod( - 'foo', 'dest', { 'proxy': true }, {}, {} + 'foo', 'dest', { 'proxy': true }, {}, {}, {} ); }, TypeError ); }, diff --git a/test/MemberBuilderValidator/inc-common.js b/test/MemberBuilderValidator/inc-common.js index e6e7233..052286f 100644 --- a/test/MemberBuilderValidator/inc-common.js +++ b/test/MemberBuilderValidator/inc-common.js @@ -87,6 +87,7 @@ exports.quickKeywordTest = function( prev_obj = {}, prev_data = prev_data || {}, name = exports.testName, + state = {}, _self = this; // convert our convenient array into a keyword obj @@ -114,7 +115,7 @@ exports.quickKeywordTest = function( var val = ( keyword_obj[ 'proxy' ] ) ? 'proxyDest': function() {}; _self.sut[ type ]( - name, val, keyword_obj, prev_data, prev_obj + name, val, keyword_obj, prev_data, prev_obj, state ); }; @@ -126,6 +127,8 @@ exports.quickKeywordTest = function( { this.assertDoesNotThrow( testfunc ); } + + this.sut.end( state ); }; From 40e287bfc3e459a84383328a7ad025c5564d3706 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 1 Feb 2014 23:15:10 -0500 Subject: [PATCH 15/52] Warning no longer issued when overriding weak method appearing later in dfn obj --- lib/MemberBuilderValidator.js | 21 +++++++++++ test/MemberBuilderValidator/MethodTest.js | 46 +++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index d0a0af9..c9c2f21 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -89,6 +89,21 @@ function _addWarn( state, member, id, warn ) } +/** + * Remove warning from validation state + * + * @param {Object} state validation state + * @param {string} member member name + * @param {string} id warning identifier + * + * @return {undefined} + */ +function _clearWarn( state, member, id, warn ) +{ + delete ( state.warn[ member ] || {} )[ id ]; +} + + /** * Validates a method declaration, ensuring that keywords are valid, * overrides make sense, etc. @@ -260,6 +275,12 @@ exports.prototype.validateMethod = function( "' without 'override' keyword" ); } + + // prevent non-override warning + if ( keywords.weak && prev_keywords[ 'override' ] ) + { + _clearWarn( state, name, 'no' ); + } } else if ( keywords[ 'override' ] ) { diff --git a/test/MemberBuilderValidator/MethodTest.js b/test/MemberBuilderValidator/MethodTest.js index 58c71bd..a32eab2 100644 --- a/test/MemberBuilderValidator/MethodTest.js +++ b/test/MemberBuilderValidator/MethodTest.js @@ -423,6 +423,52 @@ require( 'common' ).testCase( }, + /** + * The above test provides problems if we have a weak method that + * follows the definition of the override within the same definition + * object (that is---A' is defined before A where A' overrides A and A + * is weak); we must ensure that the warning is deferred until we're + * certain that we will not encounter a weak method. + */ + 'Does not throw warning when overriding a later weak method': function() + { + var _self = this; + this.warningHandler = function( warning ) + { + _self.fail( true, false, "Warning was issued." ); + }; + + this.assertDoesNotThrow( function() + { + var state = {}; + + // this should place a warning into the state + _self.sut.validateMethod( + 'foo', + function() {}, + { 'override': true }, + undefined, // no previous because weak was + undefined, // not yet encountered + state + ); + + // this should remove it upon encountering `weak' + _self.sut.validateMethod( + 'foo', + function() {}, + { 'weak': true, 'abstract': true }, + { member: function() {} }, // same as previously defined + { 'override': true }, // above + state + ); + + // hopefully we don't trigger warnings (if we do, the warning + // handler defined above will fail this test) + _self.sut.end( state ); + } ); + }, + + /** * Wait - what? That doesn't make sense from an OOP perspective, now does * it! Unfortunately, we're forced into this restriction in order to From b7a314753ace93a38dbd02ac2b1836104c910719 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 1 Feb 2014 23:15:40 -0500 Subject: [PATCH 16/52] Began implementing virtual trait methods These require special treatment with proxying. --- lib/MemberBuilder.js | 2 +- lib/MemberBuilderValidator.js | 26 ++++++--- lib/MethodWrappers.js | 2 +- lib/Trait.js | 7 ++- test/MethodWrappersTest.js | 2 +- test/Trait/VirtualTest.js | 100 ++++++++++++++++++++++++++++++++++ 6 files changed, 126 insertions(+), 13 deletions(-) create mode 100644 test/Trait/VirtualTest.js diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index 6d80a71..07c8147 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -135,7 +135,7 @@ exports.buildMethod = function( ); // we might be overriding an existing method - if ( keywords[ 'proxy' ] ) + if ( keywords[ 'proxy' ] && !( prev && keywords.weak ) ) { // TODO: Note that this is not compatible with method hiding, due to its // positioning (see hideMethod() below); address once method hiding is diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index c9c2f21..3beec53 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -231,20 +231,32 @@ exports.prototype.validateMethod = function( ); } - var lenprev = prev, - lennow = value; + + var lenprev = ( prev.__length === undefined ) + ? prev.length + : prev.__length; + + var lennow = ( value.__length === undefined ) + ? value.length + : value.__length; + + if ( keywords[ 'proxy' ] ) + { + // otherwise we'd be checking against the length of a string. + lennow = NaN; + } + if ( keywords.weak && !( prev_keywords[ 'abstract' ] ) ) { // weak abstract declaration found after its concrete // definition; check in reverse order - lenprev = value; - lennow = prev; + var tmp = lenprev; + lenprev = lennow; + lennow = tmp; } // ensure parameter list is at least the length of its supertype - if ( ( lennow.__length || lennow.length ) - < ( lenprev.__length || lenprev.length ) - ) + if ( lennow < lenprev ) { throw TypeError( "Declaration of method '" + name + "' must be compatible " + diff --git a/lib/MethodWrappers.js b/lib/MethodWrappers.js index 7677d13..086d2ad 100644 --- a/lib/MethodWrappers.js +++ b/lib/MethodWrappers.js @@ -127,7 +127,7 @@ exports.standard = { // implementations of abstract methods with param requirements (we // have no idea what we'll be proxying to at runtime, so we need to // just power through it; see test case for more info) - ret.__length = Infinity; + ret.__length = NaN; return ret; }, }; diff --git a/lib/Trait.js b/lib/Trait.js index eb48ad7..0ccf47c 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -189,7 +189,8 @@ function mixMethods( src, dest, vis, iname ) continue; } - var keywords = src[ f ].___$$keywords$$; + var keywords = src[ f ].___$$keywords$$, + vis = keywords['protected'] ? 'protected' : 'public'; // if abstract, then we are expected to provide the implementation; // otherwise, we proxy to the trait's implementation @@ -199,12 +200,12 @@ function mixMethods( src, dest, vis, iname ) // param names, since that is not [yet] important); the // visibility modified is important to prevent de-escalation // errors on override - var vis = keywords['protected'] ? 'protected' : 'public'; dest[ vis + ' weak abstract ' + f ] = src[ f ].definition; } else { - var pname = vis + ' proxy ' + f; + var virt = keywords['virtual'] ? 'weak virtual ' : '', + pname = virt + vis + ' proxy ' + f; // if we have already set up a proxy for a field of this name, // then multiple traits have defined the same concrete member diff --git a/test/MethodWrappersTest.js b/test/MethodWrappersTest.js index 28fc6ec..890a330 100644 --- a/test/MethodWrappersTest.js +++ b/test/MethodWrappersTest.js @@ -409,7 +409,7 @@ require( 'common' ).testCase( {}, null, 0, function() {}, '', {} ); - this.assertEqual( f.__length, Infinity ); + this.assertOk( !( 0 < f.__length ) ); }, } ); diff --git a/test/Trait/VirtualTest.js b/test/Trait/VirtualTest.js new file mode 100644 index 0000000..6d61cd2 --- /dev/null +++ b/test/Trait/VirtualTest.js @@ -0,0 +1,100 @@ +/** + * Tests virtual trait methods + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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' ); + }, + + + /** + * If a trait specifies a virtual method, then the class should expose + * the method as virtual. + */ + 'Class inherits virtual trait method': function() + { + var expected = 'foobar'; + + var T = this.Sut( + { + 'virtual foo': function() + { + return expected; + } + } ); + + var C = this.Class.use( T ).extend( {} ); + + // ensure that we are actually using the method + this.assertEqual( C().foo(), expected ); + + // if virtual, we should be able to override it + var expected2 = 'foobaz', + C2; + + this.assertDoesNotThrow( function() + { + C2 = C.extend( + { + 'override foo': function() + { + return expected2; + } + } ); + } ); + + this.assertEqual( C2().foo(), expected2 ); + }, + + + /** + * Virtual trait methods should be treated in a manner similar to + * abstract trait methods---a class should be able to provide its own + * concrete implementation. Note that this differs from the above test + * because we are overriding the method internally at definition time, + * not subclassing. + */ + 'Class can override virtual trait method': function() + { + var _self = this; + var T = this.Sut( + { + 'virtual foo': function() + { + // we should never execute this (unless we're broken) + _self.fail( true, false, + "Method was not overridden." + ); + } + } ); + + var expected = 'foobar'; + var C = this.Class.use( T ).extend( + { + 'override foo': function() { return expected; } + } ); + + this.assertEqual( C().foo(), expected ); + }, +} ); From a0a5c616315d9b8715adeac7460186bbd16c3623 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 2 Feb 2014 22:59:20 -0500 Subject: [PATCH 17/52] Simplified ClassBuilder.buildMembers params This cuts down on the excessive parameter length and opens up room for additional metadata generation. Some slight foreshadowing here. --- lib/ClassBuilder.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index f72c3b1..dbc259e 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -346,9 +346,11 @@ exports.prototype.build = function extend( _, __ ) this._classId, base, prop_init, - abstract_methods, - members, - static_members, + { + all: members, + 'abstract': abstract_methods, + 'static': static_members, + }, function( inst ) { return new_class.___$$svis$$; @@ -441,13 +443,16 @@ exports.prototype._getBase = function( base ) exports.prototype.buildMembers = function buildMembers( - props, class_id, base, prop_init, abstract_methods, members, - static_members, staticInstLookup + props, class_id, base, prop_init, memberdest, staticInstLookup ) { var hasOwn = Array.prototype.hasOwnProperty, defs = {}, + members = memberdest.all, + abstract_methods = memberdest['abstract'], + static_members = memberdest['static'], + smethods = static_members.methods, sprops = static_members.props, From 3c7cd0e57aeff0255c27a196c36baf91735ac915 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 2 Feb 2014 23:28:09 -0500 Subject: [PATCH 18/52] Added virtual members to class metadata There does not seem to be tests for any of the metadata at present; they are implicitly tested through various implementations that make use of them. This will also be the case here ("will"---in coming commits), but needs to change. The upcoming reflection implementation would be an excellent time to do so. --- lib/ClassBuilder.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index dbc259e..2b02134 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -308,6 +308,10 @@ exports.prototype.build = function extend( _, __ ) abstract_methods = util.clone( exports.getMeta( base ).abstractMethods ) || { __length: 0 } + + virtual_members = + util.clone( exports.getMeta( base ).virtualMembers ) + || {} ; // prevent extending final classes @@ -350,6 +354,7 @@ exports.prototype.build = function extend( _, __ ) all: members, 'abstract': abstract_methods, 'static': static_members, + 'virtual': virtual_members, }, function( inst ) { @@ -409,6 +414,7 @@ exports.prototype.build = function extend( _, __ ) // create internal metadata for the new class var meta = createMeta( new_class, base ); meta.abstractMethods = abstract_methods; + meta.virtualMembers = virtual_members; meta.name = cname; attachAbstract( new_class, abstract_methods ); @@ -449,9 +455,12 @@ exports.prototype.buildMembers = function buildMembers( var hasOwn = Array.prototype.hasOwnProperty, defs = {}, + // TODO: there does not seem to be tests for these guys; perhaps + // this can be rectified with the reflection implementation members = memberdest.all, abstract_methods = memberdest['abstract'], static_members = memberdest['static'], + virtual_members = memberdest['virtual'], smethods = static_members.methods, sprops = static_members.props, @@ -562,6 +571,11 @@ exports.prototype.buildMembers = function buildMembers( delete abstract_methods[ name ]; abstract_methods.__length--; } + + if ( keywords['virtual'] ) + { + virtual_members[ name ] = true; + } }, } ); From 66cab74cc154688794e0a9fd11389cb0160a81ea Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 3 Feb 2014 23:54:21 -0500 Subject: [PATCH 19/52] Initial trait virtual member proxy implementation This has some flaws that should be addressed, but those will be detailed in later commits; this works for now. --- lib/MethodWrappers.js | 3 +++ lib/Trait.js | 53 +++++++++++++++++++++++++++++++++++++++ test/Trait/VirtualTest.js | 37 +++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/lib/MethodWrappers.js b/lib/MethodWrappers.js index 086d2ad..c2479b9 100644 --- a/lib/MethodWrappers.js +++ b/lib/MethodWrappers.js @@ -123,6 +123,9 @@ exports.standard = { : retval; }; + // TODO: need a test for this; yes, we want to store a reference + ret.___$$proxy_to$$ = proxy_to; + // ensures that proxies can be used to provide concrete // implementations of abstract methods with param requirements (we // have no idea what we'll be proxying to at runtime, so we need to diff --git a/lib/Trait.js b/lib/Trait.js index 0ccf47c..7237b2c 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -132,10 +132,63 @@ function createConcrete( acls ) dfn[ vis + ' proxy ' + f ] = '___$$pmo$$'; } + // virtual methods need to be handled with care to ensure that we invoke + // any overrides + createVirtProxy( acls, dfn ); + return acls.extend( dfn ); } +/** + * Create virtual method proxies for all virtual members + * + * Virtual methods are a bit of hassle with traits: we are in a situation + * where we do not know at the time that the trait is created whether or not + * the virtual method has been overridden, since the class that the trait is + * mixed into may do the overriding. Therefore, we must check if an override + * has occured *when the method is invoked*; there is room for optimization + * there (by making such a determination at the time of mixin), but we'll + * leave that for later. + * + * @param {AbstractClass} acls abstract trait class + * @param {Object} dfn destination definition object + * + * @return {undefined} + */ +function createVirtProxy( acls, dfn ) +{ + var vmembers = ClassBuilder.getMeta( acls ).virtualMembers; + + // f = `field' + for ( var f in vmembers ) + { + var vis = ( acls.___$$methods$$['public'][ f ] !== undefined ) + ? 'public' + : 'protected'; + + dfn[ vis + ' virtual override ' + f ] = ( function() + { + // this is the aforementioned proxy method; see the docblock for + // more information + return function() + { + var pmo = this.___$$pmo$$, + o = pmo[ f ], + op = o.___$$proxy_to$$; + + // XXX: a better way to do this would be nice, since this + // does a substring check on every call; avoids infinite + // recursion from proxying to self + return ( o && !( op && op.substr( 0, 7 ) === '___$to$' ) ) + ? pmo[ f ].apply( pmo, arguments ) + : this.__super.apply( this, arguments ); + } + } )( f ); + } +} + + /** * Mix trait into the given definition * diff --git a/test/Trait/VirtualTest.js b/test/Trait/VirtualTest.js index 6d61cd2..abcf44a 100644 --- a/test/Trait/VirtualTest.js +++ b/test/Trait/VirtualTest.js @@ -97,4 +97,41 @@ require( 'common' ).testCase( this.assertEqual( C().foo(), expected ); }, + + + /** + * If C uses T and overrides T.Ma, and there is some method T.Mb that + * invokes T.Ma, then T.Mb should instead invoke C.Ma. + */ + 'Class-overridden virtual trait method is accessible by trait': + function() + { + var _self = this; + + var T = this.Sut( + { + 'public doFoo': function() + { + // should call overridden, not the one below + this.foo(); + }, + + // to be overridden + 'virtual protected foo': function() + { + _self.fail( true, false, "Method not overridden." ); + }, + } ); + + var called = false; + + var C = this.Class.use( T ).extend( + { + // should be called by T.doFoo + 'override protected foo': function() { called = true }, + } ); + + C().doFoo(); + this.assertOk( called ); + }, } ); From 5047d895c010b930dabd842efa2e809e4c4658f5 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 3 Feb 2014 23:55:41 -0500 Subject: [PATCH 20/52] Moved closure out of getMemberVisibility This was defining a function on every call for no particular reason other than being lazy, it seems. Performance poopoo. --- lib/MemberBuilder.js | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index 07c8147..b9012be 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -297,35 +297,39 @@ exports.buildGetterSetter = function( */ function getMemberVisibility( members, keywords, name ) { - var viserr = function() - { - throw TypeError( - "Only one access modifier may be used for definition of '" + - name + "'" - ); - } - // there's cleaner ways of doing this, but consider it loop unrolling for // performance if ( keywords[ 'private' ] ) { - ( keywords[ 'public' ] || keywords[ 'protected' ] ) && viserr(); + ( keywords[ 'public' ] || keywords[ 'protected' ] ) + && viserr( name ); return members[ 'private' ]; } else if ( keywords[ 'protected' ] ) { - ( keywords[ 'public' ] || keywords[ 'private' ] ) && viserr(); + ( keywords[ 'public' ] || keywords[ 'private' ] ) + && viserr( name ); return members[ 'protected' ]; } else { // public keyword is the default, so explicitly specifying it is only // for clarity - ( keywords[ 'private' ] || keywords[ 'protected' ] ) && viserr(); + ( keywords[ 'private' ] || keywords[ 'protected' ] ) + && viserr( name ); return members[ 'public' ]; } } +function viserr( name ) +{ + throw TypeError( + "Only one access modifier may be used for definition of '" + + name + "'" + ); +} + + /** * Scan each level of visibility for the requested member From bcada872408a8616da831fd6d9f960f82f43e8c0 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 4 Feb 2014 23:48:40 -0500 Subject: [PATCH 21/52] [bug fix] Corrected bug with implicit visibility escalation See test case comments for details. This wasted way too much of my time. --- lib/VisibilityObjectFactory.js | 26 +++++++++++++------------ test/VisibilityObjectFactoryTest.js | 30 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/lib/VisibilityObjectFactory.js b/lib/VisibilityObjectFactory.js index cf2ab5c..dab7feb 100644 --- a/lib/VisibilityObjectFactory.js +++ b/lib/VisibilityObjectFactory.js @@ -80,7 +80,7 @@ exports.prototype.setup = function setup( dest, properties, methods ) this._doSetup( dest, properties[ 'protected' ], methods[ 'protected' ], - 'public' + true ); // then add the private parts @@ -127,20 +127,21 @@ exports.prototype._createPrivateLayer = function( atop_of, properties ) /** * Set up destination object by copying over properties and methods * - * @param {Object} dest destination object - * @param {Object} properties properties to copy - * @param {Object} methods methods to copy - * @param {boolean} unless_keyword do not set if keyword is set on existing - * method + * The prot_priv parameter can be used to ignore both explicitly and + * implicitly public methods. + * + * @param {Object} dest destination object + * @param {Object} properties properties to copy + * @param {Object} methods methods to copy + * @param {boolean} prot_priv do not set unless protected or private * * @return {undefined} */ exports.prototype._doSetup = function( - dest, properties, methods, unless_keyword + dest, properties, methods, prot_priv ) { - var hasOwn = Array.prototype.hasOwnProperty, - pre = null; + var hasOwn = Array.prototype.hasOwnProperty; // copy over the methods if ( methods !== undefined ) @@ -149,7 +150,8 @@ exports.prototype._doSetup = function( { if ( hasOwn.call( methods, method_name ) ) { - pre = dest[ method_name ]; + var pre = dest[ method_name ], + kw = pre && pre.___$$keywords$$; // If requested, do not copy the method over if it already // exists in the destination object. Don't use hasOwn here; @@ -163,9 +165,9 @@ exports.prototype._doSetup = function( // protected). This is the *last* check to ensure a performance // hit is incured *only* if we're overriding protected with // protected. - if ( !unless_keyword + if ( !prot_priv || ( pre === undefined ) - || !( pre.___$$keywords$$[ unless_keyword ] ) + || ( kw[ 'private' ] || kw[ 'protected' ] ) ) { dest[ method_name ] = methods[ method_name ]; diff --git a/test/VisibilityObjectFactoryTest.js b/test/VisibilityObjectFactoryTest.js index 9f7e437..745ab44 100644 --- a/test/VisibilityObjectFactoryTest.js +++ b/test/VisibilityObjectFactoryTest.js @@ -267,6 +267,36 @@ require( 'common' ).testCase( }, + /** + * This test addresses a particularily nasty bug that wasted hours of + * development time: When a visibility modifier keyword is omitted, then + * it should be implicitly public. In this case, however, the keyword is + * not automatically added to the keyword list (maybe one day it will + * be, but for now we'll maintain the distinction); therefore, we should + * not be checking for the `public' keyword when determining if we + * should write to the protected member object. + */ + 'Public methods are not overwritten when keyword is omitted': function() + { + var f = function() {}; + f.___$$keywords$$ = {}; + + // no keywords; should be implicitly public + var dest = { fpub: f }; + + // add duplicate method to protected + this.methods[ 'protected' ].fpub = function() {}; + + this.sut.setup( dest, this.props, this.methods ); + + // ensure our public method is still referenced + this.assertStrictEqual( dest.fpub, f, + "Public methods should not be overwritten by protected methods" + ); + }, + + + /** * Same situation with private members as protected, with the exception that * we do not need to worry about the overlay problem (in regards to From 513bd1a733f06522c599b7a95e1966c9da098395 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 4 Feb 2014 23:49:25 -0500 Subject: [PATCH 22/52] Added test case for visibility escalation internally For completeness, since this is how I originally observed the issue fixed in the previous commit. --- test/Class/VisibilityTest.js | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/Class/VisibilityTest.js b/test/Class/VisibilityTest.js index 944a71d..164e90c 100644 --- a/test/Class/VisibilityTest.js +++ b/test/Class/VisibilityTest.js @@ -711,6 +711,43 @@ require( 'common' ).testCase( }, + /** + * Similar to above test, but ensure that overrides also take effect via + * the internal visibility object. + */ + 'Protected method overrides are observable by supertype': function() + { + var _self = this, + called = false; + + var C = this.Class( + { + 'public doFoo': function() + { + // will be overridden + return this.foo(); + }, + + // will be overridden + 'virtual protected foo': function() + { + _self.fail( true, false, "Method not overridden" ); + }, + } ) + .extend( + { + // should be invoked by doFoo; visibiility escalation + 'public override foo': function() + { + called = true; + }, + } ); + + C().doFoo(); + this.assertOk( called ); + }, + + /** * There was an issue where the private property object was not proxying * values to the true protected values. This would mean that when the parent From ee46fc218211b5b71c525129ddc6de214b99422e Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 4 Feb 2014 23:55:24 -0500 Subject: [PATCH 23/52] Began testing class subtyping with mixins --- test/Trait/AbstractTest.js | 38 ++++++++++++++++++++++ test/Trait/MixedExtendTest.js | 59 +++++++++++++++++++++++++++++++++++ test/Trait/VirtualTest.js | 49 +++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 test/Trait/MixedExtendTest.js diff --git a/test/Trait/AbstractTest.js b/test/Trait/AbstractTest.js index 130d971..742b5df 100644 --- a/test/Trait/AbstractTest.js +++ b/test/Trait/AbstractTest.js @@ -186,4 +186,42 @@ require( 'common' ).testCase( "Crap; order matters?!" ); }, + + + /** + * If some trait T used by abstract class C defines abstract method M, + * then some subtype C' of C should be able to provide a concrete + * definition of M such that T.M() invokes C'.M. + */ + 'Abstract method inherited from trait can be implemented by subtype': + function() + { + var T = this.Sut( + { + 'public doFoo': function() + { + // should invoke the concrete implementation + this.foo(); + }, + + 'abstract protected foo': [], + } ); + + var called = false; + + // C is a concrete class that extends an abstract class that uses + // trait T + var C = this.AbstractClass.use( T ).extend( {} ) + .extend( + { + // concrete definition that should be invoked by T.doFoo + 'protected foo': function() + { + called = true; + }, + } ); + + C().doFoo(); + this.assertOk( called ); + }, } ); diff --git a/test/Trait/MixedExtendTest.js b/test/Trait/MixedExtendTest.js new file mode 100644 index 0000000..76f8cb6 --- /dev/null +++ b/test/Trait/MixedExtendTest.js @@ -0,0 +1,59 @@ +/** + * Tests extending a class that mixes in traits + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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' ); + }, + + + /** + * The supertype should continue to work as it would without the + * subtype, which means that the supertype's traits should still be + * available. Note that ease.js does not (at least at the time of + * writing this test) check to see if a trait is no longer accessible + * due to overrides, and so a supertype's traits will always be + * instantiated. + */ + 'Subtype instantiates traits of supertype': function() + { + var called = false; + + var T = this.Sut( + { + foo: function() { called = true; }, + } ); + + // C is a subtype of a class that mixes in T + var C = this.Class.use( T ).extend( {} ) + .extend( + { + // ensure that there is no ctor-dependent trait stuff + __construct: function() {}, + } ); + + C().foo(); + this.assertOk( called ); + }, +} ); diff --git a/test/Trait/VirtualTest.js b/test/Trait/VirtualTest.js index abcf44a..df02742 100644 --- a/test/Trait/VirtualTest.js +++ b/test/Trait/VirtualTest.js @@ -134,4 +134,53 @@ require( 'common' ).testCase( C().doFoo(); this.assertOk( called ); }, + + + /** + * If a supertype mixes in a trait that provides a virtual method, a + * subtype should be able to provide its own concrete implementation. + * This is especially important to test in the case where a trait + * invokes its own virtual method---we must ensure that the message is + * properly passed to the subtype's override. + * + * For a more formal description of a similar matter, see the + * AbstractTest case; indeed, we're trying to mimic the same behavior + * that we'd expect with abstract methods. + */ + 'Subtype can override virtual method of trait mixed into supertype': + function() + { + var _self = this; + + var T = this.Sut( + { + 'public doFoo': function() + { + // this call should be passed to any overrides + return this.foo(); + }, + + // this is the one we'll try to override + 'virtual protected foo': function() + { + _self.fail( true, false, "Method not overridden." ); + }, + } ); + + var called = false; + + // C is a subtype of a class that implements T + var C = this.Class.use( T ).extend( {} ) + .extend( + { + // this should be called instead of T.foo + 'override protected foo': function() + { + called = true; + }, + } ); + + C().doFoo(); + this.assertOk( called ); + }, } ); From 451ec48a5c547f0fb6e2f8eca42f614b0128ac62 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 8 Feb 2014 00:29:25 -0500 Subject: [PATCH 24/52] Objects are now considered types of class's mixed in traits This is a consequence of ease.js' careful trait implementation that ensures that any mixed in trait retains its API in the same manner that interfaces and supertypes do. --- lib/ClassBuilder.js | 3 ++- lib/class.js | 12 +++++++++++- test/Trait/DefinitionTest.js | 22 ++++++++++++++++++++++ test/Trait/MixedExtendTest.js | 15 +++++++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 2b02134..a9902d7 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -259,7 +259,8 @@ exports.isInstanceOf = function( type, instance ) implemented = meta.implemented; i = implemented.length; - // check implemented interfaces + // check implemented interfaces et. al. (other systems may make use of + // this meta-attribute to provide references to types) while ( i-- ) { if ( implemented[ i ] === type ) diff --git a/lib/class.js b/lib/class.js index 0a0cdfa..af2d51a 100644 --- a/lib/class.js +++ b/lib/class.js @@ -384,7 +384,17 @@ function createUse( base, traits ) traits[ i ].__mixin( dfn ); } - return extend.call( null, base, dfn ); + var C = extend.call( null, base, dfn ), + meta = ClassBuilder.getMeta( C ); + + // add each trait to the list of implemented types so that the + // class is considered to be of type T in traits + for ( var i = 0, n = traits.length; i < n; i++ ) + { + meta.implemented.push( traits[ i ] ); + } + + return C; }, }; } diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index cbaff31..c4b1b26 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -245,4 +245,26 @@ require( 'common' ).testCase( this.fail( false, true, "Mixin must fail on conflict: " + desc ); }, + + + /** + * Traits in ease.js were designed in such a way that an object can be + * considered to be a type of any of the traits that its class mixes in; + * this is consistent with the concept of interfaces and provides a very + * simple and intuitive type system. + */ + 'A class is considered to be a type of each used trait': function() + { + var Ta = this.Sut( {} ), + Tb = this.Sut( {} ), + Tc = this.Sut( {} ), + o = this.Class.use( Ta, Tb ).extend( {} )(); + + // these two were mixed in + this.assertOk( this.Class.isA( Ta, o ) ); + this.assertOk( this.Class.isA( Tb, o ) ); + + // this one was not + this.assertOk( this.Class.isA( Tc, o ) === false ); + }, } ); diff --git a/test/Trait/MixedExtendTest.js b/test/Trait/MixedExtendTest.js index 76f8cb6..80e44f7 100644 --- a/test/Trait/MixedExtendTest.js +++ b/test/Trait/MixedExtendTest.js @@ -56,4 +56,19 @@ require( 'common' ).testCase( C().foo(); this.assertOk( called ); }, + + + /** + * Just as subtypes inherit the same polymorphisms with respect to + * interfaces, so too should subtypes inherit supertypes' mixed in + * traits' types. + */ + 'Subtype has same polymorphic qualities of parent mixins': function() + { + var T = this.Sut( {} ), + o = this.Class.use( T ).extend( {} ).extend( {} )(); + + // o's supertype mixes in T + this.assertOk( this.Class.isA( T, o ) ); + }, } ); From 999c10c3bfa06e9f51c3a804ae9fee2d64ca9e75 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 10 Feb 2014 00:37:25 -0500 Subject: [PATCH 25/52] Subtype mixin support --- lib/Trait.js | 33 +++++++++++++++--- test/Trait/MixedExtendTest.js | 65 +++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/lib/Trait.js b/lib/Trait.js index 7237b2c..1179079 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -294,7 +294,7 @@ function mixMethods( src, dest, vis, iname ) function addTraitInst( T, dfn ) { var tc = ( dfn.___$$tc$$ = ( dfn.___$$tc$$ || [] ) ), - iname = '___$to$' + tc.length; + iname = '___$to$' + T.__acls.__cid; // the trait object array will contain two values: the destination field // and the trait to instantiate @@ -307,7 +307,11 @@ function addTraitInst( T, dfn ) // create internal trait ctor if not available if ( dfn.___$$tctor$$ === undefined ) { - dfn.___$$tctor$$ = tctor; + // TODO: let's check for inheritance or something to avoid this weak + // definition (this prevents warnings if there is not a supertype + // that defines the trait ctor) + dfn[ 'weak virtual ___$$tctor$$' ] = function() {}; + dfn[ 'virtual override ___$$tctor$$' ] = createTctor( tc ); } return iname; @@ -327,11 +331,10 @@ function addTraitInst( T, dfn ) * * @return {undefined} */ -function tctor() +function tctor( tc ) { // instantiate all traits and assign the object to their // respective fields - var tc = this.___$$tc$$; for ( var t in tc ) { var f = tc[ t ][ 0 ], @@ -345,7 +348,29 @@ function tctor() // the intimate relationship this[ f ] = C( this.___$$vis$$ ).___$$vis$$; } + + // if we are a subtype, be sure to initialize our parent's traits + this.__super && this.__super(); }; +/** + * Create trait constructor + * + * This binds the generic trait constructor to a reference to the provided + * trait class list. + * + * @param {Object} tc trait class list + * + * @return {function()} trait constructor + */ +function createTctor( tc ) +{ + return function() + { + return tctor.call( this, tc ); + }; +} + + module.exports = Trait; diff --git a/test/Trait/MixedExtendTest.js b/test/Trait/MixedExtendTest.js index 80e44f7..4a39646 100644 --- a/test/Trait/MixedExtendTest.js +++ b/test/Trait/MixedExtendTest.js @@ -71,4 +71,69 @@ require( 'common' ).testCase( // o's supertype mixes in T this.assertOk( this.Class.isA( T, o ) ); }, + + + /** + * Subtyping should impose no limits on mixins (except for the obvious + * API compatibility restrictions inherent in OOP). + */ + 'Subtype can mix in additional traits': function() + { + var a = false, + b = false; + + var Ta = this.Sut( + { + 'public ta': function() { a = true; }, + } ), + Tb = this.Sut( + { + 'public tb': function() { b = true; }, + } ), + C = null; + + var _self = this; + this.assertDoesNotThrow( function() + { + var sup = _self.Class.use( Ta ).extend( {} ); + + // mixes in Tb; supertype already mixed in Ta + C = _self.Class.use( Tb ).extend( sup, {} ); + } ); + + this.assertDoesNotThrow( function() + { + // ensures that instantiation does not throw an error and that + // the methods both exist + var o = C(); + o.ta(); + o.tb(); + } ); + + // ensure both were properly called + this.assertOk( a ); + this.assertOk( b ); + }, + + + /** + * As a sanity check, ensure that subtyping does not override parent + * type data with respect to traits. + * + * Note that this test makes the preceding test redundant, but the + * separation is useful for debugging any potential regressions. + */ + 'Subtype trait types do not overwrite supertype types': function() + { + var Ta = this.Sut( {} ), + Tb = this.Sut( {} ), + C = this.Class.use( Ta ).extend( {} ), + o = this.Class.use( Tb ).extend( C, {} )(); + + // o's supertype mixes in Ta + this.assertOk( this.Class.isA( Ta, o ) ); + + // o mixes in Tb + this.assertOk( this.Class.isA( Tb, o ) ); + }, } ); From 897a4afab4c605289a06e54e2542a84dacd00e22 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 10 Feb 2014 23:11:34 -0500 Subject: [PATCH 26/52] Added support for mixing in traits using use method on a base class --- lib/class.js | 30 +++++++++++++++++++++++---- test/Trait/MixedExtendTest.js | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/lib/class.js b/lib/class.js index af2d51a..d74608a 100644 --- a/lib/class.js +++ b/lib/class.js @@ -374,9 +374,9 @@ function createUse( base, traits ) return { extend: function() { - var args = Array.prototype.slice.call( arguments ), - dfn = args.pop(), - base = args.pop(); + var args = Array.prototype.slice.call( arguments ), + dfn = args.pop(), + ext_base = args.pop(); // "mix" each trait into the provided definition object for ( var i = 0, n = traits.length; i < n; i++ ) @@ -384,7 +384,7 @@ function createUse( base, traits ) traits[ i ].__mixin( dfn ); } - var C = extend.call( null, base, dfn ), + var C = extend.call( null, ( base || ext_base ), dfn ), meta = ClassBuilder.getMeta( C ); // add each trait to the list of implemented types so that the @@ -495,6 +495,7 @@ function setupProps( func ) { attachExtend( func ); attachImplement( func ); + attachUse( func ); } @@ -544,3 +545,24 @@ function attachImplement( func ) }); } + +/** + * Attaches use method to the given function (class) + * + * Please see the `use' export of this module for more information. + * + * @param {function()} func function (class) to attach method to + * + * @return {undefined} + */ +function attachUse( func ) +{ + util.defineSecureProp( func, 'use', function() + { + return createUse( + func, + Array.prototype.slice.call( arguments ) + ); + } ); +} + diff --git a/test/Trait/MixedExtendTest.js b/test/Trait/MixedExtendTest.js index 4a39646..9d77e96 100644 --- a/test/Trait/MixedExtendTest.js +++ b/test/Trait/MixedExtendTest.js @@ -136,4 +136,43 @@ require( 'common' ).testCase( // o mixes in Tb this.assertOk( this.Class.isA( Tb, o ) ); }, + + + /** + * This alternative syntax mixes a trait directly into a base class and + * then omits the base class as an argument to the extend method; this + * syntax is most familiar with named classes, but we are not testing + * named classes here. + */ + 'Can mix in traits directly atop of existing class': function() + { + var called_foo = false, + called_bar = false, + called_baz = false; + + var C = this.Class( + { + 'public foo': function() { called_foo = true; }, + } ); + + var T = this.Sut( + { + 'public bar': function() { called_bar = true; }, + } ); + + // we must ensure not only that we have mixed in the trait, but that + // we have also maintained C's interface and can further extend it + var inst = C.use( T ).extend( + { + 'public baz': function() { called_baz = true; }, + } )(); + + inst.foo(); + inst.bar(); + inst.baz(); + + this.assertOk( called_foo ); + this.assertOk( called_bar ); + this.assertOk( called_baz ); + }, } ); From 455d3a58151b5e058919b0017ae27653db173f21 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 10 Feb 2014 23:14:05 -0500 Subject: [PATCH 27/52] Added immediate partial class invocation support after mixin --- lib/class.js | 55 +++++++++--------- test/Trait/ImmediateTest.js | 108 ++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 25 deletions(-) create mode 100644 test/Trait/ImmediateTest.js diff --git a/lib/class.js b/lib/class.js index d74608a..2f8ad81 100644 --- a/lib/class.js +++ b/lib/class.js @@ -371,32 +371,37 @@ function createImplement( base, ifaces, cname ) function createUse( base, traits ) { - return { - extend: function() - { - var args = Array.prototype.slice.call( arguments ), - dfn = args.pop(), - ext_base = args.pop(); - - // "mix" each trait into the provided definition object - for ( var i = 0, n = traits.length; i < n; i++ ) - { - traits[ i ].__mixin( dfn ); - } - - var C = extend.call( null, ( base || ext_base ), dfn ), - meta = ClassBuilder.getMeta( C ); - - // add each trait to the list of implemented types so that the - // class is considered to be of type T in traits - for ( var i = 0, n = traits.length; i < n; i++ ) - { - meta.implemented.push( traits[ i ] ); - } - - return C; - }, + var partial = function() + { + return partial.extend( {} ).apply( null, arguments ); }; + + partial.extend = function() + { + var args = Array.prototype.slice.call( arguments ), + dfn = args.pop(), + ext_base = args.pop(); + + // "mix" each trait into the provided definition object + for ( var i = 0, n = traits.length; i < n; i++ ) + { + traits[ i ].__mixin( dfn ); + } + + var C = extend.call( null, ( base || ext_base ), dfn ), + meta = ClassBuilder.getMeta( C ); + + // add each trait to the list of implemented types so that the + // class is considered to be of type T in traits + for ( var i = 0, n = traits.length; i < n; i++ ) + { + meta.implemented.push( traits[ i ] ); + } + + return C; + }; + + return partial; } diff --git a/test/Trait/ImmediateTest.js b/test/Trait/ImmediateTest.js new file mode 100644 index 0000000..5aaa595 --- /dev/null +++ b/test/Trait/ImmediateTest.js @@ -0,0 +1,108 @@ +/** + * Tests immediate definition/instantiation + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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' ); + }, + + + /** + * In our most simple case, mixing a trait into an empty base class and + * immediately invoking the resulting partial class (without explicitly + * extending) should have the effect of instantiating a concrete version + * of the trait (so long as that is permitted). While this test exists + * to ensure consistency throughout the system, it may be helpful in + * situations where a trait is useful on its own. + */ + 'Invoking partial class after mixin instantiates': function() + { + var called = false; + + var T = this.Sut( + { + 'public foo': function() + { + called = true; + }, + } ); + + // mixes T into an empty base class and instantiates + this.Class.use( T )().foo(); + this.assertOk( called ); + }, + + + /** + * This is the most useful and conventional form of mixin---runtime, + * atop of an existing class. In this case, we provide a short-hand form + * of instantiation to avoid the ugly pattern of `.extend( {} )()'. + */ + 'Can invoke partial mixin atop of non-empty base': function() + { + var called_foo = false, + called_bar = false; + + var C = this.Class( + { + 'public foo': function() { called_foo = true; }, + } ); + + var T = this.Sut( + { + 'public bar': function() { called_bar = true; }, + } ); + + // we must ensure not only that we have mixed in the trait, but that + // we have also maintained C's interface + var inst = C.use( T )(); + inst.foo(); + inst.bar(); + + this.assertOk( called_foo ); + this.assertOk( called_bar ); + }, + + + /** + * Ensure that the partial invocation shorthand is equivalent to the + * aforementioned `.extend( {} ).apply( null, arguments )'. + */ + 'Partial arguments are passed to class constructor': function() + { + var given = null, + expected = { foo: 'bar' }; + + var C = this.Class( + { + __construct: function() { given = arguments; }, + } ); + + var T = this.Sut( {} ); + + C.use( T )( expected ); + this.assertStrictEqual( given[ 0 ], expected ); + }, +} ); + From 26bd6b88dd7d5231580bc722a00982837c044c15 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 10 Feb 2014 23:33:07 -0500 Subject: [PATCH 28/52] Named classes now support mixins --- lib/class.js | 8 ++++++++ test/Trait/DefinitionTest.js | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/lib/class.js b/lib/class.js index 2f8ad81..1096555 100644 --- a/lib/class.js +++ b/lib/class.js @@ -300,6 +300,14 @@ function createStaging( cname ) cname ); }, + + use: function() + { + return createUse( + null, + Array.prototype.slice.call( arguments ) + ); + }, }; } diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index c4b1b26..f7726a8 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -267,4 +267,17 @@ require( 'common' ).testCase( // this one was not this.assertOk( this.Class.isA( Tc, o ) === false ); }, + + + /** + * Ensure that the named class staging object permits mixins. + */ + 'Can mix traits into named class': function() + { + var called = false, + T = this.Sut( { foo: function() { called = true; } } ); + + this.Class( 'Named' ).use( T ).extend( {} )().foo(); + this.assertOk( called ); + }, } ); From 8d81373ef8e85ef4cf1387ecb6c44890afad1185 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 13 Feb 2014 05:21:26 -0500 Subject: [PATCH 29/52] Began named trait implementation Does not yet support staging object like classes --- lib/Trait.js | 52 ++++++++++++++++++++++-- test/Trait/NamedTest.js | 87 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 test/Trait/NamedTest.js diff --git a/lib/Trait.js b/lib/Trait.js index 1179079..0e5074b 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -30,12 +30,54 @@ function Trait() case 1: return Trait.extend.apply( this, arguments ); break; + + case 2: + return createNamedTrait.apply( this, arguments ); + break; + + default: + throw Error( "Missing trait name or definition" ); } }; +/** + * Create a named trait + * + * @param {string} name trait name + * @param {Object} def trait definition + * + * @return {Function} named trait + */ +function createNamedTrait( name, dfn ) +{ + if ( arguments.length > 2 ) + { + throw Error( + "Expecting at most two arguments for definition of named " + + "Trait " + name + "'; " + arguments.length + " given" + ); + } + + if ( typeof name !== 'string' ) + { + throw Error( + "First argument of named class definition must be a string" + ); + } + + dfn.__name = name; + + return Trait.extend( dfn ); +} + + 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)'; + // we need at least one abstract member in order to declare a class as // abstract (in this case, our trait class), so let's create a dummy one // just in case DFN does not contain any abstract members itself @@ -52,9 +94,13 @@ Trait.extend = function( dfn ) // and here we can see that traits are quite literally abstract classes var tclass = AbstractClass( dfn ); - TraitType.__trait = true; - TraitType.__acls = tclass; - TraitType.__ccls = null; + TraitType.__trait = true; + TraitType.__acls = tclass; + TraitType.__ccls = null; + TraitType.toString = function() + { + return ''+name; + }; // traits are not permitted to define constructors if ( tclass.___$$methods$$['public'].__construct !== undefined ) diff --git a/test/Trait/NamedTest.js b/test/Trait/NamedTest.js new file mode 100644 index 0000000..d4a5a48 --- /dev/null +++ b/test/Trait/NamedTest.js @@ -0,0 +1,87 @@ +/** + * Tests named trait definitions + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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' ); + }, + + + /** + * If a trait is not given a name, then converting it to a string should + * indicate that it is anonymous. Further, to disambiguate from + * anonymous classes, we should further indicate that it is a trait. + * + * This test is fragile in the sense that it tests for an explicit + * string: this is intended, since some developers may rely on this + * string (even though they really should use Trait.isTrait), and so it + * should be explicitly documented. + */ + 'Anonymous trait is properly indicated when converted to string': + function() + { + var given = this.Sut( {} ).toString(); + this.assertEqual( given, '(Trait)' ); + }, + + + /** + * Analagous to named classes: we should provide the name when + * converting to a string to aid in debugging. + */ + 'Named trait contains name when converted to string': function() + { + var name = 'FooTrait', + T = this.Sut( name, {} ); + + this.assertOk( T.toString().match( name ) ); + }, + + + /** + * We assume that, if two or more arguments are provided, that the + * definition is named. + */ + 'Named trait definition cannot contain zero or more than two arguments': + function() + { + var Sut = this.Sut; + this.assertThrows( function() { Sut(); } ); + this.assertThrows( function() { Sut( 1, 2, 3 ); } ); + }, + + + /** + * Operating on the same assumption as the above test. + */ + 'First argument in named trait definition must be a string': + function() + { + var Sut = this.Sut; + this.assertThrows( function() + { + Sut( {}, {} ); + } ); + }, +} ); From 75e1470582c626a354a1ee77414ad3e87a1b6fcb Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 16 Feb 2014 23:23:11 -0500 Subject: [PATCH 30/52] Class.use now creates its own class --- lib/ClassBuilder.js | 34 +++++++++++++++++------- lib/Trait.js | 20 ++++++++------ lib/class.js | 63 ++++++++++++++++++++++++++++++++------------- 3 files changed, 82 insertions(+), 35 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index a9902d7..807ee55 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -298,6 +298,7 @@ exports.prototype.build = function extend( _, __ ) base = args.pop() || exports.ClassBase, prototype = this._getBase( base ), cname = '', + autoa = false, prop_init = this._memberBuilder.initMembers(), members = this._memberBuilder.initMembers( prototype ), @@ -331,6 +332,12 @@ exports.prototype.build = function extend( _, __ ) delete props.__name; } + // gobble up auto-abstract flag if present + if ( ( autoa = props.___$$auto$abstract$$ ) !== undefined ) + { + delete props.___$$auto$abstract$$; + } + // IE has problems with toString() if ( enum_bug ) { @@ -401,8 +408,7 @@ exports.prototype.build = function extend( _, __ ) new_class.___$$sinit$$ = staticInit; attachFlags( new_class, props ); - - validateAbstract( new_class, cname, abstract_methods ); + validateAbstract( new_class, cname, abstract_methods, autoa ); // We reduce the overall cost of this definition by defining it on the // prototype rather than during instantiation. While this does increase the @@ -588,13 +594,20 @@ exports.prototype.buildMembers = function buildMembers( /** * Validates abstract class requirements * + * We permit an `auto' flag for internal use only that will cause the + * abstract flag to be automatically set if the class should be marked as + * abstract, instead of throwing an error; this should be used sparingly and + * never exposed via a public API (for explicit use), as it goes against the + * self-documentation philosophy. + * * @param {function()} ctor class * @param {string} cname class name * @param {{__length}} abstract_methods object containing abstract methods + * @param {boolean} auto automatically flag as abstract * * @return {undefined} */ -function validateAbstract( ctor, cname, abstract_methods ) +function validateAbstract( ctor, cname, abstract_methods, auto ) { if ( ctor.___$$abstract$$ ) { @@ -606,15 +619,18 @@ function validateAbstract( ctor, cname, abstract_methods ) ); } } - else + else if ( abstract_methods.__length > 0 ) { - if ( abstract_methods.__length > 0 ) + if ( auto ) { - throw TypeError( - "Class " + ( cname || "(anonymous)" ) + " contains abstract " + - "members and must therefore be declared abstract" - ); + ctor.___$$abstract$$ = true; + return; } + + throw TypeError( + "Class " + ( cname || "(anonymous)" ) + " contains abstract " + + "members and must therefore be declared abstract" + ); } } diff --git a/lib/Trait.js b/lib/Trait.js index 0e5074b..a83f12f 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -109,9 +109,9 @@ Trait.extend = function( dfn ) } // invoked to trigger mixin - TraitType.__mixin = function( dfn ) + TraitType.__mixin = function( dfn, tc ) { - mixin( TraitType, dfn ); + mixin( TraitType, dfn, tc ); }; return TraitType; @@ -238,21 +238,25 @@ function createVirtProxy( acls, dfn ) /** * Mix trait into the given definition * - * The original object DFN is modified; it is not cloned. + * The original object DFN is modified; it is not cloned. TC should be + * initialized to an empty array; it is used to store context data for + * mixing in traits and will be encapsulated within a ctor closure (and thus + * will remain in memory). * * @param {Trait} trait trait to mix in * @param {Object} dfn definition object to merge into + * @param {Array} tc trait class context * * @return {Object} dfn */ -function mixin( trait, dfn ) +function mixin( trait, dfn, tc ) { // the abstract class hidden within the trait var acls = trait.__acls, methods = acls.___$$methods$$; // retrieve the private member name that will contain this trait object - var iname = addTraitInst( trait, dfn ); + var iname = addTraitInst( trait, dfn, tc ); mixMethods( methods['public'], dfn, 'public', iname ); mixMethods( methods['protected'], dfn, 'protected', iname ); @@ -334,13 +338,13 @@ 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 * * @return {string} private member into which C instance shall be stored */ -function addTraitInst( T, dfn ) +function addTraitInst( T, dfn, tc ) { - var tc = ( dfn.___$$tc$$ = ( dfn.___$$tc$$ || [] ) ), - iname = '___$to$' + T.__acls.__cid; + var iname = '___$to$' + T.__acls.__cid; // the trait object array will contain two values: the destination field // and the trait to instantiate diff --git a/lib/class.js b/lib/class.js index 1096555..81f56e1 100644 --- a/lib/class.js +++ b/lib/class.js @@ -379,40 +379,67 @@ function createImplement( base, ifaces, cname ) function createUse( base, traits ) { + // invoking the partially applied class will immediately complete its + // definition and instantiate it with the provided constructor arguments var partial = function() { - return partial.extend( {} ).apply( null, arguments ); + return createMixedClass( base, traits ) + .apply( null, arguments ); }; + // otherwise, its definition is deferred until additional context is + // given during the extend operation partial.extend = function() { var args = Array.prototype.slice.call( arguments ), dfn = args.pop(), ext_base = args.pop(); - // "mix" each trait into the provided definition object - for ( var i = 0, n = traits.length; i < n; i++ ) - { - traits[ i ].__mixin( dfn ); - } - - var C = extend.call( null, ( base || ext_base ), dfn ), - meta = ClassBuilder.getMeta( C ); - - // add each trait to the list of implemented types so that the - // class is considered to be of type T in traits - for ( var i = 0, n = traits.length; i < n; i++ ) - { - meta.implemented.push( traits[ i ] ); - } - - return C; + // extend the mixed class, which ensures that all super references + // are properly resolved + return extend.call( null, + createMixedClass( ( base || ext_base ), traits ), + dfn + ); }; return partial; } +function createMixedClass( base, traits ) +{ + // generated definition for our [abstract] class that will mix in each + // of the provided traits; it will automatically be marked as abstract + // if needed + var dfn = { ___$$auto$abstract$$: true }; + + // this object is used as a class-specific context for storing trait + // data; it will be encapsulated within a ctor closure and will not be + // attached to any class + var tc = []; + + // "mix" each trait into the class definition object + for ( var i = 0, n = traits.length; i < n; i++ ) + { + traits[ i ].__mixin( dfn, tc ); + } + + // create the mixed class from the above generated definition + var C = extend.call( null, base, dfn ), + meta = ClassBuilder.getMeta( C ); + + // add each trait to the list of implemented types so that the + // class is considered to be of type T in traits + for ( var i = 0, n = traits.length; i < n; i++ ) + { + meta.implemented.push( traits[ i ] ); + } + + return C; +} + + /** * Mimics class inheritance * From 6473cf35ae7553ae8f7db08d2cc19710eba353dc Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 19 Feb 2014 00:26:57 -0500 Subject: [PATCH 31/52] Began Scala-influenced linearization implementation More information on this implementation and the rationale behind it will appear in the manual. See future commits. (Note the TODOs; return values aren't quite right here, but that will be handled in the next commit.) --- lib/MethodWrappers.js | 3 -- lib/Trait.js | 66 ++++++++++++++++++++------- test/Trait/LinearizationTest.js | 80 +++++++++++++++++++++++++++++++++ test/Trait/VirtualTest.js | 17 ++++--- 4 files changed, 142 insertions(+), 24 deletions(-) create mode 100644 test/Trait/LinearizationTest.js diff --git a/lib/MethodWrappers.js b/lib/MethodWrappers.js index c2479b9..086d2ad 100644 --- a/lib/MethodWrappers.js +++ b/lib/MethodWrappers.js @@ -123,9 +123,6 @@ exports.standard = { : retval; }; - // TODO: need a test for this; yes, we want to store a reference - ret.___$$proxy_to$$ = proxy_to; - // ensures that proxies can be used to provide concrete // implementations of abstract methods with param requirements (we // have no idea what we'll be proxying to at runtime, so we need to diff --git a/lib/Trait.js b/lib/Trait.js index a83f12f..7d1ed7e 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -213,23 +213,31 @@ function createVirtProxy( acls, dfn ) ? 'public' : 'protected'; + // this is the aforementioned proxy method; see the docblock for + // more information dfn[ vis + ' virtual override ' + f ] = ( function() { - // this is the aforementioned proxy method; see the docblock for - // more information return function() { var pmo = this.___$$pmo$$, - o = pmo[ f ], - op = o.___$$proxy_to$$; + o = pmo[ f ]; - // XXX: a better way to do this would be nice, since this - // does a substring check on every call; avoids infinite - // recursion from proxying to self - return ( o && !( op && op.substr( 0, 7 ) === '___$to$' ) ) - ? pmo[ f ].apply( pmo, arguments ) + // proxy to virtual override from the class we are mixed + // into, if found; otherwise, proxy to our supertype + return ( o ) + ? o.apply( pmo, arguments ) : this.__super.apply( this, arguments ); - } + }; + } )( f ); + + // this guy bypasses the above virtual override check, which is + // necessary in certain cases to prevent infinte recursion + dfn[ vis + ' virtual __$$' + f ] = ( function( f ) + { + return function() + { + this.___$$parent$$[ f ].apply( this, arguments ); + }; } )( f ); } } @@ -271,7 +279,7 @@ function mixin( trait, dfn, tc ) * @param {Object} src visibility object to scavenge from * @param {Object} dest destination definition object * @param {string} vis visibility modifier - * @param {string} ianem proxy destination (trait instance) + * @param {string} iname proxy destination (trait instance) * * @return {undefined} */ @@ -307,8 +315,9 @@ function mixMethods( src, dest, vis, iname ) } else { - var virt = keywords['virtual'] ? 'weak virtual ' : '', - pname = virt + vis + ' proxy ' + f; + var vk = keywords['virtual'], + virt = vk ? 'weak virtual ' : '', + pname = ( vk ? '' : 'proxy ' ) + virt + vis + ' ' + f; // if we have already set up a proxy for a field of this name, // then multiple traits have defined the same concrete member @@ -318,9 +327,36 @@ function mixMethods( src, dest, vis, iname ) throw Error( "Trait member conflict: `" + f + "'" ); } + // if non-virtual, a normal proxy should do + // TODO: test return value; see below + if ( !( keywords['virtual'] ) ) + { + dest[ pname ] = iname; + continue; + } + // proxy this method to what will be the encapsulated trait - // object - dest[ pname ] = iname; + // object (note that we do not use the proxy keyword here + // beacuse we are not proxying to a method of the same name) + dest[ pname ] = ( function( f ) + { + return function() + { + var pdest = this[ iname ]; + + // invoke the direct method on the trait instance; this + // bypasses the virtual override check on the trait + // method to ensure that it is invoked without + // additional overhead or confusion + var ret = pdest[ '__$$' + f ].apply( pdest, arguments ); + + // TODO: test return value + // if the trait returns itself, return us instead + return ( ret === iname ) + ? this + : ret; + }; + } )( f ); } } } diff --git a/test/Trait/LinearizationTest.js b/test/Trait/LinearizationTest.js new file mode 100644 index 0000000..88d0728 --- /dev/null +++ b/test/Trait/LinearizationTest.js @@ -0,0 +1,80 @@ +/** + * Tests trait/class linearization + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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 . + * + * GNU ease.js adopts Scala's concept of `linearization' with respect to + * resolving calls to supertypes; the tests that follow provide a detailed + * description of the concept, but readers may find it helpful to read + * through the ease.js manual or Scala documentation. + */ + +require( 'common' ).testCase( +{ + caseSetUp: function() + { + this.Sut = this.require( 'Trait' ); + this.Class = this.require( 'class' ); + }, + + + /** + * When a class mixes in a trait that defines some method M, and then + * overrides it as M', then this.__super within M' should refer to M. + * Note that this does not cause any conflicts with any class supertypes + * that may define a method of the same name as M, because M must have + * been an override, otherwise an error would have occurred. + */ + 'Class super call refers to mixin that is part of a class definition': + function() + { + var _self = this, + scalled = false; + + var T = this.Sut( + { + // after mixin, this should be the super method + 'virtual public foo': function() + { + scalled = true; + }, + } ); + + this.Class.use( T ).extend( + { + // overrides mixed-in foo + 'override public foo': function() + { + // should invoke T.foo + try + { + this.__super(); + } + catch ( e ) + { + _self.fail( false, true, + "Super invocation failure: " + e.message + ); + } + }, + } )().foo(); + + this.assertOk( scalled ); + }, +} ); + diff --git a/test/Trait/VirtualTest.js b/test/Trait/VirtualTest.js index df02742..82ac37f 100644 --- a/test/Trait/VirtualTest.js +++ b/test/Trait/VirtualTest.js @@ -17,6 +17,9 @@ * * You should have received a copy of the GNU General Public License * along with this program. If not, see . + * + * Note that tests for super calls are contained within LinearizationTest; + * these test cases simply ensure that overrides are actually taking place. */ require( 'common' ).testCase( @@ -34,23 +37,24 @@ require( 'common' ).testCase( */ 'Class inherits virtual trait method': function() { - var expected = 'foobar'; + var called = false; var T = this.Sut( { 'virtual foo': function() { - return expected; + called = true; } } ); var C = this.Class.use( T ).extend( {} ); // ensure that we are actually using the method - this.assertEqual( C().foo(), expected ); + C().foo(); + this.assertOk( called, "Virtual method not called" ); // if virtual, we should be able to override it - var expected2 = 'foobaz', + var called2 = false, C2; this.assertDoesNotThrow( function() @@ -59,12 +63,13 @@ require( 'common' ).testCase( { 'override foo': function() { - return expected2; + called2 = true; } } ); } ); - this.assertEqual( C2().foo(), expected2 ); + C2().foo(); + this.assertOk( called2, "Method not overridden" ); }, From c8023cb382f601ce3ecdc0afa1b23bc21a5375bb Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 20 Feb 2014 23:17:04 -0500 Subject: [PATCH 32/52] Trait method return value implementation correction and testing --- lib/Trait.js | 6 +- test/Trait/DefinitionTest.js | 24 ++++++++ test/Trait/VirtualTest.js | 113 +++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 4 deletions(-) diff --git a/lib/Trait.js b/lib/Trait.js index 7d1ed7e..60ff786 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -236,7 +236,7 @@ function createVirtProxy( acls, dfn ) { return function() { - this.___$$parent$$[ f ].apply( this, arguments ); + return this.___$$parent$$[ f ].apply( this, arguments ); }; } )( f ); } @@ -328,7 +328,6 @@ function mixMethods( src, dest, vis, iname ) } // if non-virtual, a normal proxy should do - // TODO: test return value; see below if ( !( keywords['virtual'] ) ) { dest[ pname ] = iname; @@ -350,9 +349,8 @@ function mixMethods( src, dest, vis, iname ) // additional overhead or confusion var ret = pdest[ '__$$' + f ].apply( pdest, arguments ); - // TODO: test return value // if the trait returns itself, return us instead - return ( ret === iname ) + return ( ret === pdest ) ? this : ret; }; diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index f7726a8..da9eb67 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -280,4 +280,28 @@ require( 'common' ).testCase( this.Class( 'Named' ).use( T ).extend( {} )().foo(); this.assertOk( called ); }, + + + /** + * When a trait is mixed into a class, it acts as though it is part of + * that class. Therefore, it should stand to reason that, when a mixed + * in method returns `this', it should actually return the instance of + * the class that it is mixed into (in the case of this test, its + * private member object, since that's our context when invoking the + * trait method). + */ + 'Trait method that returns self will return containing class': + function() + { + var _self = this, + T = this.Sut( { foo: function() { return this; } } ); + + this.Class.use( T ).extend( + { + go: function() + { + _self.assertStrictEqual( this, this.foo() ); + }, + } )().go(); + }, } ); diff --git a/test/Trait/VirtualTest.js b/test/Trait/VirtualTest.js index 82ac37f..b18feef 100644 --- a/test/Trait/VirtualTest.js +++ b/test/Trait/VirtualTest.js @@ -188,4 +188,117 @@ require( 'common' ).testCase( C().doFoo(); this.assertOk( called ); }, + + + /** + * This is the same concept as the non-virtual test found in the + * DefinitionTest case: since a trait is mixed into a class, if it + * returns itself, then it should in actuality return the instance of + * the class it is mixed into. + */ + 'Virtual trait method returning self returns class instance': + function() + { + var _self = this; + + var T = this.Sut( { 'virtual foo': function() { return this; } } ); + + this.Class.use( T ).extend( + { + go: function() + { + _self.assertStrictEqual( this, this.foo() ); + }, + } )().go(); + }, + + + /** + * Same concept as the above test case, but ensures that invoking the + * super method does not screw anything up. + */ + 'Overridden virtual trait method returning self returns class instance': + function() + { + var _self = this; + + var T = this.Sut( { 'virtual foo': function() { return this; } } ); + + this.Class.use( T ).extend( + { + 'override foo': function() + { + return this.__super(); + }, + + go: function() + { + _self.assertStrictEqual( this, this.foo() ); + }, + } )().go(); + }, + + + /** + * When a trait method is overridden, ensure that the data are properly + * proxied back to the caller. This differs from the above tests, which + * just make sure that the method is actually overridden and invoked. + */ + 'Data are properly returned from trait override super call': function() + { + var _self = this, + expected = {}; + + var T = this.Sut( + { + 'virtual foo': function() { return expected; } + } ); + + this.Class.use( T ).extend( + { + 'override foo': function() + { + _self.assertStrictEqual( expected, this.__super() ); + }, + } )().foo(); + }, + + + /** + * When a trait method is overridden by the class that it is mixed into, + * and the super method is called, then the trait method should execute + * within the private member context of the trait itself (as if it were + * never overridden). Some kinky stuff would have to be going on (at + * least in the implementation at the time this was written) for this + * test to fail, but let's be on the safe side. + */ + 'Super trait method overrided in class executed within private context': + function() + { + var expected = {}; + + var T = this.Sut( + { + 'virtual foo': function() + { + // should succeed + return this.priv(); + }, + + 'private priv': function() + { + return expected; + }, + } ); + + this.assertStrictEqual( expected, + this.Class.use( T ).extend( + { + 'override virtual foo': function() + { + return this.__super(); + }, + } )().foo() + ); + }, } ); From 14bd55236145465f65bbdf15b9682f3b0b8e7444 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 28 Feb 2014 23:55:24 -0500 Subject: [PATCH 33/52] Trait can now implement interfaces Note the incomplete test case: the next commit will introduce the ability for mixins to override methods that may have already been defined. --- lib/Trait.js | 96 ++++++++++++++++- lib/class.js | 4 +- test/Trait/ClassVirtualTest.js | 184 +++++++++++++++++++++++++++++++++ 3 files changed, 279 insertions(+), 5 deletions(-) create mode 100644 test/Trait/ClassVirtualTest.js diff --git a/lib/Trait.js b/lib/Trait.js index 60ff786..e7544de 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -74,6 +74,9 @@ function createNamedTrait( name, dfn ) Trait.extend = function( dfn ) { + // we may have been passed some additional metadata + var meta = this.__$$meta || {}; + // 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)'; @@ -91,8 +94,15 @@ Trait.extend = function( dfn ) throw Error( "Cannot instantiate trait" ); }; + // implement interfaces if indicated + var base = AbstractClass; + if ( meta.ifaces ) + { + base = base.implement.apply( null, meta.ifaces ); + } + // and here we can see that traits are quite literally abstract classes - var tclass = AbstractClass( dfn ); + var tclass = base.extend( dfn ); TraitType.__trait = true; TraitType.__acls = tclass; @@ -114,10 +124,33 @@ Trait.extend = function( dfn ) mixin( TraitType, dfn, tc ); }; + // mixes in implemented types + TraitType.__mixinImpl = function( dest_meta ) + { + mixinImpl( tclass, dest_meta ); + }; + return TraitType; }; +Trait.implement = function() +{ + var ifaces = arguments; + + return { + extend: function() + { + // pass our interface metadata as the invocation context + return Trait.extend.apply( + { __$$meta: { ifaces: ifaces } }, + arguments + ); + }, + }; +}; + + Trait.isTrait = function( trait ) { return !!( trait || {} ).__trait; @@ -260,16 +293,71 @@ function createVirtProxy( acls, dfn ) function mixin( trait, dfn, tc ) { // the abstract class hidden within the trait - var acls = trait.__acls, - methods = acls.___$$methods$$; + var acls = trait.__acls; // retrieve the private member name that will contain this trait object var iname = addTraitInst( trait, dfn, tc ); + // 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 ); + return dfn; +} + + +/** + * Recursively mix in class methods + * + * If CLS extends another class, its methods will be recursively processed + * to ensure that the entire prototype chain is properly proxied. + * + * For an explanation of the iname parameter, see the mixin function. + * + * @param {Class} cls class to mix in + * @param {Object} dfn definition object to merge into + * @param {string} iname trait object private member instance name + * + * @return {undefined} + */ +function mixinCls( cls, dfn, iname ) +{ + var methods = cls.___$$methods$$; + mixMethods( methods['public'], dfn, 'public', iname ); mixMethods( methods['protected'], dfn, 'protected', iname ); - return dfn; + // if this class inherits from another class that is *not* the base + // class, recursively process its methods; otherwise, we will have + // incompletely proxied the prototype chain + var parent = methods['public'].___$$parent$$; + if ( parent && ( parent.constructor !== ClassBuilder.ClassBase ) ) + { + mixinCls( parent.constructor, dfn, iname ); + } +} + + +/** + * Mix implemented types into destination object + * + * The provided destination object will ideally be the `implemented' array + * of the destination class's meta object. + * + * @param {Class} cls source class + * @param {Object} dest_meta destination object to copy into + * + * @return {undefined} + */ +function mixinImpl( cls, dest_meta ) +{ + var impl = ClassBuilder.getMeta( cls ).implemented || [], + i = impl.length; + + while ( i-- ) + { + // TODO: this could potentially result in duplicates + dest_meta.push( impl[ i ] ); + } } diff --git a/lib/class.js b/lib/class.js index 81f56e1..2d4b93d 100644 --- a/lib/class.js +++ b/lib/class.js @@ -431,9 +431,11 @@ function createMixedClass( base, traits ) // add each trait to the list of implemented types so that the // class is considered to be of type T in traits + var impl = meta.implemented; for ( var i = 0, n = traits.length; i < n; i++ ) { - meta.implemented.push( traits[ i ] ); + impl.push( traits[ i ] ); + traits[ i ].__mixinImpl( impl ); } return C; diff --git a/test/Trait/ClassVirtualTest.js b/test/Trait/ClassVirtualTest.js new file mode 100644 index 0000000..7cdcb5b --- /dev/null +++ b/test/Trait/ClassVirtualTest.js @@ -0,0 +1,184 @@ +/** + * Tests overriding virtual class methods using mixins + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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 . + * + * These tests vary from those in VirtualTest in that, rather than a class + * overriding a virtual method defined within a trait, a trait is overriding + * a method in the class that it is mixed into. In particular, since + * overrides require that the super method actually exist, this means that a + * trait must implement or extend a common interface. + * + * It is this very important (and powerful) system that allows traits to be + * used as stackable modifications, similar to how one would use the + * decorator pattern (but more tightly coupled). + */ + +require( 'common' ).testCase( +{ + caseSetUp: function() + { + this.Sut = this.require( 'Trait' ); + this.Class = this.require( 'class' ); + this.AbstractClass = this.require( 'class_abstract' ); + this.Interface = this.require( 'interface' ); + }, + + + /** + * A trait may implement an interface I for a couple of reasons: to have + * the class mixed into be considered to of type I and to override + * methods. But, regardless of the reason, let's start with the + * fundamentals. + */ + 'Traits may implement an interface': function() + { + var _self = this; + + // simply make sure that the API is supported; nothing more. + this.assertDoesNotThrow( function() + { + _self.Sut.implement( _self.Interface( {} ) ).extend( {} ); + } ); + }, + + + /** + * We would expect that the default behavior of implementing an + * interface I into a trait would create a trait with all abstract + * methods defined by I. + */ + 'Traits implementing interfaces define abstract methods': function() + { + var I = this.Interface( { foo: [], bar: [] } ), + T = this.Sut.implement( I ).extend( {} ); + + var Class = this.Class, + AbstractClass = this.AbstractClass; + + // T should contain both foo and bar as abstract methods, which we + // will test indirectly in the assertions below + + // should fail because of abstract foo and bar + this.assertThrows( function() + { + Class.use( T ).extend( {} ); + } ); + + // should succeed, since we can have abstract methods within an + // abstract class + this.assertDoesNotThrow( function() + { + AbstractClass.use( T ).extend( {} ); + } ); + + // one remaining abstract method + this.assertDoesNotThrow( function() + { + AbstractClass.use( T ).extend( { foo: function() {} } ); + } ); + + // both concrete + this.assertDoesNotThrow( function() + { + Class.use( T ).extend( + { + foo: function() {}, + bar: function() {}, + } ); + } ); + }, + + + /** + * Just as classes implementing interfaces may choose to immediately + * provide concrete definitions for the methods declared in the + * interface (instead of becoming an abstract class), so too may traits. + */ + 'Traits may provide concrete methods for interfaces': function() + { + var called = false; + + var I = this.Interface( { foo: [] } ), + T = this.Sut.implement( I ).extend( + { + foo: function() + { + called = true; + }, + } ); + + var Class = this.Class; + this.assertDoesNotThrow( function() + { + // should invoke concrete foo; class definition should not fail, + // because foo is no longer abstract + Class.use( T )().foo(); + } ); + + this.assertOk( called ); + }, + + + /** + * Instances of class C mixing in some trait T implementing I will be + * considered to be of type I, since any method of I would either be + * defined within T, or would be implicitly abstract in T, requiring its + * definition within C; otherwise, C would have to be declared astract. + */ + 'Instance of class mixing in trait implementing I is of type I': + function() + { + var I = this.Interface( {} ), + T = this.Sut.implement( I ).extend( {} ); + + this.assertOk( + this.Class.isA( I, this.Class.use( T )() ) + ); + }, + + + /** + * The API for multiple interfaces should be the same for traits as it + * is for classes. + */ + 'Trait can implement multiple interfaces': function() + { + var Ia = this.Interface( {} ), + Ib = this.Interface( {} ), + T = this.Sut.implement( Ia, Ib ).extend( {} ), + o = this.Class.use( T ).extend( {} )(); + + this.assertOk( this.Class.isA( Ia, o ) ); + this.assertOk( this.Class.isA( Ib, o ) ); + }, + + + /** + * This is a concept borrowed from Scala: consider class C and trait T, + * both implementing interface I which declares method M. T should be + * able to override C.M so long as it is concrete, but to do so, we need + * some way of telling ease.js that we are overriding at time of mixin; + * otherwise, override does not make sense, because I.M is clearly + * abstract and there is nothing to override. + */ + 'Trait can override virtual concrete interface methods at mixin': + function() + { + }, +} ); From 8480d8f92c45a1c63d3f4f140619aff48cbf2666 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 4 Mar 2014 00:19:39 -0500 Subject: [PATCH 34/52] Added support for abstract overrides --- lib/MemberBuilder.js | 51 ++++++++++++++++++++++++++++-- lib/MemberBuilderValidator.js | 19 +++++++++-- lib/Trait.js | 16 ++++++---- lib/util.js | 5 ++- test/Trait/ClassVirtualTest.js | 33 ++++++++++++++++++- test/Trait/LinearizationTest.js | 47 +++++++++++++++++++++++++-- test/Util/PropParseKeywordsTest.js | 27 ++++++++++++++++ 7 files changed, 183 insertions(+), 15 deletions(-) diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index b9012be..ab568e6 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -146,7 +146,7 @@ exports.buildMethod = function( } else if ( prev ) { - if ( keywords.weak ) + if ( keywords.weak && !( prev_keywords[ 'abstract' ] ) ) { // another member of the same name has been found; discard the // weak declaration @@ -154,9 +154,15 @@ exports.buildMethod = function( } else if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] ) { + // if we have the `abstract' keyword at this point, then we are + // an abstract override + var override = ( keywords[ 'abstract' ] ) + ? aoverride( name ) + : prev; + // override the method dest[ name ] = this._overrideMethod( - prev, value, instCallback, cid + override, value, instCallback, cid ); } else @@ -185,6 +191,47 @@ exports.buildMethod = function( }; +/** + * Creates an abstract override super method proxy to NAME + * + * This is a fairly abstract concept that is disastrously confusing without + * having been put into the proper context: This function is intended to be + * used as a super method for a method override in the case of abstract + * overrides. It only makes sense to be used, at least at this time, with + * mixins. + * + * When called, the bound context (`this') will be the private member object + * of the caller, which should contain a reference to the protected member + * object of the supertype to proxy to. It is further assumed that the + * protected member object (pmo) defines NAME such that it proxies to a + * mixin; this means that invoking it could result in an infinite loop. We + * therefore skip directly to the super-super method, which will be the + * method we are interested in proxying to. + * + * There is one additional consideration: If this super method is proxying + * from a mixin instance into a class, then it is important that we bind the + * calling context to the pmo instaed of our own context; otherwise, we'll + * be executing within the context of the trait, without access to the + * members of the supertype that we are proxying to! The pmo will be used by + * the ease.js method wrapper to look up the proper private member object, + * so it is not a problem that the pmo is being passed in. + * + * That's a lot of text for such a small amount of code. + * + * @param {string} name name of method to proxy to + * + * @return {Function} abstract override super method proxy + */ +function aoverride( name ) +{ + return function() + { + return this.___$$pmo$$.___$$parent$$[ name ] + .apply( this.___$$pmo$$, arguments ); + }; +} + + /** * Copies a property to the appropriate member prototype, depending on * visibility, and assigns necessary metadata from keywords diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index 3beec53..e6e60cf 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -213,9 +213,22 @@ exports.prototype.validateMethod = function( // disallow overriding non-virtual methods if ( keywords[ 'override' ] && !( prev_keywords[ 'virtual' ] ) ) { - throw TypeError( - "Cannot override non-virtual method '" + name + "'" - ); + if ( !( keywords[ 'abstract' ] ) ) + { + throw TypeError( + "Cannot override non-virtual method '" + name + "'" + ); + } + + // at this point, we have `abstract override' + if ( !( prev_keywords[ 'abstract' ] ) ) + { + // TODO: test me + throw TypeError( + "Cannot perform abstract override on non-abstract " + + "method '" + name + "'" + ); + } } // do not allow overriding concrete methods with abstract unless the diff --git a/lib/Trait.js b/lib/Trait.js index e7544de..27da48d 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -175,8 +175,10 @@ function createConcrete( acls ) var dfn = { 'protected ___$$trait$$': function() {}, - // protected member object - 'private ___$$pmo$$': null, + // protected member object (we define this as protected so that the + // parent ACLS has access to it (!), which is not prohibited since + // JS does not provide a strict typing mechanism...this is a kluge) + 'protected ___$$pmo$$': null, __construct: function( pmo ) { this.___$$pmo$$ = pmo; @@ -393,7 +395,7 @@ function mixMethods( src, dest, vis, iname ) // if abstract, then we are expected to provide the implementation; // otherwise, we proxy to the trait's implementation - if ( keywords['abstract'] ) + if ( keywords[ 'abstract' ] && !( keywords[ 'override' ] ) ) { // copy the abstract definition (N.B. this does not copy the // param names, since that is not [yet] important); the @@ -403,9 +405,10 @@ function mixMethods( src, dest, vis, iname ) } else { - var vk = keywords['virtual'], + var vk = keywords['virtual'], virt = vk ? 'weak virtual ' : '', - pname = ( vk ? '' : 'proxy ' ) + virt + vis + ' ' + f; + ovr = ( keywords['override'] ) ? 'override ' : '', + pname = ( vk ? '' : 'proxy ' ) + virt + ovr + vis + ' ' + f; // if we have already set up a proxy for a field of this name, // then multiple traits have defined the same concrete member @@ -416,7 +419,7 @@ function mixMethods( src, dest, vis, iname ) } // if non-virtual, a normal proxy should do - if ( !( keywords['virtual'] ) ) + if ( !( keywords[ 'virtual' ] ) ) { dest[ pname ] = iname; continue; @@ -546,3 +549,4 @@ function createTctor( tc ) module.exports = Trait; + diff --git a/lib/util.js b/lib/util.js index eb5b274..2cc46f5 100644 --- a/lib/util.js +++ b/lib/util.js @@ -305,7 +305,10 @@ exports.propParse = function( data, options ) name = parse_data.name || prop; keywords = parse_data.keywords || {}; - if ( options.assumeAbstract || keywords[ 'abstract' ] ) + // note the exception for abstract overrides + if ( options.assumeAbstract + || ( keywords[ 'abstract' ] && !( keywords[ 'override' ] ) ) + ) { // may not be set if assumeAbstract is given keywords[ 'abstract' ] = true; diff --git a/test/Trait/ClassVirtualTest.js b/test/Trait/ClassVirtualTest.js index 7cdcb5b..36e1613 100644 --- a/test/Trait/ClassVirtualTest.js +++ b/test/Trait/ClassVirtualTest.js @@ -177,8 +177,39 @@ require( 'common' ).testCase( * otherwise, override does not make sense, because I.M is clearly * abstract and there is nothing to override. */ - 'Trait can override virtual concrete interface methods at mixin': + 'Mixin can override virtual concrete method defined by interface': function() { + var called = false, + I = this.Interface( { foo: [] } ); + + var T = this.Sut.implement( I ).extend( + { + // the keyword combination `abstract override' indicates that we + // should override whatever concrete implementation was defined + // before our having been mixed in + 'abstract override foo': function() + { + called = true; + }, + } ); + + var _self = this; + var C = this.Class.implement( I ).extend( + { + // this should be overridden by the mixin and should therefore + // never be called (for __super tests, see LinearizationTest) + 'virtual foo': function() + { + _self.fail( false, true, + "Concrete class method was not overridden by mixin" + ); + }, + } ); + + // mixing in a trait atop of C should yield the results described + // above due to the `abstract override' keyword combination + C.use( T )().foo(); + this.assertOk( called ); }, } ); diff --git a/test/Trait/LinearizationTest.js b/test/Trait/LinearizationTest.js index 88d0728..3906bd1 100644 --- a/test/Trait/LinearizationTest.js +++ b/test/Trait/LinearizationTest.js @@ -28,8 +28,9 @@ require( 'common' ).testCase( { caseSetUp: function() { - this.Sut = this.require( 'Trait' ); - this.Class = this.require( 'class' ); + this.Sut = this.require( 'Trait' ); + this.Class = this.require( 'class' ); + this.Interface = this.require( 'interface' ); }, @@ -76,5 +77,47 @@ require( 'common' ).testCase( this.assertOk( scalled ); }, + + + /** + * If a trait overrides a method of a class that it is mixed into, then + * super calls within the trait method should resolve to the class + * method. + */ + 'Mixin overriding class method has class method as super method': + function() + { + var _self = this; + + var expected = {}, + I = this.Interface( { foo: [] } ); + + var T = this.Sut.implement( I ).extend( + { + // see ClassVirtualTest case for details on this + 'abstract override foo': function() + { + // should reference C.foo + return this.__super( expected ); + }, + } ); + + var priv_expected = Math.random(); + + var C = this.Class.implement( I ).extend( + { + // asserting on this value will ensure that the below method is + // invoked in the proper context + 'private _priv': priv_expected, + + 'virtual foo': function( given ) + { + _self.assertEqual( priv_expected, this._priv ); + return given; + }, + } ); + + this.assertStrictEqual( C.use( T )().foo(), expected ); + }, } ); diff --git a/test/Util/PropParseKeywordsTest.js b/test/Util/PropParseKeywordsTest.js index 5f8e45a..d6378ec 100644 --- a/test/Util/PropParseKeywordsTest.js +++ b/test/Util/PropParseKeywordsTest.js @@ -55,6 +55,33 @@ require( 'common' ).testCase( }, + /** + * As an exception to the above rule, a method shall not considered to be + * abstract if the `override' keyword is too provided (an abstract + * override---see the trait tests for more information). + */ + 'Not considered abstract when `override\' also provided': function() + { + var _self = this; + + var data = { 'abstract override foo': function() {} }, + found = null; + + this.Sut.propParse( data, { + method: function ( name, func, is_abstract ) + { + _self.assertOk( is_abstract === false ); + _self.assertEqual( typeof func, 'function' ); + _self.assertOk( _self.Sut.isAbstractMethod( func ) === false ); + + found = name; + }, + } ); + + this.assertEqual( found, 'foo' ); + }, + + /** * The idea behind supporting this functionality---which is unsued at * the time of writing this test---is to allow eventual customization of From 88713987e2932d5ac5e4e323be951c3bb8d74629 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 5 Mar 2014 23:42:48 -0500 Subject: [PATCH 35/52] Mixin use method calls can now be chained Syntatic sugar; could have previously extended explicitly and then mixed in. --- lib/class.js | 7 +++++++ test/Trait/MixedExtendTest.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/class.js b/lib/class.js index 2d4b93d..9bd0c92 100644 --- a/lib/class.js +++ b/lib/class.js @@ -403,6 +403,13 @@ function createUse( base, traits ) ); }; + // syntatic sugar to avoid the aruduous and seemingly pointless `extend' + // call simply to mix in another trait + partial.use = function() + { + return partial.extend( {} ).use.apply( null, arguments ); + }; + return partial; } diff --git a/test/Trait/MixedExtendTest.js b/test/Trait/MixedExtendTest.js index 9d77e96..879ceff 100644 --- a/test/Trait/MixedExtendTest.js +++ b/test/Trait/MixedExtendTest.js @@ -175,4 +175,38 @@ require( 'common' ).testCase( this.assertOk( called_bar ); this.assertOk( called_baz ); }, + + + /** + * This test ensures that we can mix in traits using the syntax + * C.use(T1).use(T2), and so on; this may be necessary to disambiguate + * overrides if T1 and T2 provide definitions for the same method (and + * so the syntax C.use(T1, T2) cannot be used). This syntax is also + * important for the concept of stackable traits (see + * LinearizationTest). + * + * Note that this differs from C.use(T1).use(T2).extend({}); we're + * talking about C.extend({}).use(T1).use(T2). Therefore, this can be + * considered to be syntatic sugar for + * C.use( T1 ).extend( {} ).use( T2 ). + */ + 'Can chain use calls': function() + { + var T1 = this.Sut( { foo: function() {} } ), + T2 = this.Sut( { bar: function() {} } ), + C = null; + + var Class = this.Class; + this.assertDoesNotThrow( function() + { + C = Class.extend( {} ).use( T1 ).use( T2 ); + } ); + + // ensure that the methods were actually mixed in + this.assertDoesNotThrow( function() + { + C().foo(); + C().bar(); + } ); + }, } ); From 311496fbe4425cacc4bb92d231f896acbe7d0830 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 6 Mar 2014 00:57:24 -0500 Subject: [PATCH 36/52] Virtual mixin need not be weak This is a relic from the initial implementation, before the abstract/concrete separation. --- lib/Trait.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Trait.js b/lib/Trait.js index 27da48d..ff335a7 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -406,7 +406,7 @@ function mixMethods( src, dest, vis, iname ) else { var vk = keywords['virtual'], - virt = vk ? 'weak virtual ' : '', + virt = vk ? 'virtual ' : '', ovr = ( keywords['override'] ) ? 'override ' : '', pname = ( vk ? '' : 'proxy ' ) + virt + ovr + vis + ' ' + f; From dd7b06247459017850672f8a992a66b4c6b2a58f Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 6 Mar 2014 01:50:00 -0500 Subject: [PATCH 37/52] Base class now has __cid assigned to 0 --- lib/ClassBuilder.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 807ee55..5108227 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -148,6 +148,9 @@ function ClassBuilder( member_builder, visibility_factory ) */ exports.ClassBase = function Class() {}; +// the base class has the class identifier 0 +util.defineSecureProp( exports.ClassBase, '__cid', 0 ); + /** * Default static property method From 3005cda5439326a89efd350754e392a2035cea10 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 6 Mar 2014 01:51:49 -0500 Subject: [PATCH 38/52] Support for stacked mixins The concept of stacked traits already existed in previous commits, but until now, mixins could not be stacked without some ugly errors. This also allows mixins to be stacked atop of themselves, duplicating their effect. This would naturally have limited use, but it's there. This differs slightly from Scala. For example, consider this ease.js mixin: C.use( T ).use( T )() This is perfectly valid---it has the effect of stacking T twice. In reality, ease.js is doing this: - C' = C.use( T ); - new C'.use( T ); That is, it each call to `use' creates another class with T mixed in. Scala, on the other hand, complains in this situation: new C with T with T will produce an error stating that "trait T is inherited twice". You can work around this, however, by doing this: class Ca extends T new Ca with T In fact, this is precisely what ease.js is doing, as mentioned above; the "use.use" syntax is merely shorthand for this: new C.use( T ).extend( {} ).use( T ) Just keep that in mind. --- lib/MemberBuilder.js | 2 +- lib/Trait.js | 49 +++++++++++++------- lib/class.js | 2 +- test/Trait/LinearizationTest.js | 80 +++++++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 19 deletions(-) diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index ab568e6..a184d64 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -226,7 +226,7 @@ function aoverride( name ) { return function() { - return this.___$$pmo$$.___$$parent$$[ name ] + return this.___$$super$$.prototype[ name ] .apply( this.___$$pmo$$, arguments ); }; } diff --git a/lib/Trait.js b/lib/Trait.js index ff335a7..279297e 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -119,9 +119,9 @@ Trait.extend = function( dfn ) } // invoked to trigger mixin - TraitType.__mixin = function( dfn, tc ) + TraitType.__mixin = function( dfn, tc, base ) { - mixin( TraitType, dfn, tc ); + mixin( TraitType, dfn, tc, base ); }; // mixes in implemented types @@ -178,10 +178,14 @@ function createConcrete( acls ) // protected member object (we define this as protected so that the // parent ACLS has access to it (!), which is not prohibited since // JS does not provide a strict typing mechanism...this is a kluge) + // and target supertype---that is, what __super calls should + // referene 'protected ___$$pmo$$': null, - __construct: function( pmo ) + 'protected ___$$super$$': null, + __construct: function( base, pmo ) { - this.___$$pmo$$ = pmo; + this.___$$super$$ = base; + this.___$$pmo$$ = pmo; }, // mainly for debugging; should really never see this. @@ -289,16 +293,17 @@ function createVirtProxy( acls, dfn ) * @param {Trait} trait trait to mix in * @param {Object} dfn definition object to merge into * @param {Array} tc trait class context + * @param {Class} base target supertyep * * @return {Object} dfn */ -function mixin( trait, dfn, tc ) +function mixin( trait, dfn, tc, base ) { // the abstract class hidden within the trait var acls = trait.__acls; // retrieve the private member name that will contain this trait object - var iname = addTraitInst( trait, dfn, tc ); + var iname = addTraitInst( trait, dfn, tc, base ); // recursively mix in trait's underlying abstract class (ensuring that // anything that the trait inherits from is also properly mixed in) @@ -461,15 +466,21 @@ function mixMethods( src, dest, vis, iname ) * Here, `tc' and `to' are understood to be, respectively, ``trait class'' * and ``trait object''. * - * @param {Class} T trait - * @param {Object} dfn definition object of class being mixed into - * @param {Array} tc trait class object + * @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 * * @return {string} private member into which C instance shall be stored */ -function addTraitInst( T, dfn, tc ) +function addTraitInst( T, dfn, tc, base ) { - var iname = '___$to$' + T.__acls.__cid; + var base_cid = base.__cid; + + // creates a property of the form ___$to$N$M to hold the trait object + // reference; M is required because of the private member restrictions + // imposed to be consistent with pre-ES5 fallback + var iname = '___$to$' + T.__acls.__cid + '$' + base_cid; // the trait object array will contain two values: the destination field // and the trait to instantiate @@ -486,7 +497,7 @@ function addTraitInst( T, dfn, tc ) // definition (this prevents warnings if there is not a supertype // that defines the trait ctor) dfn[ 'weak virtual ___$$tctor$$' ] = function() {}; - dfn[ 'virtual override ___$$tctor$$' ] = createTctor( tc ); + dfn[ 'virtual override ___$$tctor$$' ] = createTctor( tc, base ); } return iname; @@ -504,9 +515,12 @@ function addTraitInst( T, dfn, tc ) * This will lazily create the concrete trait class if it does not already * exist, which saves work if the trait is never used. * + * @param {Object} tc trait class list + * @param {Class} base target supertype + * * @return {undefined} */ -function tctor( tc ) +function tctor( tc, base ) { // instantiate all traits and assign the object to their // respective fields @@ -521,7 +535,7 @@ function tctor( tc ) // (but not private); in return, we will use its own protected // visibility object to gain access to its protected members...quite // the intimate relationship - this[ f ] = C( this.___$$vis$$ ).___$$vis$$; + this[ f ] = C( base, this.___$$vis$$ ).___$$vis$$; } // if we are a subtype, be sure to initialize our parent's traits @@ -535,15 +549,16 @@ function tctor( tc ) * This binds the generic trait constructor to a reference to the provided * trait class list. * - * @param {Object} tc trait class list + * @param {Object} tc trait class list + * @param {Class} base target supertype * * @return {function()} trait constructor */ -function createTctor( tc ) +function createTctor( tc, base ) { return function() { - return tctor.call( this, tc ); + return tctor.call( this, tc, base ); }; } diff --git a/lib/class.js b/lib/class.js index 9bd0c92..6454a2c 100644 --- a/lib/class.js +++ b/lib/class.js @@ -429,7 +429,7 @@ function createMixedClass( base, traits ) // "mix" each trait into the class definition object for ( var i = 0, n = traits.length; i < n; i++ ) { - traits[ i ].__mixin( dfn, tc ); + traits[ i ].__mixin( dfn, tc, ( base || ClassBuilder.ClassBase ) ); } // create the mixed class from the above generated definition diff --git a/test/Trait/LinearizationTest.js b/test/Trait/LinearizationTest.js index 3906bd1..579751b 100644 --- a/test/Trait/LinearizationTest.js +++ b/test/Trait/LinearizationTest.js @@ -119,5 +119,85 @@ require( 'common' ).testCase( this.assertStrictEqual( C.use( T )().foo(), expected ); }, + + + /** + * Similar in spirit to the previous test: a supertype with a mixin + * should be treated just as any other class. + * + * Another way of phrasing this test is: "traits are stackable". + * Importantly, this also means that `virtual' must play nicely with + * `abstract override'. + */ + 'Mixin overriding another mixin method M has super method M': function() + { + var called = {}; + + var I = this.Interface( { foo: [] } ); + + var Ta = this.Sut.implement( I ).extend( + { + 'virtual abstract override foo': function() + { + called.a = true; + this.__super(); + }, + } ); + + var Tb = this.Sut.implement( I ).extend( + { + 'abstract override foo': function() + { + called.b = true; + this.__super(); + }, + } ); + + this.Class.implement( I ).extend( + { + 'virtual foo': function() { called.base = true; }, + } ).use( Ta ).use( Tb )().foo(); + + this.assertOk( called.a ); + this.assertOk( called.b ); + this.assertOk( called.base ); + }, + + + /** + * Essentially the same as the above test, but ensures that a mixin can + * be stacked multiple times atop of itself with no ill effects. We + * assume that all else is working (per the previous test). + * + * The number of times we stack the mixin is not really relevant, so + * long as it is >= 2; we did 3 here just for the hell of it to + * demonstrate that there is ideally no limit. + */ + 'Mixin can be mixed in atop of itself': function() + { + var called = 0, + calledbase = false; + + var I = this.Interface( { foo: [] } ); + + var T = this.Sut.implement( I ).extend( + { + 'virtual abstract override foo': function() + { + called++; + this.__super(); + }, + } ); + + this.Class.implement( I ).extend( + { + 'virtual foo': function() { calledbase = true; }, + } ).use( T ).use( T ).use( T )().foo(); + + + // mixed in thrice, so it should have stacked thrice + this.assertEqual( called, 3 ); + this.assertOk( calledbase ); + }, } ); From 55e993a74d1d0a7657c9f3e8864a71ac04fa34da Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 7 Mar 2014 01:07:46 -0500 Subject: [PATCH 39/52] Non-private properties now expressly prohibited in trait dfns :O What?! See Trait/PropertyTest for more information on why this is the case, at least for now. --- lib/Trait.js | 31 ++++++++++++++++ test/Trait/PropertyTest.js | 73 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 test/Trait/PropertyTest.js diff --git a/lib/Trait.js b/lib/Trait.js index 279297e..31de351 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -118,6 +118,10 @@ Trait.extend = function( dfn ) throw Error( "Traits may not define __construct" ); } + // traits have property restrictions + validateProps( tclass.___$$props$$['public'] ); + validateProps( tclass.___$$props$$['protected'] ); + // invoked to trigger mixin TraitType.__mixin = function( dfn, tc, base ) { @@ -134,6 +138,33 @@ Trait.extend = function( dfn ) }; +/** + * Throws error if non-internal property is found within PROPS + * + * For details and rationale, see the Trait/PropertyTest case. + * + * @param {Object} props properties to prohibit + * + * @return {undefined} + */ +function validateProps( props ) +{ + for ( var f in props ) + { + // ignore internal properties + if ( f.substr( 0, 3 ) === '___' ) + { + continue; + } + + throw Error( + "Cannot define property `" + f + "'; only private " + + "properties are permitted within Trait definitions" + ); + } +} + + Trait.implement = function() { var ifaces = arguments; diff --git a/test/Trait/PropertyTest.js b/test/Trait/PropertyTest.js new file mode 100644 index 0000000..f288841 --- /dev/null +++ b/test/Trait/PropertyTest.js @@ -0,0 +1,73 @@ +/** + * Tests trait properties + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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 . + * + * Or, rather, lack thereof, at least for the time being---this is something + * that is complicated by pre-ES5 fallback and, while a solution is + * possible, it is not performant in the case of a fallback and would muddy + * up ease.js' code. + */ + +require( 'common' ).testCase( +{ + caseSetUp: function() + { + this.Sut = this.require( 'Trait' ); + }, + + + /** + * Since private properties cannot be accessed by anything besides the + * trait itself, they cannot interfere with anything else and should be + * permitted. Indeed, it would be obsurd to think otherwise, since the + * trait should be able to maintain its own local state. + */ + 'Private trait properties are permitted': function() + { + var Sut = this.Sut; + this.assertDoesNotThrow( function() + { + Sut( { 'private _foo': 'bar' } ); + } ); + }, + + + /** + * See the description at the top of this file. This is something that + * may be addressed in future releases. + * + * Rather than simply ignoring them, we should notify the user that + * their code is not going to work as intended and prevent bugs + * associated with it. + */ + 'Public and protected trait properties are prohibited': function() + { + var Sut = this.Sut; + + this.assertThrows( function() + { + Sut( { 'public foo': 'bar' } ); + } ); + + this.assertThrows( function() + { + Sut( { 'protected foo': 'bar' } ); + } ); + }, +} ); From 433dd4fb7a15f60c5f711170fb45610573f3d55e Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 7 Mar 2014 01:55:46 -0500 Subject: [PATCH 40/52] Trait definition and mixin performance test cases Does not yet include many more detailed tests, such as method invocation times, which will be of particular interest. While definitions are indeed interesting, they often occur when a program is loading---when the user is expecting to wait. Not so for method invocations. --- test/perf/perf-trait-define.js | 40 ++++++++++ test/perf/perf-trait-methods.js | 126 ++++++++++++++++++++++++++++++++ test/perf/perf-trait-mixin.js | 119 ++++++++++++++++++++++++++++++ 3 files changed, 285 insertions(+) create mode 100644 test/perf/perf-trait-define.js create mode 100644 test/perf/perf-trait-methods.js create mode 100644 test/perf/perf-trait-mixin.js diff --git a/test/perf/perf-trait-define.js b/test/perf/perf-trait-define.js new file mode 100644 index 0000000..f4d6af7 --- /dev/null +++ b/test/perf/perf-trait-define.js @@ -0,0 +1,40 @@ +/** + * Tests amount of time taken to declare N anonymous traits + * + * Copyright (C) 2010, 2011, 2013 Mike Gerwitz + * + * 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 . + * + * Contrast with respective class test. + */ + +var common = require( __dirname + '/common.js' ), + Trait = common.require( 'Trait' ), + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Trait( {} ); + } + +}, count, 'Declare ' + count + ' empty anonymous traits' ); diff --git a/test/perf/perf-trait-methods.js b/test/perf/perf-trait-methods.js new file mode 100644 index 0000000..4a03efd --- /dev/null +++ b/test/perf/perf-trait-methods.js @@ -0,0 +1,126 @@ +/** + * Tests amount of time taken defining and invoking methods passing through + * traits + * + * Copyright (C) 2010, 2011, 2013 Mike Gerwitz + * + * 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 . + * + * Contrast with respective class test. + */ + +var common = require( __dirname + '/common.js' ), + Trait = common.require( 'Trait' ), + Class = common.require( 'class' ), + Interface = common.require( 'interface' ), + + count = 1000 +; + + +var I = Interface( +{ + a: [], + b: [], + c: [], +} ); + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Trait( + { + a: function() {}, + b: function() {}, + c: function() {}, + } ); + } + +}, count, +'Declare ' + count + ' empty anonymous traits with few concrete methods' ); + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Trait( + { + 'virtual a': function() {}, + 'virtual b': function() {}, + 'virtual c': function() {}, + } ); + } + +}, count, +'Declare ' + count + ' empty anonymous traits with few virtual methods' ); + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Trait( + { + 'abstract a': [], + 'abstract b': [], + 'abstract c': [], + } ); + } + +}, count, +'Declare ' + count + ' empty anonymous traits with few abstract methods' ); + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Trait.implement( I ).extend( {} ); + } + +}, count, +'Declare ' + count + ' empty anonymous traits implementing interface ' + + 'with few methods' ); + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Trait.implement( I ).extend( + { + 'abstract override a': function() {}, + 'abstract override b': function() {}, + 'abstract override c': function() {}, + } ); + } + +}, count, +'Declare ' + count + ' empty anonymous traits with few ' + + 'abstract overrides, implementing interface' ); diff --git a/test/perf/perf-trait-mixin.js b/test/perf/perf-trait-mixin.js new file mode 100644 index 0000000..23d0200 --- /dev/null +++ b/test/perf/perf-trait-mixin.js @@ -0,0 +1,119 @@ +/** + * Tests amount of time taken to declare N classes mixing in traits of + * various sorts + * + * Copyright (C) 2010, 2011, 2013 Mike Gerwitz + * + * 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 . + * + * Contrast with respective class test. + */ + +var common = require( __dirname + '/common.js' ), + Trait = common.require( 'Trait' ), + Class = common.require( 'class' ), + Interface = common.require( 'interface' ), + + count = 1000 +; + +// we don't care about declare time; we're testing mixin time +var Te = Trait( {} ); + +var Tv = Trait( +{ + 'virtual a': function() {}, + 'virtual b': function() {}, + 'virtual c': function() {}, +} ); + +var I = Interface( +{ + a: [], + b: [], + c: [], +} ); +var Cv = Class.implement( I ).extend( +{ + 'virtual a': function() {}, + 'virtual b': function() {}, + 'virtual c': function() {}, +} ); + +var To = Trait.implement( I ).extend( +{ + 'virtual abstract override a': function() {}, + 'virtual abstract override b': function() {}, + 'virtual abstract override c': function() {}, +} ); + + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + // extend to force lazy mixin + Class.use( Te ).extend( {} ); + } + +}, count, 'Mix in ' + count + ' empty traits' ); + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + // extend to force lazy mixin + Class.use( Tv ).extend( {} ); + } + +}, count, 'Mix in ' + count + ' traits with few virtual methods' ); + + +// now override 'em +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class.use( Tv ).extend( + { + 'override virtual a': function() {}, + 'override virtual b': function() {}, + 'override virtual c': function() {}, + } ); + } + +}, count, 'Mix in and override ' + count + + ' traits with few virtual methods' ); + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Cv.use( To ).extend( {} ); + } + +}, count, 'Mix in trait that overrides class methods' ); From a7e381a31ec583b8ba3305b30cf3a0db0bb4c590 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 8 Mar 2014 03:55:48 -0500 Subject: [PATCH 41/52] Mixing method invocation performance tests As expected, mixin method invocation is dramatically slower than conventional class method definitions. However, it is a bit slower than I had anticipated; future releases will definately need to take a look at improving performance, which should happen anyway, since the trait implementation takes the easy way out in a number of instances. Let's get an initial release first. --- test/perf/perf-trait-invoke-method.js | 170 ++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 test/perf/perf-trait-invoke-method.js diff --git a/test/perf/perf-trait-invoke-method.js b/test/perf/perf-trait-invoke-method.js new file mode 100644 index 0000000..948a3a0 --- /dev/null +++ b/test/perf/perf-trait-invoke-method.js @@ -0,0 +1,170 @@ +/** + * Tests amount of time taken to apply trait (mixin) methods + * + * Copyright (C) 2014 Mike Gerwitz + * + * 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 . + * + * Contrast with respective class tests. + * + * Note that there is a lot of code duplication; this is to reduce + * unnecessary lookups for function invocation to gain a more accurate + * estimate of invocation time (e.g., no foo[ bar ]()). + * + * Traits are expected to be considerably slower than conventional classes + * due to their very nature---dynamically bound methods. This should not be + * alarming under most circumstances, as the method invocation is still + * likely much faster than any serious logic contained within the method; + * however, performance issues could manifest when recursing heavily, so be + * cognisant of such. + * + * There is, at least at the time of writing this message, much room for + * optimization in the trait implementation. + */ + +var common = require( __dirname + '/common.js' ), + Trait = common.require( 'Trait' ), + Class = common.require( 'class' ), + Interface = common.require( 'interface' ), + + // misc. var used to ensure that v8 does not optimize away empty + // functions + x = 0, + + // method invocation is pretty speedy, so we need a lot of iterations + count = 500000 +; + +// objects should be pre-created; we don't care about the time taken to +// instantiate them +var T = Trait( +{ + 'public pub': function() { x++ }, + 'protected prot': function() { x++ }, + + 'virtual public vpub': function() { x++ }, + 'virtual public vprot': function() { x++ }, + + 'virtual public vopub': function() { x++ }, + 'virtual public voprot': function() { x++ }, +} ); + +var I = Interface( +{ + 'public aopub': [], + 'public vaopub': [], +} ); + +var Ta = Trait.implement( I ).extend( +{ + // TODO: protected once we support extending classes + 'abstract public override aopub': function() { x++ }, + 'virtual abstract public override vaopub': function() { x++ }, +} ); + +var o = Class.use( T ).extend( +{ + // overrides T mixin + 'override public vopub': function() { x++ }, + 'override public voprot': function() { x++ }, + + // overridden by Ta mixin + 'virtual public aopub': function() { x++ }, + 'virtual public vaopub': function() { x++ }, + + + 'public internalTest': function() + { + var _self = this; + + common.test( function() + { + var i = count; + while ( i-- ) _self.pub(); + }, count, "Invoke public mixin method internally" ); + + + common.test( function() + { + var i = count; + while ( i-- ) _self.prot(); + }, count, "Invoke protected mixin method internally" ); + + vtest( this, "internally" ); + }, +} ).use( Ta )(); + + +common.test( function() +{ + var i = count; + while ( i-- ) + { + o.pub(); + } +}, count, "Invoke public mixin method externally" ); + + +// run applicable external virtual tests +vtest( o, "externally" ); + + +function vtest( context, s ) +{ + common.test( function() + { + var i = count; + while ( i-- ) context.vpub(); + }, count, "Invoke public virtual mixin method " + s ); + + common.test( function() + { + var i = count; + while ( i-- ) context.vopub(); + }, count, "Invoke public overridden virtual mixin method " + s ); + + common.test( function() + { + var i = count; + while ( i-- ) context.aopub(); + }, count, "Invoke public abstract override mixin method " + s ); + + common.test( function() + { + var i = count; + while ( i-- ) context.vaopub(); + }, count, "Invoke public virtual abstract override mixin method " + s ); + + + if ( !( context.vprot ) ) return; + + + common.test( function() + { + var i = count; + while ( i-- ) context.vprot(); + }, count, "Invoke protected virtual mixin method " + s ); + + common.test( function() + { + var i = count; + while ( i-- ) context.voprot(); + }, count, "Invoke protected overridden virtual mixin method " + s ); +} + + +// run tests internally +o.internalTest(); From 696b8d05a6ff6d0a1d96f4f3e1dd2fe6ff3aafb3 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 8 Mar 2014 23:52:47 -0500 Subject: [PATCH 42/52] Class definition mixin now requires explicit extend See the rather verbose docblocks in this diff for more information. Additional rationale will be contained in the commits that follow. --- lib/class.js | 92 +++++++++++++++++++++++++++++++--- test/Trait/ClassVirtualTest.js | 4 +- test/Trait/DefinitionTest.js | 51 ++++++++++++++++++- test/Trait/ImmediateTest.js | 5 +- 4 files changed, 139 insertions(+), 13 deletions(-) diff --git a/lib/class.js b/lib/class.js index 6454a2c..69d8553 100644 --- a/lib/class.js +++ b/lib/class.js @@ -45,6 +45,8 @@ var util = require( __dirname + '/util' ), ) ; +var _nullf = function() { return null; } + /** * This module may be invoked in order to provide a more natural looking class @@ -120,11 +122,25 @@ module.exports.implement = function( interfaces ) }; +/** + * Mix a trait into a class + * + * The ultimate intent of this depends on the ultimate `extend' call---if it + * extends another class, then the traits will be mixed into that class; + * otherwise, the traits will be mixed into the base class. In either case, + * a final `extend' call is necessary to complete the definition. An attempt + * to instantiate the return value before invoking `extend' will result in + * an exception. + * + * @param {Array.} traits traits to mix in + * + * @return {Function} staging object for class definition + */ module.exports.use = function( traits ) { // consume traits onto an empty base return createUse( - null, + _nullf, Array.prototype.slice.call( arguments ) ); }; @@ -304,7 +320,7 @@ function createStaging( cname ) use: function() { return createUse( - null, + _nullf, Array.prototype.slice.call( arguments ) ); }, @@ -373,27 +389,79 @@ function createImplement( base, ifaces, cname ) def ); }, + + use: function() + { + var traits = Array.prototype.slice.call( arguments ); + return createUse( + function() { return base; }, + traits + ); + }, }; } -function createUse( base, traits ) +/** + * Create a staging object representing an eventual mixin + * + * This staging objects prepares a class definition for trait mixin. In + * particular, the returned staging object has the following features: + * - invoking it will, if mixing into an existing (non-base) class without + * subclassing, immediately complete the mixin and instantiate the + * generated class; + * - calling `use' has the effect of chaining mixins, stacking them atop + * of one-another; and + * - invoking `extend' will immediately complete the mixin, resulting in a + * subtype of the base. + * + * Mixins are performed lazily---the actual mixin will not take place until + * the final `extend' call, which may be implicit by invoking the staging + * object (performing instantiation). + * + * The third argument determines whether or not a final `extend' call must + * be explicit: in this case, any instantiation attempts will result in an + * exception being thrown. + * + * @param {function()} basef returns base from which to lazily + * extend + * @param {Array.} traits traits to mix in + * @param {boolean} nonbase extending from a non-base class + * (setting will permit instantiation + * with implicit extend) + * + * @return {Function} staging object for mixin + */ +function createUse( basef, traits, nonbase ) { // invoking the partially applied class will immediately complete its // definition and instantiate it with the provided constructor arguments var partial = function() { - return createMixedClass( base, traits ) + // this argument will be set only in the case where an existing + // (non-base) class is extended, meaning that an explict Class or + // AbstractClass was not provided + if ( !( nonbase ) ) + { + throw TypeError( + "Cannot instantiate incomplete class definition; did " + + "you forget to call `extend'?" + ); + } + + return createMixedClass( basef(), traits ) .apply( null, arguments ); }; + // otherwise, its definition is deferred until additional context is // given during the extend operation partial.extend = function() { var args = Array.prototype.slice.call( arguments ), dfn = args.pop(), - ext_base = args.pop(); + ext_base = args.pop(), + base = basef(); // extend the mixed class, which ensures that all super references // are properly resolved @@ -407,7 +475,14 @@ function createUse( base, traits ) // call simply to mix in another trait partial.use = function() { - return partial.extend( {} ).use.apply( null, arguments ); + return createUse( + function() + { + return partial.extend( {} ) + }, + Array.prototype.slice.call( arguments ), + nonbase + ); }; return partial; @@ -609,8 +684,9 @@ function attachUse( func ) util.defineSecureProp( func, 'use', function() { return createUse( - func, - Array.prototype.slice.call( arguments ) + function() { return func; }, + Array.prototype.slice.call( arguments ), + true ); } ); } diff --git a/test/Trait/ClassVirtualTest.js b/test/Trait/ClassVirtualTest.js index 36e1613..49dcf6f 100644 --- a/test/Trait/ClassVirtualTest.js +++ b/test/Trait/ClassVirtualTest.js @@ -128,7 +128,7 @@ require( 'common' ).testCase( { // should invoke concrete foo; class definition should not fail, // because foo is no longer abstract - Class.use( T )().foo(); + Class.use( T ).extend( {} )().foo(); } ); this.assertOk( called ); @@ -148,7 +148,7 @@ require( 'common' ).testCase( T = this.Sut.implement( I ).extend( {} ); this.assertOk( - this.Class.isA( I, this.Class.use( T )() ) + this.Class.isA( I, this.Class.use( T ).extend( {} )() ) ); }, diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index da9eb67..5373630 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -23,8 +23,10 @@ require( 'common' ).testCase( { caseSetUp: function() { - this.Sut = this.require( 'Trait' ); - this.Class = this.require( 'class' ); + this.Sut = this.require( 'Trait' ); + this.Class = this.require( 'class' ); + this.Interface = this.require( 'interface' ); + this.AbstractClass = this.require( 'class_abstract' ); // means of creating anonymous traits this.ctor = [ @@ -282,6 +284,51 @@ require( 'common' ).testCase( }, + /** + * When explicitly defining a class (that is, not mixing into an + * existing class definition), which involves the use of Class or + * AbstractClass, mixins must be terminated with a call to `extend'. + * This allows the system to make a final determination as to whether + * the resulting class is abstract. + * + * Contrast this with Type.use( T )( ... ), where Type is not the base + * class (Class) or AbstractClass. + */ + 'Explicit class definitions must be terminated by an extend call': + function() + { + var _self = this, + Ta = this.Sut( { foo: function() {} } ), + Tb = this.Sut( { bar: function() {} } ); + + // does not complete with call to `extend' + this.assertThrows( function() + { + _self.Class.use( Ta )(); + }, TypeError ); + + // nested uses; does not complete + this.assertThrows( function() + { + _self.Class.use( Ta ).use( Tb )(); + }, TypeError ); + + // similar to above, with abstract; note that we're checking for + // TypeError here + this.assertThrows( function() + { + _self.AbstractClass.use( Ta )(); + }, TypeError ); + + // does complete; OK + this.assertDoesNotThrow( function() + { + _self.Class.use( Ta ).extend( {} )(); + _self.Class.use( Ta ).use( Tb ).extend( {} )(); + } ); + }, + + /** * When a trait is mixed into a class, it acts as though it is part of * that class. Therefore, it should stand to reason that, when a mixed diff --git a/test/Trait/ImmediateTest.js b/test/Trait/ImmediateTest.js index 5aaa595..5a730f9 100644 --- a/test/Trait/ImmediateTest.js +++ b/test/Trait/ImmediateTest.js @@ -35,6 +35,9 @@ require( 'common' ).testCase( * of the trait (so long as that is permitted). While this test exists * to ensure consistency throughout the system, it may be helpful in * situations where a trait is useful on its own. + * + * Note that we cannot simply use Class.use( T ), because this sets up a + * concrete class definition, not an immediate mixin. */ 'Invoking partial class after mixin instantiates': function() { @@ -49,7 +52,7 @@ require( 'common' ).testCase( } ); // mixes T into an empty base class and instantiates - this.Class.use( T )().foo(); + this.Class.extend( {} ).use( T )().foo(); this.assertOk( called ); }, From 255a60e4255fdc2a0413071799918966c8bf0c3f Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 9 Mar 2014 21:13:11 -0400 Subject: [PATCH 43/52] Implemented and abstract with mixins properly handled Classes will now properly be recognized as concrete or abstract when mixing in any number of traits, optionally in conjunction with interfaces. --- lib/class.js | 22 +++++- lib/class_abstract.js | 17 ++++- test/Trait/AbstractTest.js | 136 +++++++++++++++++++++++++++++++++++ test/Trait/DefinitionTest.js | 39 ++++++++++ 4 files changed, 210 insertions(+), 4 deletions(-) diff --git a/lib/class.js b/lib/class.js index 69d8553..ec6d6b8 100644 --- a/lib/class.js +++ b/lib/class.js @@ -346,7 +346,7 @@ function createImplement( base, ifaces, cname ) { // Defer processing until after extend(). This also ensures that implement() // returns nothing usable. - return { + var partial = { extend: function() { var args = Array.prototype.slice.call( arguments ), @@ -390,15 +390,25 @@ function createImplement( base, ifaces, cname ) ); }, + // TODO: this is a naive implementation that works, but could be + // much more performant (it creates a subtype before mixing in) use: function() { var traits = Array.prototype.slice.call( arguments ); return createUse( - function() { return base; }, + function() { return partial.__createBase(); }, traits ); }, + + // allows overriding default behavior + __createBase: function() + { + return partial.extend( {} ); + }, }; + + return partial; } @@ -478,13 +488,19 @@ function createUse( basef, traits, nonbase ) return createUse( function() { - return partial.extend( {} ) + return partial.__createBase(); }, Array.prototype.slice.call( arguments ), nonbase ); }; + // allows overriding default behavior + partial.__createBase = function() + { + return partial.extend( {} ); + }; + return partial; } diff --git a/lib/class_abstract.js b/lib/class_abstract.js index c52df72..4654cb6 100644 --- a/lib/class_abstract.js +++ b/lib/class_abstract.js @@ -122,7 +122,8 @@ function markAbstract( args ) function abstractOverride( obj ) { var extend = obj.extend, - impl = obj.implement; + impl = obj.implement, + use = obj.use; // wrap and apply the abstract flag, only if the method is defined (it // may not be under all circumstances, e.g. after an implement()) @@ -131,6 +132,12 @@ function abstractOverride( obj ) return abstractOverride( impl.apply( this, arguments ) ); } ); + var mixin = false; + use && ( obj.use = function() + { + return abstractOverride( use.apply( this, arguments ) ); + } ); + // wrap extend, applying the abstract flag obj.extend = function() { @@ -138,6 +145,14 @@ function abstractOverride( obj ) return extend.apply( this, arguments ); }; + // used by mixins; we need to mark the intermediate subtype as abstract, + // but ensure we don't throw any errors if no abstract members are mixed + // in (since thay may be mixed in later on) + obj.__createBase = function() + { + return extend( { ___$$auto$abstract$$: true } ); + }; + return obj; } diff --git a/test/Trait/AbstractTest.js b/test/Trait/AbstractTest.js index 742b5df..9f59e32 100644 --- a/test/Trait/AbstractTest.js +++ b/test/Trait/AbstractTest.js @@ -224,4 +224,140 @@ require( 'common' ).testCase( C().doFoo(); this.assertOk( called ); }, + + + /** + * Ensure that chained mixins (that is, calling `use' multiple times + * independently) maintains the use of AbstractClass, and properly + * performs the abstract check at the final `extend' call. + */ + 'Chained mixins properly carry abstract flag': function() + { + var _self = this, + Ta = this.Sut( { foo: function() {} } ), + Tc = this.Sut( { baz: function() {} } ), + Tab = this.Sut( { 'abstract baz': [] } ); + + // ensure that abstract definitions are carried through properly + this.assertDoesNotThrow( function() + { + // single, abstract + _self.assertOk( + _self.AbstractClass + .use( Tab ) + .extend( {} ) + .isAbstract() + ); + + // single, concrete + _self.assertOk( + _self.AbstractClass + .use( Ta ) + .extend( { 'abstract baz': [] } ) + .isAbstract() + ); + + // chained, both + _self.assertOk( + _self.AbstractClass + .use( Ta ) + .use( Tab ) + .extend( {} ) + .isAbstract() + + ); + _self.assertOk( + _self.AbstractClass + .use( Tab ) + .use( Ta ) + .extend( {} ) + .isAbstract() + ); + } ); + + // and then ensure that we will properly throw an exception if not + this.assertThrows( function() + { + // not abstract + _self.AbstractClass.use( Tc ).extend( {} ); + } ); + + this.assertThrows( function() + { + // initially abstract, but then not (by extend) + _self.AbstractClass.use( Tab ).extend( + { + // concrete definition; no longer abstract + baz: function() {}, + } ); + } ); + + this.assertThrows( function() + { + // initially abstract, but then second mix provides a concrete + // definition + _self.AbstractClass.use( Tab ).use( Tc ).extend( {} ); + } ); + }, + + + /** + * Mixins can make a class auto-abstract (that is, not require the use + * of AbstractClass for the mixin) in order to permit the use of + * Type.use when the intent is not to subclass, but to decorate (yes, + * the result is still a subtype). Let's make sure that we're not + * breaking the AbstractClass requirement, whose sole purpose is to aid + * in documentation by creating self-documenting code. + */ + 'Explicitly-declared class will not be automatically abstract': + function() + { + var _self = this, + Tc = this.Sut( { foo: function() {} } ), + Ta = this.Sut( { 'abstract foo': [], } ); + + // if we provide no abstract methods, then declaring the class as + // abstract should result in an error + this.assertThrows( function() + { + // no abstract methods + _self.assertOk( !( + _self.AbstractClass.use( Tc ).extend( {} ).isAbstract() + ) ); + } ); + + // similarily, if we provide abstract methods, then there should be + // no error + this.assertDoesNotThrow( function() + { + // abstract methods via extend + _self.assertOk( + _self.AbstractClass.use( Tc ).extend( + { + 'abstract bar': [], + } ).isAbstract() + ); + + // abstract via trait + _self.assertOk( + _self.AbstractClass.use( Ta ).extend( {} ).isAbstract() + ); + } ); + + // if we provide abstract methods, then we should not be able to + // declare a class as concrete + this.assertThrows( function() + { + _self.Class.use( Tc ).extend( + { + 'abstract bar': [], + } ); + } ); + + // similar to above, but via trait + this.assertThrows( function() + { + _self.Class.use( Ta ).extend(); + } ); + }, } ); diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index 5373630..465f0a4 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -59,6 +59,8 @@ require( 'common' ).testCase( { 'protected foo': function() {} }, ], ]; + + this.base = [ this.Class ]; }, @@ -329,6 +331,43 @@ require( 'common' ).testCase( }, + /** + * Ensure that the staging object created by the `implement' call + * exposes a `use' method (and properly applies it). + */ + 'Can mix traits into class after implementing interface': function() + { + var _self = this, + called = false, + + T = this.Sut( { foo: function() { called = true; } } ), + I = this.Interface( { bar: [] } ), + A = null; + + // by declaring this abstract, we ensure that the interface was + // actually implemented (otherwise, all methods would be concrete, + // resulting in an error) + this.assertDoesNotThrow( function() + { + A = _self.AbstractClass.implement( I ).use( T ).extend( {} ); + _self.assertOk( A.isAbstract() ); + } ); + + // ensure that we actually fail if there's no interface implemented + // (and thus no abstract members); if we fail and the previous test + // succeeds, that implies that somehow the mixin is causing the + // class to become abstract, and that is an issue (and the reason + // for this seemingly redundant test) + this.assertThrows( function() + { + _self.Class.implement( I ).use( T ).extend( {} ); + } ); + + A.extend( { bar: function() {} } )().foo(); + this.assertOk( called ); + }, + + /** * When a trait is mixed into a class, it acts as though it is part of * that class. Therefore, it should stand to reason that, when a mixed From c5b6562d349f2dc8b6e279b665a296fca90521f4 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 10 Mar 2014 00:38:25 -0400 Subject: [PATCH 44/52] Mention of "Traits as mixins" in README.md and manual No documentation yet; this is planned for the 0.2.1 release to prevent further delays of the first GNU release. --- README.md | 1 + doc/about.texi | 1 + 2 files changed, 2 insertions(+) diff --git a/README.md b/README.md index 2e4f12a..d900169 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ Current support includes: * Classical inheritance * Abstract classes and methods * Interfaces +* Traits as mixins * Visibility (public, protected, and private members) * Static and constant members diff --git a/doc/about.texi b/doc/about.texi index 15d4660..1f98080 100644 --- a/doc/about.texi +++ b/doc/about.texi @@ -20,6 +20,7 @@ Current support includes: @item Classical inheritance @item Abstract classes and methods @item Interfaces +@item Traits as mixins @item Visibility (public, protected, and private members) @item Static, constant, and final members @end itemize From 7d27bc7969ed2b72eab7c7e5a4f75dcc7057b374 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 10 Mar 2014 00:40:26 -0400 Subject: [PATCH 45/52] Exposing Trait module Oh boy oh boy oh boy! --- index.js | 1 + test/IndexTest.js | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/index.js b/index.js index 3edd1c3..ad7069e 100644 --- a/index.js +++ b/index.js @@ -23,5 +23,6 @@ exports.Class = require( __dirname + '/lib/class' ); exports.AbstractClass = require( __dirname + '/lib/class_abstract' ); exports.FinalClass = require( __dirname + '/lib/class_final' ); exports.Interface = require( __dirname + '/lib/interface' ); +exports.Trait = require( __dirname + '/lib/Trait' ); exports.version = require( __dirname + '/lib/version' ); diff --git a/test/IndexTest.js b/test/IndexTest.js index a1b22a7..6abe16e 100644 --- a/test/IndexTest.js +++ b/test/IndexTest.js @@ -55,6 +55,10 @@ require( 'common' ).testCase( { this.exportedAs( 'Interface', 'interface' ); }, + 'Trait module is exported as `Trait\'': function() + { + this.exportedAs( 'Trait', 'Trait' ); + }, 'Version information is exported as `version\'': function() { this.exportedAs( 'version', 'version' ); From 3d474430464ff39bee817ce6af8851896e632383 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 10 Mar 2014 01:34:31 -0400 Subject: [PATCH 46/52] Refactored ClassBuilder.buildMembers (dynamic prop parse context) The parser methods are now split into their own functions. This has a number of benefits: The most immediate is the commit that will follow. The second benefit is that the function is no longer a closure---all context information is passed into it, and so it can be optimized by the JavaScript engine accordingly. --- lib/ClassBuilder.js | 282 ++++++++++++++++++++----------------- lib/util.js | 10 +- test/Util/PropParseTest.js | 42 ++++++ 3 files changed, 199 insertions(+), 135 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 5108227..b351dd6 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -28,6 +28,9 @@ var util = require( __dirname + '/util' ), warn = require( __dirname + '/warn' ), Warning = warn.Warning, + hasOwn = Object.prototype.hasOwnProperty, + + /** * IE contains a nasty enumeration "bug" (poor implementation) that makes * toString unenumerable. This means that, if you do obj.toString = foo, @@ -462,135 +465,157 @@ exports.prototype.buildMembers = function buildMembers( props, class_id, base, prop_init, memberdest, staticInstLookup ) { - var hasOwn = Array.prototype.hasOwnProperty, - defs = {}, + var context = { + _cb: this, + + // arguments + prop_init: prop_init, + class_id: class_id, + base: base, + staticInstLookup: staticInstLookup, + + defs: {}, + + // holds member builder state + state: {}, // TODO: there does not seem to be tests for these guys; perhaps // this can be rectified with the reflection implementation - members = memberdest.all, - abstract_methods = memberdest['abstract'], - static_members = memberdest['static'], - virtual_members = memberdest['virtual'], - - smethods = static_members.methods, - sprops = static_members.props, - - // holds member builder state - state = {}, - - _self = this - ; + members: memberdest.all, + abstract_methods: memberdest['abstract'], + static_members: memberdest['static'], + virtual_members: memberdest['virtual'], + }; util.propParse( props, { - each: function( name, value, keywords ) - { - // disallow use of our internal __initProps() method - if ( reserved_members[ name ] === true ) - { - throw Error( name + " is reserved" ); - } - - // if a member was defined multiple times in the same class - // declaration, throw an error (unless the `weak' keyword is - // provided, which exists to accomodate this situation) - if ( hasOwn.call( defs, name ) - && !( keywords['weak'] || defs[ name ].weak ) - ) - { - throw Error( - "Cannot redefine method '" + name + "' in same declaration" - ); - } - - // keep track of the definitions (only during class declaration) - // to catch duplicates - defs[ name ] = keywords; - }, - - property: function( name, value, keywords ) - { - var dest = ( keywordStatic( keywords ) ) ? sprops : prop_init; - - // build a new property, passing in the other members to compare - // against for preventing nonsensical overrides - _self._memberBuilder.buildProp( - dest, null, name, value, keywords, base - ); - }, - - getset: function( name, get, set, keywords ) - { - var dest = ( keywordStatic( keywords ) ) ? smethods : members, - is_static = keywordStatic( keywords ), - instLookup = ( ( is_static ) - ? staticInstLookup - : exports.getMethodInstance - ); - - _self._memberBuilder.buildGetterSetter( - dest, null, name, get, set, keywords, instLookup, class_id, base - ); - }, - - method: function( name, func, is_abstract, keywords ) - { - var is_static = keywordStatic( keywords ), - dest = ( is_static ) ? smethods : members, - instLookup = ( is_static ) - ? staticInstLookup - : exports.getMethodInstance - ; - - // constructor check - if ( public_methods[ name ] === true ) - { - if ( keywords[ 'protected' ] || keywords[ 'private' ] ) - { - throw TypeError( - name + " must be public" - ); - } - } - - var used = _self._memberBuilder.buildMethod( - dest, null, name, func, keywords, instLookup, - class_id, base, state - ); - - // do nothing more if we didn't end up using this definition - // (this may be the case, for example, with weak members) - if ( !used ) - { - return; - } - - // note the concrete method check; this ensures that weak - // abstract methods will not count if a concrete method of the - // smae name has already been seen - if ( is_abstract ) - { - abstract_methods[ name ] = true; - abstract_methods.__length++; - } - else if ( ( hasOwn.call( abstract_methods, name ) ) - && ( is_abstract === false ) - ) - { - // if this was a concrete method, then it should no longer - // be marked as abstract - delete abstract_methods[ name ]; - abstract_methods.__length--; - } - - if ( keywords['virtual'] ) - { - virtual_members[ name ] = true; - } - }, - } ); + each: _parseEach, + property: _parseProp, + getset: _parseGetSet, + method: _parseMethod, + }, context ); // process accumulated member state - this._memberBuilder.end( state ); + this._memberBuilder.end( context.state ); +} + + +function _parseEach( name, value, keywords ) +{ + var defs = this.defs; + + // disallow use of our internal __initProps() method + if ( reserved_members[ name ] === true ) + { + throw Error( name + " is reserved" ); + } + + // if a member was defined multiple times in the same class + // declaration, throw an error (unless the `weak' keyword is + // provided, which exists to accomodate this situation) + if ( hasOwn.call( defs, name ) + && !( keywords['weak'] || defs[ name ].weak ) + ) + { + throw Error( + "Cannot redefine method '" + name + "' in same declaration" + ); + } + + // keep track of the definitions (only during class declaration) + // to catch duplicates + defs[ name ] = keywords; +} + + +function _parseProp( name, value, keywords ) +{ + var dest = ( keywordStatic( keywords ) ) + ? this.static_members.props + : this.prop_init; + + // build a new property, passing in the other members to compare + // against for preventing nonsensical overrides + this._cb._memberBuilder.buildProp( + dest, null, name, value, keywords, this.base + ); +} + + +function _parseGetSet( name, get, set, keywords ) +{ + var dest = ( keywordStatic( keywords ) ) + ? this.static_members.methods + : this.members, + + is_static = keywordStatic( keywords ), + instLookup = ( ( is_static ) + ? this.staticInstLookup + : exports.getMethodInstance + ); + + this._cb._memberBuilder.buildGetterSetter( + dest, null, name, get, set, keywords, instLookup, + this.class_id, this.base + ); +} + + +function _parseMethod( name, func, is_abstract, keywords ) +{ + var is_static = keywordStatic( keywords ), + dest = ( is_static ) + ? this.static_members.methods + : this.members, + instLookup = ( is_static ) + ? this.staticInstLookup + : exports.getMethodInstance + ; + + // constructor check + if ( public_methods[ name ] === true ) + { + if ( keywords[ 'protected' ] || keywords[ 'private' ] ) + { + throw TypeError( + name + " must be public" + ); + } + } + + var used = this._cb._memberBuilder.buildMethod( + dest, null, name, func, keywords, instLookup, + this.class_id, this.base, this.state + ); + + // do nothing more if we didn't end up using this definition + // (this may be the case, for example, with weak members) + if ( !used ) + { + return; + } + + // note the concrete method check; this ensures that weak + // abstract methods will not count if a concrete method of the + // smae name has already been seen + if ( is_abstract ) + { + this.abstract_methods[ name ] = true; + this.abstract_methods.__length++; + } + else if ( ( hasOwn.call( this.abstract_methods, name ) ) + && ( is_abstract === false ) + ) + { + // if this was a concrete method, then it should no longer + // be marked as abstract + delete this.abstract_methods[ name ]; + this.abstract_methods.__length--; + } + + if ( keywords['virtual'] ) + { + this.virtual_members[ name ] = true; + } } @@ -741,9 +766,7 @@ exports.prototype.createConcreteCtor = function( cname, members ) // Provide a more intuitive string representation of the class // instance. If a toString() method was already supplied for us, // use that one instead. - if ( !( Object.prototype.hasOwnProperty.call( - members[ 'public' ], 'toString' - ) ) ) + if ( !( hasOwn.call( members[ 'public' ], 'toString' ) ) ) { // use __toString if available (see enum_bug), otherwise use // our own defaults @@ -991,8 +1014,7 @@ function attachStatic( ctor, members, base, inheriting ) // we use hasOwnProperty to ensure that undefined values will not // cause us to continue checking the parent, thereby potentially // failing to set perfectly legal values - var has = Object.prototype.hasOwnProperty, - found = false, + var found = false, // Determine if we were invoked in the context of a class. If // so, use that. Otherwise, use ourself. @@ -1011,16 +1033,16 @@ function attachStatic( ctor, members, base, inheriting ) // available and we are internal (within a method), we can move on // to check other levels of visibility. `found` will contain the // visibility level the property was found in, or false. - found = has.call( props[ 'public' ], prop ) && 'public'; + found = hasOwn.call( props[ 'public' ], prop ) && 'public'; if ( !found && _self._spropInternal ) { // Check for protected/private. We only check for private // properties if we are not currently checking the properties of // a subtype. This works because the context is passed to each // recursive call. - found = has.call( props[ 'protected' ], prop ) && 'protected' + found = hasOwn.call( props[ 'protected' ], prop ) && 'protected' || !in_subtype - && has.call( props[ 'private' ], prop ) && 'private' + && hasOwn.call( props[ 'private' ], prop ) && 'private' ; } diff --git a/lib/util.js b/lib/util.js index 2cc46f5..868c6c8 100644 --- a/lib/util.js +++ b/lib/util.js @@ -257,7 +257,7 @@ exports.copyTo = function( dest, src, deep ) * * @return undefined */ -exports.propParse = function( data, options ) +exports.propParse = function( data, options, context ) { // todo: profile; function calls are more expensive than if statements, so // it's probably a better idea not to use fvoid @@ -327,13 +327,13 @@ exports.propParse = function( data, options ) // if an 'each' callback was provided, pass the data before parsing it if ( callbackEach ) { - callbackEach.call( callbackEach, name, value, keywords ); + callbackEach.call( context, name, value, keywords ); } // getter/setter if ( getter || setter ) { - callbackGetSet.call( callbackGetSet, + callbackGetSet.call( context, name, getter, setter, keywords ); } @@ -341,7 +341,7 @@ exports.propParse = function( data, options ) else if ( ( typeof value === 'function' ) || ( keywords[ 'proxy' ] ) ) { callbackMethod.call( - callbackMethod, + context, name, value, exports.isAbstractMethod( value ), @@ -351,7 +351,7 @@ exports.propParse = function( data, options ) // simple property else { - callbackProp.call( callbackProp, name, value, keywords ); + callbackProp.call( context, name, value, keywords ); } } }; diff --git a/test/Util/PropParseTest.js b/test/Util/PropParseTest.js index 7257946..6c2910c 100644 --- a/test/Util/PropParseTest.js +++ b/test/Util/PropParseTest.js @@ -210,4 +210,46 @@ require( 'common' ).testCase( propParse( { 'abstract foo': [ 'valid_name' ] }, {} ); }, SyntaxError ); }, + + + /** + * The motivation behind this feature is to reduce the number of closures + * necessary to perform a particular task: this allows binding `this' of the + * handler to a custom context. + */ + 'Supports dynamic context to handlers': function() + { + var _self = this; + context = {}; + + // should trigger all of the handlers + var all = { + prop: 'prop', + method: function() {}, + }; + + // run test on getters/setters only if supported by the environment + if ( this.hasGetSet ) + { + Object.defineProperty( all, 'getset', { + get: ( get = function () {} ), + set: ( set = function () {} ), + + enumerable: true, + } ); + } + + function _chk() + { + _self.assertStrictEqual( this, context ); + } + + // check each supported handler for conformance + this.Sut.propParse( all, { + each: _chk, + property: _chk, + getset: _chk, + method: _chk, + }, context ); + }, } ); From 316a7dd703c3adad727e437450cdea8817ff95a9 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 11 Mar 2014 06:36:45 -0400 Subject: [PATCH 47/52] Refactored Traits to use propParse hooks --- lib/ClassBuilder.js | 39 +++++++++++++++++++++++++--- lib/Trait.js | 63 +++++++++++++++++++++++++++++++-------------- 2 files changed, 80 insertions(+), 22 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index b351dd6..5d29a29 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -487,14 +487,47 @@ exports.prototype.buildMembers = function buildMembers( virtual_members: memberdest['virtual'], }; - util.propParse( props, { + // default member handlers for parser + var handlers = { each: _parseEach, property: _parseProp, getset: _parseGetSet, method: _parseMethod, - }, context ); + }; - // process accumulated member state + // a custom parser may be provided to hook the below property parser; + // this can be done to save time on post-processing, or alter the + // default behavior of the parser + if ( props.___$$parser$$ ) + { + // this isn't something that we actually want to parse + var parser = props.___$$parser$$; + delete props.___$$parser$$; + + function hjoin( name, orig ) + { + handlers[ name ] = function() + { + var args = Array.prototype.slice.call( arguments ); + + // invoke the custom handler with the original handler as + // its last argument (which the custom handler may choose + // not to invoke at all) + args.push( orig ); + parser[ name ].apply( context, args ); + }; + } + + // this avoids a performance penalty unless the above property is + // set + parser.each && hjoin( 'each', handlers.each ); + parser.property && hjoin( 'property', handlers.property ); + parser.getset && hjoin( 'getset', handlers.getset ); + parser.method && hjoin( 'method', handlers.method ); + } + + // parse members and process accumulated member state + util.propParse( props, handlers, context ); this._memberBuilder.end( context.state ); } diff --git a/lib/Trait.js b/lib/Trait.js index 31de351..c2bb77b 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -86,6 +86,12 @@ Trait.extend = function( dfn ) // just in case DFN does not contain any abstract members itself dfn[ 'abstract protected ___$$trait$$' ] = []; + // augment the parser to handle our own oddities + dfn.___$$parser$$ = { + each: _parseMember, + property: _parseProps, + }; + // give the abstract trait class a distinctive name for debugging dfn.__name = '#AbstractTrait#'; @@ -112,16 +118,6 @@ Trait.extend = function( dfn ) return ''+name; }; - // traits are not permitted to define constructors - if ( tclass.___$$methods$$['public'].__construct !== undefined ) - { - throw Error( "Traits may not define __construct" ); - } - - // traits have property restrictions - validateProps( tclass.___$$props$$['public'] ); - validateProps( tclass.___$$props$$['protected'] ); - // invoked to trigger mixin TraitType.__mixin = function( dfn, tc, base ) { @@ -138,30 +134,59 @@ Trait.extend = function( dfn ) }; +/** + * Verifies trait member restrictions + * + * @param {string} name property name + * @param {*} value property value + * @param {Object} keywords property keywords + * @param {Function} h original handler that we replaced + * + * @return {undefined} + */ +function _parseMember( name, value, keywords, h ) +{ + // traits are not permitted to define constructors + if ( name === '__construct' ) + { + throw Error( "Traits may not define __construct" ); + } + + // apply original handler + h.apply( this, arguments ); +} + + /** * Throws error if non-internal property is found within PROPS * * For details and rationale, see the Trait/PropertyTest case. * - * @param {Object} props properties to prohibit + * @param {string} name property name + * @param {*} value property value + * @param {Object} keywords property keywords + * @param {Function} h original handler that we replaced * * @return {undefined} */ -function validateProps( props ) +function _parseProps( name, value, keywords, h ) { - for ( var f in props ) + // ignore internal properties + if ( name.substr( 0, 3 ) === '___' ) { - // ignore internal properties - if ( f.substr( 0, 3 ) === '___' ) - { - continue; - } + return; + } + if ( !( keywords['private'] ) ) + { throw Error( - "Cannot define property `" + f + "'; only private " + + "Cannot define property `" + name + "'; only private " + "properties are permitted within Trait definitions" ); } + + // apply original handler + h.apply( this, arguments ); } From eab4dbfe4d3554f068ce8e3b515d371232acad36 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 11 Mar 2014 06:37:16 -0400 Subject: [PATCH 48/52] Throwing exception on trait static member These will be supported in future versions; this is not something that I want to rush, nor is it something I want to hold up the first GNU release; it is likely to be a much lesser-used feature. --- lib/Trait.js | 9 +++++++++ test/Trait/DefinitionTest.js | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/lib/Trait.js b/lib/Trait.js index c2bb77b..f1f40da 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -152,6 +152,15 @@ function _parseMember( name, value, keywords, h ) throw Error( "Traits may not define __construct" ); } + // will be supported in future versions + if ( keywords['static'] ) + { + throw Error( + "Cannot define member `" + name + "'; static trait " + + "members are currently unsupported" + ); + } + // apply original handler h.apply( this, arguments ); } diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index 465f0a4..fba9190 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -390,4 +390,27 @@ require( 'common' ).testCase( }, } )().go(); }, + + + /** + * Support for static members will be added in future versions; this is + * not something that the author wanted to rush for the first trait + * release, as static members have their own odd quirks. + */ + 'Trait static members are prohibited': function() + { + var Sut = this.Sut; + + // property + this.assertThrows( function() + { + Sut( { 'static private foo': 'prop' } ); + } ); + + // method + this.assertThrows( function() + { + Sut( { 'static foo': function() {} } ); + } ); + }, } ); From bfadd9fce91b1c2fc6e5b7e677b46e5fde9f97df Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 11 Mar 2014 06:46:12 -0400 Subject: [PATCH 49/52] Added missing trait docblocks --- lib/Trait.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/lib/Trait.js b/lib/Trait.js index f1f40da..90687d6 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -23,6 +23,15 @@ var AbstractClass = require( __dirname + '/class_abstract' ), ClassBuilder = require( __dirname + '/ClassBuilder' ); +/** + * Trait constructor / base object + * + * The interpretation of the argument list varies by number. Further, + * various trait methods may be used as an alternative to invoking this + * constructor. + * + * @return {Function} trait + */ function Trait() { switch ( arguments.length ) @@ -199,6 +208,18 @@ function _parseProps( name, value, keywords, h ) } +/** + * Implement one or more interfaces + * + * Implementing an interface into a trait has the same effect as it does + * within classes in that it will automatically define abstract methods + * unless a concrete method is provided. Further, the class that the trait + * is mixed into will act as though it implemented the interfaces. + * + * @param {...Function} interfaces interfaces to implement + * + * @return {Object} staged trait object + */ Trait.implement = function() { var ifaces = arguments; @@ -216,6 +237,13 @@ Trait.implement = function() }; +/** + * Determines if the provided value references a trait + * + * @param {*} trait value to check + * + * @return {boolean} whether the provided value references a trait + */ Trait.isTrait = function( trait ) { return !!( trait || {} ).__trait; From cc43f4b339a970101eaaf142d6d7ef255454f822 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 11 Mar 2014 06:58:39 -0400 Subject: [PATCH 50/52] Prohibiting trait getters/setters --- lib/Trait.js | 23 ++++++++++++++++++++++ test/Trait/DefinitionTest.js | 38 ++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/lib/Trait.js b/lib/Trait.js index 90687d6..3aa9e6c 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -99,6 +99,7 @@ Trait.extend = function( dfn ) dfn.___$$parser$$ = { each: _parseMember, property: _parseProps, + getset: _parseGetSet, }; // give the abstract trait class a distinctive name for debugging @@ -208,6 +209,28 @@ function _parseProps( name, value, keywords, h ) } +/** + * Immediately throws an exception, as getters/setters are unsupported + * + * This is a temporary restriction; they will be supported in future + * releases. + * + * @param {string} name property name + * @param {*} value property value + * @param {Object} keywords property keywords + * @param {Function} h original handler that we replaced + * + * @return {undefined} + */ +function _parseGetSet( name, value, keywords, h ) +{ + throw Error( + "Cannot define property `" + name + "'; getters/setters are " + + "currently unsupported" + ); +} + + /** * Implement one or more interfaces * diff --git a/test/Trait/DefinitionTest.js b/test/Trait/DefinitionTest.js index fba9190..21325ec 100644 --- a/test/Trait/DefinitionTest.js +++ b/test/Trait/DefinitionTest.js @@ -28,6 +28,10 @@ require( 'common' ).testCase( this.Interface = this.require( 'interface' ); this.AbstractClass = this.require( 'class_abstract' ); + this.hasGetSet = !( + this.require( 'util' ).definePropertyFallback() + ); + // means of creating anonymous traits this.ctor = [ this.Sut.extend, @@ -413,4 +417,38 @@ require( 'common' ).testCase( Sut( { 'static foo': function() {} } ); } ); }, + + + /** + * For the same reasons as static members (described immediately above), + * getters/setters are unsupported until future versions. + * + * Note that we use defineProperty instead of the short-hand object + * literal notation to avoid syntax errors in pre-ES5 environments. + */ + 'Trait getters and setters are prohibited': function() + { + // perform these tests only when getters/setters are supported by + // our environment + if ( !( this.hasGetSet ) ) + { + return; + } + + var Sut = this.Sut; + + this.assertThrows( function() + { + var dfn = {}; + Object.defineProperty( dfn, 'foo', + { + get: function() {}, + set: function() {}, + + enumerable: true, + } ); + + Sut( dfn ); + } ); + }, } ); From 0713a9f3d0f39eb41a88baf911162c8c7315db06 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 15 Mar 2014 01:47:31 -0400 Subject: [PATCH 51/52] Deleted unnecessary trait abstract method in favor of auto-abstract flag This method was added before this flag existed. --- lib/ClassBuilder.js | 2 +- lib/Trait.js | 13 +++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 5d29a29..91a072b 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -672,7 +672,7 @@ function validateAbstract( ctor, cname, abstract_methods, auto ) { if ( ctor.___$$abstract$$ ) { - if ( abstract_methods.__length === 0 ) + if ( !auto && ( abstract_methods.__length === 0 ) ) { throw TypeError( "Class " + ( cname || "(anonymous)" ) + " was declared as " + diff --git a/lib/Trait.js b/lib/Trait.js index 3aa9e6c..e1c18ff 100644 --- a/lib/Trait.js +++ b/lib/Trait.js @@ -90,11 +90,6 @@ Trait.extend = function( dfn ) // object will be used to define the hidden abstract class) var name = dfn.__name || '(Trait)'; - // we need at least one abstract member in order to declare a class as - // abstract (in this case, our trait class), so let's create a dummy one - // just in case DFN does not contain any abstract members itself - dfn[ 'abstract protected ___$$trait$$' ] = []; - // augment the parser to handle our own oddities dfn.___$$parser$$ = { each: _parseMember, @@ -102,6 +97,10 @@ Trait.extend = function( dfn ) getset: _parseGetSet, }; + // automatically mark ourselves as abstract if an abstract method is + // provided + dfn.___$$auto$abstract$$ = true; + // give the abstract trait class a distinctive name for debugging dfn.__name = '#AbstractTrait#'; @@ -289,8 +288,6 @@ function createConcrete( acls ) // a constructor that accepts the protected member object of the // containing class var dfn = { - 'protected ___$$trait$$': function() {}, - // protected member object (we define this as protected so that the // parent ACLS has access to it (!), which is not prohibited since // JS does not provide a strict typing mechanism...this is a kluge) @@ -506,7 +503,7 @@ function mixMethods( src, dest, vis, iname ) // 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 - if ( !( src[ f ].___$$keywords$$ ) || f === '___$$trait$$' ) + if ( !( src[ f ].___$$keywords$$ ) ) { continue; } From 00a4ff67868d16b3b489a31d8b4be025d2a90428 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 15 Mar 2014 21:27:39 -0400 Subject: [PATCH 52/52] README.traits containing remaining TODOs --- README.traits | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 README.traits diff --git a/README.traits b/README.traits new file mode 100644 index 0000000..437da55 --- /dev/null +++ b/README.traits @@ -0,0 +1,98 @@ +GNU ease.js Traits +================== +The trait implementation is not yet complete; this is the list of known +issues/TODOs. If you discover any problems, please send an e-mail to +bug-easejs@gnu.org. + +Aside from the issues below, traits are stable and ready to be used in +production. See the test cases and performance tests for more information +and a plethora of examples until the documentation is complete. + + +TODO: Trait Extending +--------------------- +Currently, the only way for a trait to override methods of a class it is +being mixed into is to implement a common interface. Traits should +alternatively be able to "extend" classes, which will have effects similar +to Scala in that the trait can only be mixed into that class. Further, +traits should be able to extend and mix in other traits (though such should +be done conservatively). + + +TODO: Documentation +------------------- +Due to the trait implementation taking longer than expected to complete, and +the importance of the first GNU release, trait documentation is not yet +complete. Instead, traits have been released as a development preview, with +the test cases and performance tests serving as interim documentation. + +Comprehensive documentation, including implementation details and rationale, +will be available shortly. + + +TODO: Static members +-------------------- +Static members are currently unsupported. There is no particular difficulty +in implementing them---the author didn't want it to hold up an initial +release (the first GNU release) even further. + + +TODO: Getters/setters +--------------------- +Getters and setters, although they act like properties, should be treated as +though they are methods. Further, they do not suffer from the same +complications as properties, because they are only available in an ES5 +environment (as an ECMAScript language feature). + + +TODO: Mixin Caching +------------------- +The pattern Type.use(...)(...)---that is, mix a trait into a class and +immediate instantiate the result---is a common idiom that can often be +better for self-documentation than storing the resulting class in another +variable before instantiation. Currently, it's also a terrible thing to do +in any sort of loop, as it re-mixes each and every time. + +We should introduce a caching system to avoid that cost and make it fairly +cheap to use such an idiom. Further, this would permit the Scala-like +ability to use Type.use in Class.isA checks. + + +TODO: Public/Protected Property Support +--------------------------------------- +Private properties are currently supported on traits because they do not +affect the API of the type they are mixed into. However, due to limitations +of pre-ES5 environments, implementing public and protected member epoxying +becomes ugly in the event of a fallback, amounting essentially to +re-assignment before/after trait method proxying. It is possible, though. + +This is not a necessary, or recommended, feature---one should aim to +encapsulate all data, not expose it---but it does have its legitimate uses. +As such, this is not a high-priority item. + + +TODO: Trait-specific error messages +----------------------------------- +All error messages resulting from traits should refer to the trait by name +and any problem members by name, and should offer context-specific +suggestions for resolution. Currently, the errors may be more general and +may reflect the internal construction of traits, which will be rather +confusing to users. + + +TODO: Performance enhancements +------------------------------ +The current trait implementation works well, but is relatively slow +(compared to how performant it could be). While this is sufficient for most +users' uses, there is plenty of room for improvement. Until that time, be +mindful of the performance test cases in the `test/perf' directory. + + +TODO: Intermediate object as class +---------------------------------- +The immediate syntax---Foo.use(T)()---is a short-hand equivalent of +Foo.use(T).extend({})(). As such, for consistency, Class.isA should consider +the intermediate object returned by a call to `use' to be a class. + +If we are to do so, though, we must make sure that the entire class API is +supported.