Correct trait->class calling context on class supertype
See test cases for more information. This was a pretty unfortunate and nasty bug that I discovered while working on a project that uses easejs; it wasn't something that was found previously because this support was only added relatively recently, and this problem does not exist if an interface is used. * lib/Trait.js (bindSuperCtx): Add function. (tctor): Use it. * test/Trait/ScopeTest.js: Add calling context tests.master
parent
748ceaf0bf
commit
04e98e682e
32
lib/Trait.js
32
lib/Trait.js
|
@ -908,6 +908,10 @@ function tctor( tc, base, privsym )
|
||||||
// the intimate relationship
|
// the intimate relationship
|
||||||
this[ f ] = C( base, this[ privsym ].vis )[ privsym ].vis;
|
this[ f ] = C( base, this[ privsym ].vis )[ privsym ].vis;
|
||||||
|
|
||||||
|
// rebind trait's supertype context (if any) to our own, causing us
|
||||||
|
// to share state
|
||||||
|
bindSuperCtx( this[ f ], this, privsym );
|
||||||
|
|
||||||
// this has been previously validated to ensure that it is a
|
// this has been previously validated to ensure that it is a
|
||||||
// function
|
// function
|
||||||
this[ f ].__mixin && this[ f ].__mixin.apply(
|
this[ f ].__mixin && this[ f ].__mixin.apply(
|
||||||
|
@ -920,6 +924,34 @@ function tctor( tc, base, privsym )
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Bind the supertype context to that of another instance
|
||||||
|
*
|
||||||
|
* The source instance FROM is expected to have a `___$$super$$' property
|
||||||
|
* holding a reference to the supertype. The context associated with the
|
||||||
|
* supertype's class id X in FROM will then be reassigned by reference to
|
||||||
|
* the context X of instance TO.
|
||||||
|
*
|
||||||
|
* This has the effect of ensuring that, whenever a call enters the
|
||||||
|
* supertype's context in FROM, it will use all state from the context of
|
||||||
|
* TO. For mixins, this means that---despite the trait itself inheriting
|
||||||
|
* from the class---we will use the private state of the _mixer_, not our
|
||||||
|
* own.
|
||||||
|
*
|
||||||
|
* This is all done without any loss in performance, since calling context
|
||||||
|
* is immediately bound rather than resolved during each method call.
|
||||||
|
*
|
||||||
|
* TODO: make into a standard API for class manipulation
|
||||||
|
*/
|
||||||
|
function bindSuperCtx( from, to, privsym )
|
||||||
|
{
|
||||||
|
var ctx = from[ privsym ].vis,
|
||||||
|
cid = ctx.___$$super$$.__cid;
|
||||||
|
|
||||||
|
ctx[ privsym ].vis[ cid ] = to[ privsym ].vis[ cid ];
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create trait constructor
|
* Create trait constructor
|
||||||
*
|
*
|
||||||
|
|
|
@ -17,14 +17,19 @@
|
||||||
*
|
*
|
||||||
* You should have received a copy of the GNU General Public License
|
* You should have received a copy of the GNU General Public License
|
||||||
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
*
|
||||||
|
* These tests could possibly duplicate tests elsewhere; that's fine, as
|
||||||
|
* this is a vital concept that wouldn't hurt to be reiterated in a
|
||||||
|
* different context (no pun intended).
|
||||||
*/
|
*/
|
||||||
|
|
||||||
require( 'common' ).testCase(
|
require( 'common' ).testCase(
|
||||||
{
|
{
|
||||||
caseSetUp: function()
|
caseSetUp: function()
|
||||||
{
|
{
|
||||||
this.Sut = this.require( 'Trait' );
|
this.Sut = this.require( 'Trait' );
|
||||||
this.Class = this.require( 'class' );
|
this.Class = this.require( 'class' );
|
||||||
|
this.Interface = this.require( 'interface' );
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|
||||||
|
@ -149,4 +154,218 @@ require( 'common' ).testCase(
|
||||||
} )().callFoo();
|
} )().callFoo();
|
||||||
} );
|
} );
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* When a class makes a call to a trait method, the calling context
|
||||||
|
* should be that of the trait itself (that is, the trait has its own
|
||||||
|
* internal state).
|
||||||
|
*/
|
||||||
|
'Class->trait calling context binds to trait': function()
|
||||||
|
{
|
||||||
|
var T = this.Sut(
|
||||||
|
{
|
||||||
|
'private _foo': [],
|
||||||
|
_givenMixin: null,
|
||||||
|
|
||||||
|
// must be properly bound before mixin
|
||||||
|
__mixin: function()
|
||||||
|
{
|
||||||
|
this._givenMixin = this.get();
|
||||||
|
},
|
||||||
|
|
||||||
|
push: function( item )
|
||||||
|
{
|
||||||
|
this._foo.push( item );
|
||||||
|
},
|
||||||
|
|
||||||
|
// make sure calling context is preserved on override
|
||||||
|
'virtual overridePush': function( item )
|
||||||
|
{
|
||||||
|
this._foo.push( item );
|
||||||
|
},
|
||||||
|
|
||||||
|
get: function()
|
||||||
|
{
|
||||||
|
return this._foo;
|
||||||
|
},
|
||||||
|
|
||||||
|
getGivenMixin: function()
|
||||||
|
{
|
||||||
|
return this._givenMixin;
|
||||||
|
},
|
||||||
|
} );
|
||||||
|
|
||||||
|
var inst = this.Class.use( T ).extend(
|
||||||
|
{
|
||||||
|
// ensure calling context on T
|
||||||
|
superPush: function( item )
|
||||||
|
{
|
||||||
|
this.push( item );
|
||||||
|
},
|
||||||
|
|
||||||
|
'override overridePush': function( item )
|
||||||
|
{
|
||||||
|
this.__super( item );
|
||||||
|
},
|
||||||
|
} )();
|
||||||
|
|
||||||
|
inst.push( 'a' );
|
||||||
|
inst.superPush( 'b' );
|
||||||
|
inst.overridePush( 'c' );
|
||||||
|
|
||||||
|
this.assertDeepEqual( [ 'a', 'b', 'c' ], inst.get() );
|
||||||
|
this.assertStrictEqual( inst.get(), inst.getGivenMixin() );
|
||||||
|
},
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* This test focuses on an implementation detail: that traits extending
|
||||||
|
* classes literally extend that class. The problem there is that,
|
||||||
|
* because of this detail, calling one of the supertypes methods is
|
||||||
|
* going to apply the method within the context of _that
|
||||||
|
* trait_. Remember: each object has private state associated with each
|
||||||
|
* class in its hierarchy. So the class C containing the mixin of trait
|
||||||
|
* T has it's own state S_c, and T has its own state T_c because of the
|
||||||
|
* extension. Given C#Foo, calling T#Foo applies T_c rather than the
|
||||||
|
* intended C_c. That is, without proper care.
|
||||||
|
*
|
||||||
|
* This tests to make sure the context has been properly rebound to the
|
||||||
|
* mixer.
|
||||||
|
*/
|
||||||
|
'Trait->class calling context binds to class': function()
|
||||||
|
{
|
||||||
|
var C = this.Class(
|
||||||
|
{
|
||||||
|
'private _stack': [],
|
||||||
|
|
||||||
|
'virtual push': function( item )
|
||||||
|
{
|
||||||
|
this._stack.push( item );
|
||||||
|
},
|
||||||
|
|
||||||
|
// non-virtual, test fall-through
|
||||||
|
getStack: function()
|
||||||
|
{
|
||||||
|
return this._stack;
|
||||||
|
},
|
||||||
|
} );
|
||||||
|
|
||||||
|
var T = this.Sut.extend( C,
|
||||||
|
{
|
||||||
|
_givenMixin: null,
|
||||||
|
|
||||||
|
// proper context set before __mixin
|
||||||
|
__mixin: function()
|
||||||
|
{
|
||||||
|
this._givenMixin = this.getStack();
|
||||||
|
},
|
||||||
|
|
||||||
|
// proper context to __super
|
||||||
|
'override push': function( item )
|
||||||
|
{
|
||||||
|
this.__super( item );
|
||||||
|
},
|
||||||
|
|
||||||
|
// proper context to parent `getStack'
|
||||||
|
getSuperStack: function()
|
||||||
|
{
|
||||||
|
return this.getStack();
|
||||||
|
},
|
||||||
|
|
||||||
|
getGivenMixin: function()
|
||||||
|
{
|
||||||
|
return this._givenMixin;
|
||||||
|
},
|
||||||
|
} );
|
||||||
|
|
||||||
|
var stack = C.use( T )();
|
||||||
|
stack.push( 'a' );
|
||||||
|
|
||||||
|
// proper context to parent method call (non-__super)
|
||||||
|
this.assertStrictEqual( stack.getStack(), stack.getSuperStack() );
|
||||||
|
|
||||||
|
// proper context to __super
|
||||||
|
this.assertDeepEqual( [ 'a' ], stack.getStack() );
|
||||||
|
|
||||||
|
// context available before __mixin
|
||||||
|
this.assertStrictEqual( stack.getStack(), stack.getGivenMixin() );
|
||||||
|
},
|
||||||
|
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Similar to the above, except that we extend an interface rather than
|
||||||
|
* a base class.
|
||||||
|
*
|
||||||
|
* Notice how T here implements I rather than extending C, and
|
||||||
|
* consequently uses `abstract override' in place of `override'.
|
||||||
|
*
|
||||||
|
* What is interesting in this case is whether this test fails when the
|
||||||
|
* previous does not, or vice-versa (such was the case when this test
|
||||||
|
* was introduced).
|
||||||
|
*/
|
||||||
|
'Trait->interface calling context binds to implementing class': function()
|
||||||
|
{
|
||||||
|
var I = this.Interface(
|
||||||
|
{
|
||||||
|
push: [ 'item' ],
|
||||||
|
getStack: [],
|
||||||
|
} );
|
||||||
|
|
||||||
|
var C = this.Class.implement( I ).extend(
|
||||||
|
{
|
||||||
|
'private _stack': [],
|
||||||
|
|
||||||
|
'virtual push': function( item )
|
||||||
|
{
|
||||||
|
this._stack.push( item );
|
||||||
|
},
|
||||||
|
|
||||||
|
// non-virtual, test fall-through
|
||||||
|
getStack: function()
|
||||||
|
{
|
||||||
|
return this._stack;
|
||||||
|
},
|
||||||
|
} );
|
||||||
|
|
||||||
|
var T = this.Sut.implement( I ).extend(
|
||||||
|
{
|
||||||
|
_givenMixin: null,
|
||||||
|
|
||||||
|
// proper context set before __mixin
|
||||||
|
__mixin: function()
|
||||||
|
{
|
||||||
|
this._givenMixin = this.getStack();
|
||||||
|
},
|
||||||
|
|
||||||
|
// proper context to __super
|
||||||
|
'abstract override push': function( item )
|
||||||
|
{
|
||||||
|
this.__super( item );
|
||||||
|
},
|
||||||
|
|
||||||
|
// proper context to parent `getStack'
|
||||||
|
getSuperStack: function()
|
||||||
|
{
|
||||||
|
return this.getStack();
|
||||||
|
},
|
||||||
|
|
||||||
|
getGivenMixin: function()
|
||||||
|
{
|
||||||
|
return this._givenMixin;
|
||||||
|
},
|
||||||
|
} );
|
||||||
|
|
||||||
|
var stack = C.use( T )();
|
||||||
|
stack.push( 'a' );
|
||||||
|
|
||||||
|
// proper context to parent method call (non-__super)
|
||||||
|
this.assertStrictEqual( stack.getStack(), stack.getSuperStack() );
|
||||||
|
|
||||||
|
// proper context to __super
|
||||||
|
this.assertDeepEqual( [ 'a' ], stack.getStack() );
|
||||||
|
|
||||||
|
// context available before __mixin
|
||||||
|
this.assertStrictEqual( stack.getStack(), stack.getGivenMixin() );
|
||||||
|
},
|
||||||
} );
|
} );
|
||||||
|
|
Loading…
Reference in New Issue