From 3724b1bc0de5c72e678492c5fdbe557673021702 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 23 Jan 2014 01:00:16 -0500 Subject: [PATCH] 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 ); },