From 4e2af2333d55bdd0f3adce543bff8d13976d8431 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 2 Nov 2011 23:28:23 -0400 Subject: [PATCH] [#25] Now injecting MemberBuilderValidator into MemberBuilder --- lib/MemberBuilder.js | 9 +-- lib/MemberBuilderValidator.js | 2 +- lib/class.js | 3 +- lib/interface.js | 14 ++++- test/MemberBuilder/VisibilityTest.js | 3 +- test/inc-testcase.js | 33 +++++++++- test/test-class_builder-const.js | 12 ++-- test/test-class_builder-static.js | 89 +++++++++++++++------------ test/test-class_builder-visibility.js | 3 +- 9 files changed, 107 insertions(+), 61 deletions(-) diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index 1e16384..341d02e 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -30,27 +30,24 @@ var util = require( __dirname + '/util' ), Warning = require( __dirname + '/warn' ).Warning, - visibility = [ 'public', 'protected', 'private' ], - - validate = require( __dirname + '/MemberBuilderValidator' )() + visibility = [ 'public', 'protected', 'private' ] ; /** * Responsible for building class members */ -module.exports = function MemberBuilder( wrap_method, wrap_override ) +module.exports = function MemberBuilder( wrap_method, wrap_override, validate ) { // permit omitting 'new' keyword if ( !( this instanceof module.exports ) ) { - return new module.exports( wrap_method, wrap_override ); + return new module.exports( wrap_method, wrap_override, validate ); } this._wrapMethod = wrap_method; this._wrapOverride = wrap_override; - // XXX: temporarily tightly coupled this._validate = validate; }; diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index 7562b15..5f15d79 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -246,7 +246,7 @@ exports.prototype.validateGetterSetter = function( ) { var prev = ( prev_data ) ? prev_data.member : null, - prev_gs = ( ( prev_data && (prev_data.get || prev_data.set ) ) + prev_gs = ( ( prev_data && ( prev_data.get || prev_data.set ) ) ? true : false ) diff --git a/lib/class.js b/lib/class.js index 726b31d..f084362 100644 --- a/lib/class.js +++ b/lib/class.js @@ -31,7 +31,8 @@ var util = require( __dirname + '/util' ), class_builder = ClassBuilder( require( __dirname + '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), - MethodWrapperFactory( wrappers.wrapOverride ) + MethodWrapperFactory( wrappers.wrapOverride ), + require( __dirname + '/MemberBuilderValidator' )() ), require( __dirname + '/VisibilityObjectFactoryFactory' ) .fromEnvironment() diff --git a/lib/interface.js b/lib/interface.js index 221396b..a23370e 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -23,8 +23,18 @@ */ var util = require( __dirname + '/util' ), - member_builder = require( __dirname + '/MemberBuilder' )(), - Class = require( __dirname + '/class' ); + + MethodWrapperFactory = require( __dirname + '/MethodWrapperFactory' ), + wrappers = require( __dirname + '/MethodWrappers' ).standard, + + member_builder = require( __dirname + '/MemberBuilder' )( + MethodWrapperFactory( wrappers.wrapNew ), + MethodWrapperFactory( wrappers.wrapOverride ), + require( __dirname + '/MemberBuilderValidator' )() + ), + + Class = require( __dirname + '/class' ) +; /** diff --git a/test/MemberBuilder/VisibilityTest.js b/test/MemberBuilder/VisibilityTest.js index 2e282f0..72c36b5 100644 --- a/test/MemberBuilder/VisibilityTest.js +++ b/test/MemberBuilder/VisibilityTest.js @@ -214,7 +214,8 @@ require( 'common' ).testCase( ); this.sut = this.require( 'MemberBuilder' )( - stubFactory, stubFactory + stubFactory, stubFactory, + this.getMock( 'MemberBuilderValidator' ) ); this.members = this.sut.initMembers(); diff --git a/test/inc-testcase.js b/test/inc-testcase.js index bf625a8..6d60d3b 100644 --- a/test/inc-testcase.js +++ b/test/inc-testcase.js @@ -2,7 +2,10 @@ var assert = require( 'assert' ), assert_wrapped = {}, - acount = 0; + acount = 0, + + common_require = require( __dirname + '/common' ).require +; // wrap each of the assertions so that we can keep track of the number of times @@ -114,6 +117,30 @@ module.exports = function( test_case ) }; +function getMock( proto ) +{ + var P = common_require( proto ), + Mock = function() {}, + + proto = Mock.prototype = new P() + ; + + for ( i in proto ) + { + // only mock out methods + if ( typeof proto[ i ] !== 'function' ) + { + continue; + } + + // clear the method + proto[ i ] = function() {}; + } + + return new Mock(); +} + + /** * Prepare assertion methods on context * @@ -122,7 +149,7 @@ module.exports = function( test_case ) function prepareCaseContext() { return { - require: require( __dirname + '/common' ).require, + require: common_require, fail: assert_wrapped.fail, assertOk: assert_wrapped.ok, @@ -135,6 +162,8 @@ function prepareCaseContext() assertDoesNotThrow: assert_wrapped.doesNotThrow, assertIfError: assert_wrapped.ifError, incAssertCount: incAssertCount, + + getMock: getMock, }; } diff --git a/test/test-class_builder-const.js b/test/test-class_builder-const.js index af2b217..9c50be0 100644 --- a/test/test-class_builder-const.js +++ b/test/test-class_builder-const.js @@ -38,16 +38,16 @@ require( 'common' ).testCase( this.builder = ClassBuilder( this.require( '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), - MethodWrapperFactory( wrappers.wrapOverride ) + MethodWrapperFactory( wrappers.wrapOverride ), + this.getMock( 'MemberBuilderValidator' ) ), this.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment() ) }, - /** - * The const keyword should result in a static property. The rationale for this - * is that, if a value is constant, then instances do not make sense. + /** The const keyword should result in a static property. The rationale for + * this is that, if a value is constant, then instances do not make sense. */ 'const keyword declares properties as static': function() { @@ -113,8 +113,8 @@ require( 'common' ).testCase( }, } ), - // be sure to override each method to ensure we're checking references - // on the subtype, *not* the parent type + // be sure to override each method to ensure we're checking + // references on the subtype, *not* the parent type SubFoo = this.builder.build( Foo, { 'public static getProt': function() diff --git a/test/test-class_builder-static.js b/test/test-class_builder-static.js index 4de7961..65188c1 100644 --- a/test/test-class_builder-static.js +++ b/test/test-class_builder-static.js @@ -29,20 +29,25 @@ var common = require( './common' ), // dependencies should not be necessary for testing ClassBuilder = common.require( '/ClassBuilder' ), MethodWrapperFactory = common.require( '/MethodWrapperFactory' ), - wrappers = common.require( '/MethodWrappers' ).standard, - - builder = ClassBuilder( - common.require( '/MemberBuilder' )( - MethodWrapperFactory( wrappers.wrapNew ), - MethodWrapperFactory( wrappers.wrapOverride ) - ), - common.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment() - ) + wrappers = common.require( '/MethodWrappers' ).standard ; require( 'common' ).testCase( { + setUp: function() + { + this.builder = ClassBuilder( + common.require( '/MemberBuilder' )( + MethodWrapperFactory( wrappers.wrapNew ), + MethodWrapperFactory( wrappers.wrapOverride ), + this.getMock( 'MemberBuilderValidator' ) + ), + common.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment() + ); + }, + + /** * To provide access to static members, this.__self is made available inside * of instances. @@ -50,7 +55,7 @@ require( 'common' ).testCase( 'Self property references class definition': function() { var val = [ 'baz' ], - Foo = builder.build( + Foo = this.builder.build( { 'public test': function() { @@ -77,7 +82,7 @@ require( 'common' ).testCase( */ 'Static property lookup returns undefined if not found': function() { - var result = builder.build( {} ).$( 'foo' ); + var result = this.builder.build( {} ).$( 'foo' ); this.assertEqual( result, undefined, "Static property getter should return undefined if not found" @@ -93,7 +98,7 @@ require( 'common' ).testCase( 'Static property accessor is not enumerable': function() { var get = Object.getOwnPropertyDescriptor, - Foo = builder.build( {} ); + Foo = this.builder.build( {} ); // don't perform the test if unsupported if ( fallback ) @@ -117,7 +122,7 @@ require( 'common' ).testCase( { var val = 'foo', val2 = 'bar', - Foo = builder.build( + Foo = this.builder.build( { 'public static foo': val, @@ -198,7 +203,7 @@ require( 'common' ).testCase( } ); // define the class - var Foo = builder.build( def ); + var Foo = this.builder.build( def ); this.assertEqual( Foo.foo, val, "Public static getters are accessible via class definition" @@ -240,7 +245,7 @@ require( 'common' ).testCase( 'Static methods not bound to instance': function() { var result = null, - Foo = builder.build( + Foo = this.builder.build( { 'public static foo': function() { @@ -285,15 +290,15 @@ require( 'common' ).testCase( } var baz = 'foobar', - Foo = builder.build( def ), + Foo = this.builder.build( def ), // extends from the parent and adds an additional - SubFoo = builder.build( Foo, { 'public static baz': baz } ), + SubFoo = this.builder.build( Foo, { 'public static baz': baz } ), // simply extends from the parent (also serves as a check to ensure // that static members of *all* parents are inherited, not just the // immediate) - SubSubFoo = builder.build( SubFoo, {} ) + SubSubFoo = this.builder.build( SubFoo, {} ) ; // properties @@ -358,11 +363,11 @@ require( 'common' ).testCase( var val = [ 1, 2, 3 ], val2 = [ 'a', 'b', 'c' ], - Foo = builder.build( + Foo = this.builder.build( { 'public static bar': val, } ), - SubFoo = builder.build( Foo, {} ) + SubFoo = this.builder.build( Foo, {} ) ; // the properties should reference the same object @@ -403,7 +408,7 @@ require( 'common' ).testCase( 'Setting static props to undefined will not corrupt lookup': function() { var val = 'baz', - Foo = builder.build( + Foo = this.builder.build( { 'public static foo': '', } ) @@ -437,12 +442,12 @@ require( 'common' ).testCase( */ 'Static property setters return proper context': function() { - var Foo = builder.build( + var Foo = this.builder.build( { 'public static foo': '', } ), - SubFoo = builder.build( Foo, {} ) + SubFoo = this.builder.build( Foo, {} ) ; this.assertOk( Foo.$( 'foo', 'val' ) === Foo, @@ -462,12 +467,14 @@ require( 'common' ).testCase( */ 'Attempting to set undeclared static prop results in exception': function() { + var _self = this; + this.assertThrows( function() { // should throw an exception since property 'foo' has not been // declared - builder.build( {} ).$( 'foo', 'val' ); + _self.builder.build( {} ).$( 'foo', 'val' ); }, ReferenceError, "Attempting to set an undeclaraed static property results in an " + @@ -483,7 +490,7 @@ require( 'common' ).testCase( 'Protected static members are available inside class only': function() { var val = 'foo', - Foo = builder.build( + Foo = this.builder.build( { 'protected static prop': val, @@ -577,7 +584,7 @@ require( 'common' ).testCase( } ); // define the class - var Foo = builder.build( def ); + var Foo = this.builder.build( def ); this.assertEqual( Foo.getProp(), val, "Protected static getters are accessible from within the class" @@ -630,9 +637,9 @@ require( 'common' ).testCase( }; } - var Foo = builder.build( def ), + var Foo = this.builder.build( def ), - SubFoo = builder.build( Foo, + SubFoo = this.builder.build( Foo, { 'public static bar': function() { @@ -655,7 +662,7 @@ require( 'common' ).testCase( }, } ), - SubSubFoo = builder.build( SubFoo, {} ) + SubSubFoo = this.builder.build( SubFoo, {} ) ; this.assertEqual( SubFoo.bar(), val, @@ -715,7 +722,7 @@ require( 'common' ).testCase( 'Private static members are available inside class only': function() { var val = 'foo', - Foo = builder.build( + Foo = this.builder.build( { 'private static prop': val, @@ -792,9 +799,9 @@ require( 'common' ).testCase( } ); } - var Foo = builder.build( def ), + var Foo = this.builder.build( def ), - SubFoo = builder.build( Foo, + SubFoo = this.builder.build( Foo, { 'public static getPriv': function() { @@ -877,7 +884,7 @@ require( 'common' ).testCase( } ); // define the class - var Foo = builder.build( def ); + var Foo = this.builder.build( def ); this.assertEqual( Foo.getProp(), val, "Private static getters are accessible from within the class" @@ -901,13 +908,13 @@ require( 'common' ).testCase( 'Static methods can be overridden by subtypes': function() { var val = 'bar', - Foo = builder.build( + Foo = this.builder.build( { 'public static foo': function() {}, 'protected static bar': function() {}, } ), - SubFoo = builder.build( Foo, + SubFoo = this.builder.build( Foo, { 'public static foo': function() { @@ -945,12 +952,12 @@ require( 'common' ).testCase( 'Cannot exploit accessor method to gain access to parent private props': function() { - var Foo = builder.build( + var Foo = this.builder.build( { 'private static foo': 'bar', } ), - SubFoo = builder.build( Foo, + SubFoo = this.builder.build( Foo, { 'public static getParentPrivate': function() { @@ -974,7 +981,7 @@ require( 'common' ).testCase( var val_orig = 'foobaz', val = 'foobar', - Foo = builder.build( + Foo = this.builder.build( { 'public static prop': val_orig, @@ -994,7 +1001,7 @@ require( 'common' ).testCase( }, } ), - SubFoo = builder.build( Foo, + SubFoo = this.builder.build( Foo, { 'public static prop': val, @@ -1037,7 +1044,7 @@ require( 'common' ).testCase( 'Calls to parent static methods retain private member access': function() { var val = 'foobar', - Foo = builder.build( + Foo = this.builder.build( { 'private static _priv': val, @@ -1047,7 +1054,7 @@ require( 'common' ).testCase( }, } ), - SubFoo = builder.build( Foo, + SubFoo = this.builder.build( Foo, { 'public static getPriv2': function() { diff --git a/test/test-class_builder-visibility.js b/test/test-class_builder-visibility.js index 487b2da..5634578 100644 --- a/test/test-class_builder-visibility.js +++ b/test/test-class_builder-visibility.js @@ -43,7 +43,8 @@ require( 'common' ).testCase( this.builder = ClassBuilder( this.require( '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), - MethodWrapperFactory( wrappers.wrapOverride ) + MethodWrapperFactory( wrappers.wrapOverride ), + this.getMock( 'MemberBuilderValidator' ) ), this.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment() );