From a929c42a3ffaf31f011c4b921c2a26deb014d324 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 11 Jun 2011 20:55:45 -0400 Subject: [PATCH 01/17] Corrected Makefile; moved src_* back to root Makefile - Was causing combine target to not recognize file changes --- Makefile | 3 +++ doc/Makefile | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 428c3f1..6247746 100644 --- a/Makefile +++ b/Makefile @@ -10,6 +10,9 @@ path_perf_test=${path_test}/perf perf_tests := $(shell find "$(path_perf_test)" -name 'perf-*.js') +src_js := index.js $(wildcard $(path_lib)/*.js) +src_tests := index.js $(wildcard $(path_test)/test-*) + path_doc := ./doc combine=${path_tools}/combine diff --git a/doc/Makefile b/doc/Makefile index 87661ba..87e3425 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -15,8 +15,6 @@ path_manual_texi=${path_doc}/manual.texi path_info_install := /usr/local/share/info -src_js := index.js $(wildcard $(path_lib)/*.js) -src_tests := index.js $(wildcard $(path_test)/test-*) doc_src := $(wildcard $(path_doc)/*.texi) doc_imgs := $(patsubst %.dia, %.png, $(wildcard $(path_doc_img)/*.dia)) doc_imgs_txt := $(patsubst %.dia, %.png, $(wildcard $(path_doc_img)/*.txt)) From d6873d1cc931a6067c0431b161d4ba4a36658b03 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 11 Jun 2011 21:32:11 -0400 Subject: [PATCH 02/17] Corrected client-side assert.deepEqual() to only perform object/array operations if both provided values are objects/arrays respectively --- tools/combine-test.tpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/combine-test.tpl b/tools/combine-test.tpl index 3e07c31..08540b9 100644 --- a/tools/combine-test.tpl +++ b/tools/combine-test.tpl @@ -73,7 +73,7 @@ module.assert = { exports: { return; } - if ( cmp instanceof Array ) + if ( ( cmp instanceof Array ) && ( val instanceof Array ) ) { var i = 0, len = cmp.length; @@ -86,7 +86,7 @@ module.assert = { exports: { return; } - else if ( cmp instanceof Object ) + else if ( ( typeof cmp === 'object' ) && ( typeof val === 'object' ) ) { for ( var i in cmp ) { From 75059ad030a9d06dd8274585e0fd953bdc99f7d6 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 11 Jun 2011 21:47:57 -0400 Subject: [PATCH 03/17] Added util.get{Own,}PropertyDescriptor --- lib/util.js | 79 +++++++++++++ test/test-util-get-property-descriptor.js | 137 ++++++++++++++++++++++ 2 files changed, 216 insertions(+) create mode 100644 test/test-util-get-property-descriptor.js diff --git a/lib/util.js b/lib/util.js index afd599d..d14de40 100644 --- a/lib/util.js +++ b/lib/util.js @@ -419,6 +419,85 @@ exports.arrayShrink = function( items ) }; +/** + * Uses Object.getOwnPropertyDescriptor if available, otherwise provides our own + * implementation to fall back on + * + * If the environment does not support retrieving property descriptors (ES5), + * then the following will be true: + * - get/set will always be undefined + * - writable, enumerable and configurable will always be true + * - value will be the value of the requested property on the given object + * + * @param {Object} obj object to check property on + * @param {string} prop property to retrieve descriptor for + * + * @return {Object} descriptor for requested property or undefined if not found + */ +exports.getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor + || function( obj, prop ) + { + if ( !Object.prototype.hasOwnProperty.call( obj, prop ) ) + { + return undefined; + } + + // fallback response + return { + get: undefined, + set: undefined, + + writable: true, + enumerable: true, + configurable: true, + + value: obj[ prop ], + }; + }; + + +/** + * Travels down the prototype chain of the given object in search of the + * requested property and returns its descriptor + * + * This operates as Object.getOwnPropertyDescriptor(), except that it traverses + * the prototype chain. For environments that do not support __proto__, it will + * not traverse the prototype chain and essentially serve as an alias for + * getOwnPropertyDescriptor(). + * + * @param {Object} obj object to check property on + * @param {string} prop property to retrieve descriptor for + * + * @return {Object} descriptor for requested property or undefined if not found + */ +exports.getPropertyDescriptor = function( obj, prop ) +{ + // note that this uses util's function, not Object's + var desc = exports.getOwnPropertyDescriptor( obj, prop ); + + // if we didn't find a descriptor and a prototype is available, recurse down + // the prototype chain + if ( !desc && obj.__proto__ ) + { + return exports.getPropertyDescriptor( obj.__proto__, prop ); + } + + // return the descriptor or undefined if no prototype is available + return desc; +}; + + +/** + * Indicates whether or not the getPropertyDescriptor method is capable of + * traversing the prototype chain + * + * @type {boolean} + */ +exports.defineSecureProp( exports.getPropertyDescriptor, 'canTraverse', + ( {}.__proto__ ) ? true : false +); + + /** * Appropriately returns defineSecureProp implementation to avoid check on each * invocation diff --git a/test/test-util-get-property-descriptor.js b/test/test-util-get-property-descriptor.js new file mode 100644 index 0000000..c33d914 --- /dev/null +++ b/test/test-util-get-property-descriptor.js @@ -0,0 +1,137 @@ +/** + * Tests util.getPropertyDescriptor + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser 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 Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package test + */ + +var common = require( './common' ), + assert = require( 'assert' ), + util = common.require( 'util' ), + + get_set = !( util.definePropertyFallback() ) +; + + +/** + * If Object.getOwnPropertyDescriptor is provided by our environment, it should + * be used by util + */ +( function testUtilGetOwnPropertyDescriptorIsObjectsIfAvailable() +{ + if ( Object.getOwnPropertyDescriptor ) + { + assert.strictEqual( + util.getOwnPropertyDescriptor, + Object.getOwnPropertyDescriptor, + "Util should use Object.getOwnPropertyDescriptor if available" + ); + } +} )(); + + +/** + * The function should provide a boolean value indicating whether it can + * traverse the prototype chain + */ +( function testIndicatesWhetherTraversalIsPossible() +{ + var traversable = ( {}.__proto__ ) ? true : false; + + assert.equal( util.getPropertyDescriptor.canTraverse, traversable, + "Indicates whether traversal is possible" + ); +} )(); + + +/** + * We don't want tricksters to get funky with our system + */ +( function testTraversablePropertyIsNonWritable() +{ + var getDesc; + + if ( get_set ) + { + assert.equal( + Object.getOwnPropertyDescriptor( + util.getPropertyDescriptor, 'canTraverse' + ).writable, + false, + "Should not be able to alter canTravese value" + ); + } +} )(); + + +/** + * The return value should mimic Object.getOwnPropertyDescriptor() if we're not + * having to traverse the prototype chain + */ +( function testActsExactlyAsGetOwnPropertyDescriptorInEs5SystemsOnSameObject() +{ + var obj = { foo: 'bar' }, + desc1 = util.getOwnPropertyDescriptor( obj, 'foo' ), + desc2 = util.getPropertyDescriptor( obj, 'foo' ) + ; + + assert.deepEqual( desc1, desc2, + "When operating one level deep, should return same as " + + "Object.getOwnPropertyDescriptor" + ); +} )(); + + +/** + * If we *do* have to start traversing the prototype chain (which + * Object.getOwnPropertyDescriptor() cannot do), then it should be as if we + * called Object.getOwnPropertyDescriptor() on the object in the prototype chain + * containing the requested property. + */ +( function testTraversesThePrototypeChain() +{ + // if we cannot traverse the prototype chain, this test is pointless + if ( !util.getPropertyDescriptor.canTraverse ) + { + return; + } + + var proto = { foo: 'bar' }, + obj = function() {} + ; + + obj.prototype = proto; + + // to give ourselves the prototype chain (we don't want to set __proto__ + // because this test will also be run on pre-ES5 engines) + var inst = new obj(), + + // get the actual descriptor + expected = util.getOwnPropertyDescriptor( proto, 'foo' ), + + // attempt to gather the descriptor from the prototype chain + given = util.getPropertyDescriptor( inst, 'foo' ) + ; + + assert.deepEqual( given, expected, + "Properly traverses the prototype chain to retrieve the descriptor" + ); +} )(); + From 6d31bf1084923be7a34168b386c31cb2aa315004 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 12 Jun 2011 00:36:52 -0400 Subject: [PATCH 04/17] Now passing base into member builder for getters/setters --- lib/class_builder.js | 4 ++-- lib/member_builder.js | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/class_builder.js b/lib/class_builder.js index c19ed0f..5a63dc1 100644 --- a/lib/class_builder.js +++ b/lib/class_builder.js @@ -541,7 +541,7 @@ function buildMembers( var dest = ( keywordStatic( keywords ) ) ? smethods : members; member_builder.buildGetter( - dest, null, name, value, keywords + dest, null, name, value, keywords, base ); }, @@ -550,7 +550,7 @@ function buildMembers( var dest = ( keywordStatic( keywords ) ) ? smethods : members; member_builder.buildSetter( - dest, null, name, value, keywords + dest, null, name, value, keywords, base ); }, diff --git a/lib/member_builder.js b/lib/member_builder.js index f47cb4c..7a071c3 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -282,9 +282,11 @@ exports.buildProp = function( members, meta, name, value, keywords, base ) * * @param {Object.} keywords parsed keywords * + * @param {Object=} base optional base object to scan + * * @return {undefined} */ -exports.buildGetter = function( members, meta, name, value, keywords ) +exports.buildGetter = function( members, meta, name, value, keywords, base ) { Object.defineProperty( getMemberVisibility( members, keywords ), @@ -312,9 +314,11 @@ exports.buildGetter = function( members, meta, name, value, keywords ) * * @param {Object.} keywords parsed keywords * + * @param {Object=} base optional base object to scan + * * @return {undefined} */ -exports.buildSetter = function( members, meta, name, value, keywords ) +exports.buildSetter = function( members, meta, name, value, keywords, base ) { Object.defineProperty( getMemberVisibility( members, keywords ), From 51e27334cd054dc6fbfc3abf62888812ec58531a Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 12 Jun 2011 01:07:43 -0400 Subject: [PATCH 05/17] Can no longer override methods with getters/setters --- lib/member_builder.js | 37 +++++++++++++++ test/test-member_builder-gettersetter.js | 60 ++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/lib/member_builder.js b/lib/member_builder.js index 7a071c3..cab1729 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -288,6 +288,8 @@ exports.buildProp = function( members, meta, name, value, keywords, base ) */ exports.buildGetter = function( members, meta, name, value, keywords, base ) { + validateGetterSetter( members, keywords, name, base ); + Object.defineProperty( getMemberVisibility( members, keywords ), name, @@ -320,6 +322,8 @@ exports.buildGetter = function( members, meta, name, value, keywords, base ) */ exports.buildSetter = function( members, meta, name, value, keywords, base ) { + validateGetterSetter( members, keywords, name, base ); + Object.defineProperty( getMemberVisibility( members, keywords ), name, @@ -334,6 +338,39 @@ exports.buildSetter = function( members, meta, name, value, keywords, base ) }; +/** + * Performs common validations on getters/setters + * + * If a problem is found, an exception will be thrown. + * + * @param {{public: Object, protected: Object, private: Object}} members + * + * @param {Object.} keywords parsed keywords + * @param {string} name getter/setter name + * @param {Object} base optional base to parse + */ +function validateGetterSetter( members, keywords, name, base ) +{ + var prev_data = scanMembers( members, name, base ), + prev = ( prev_data ) ? prev_data.member : null, + + prev_keywords = ( prev && prev.___$$keywords$$ ) + ? prev.___$$keywords$$ + : {} + ; + + if ( prev ) + { + if ( typeof prev === 'function' ) + { + throw TypeError( + "Cannot override method '" + name + "' with getter/setter" + ); + } + } +} + + /** * Returns member prototype to use for the requested visibility * diff --git a/test/test-member_builder-gettersetter.js b/test/test-member_builder-gettersetter.js index 6a3ece8..8d1d966 100644 --- a/test/test-member_builder-gettersetter.js +++ b/test/test-member_builder-gettersetter.js @@ -56,15 +56,41 @@ function setUp() /** * Partially applied function to quickly build getter from common test data */ -function buildGetterSetterQuick( keywords, val ) +function buildGetterSetterQuick( keywords, val, preserve_prior, use ) { + preserve_prior = !!preserve_prior; + use = ( use === undefined ) ? 0 : +use; + keywords = keywords || {}; val = val || value; - setUp(); + if ( !preserve_prior ) + { + setUp(); + } - buildGetter( members, meta, name, val, keywords ); - buildSetter( members, meta, name, val, keywords ); + if ( use == 0 || use == 1 ) + { + buildGetter( members, meta, name, val, keywords ); + } + if ( use == 0 || use == 2 ) + { + buildSetter( members, meta, name, val, keywords ); + } +} + + +function testEach( test ) +{ + test( 'getter', function( keywords, val, preserve ) + { + buildGetterSetterQuick.call( this, keywords, val, preserve, 1 ); + } ); + + test( 'setter', function( keywords, val, preserve ) + { + buildGetterSetterQuick.call( this, keywords, val, preserve, 2 ); + } ); } @@ -176,3 +202,29 @@ function assertOnlyVisibility( vis, name, value, message ) } )(); + +/** + * Getters/setters should not be able to override methods, for the obvious + * reason that they are two different types and operate entirely differently. Go + * figure. + */ +testEach( function testCannotOverrideMethodWithGetterOrSetter( type, build ) +{ + setUp(); + + // method + members[ 'public' ][ name ] = function() {}; + + try + { + // attempt to override method with getter/setter (should fail) + build( { 'public': true }, null, true ); + } + catch ( e ) + { + return; + } + + assert.fail( type + " should not be able to override methods"); +} ); + From c83def013abaab8b6fa9f906483c0cef5a7cb00c Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 14 Jun 2011 17:55:24 -0400 Subject: [PATCH 06/17] Added test to ensure getter/setter method override error contains name --- test/test-member_builder-gettersetter.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test-member_builder-gettersetter.js b/test/test-member_builder-gettersetter.js index 8d1d966..4708d27 100644 --- a/test/test-member_builder-gettersetter.js +++ b/test/test-member_builder-gettersetter.js @@ -222,6 +222,9 @@ testEach( function testCannotOverrideMethodWithGetterOrSetter( type, build ) } catch ( e ) { + assert.ok( e.message.search( name ) !== -1, + "Method override error message should contain getter/setter name" + ); return; } From fb155d8df5670e7a98f4d1e7e212b4fd74d7d233 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 14 Jun 2011 18:24:18 -0400 Subject: [PATCH 07/17] Getters/setters can no longer override properties --- lib/member_builder.js | 17 ++++++++++---- test/test-member_builder-gettersetter.js | 30 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/lib/member_builder.js b/lib/member_builder.js index cab1729..5b8571f 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -361,10 +361,14 @@ function validateGetterSetter( members, keywords, name, base ) if ( prev ) { - if ( typeof prev === 'function' ) + // To speed up the system we'll simply check for a getter/setter, rather + // than checking separately for methods/properties. This is at the + // expense of more detailed error messages. They'll live. + if ( !( prev_data.get || prev_data.set ) ) { throw TypeError( - "Cannot override method '" + name + "' with getter/setter" + "Cannot override method or property '" + name + + "' with getter/setter" ); } } @@ -430,10 +434,15 @@ function scanMembers( members, name, base ) // locate requested member by scanning each level of visibility while ( i-- ) { - if ( member = members[ visibility[ i ] ][ name ] ) + var visobj = members[ visibility[ i ] ]; + + // in order to support getters/setters, we must go off of the descriptor + if ( member = util.getPropertyDescriptor( visobj, name ) ) { return { - member: member, + get: member.get, + set: member.set, + member: member.value, visibility: ( ( fallback ) ? 0 : i ), }; } diff --git a/test/test-member_builder-gettersetter.js b/test/test-member_builder-gettersetter.js index 4708d27..d17d461 100644 --- a/test/test-member_builder-gettersetter.js +++ b/test/test-member_builder-gettersetter.js @@ -231,3 +231,33 @@ testEach( function testCannotOverrideMethodWithGetterOrSetter( type, build ) assert.fail( type + " should not be able to override methods"); } ); + +/** + * Getters/setters should not be able to override properties. While, at first, + * this concept may seem odd, keep in mind that the parent would likely not + * expect a subtype to be able to override property assignments. This could open + * up holes to exploit the parent class. + */ +testEach( function testCannotOverridePropertiesWithGetterOrSetter( type, build ) +{ + setUp(); + + // declare a property + members[ 'public' ][ name ] = 'foo'; + + try + { + // attempt to override property with getter/setter (should fail) + build( { 'public': true }, null, true ); + } + catch ( e ) + { + assert.ok( e.message.search( name ) !== -1, + "Property override error message should contain getter/setter name" + ); + return; + } + + assert.fail( type + " should not be able to override properties" ); +} ); + From 394b4981bf48e41966c31ab8b997e20e80954e43 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 14 Jun 2011 19:44:11 -0400 Subject: [PATCH 08/17] Can no longer override getters/setters with methods --- lib/member_builder.js | 8 ++++ test/test-member_builder-method.js | 72 ++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/lib/member_builder.js b/lib/member_builder.js index 5b8571f..a3c6225 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -153,6 +153,14 @@ function validateMethod( keywords, prev_data, value, name ) ); } + // do not allow overriding getters/setters + if ( prev_data && ( prev_data.get || prev_data.set ) ) + { + throw TypeError( + "Cannot override getter/setter '" + name + "' with method" + ); + } + // search for any previous instances of this member if ( prev ) { diff --git a/test/test-member_builder-method.js b/test/test-member_builder-method.js index e6f7775..e092c69 100644 --- a/test/test-member_builder-method.js +++ b/test/test-member_builder-method.js @@ -291,3 +291,75 @@ mb_common.assertCommon(); }, TypeError, "Cannot declare private abstract method" ); } )(); + +/** + * While getters are technically methods, it doesn't make sense to override + * getters/setters with methods because they are fundamentally different. + */ +( function testCannotOverrideGetters() +{ + mb_common.members[ 'public' ] = {}; + Object.defineProperty( mb_common.members[ 'public' ], mb_common.name, { + get: function() {}, + } ); + + try + { + mb_common.value = function() {}; + mb_common.buildMemberQuick( {}, true ); + } + catch ( e ) + { + assert.ok( e.message.search( mb_common.name ) !== -1, + "Method override getter failure should contain method name" + ); + + // ensure we have the correct error + assert.ok( e.message.search( 'getter' ) !== -1, + "Proper error is thrown for getter override failure" + ); + + return; + } + + assert.fail( + "Should not be permitted to override getters with methods" + ); +} )(); + + +/** + * While setters are technically methods, it doesn't make sense to override + * getters/setters with methods because they are fundamentally different. + */ +( function testCannotOverrideSetters() +{ + mb_common.members[ 'public' ] = {}; + Object.defineProperty( mb_common.members[ 'public' ], mb_common.name, { + set: function() {}, + } ); + + try + { + mb_common.value = function() {}; + mb_common.buildMemberQuick( {}, true ); + } + catch ( e ) + { + assert.ok( e.message.search( mb_common.name ) !== -1, + "Method override setter failure should contain method name" + ); + + // ensure we have the correct error + assert.ok( e.message.search( 'setter' ) !== -1, + "Proper error is thrown for setter override failure" + ); + + return; + } + + assert.fail( + "Should not be permitted to override setters with methods" + ); +} )(); + From 5065525f0d908ee846c72999de012a6429808e77 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 14 Jun 2011 22:18:26 -0400 Subject: [PATCH 09/17] Can no longer override getters/setters with properties --- lib/member_builder.js | 8 ++++ test/test-member_builder-prop.js | 72 ++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/lib/member_builder.js b/lib/member_builder.js index a3c6225..3b6e7e7 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -240,6 +240,14 @@ exports.buildProp = function( members, meta, name, value, keywords, base ) ); } + // do not allow overriding getters/setters + if ( prev_data && ( prev_data.get || prev_data.set ) ) + { + throw TypeError( + "Cannot override getter/setter '" + name + "' with property" + ); + } + // do not permit visibility de-escalation if ( prev && ( prev_data.visibility < getVisibilityValue( keywords ) ) ) { diff --git a/test/test-member_builder-prop.js b/test/test-member_builder-prop.js index b1afbb3..07b4048 100644 --- a/test/test-member_builder-prop.js +++ b/test/test-member_builder-prop.js @@ -66,3 +66,75 @@ mb_common.assertCommon(); }, TypeError, "Cannot declare abstract property" ); } )(); + +/** + * While getters act as properties, it doesn't make sense to override + * getters/setters with properties because they are fundamentally different. + */ +( function testCannotOverrideGetters() +{ + mb_common.members[ 'public' ] = {}; + Object.defineProperty( mb_common.members[ 'public' ], mb_common.name, { + get: function() {}, + } ); + + try + { + mb_common.value = 'foo'; + mb_common.buildMemberQuick( {}, true ); + } + catch ( e ) + { + assert.ok( e.message.search( mb_common.name ) !== -1, + "Property override getter failure should contain property name" + ); + + // ensure we have the correct error + assert.ok( e.message.search( 'getter' ) !== -1, + "Proper error is thrown for getter override failure" + ); + + return; + } + + assert.fail( + "Should not be permitted to override getters with properties" + ); +} )(); + + +/** + * While setters act as properties, it doesn't make sense to override + * getters/setters with properties because they are fundamentally different. + */ +( function testCannotOverrideSetters() +{ + mb_common.members[ 'public' ] = {}; + Object.defineProperty( mb_common.members[ 'public' ], mb_common.name, { + set: function() {}, + } ); + + try + { + mb_common.value = 'foo'; + mb_common.buildMemberQuick( {}, true ); + } + catch ( e ) + { + assert.ok( e.message.search( mb_common.name ) !== -1, + "Property override setter failure should contain method name" + ); + + // ensure we have the correct error + assert.ok( e.message.search( 'setter' ) !== -1, + "Proper error is thrown for setter override failure" + ); + + return; + } + + assert.fail( + "Should not be permitted to override setters with properties" + ); +} )(); + From a1585a351e6d09407b97ff2cb7ec991a5950aeec Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 14 Jun 2011 23:00:37 -0400 Subject: [PATCH 10/17] Not running getter/setter override tests if unsupported by environment --- test/test-member_builder-method.js | 13 ++++++++++++- test/test-member_builder-prop.js | 13 ++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/test/test-member_builder-method.js b/test/test-member_builder-method.js index e092c69..7360f84 100644 --- a/test/test-member_builder-method.js +++ b/test/test-member_builder-method.js @@ -25,7 +25,8 @@ var common = require( './common' ), assert = require( 'assert' ), mb_common = require( __dirname + '/inc-member_builder-common' ), - builder = common.require( 'member_builder' ) + builder = common.require( 'member_builder' ), + util = common.require( 'util' ) ; mb_common.funcVal = 'foobar'; @@ -298,6 +299,11 @@ mb_common.assertCommon(); */ ( function testCannotOverrideGetters() { + if ( util.definePropertyFallback() ) + { + return; + } + mb_common.members[ 'public' ] = {}; Object.defineProperty( mb_common.members[ 'public' ], mb_common.name, { get: function() {}, @@ -334,6 +340,11 @@ mb_common.assertCommon(); */ ( function testCannotOverrideSetters() { + if ( util.definePropertyFallback() ) + { + return; + } + mb_common.members[ 'public' ] = {}; Object.defineProperty( mb_common.members[ 'public' ], mb_common.name, { set: function() {}, diff --git a/test/test-member_builder-prop.js b/test/test-member_builder-prop.js index 07b4048..f568868 100644 --- a/test/test-member_builder-prop.js +++ b/test/test-member_builder-prop.js @@ -25,7 +25,8 @@ var common = require( './common' ), assert = require( 'assert' ), mb_common = require( __dirname + '/inc-member_builder-common' ), - builder = common.require( 'member_builder' ) + builder = common.require( 'member_builder' ), + util = common.require( 'util' ) ; @@ -73,6 +74,11 @@ mb_common.assertCommon(); */ ( function testCannotOverrideGetters() { + if ( util.definePropertyFallback() ) + { + return; + } + mb_common.members[ 'public' ] = {}; Object.defineProperty( mb_common.members[ 'public' ], mb_common.name, { get: function() {}, @@ -109,6 +115,11 @@ mb_common.assertCommon(); */ ( function testCannotOverrideSetters() { + if ( util.definePropertyFallback() ) + { + return; + } + mb_common.members[ 'public' ] = {}; Object.defineProperty( mb_common.members[ 'public' ], mb_common.name, { set: function() {}, From e2581deb9091033c3c19c0c9255927279414a51c Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 29 Jun 2011 20:49:56 -0400 Subject: [PATCH 11/17] Added Warning --- lib/warn.js | 67 ++++++++++++++++++++++++ test/test-warn-exception.js | 101 ++++++++++++++++++++++++++++++++++++ tools/combine | 2 +- 3 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 lib/warn.js create mode 100644 test/test-warn-exception.js diff --git a/lib/warn.js b/lib/warn.js new file mode 100644 index 0000000..b9983f8 --- /dev/null +++ b/lib/warn.js @@ -0,0 +1,67 @@ +/** + * ease.js warning system + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser 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 Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package core + */ + + +/** + * Permits wrapping an exception as a warning + * + * Warnings are handled differently by the system, depending on the warning + * level that has been set. + * + * @param {Error} e exception (error) to wrap + * + * @return {Warning} new warning instance + */ +var Warning = exports.Warning = function( e ) +{ + // allow instantiation without use of 'new' keyword + if ( !( this instanceof Warning ) ) + { + return new Warning( e ); + } + + // ensure we're wrapping an exception + if ( !( e instanceof Error ) ) + { + throw TypeError( "Must provide exception to wrap" ); + } + + // copy over the message for convenience + this.message = e.message; + this._error = e; +}; + +Warning.prototype = Error(); + + +/** + * Return the error wrapped by the warning + * + * @return {Error} wrapped error + */ +Warning.prototype.getError = function() +{ + return this._error; +}; + diff --git a/test/test-warn-exception.js b/test/test-warn-exception.js new file mode 100644 index 0000000..72cac1a --- /dev/null +++ b/test/test-warn-exception.js @@ -0,0 +1,101 @@ +/** + * Tests the Warning prototype + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser 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 Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package test + */ + +var common = require( './common' ), + assert = require( 'assert' ), + Warning = common.require( 'warn' ).Warning +; + + +/** + * Warning's prototype should be Error to ensure instanceof() checks work + * properly + */ +( function testWarningIsAvailableAndHasErrorAsPrototype() +{ + assert.ok( ( Warning.prototype instanceof Error ), + "Warning should be an instance of Error" + ); +} )(); + + +/** + * Just as with the other Error classes, as well as all ease.js classes, the + * 'new' keyword should be optional when instantiating the class + */ +( function testNewKeywordIsNotRequiredForInstantiation() +{ + assert.ok( Warning( Error( '' ) ) instanceof Warning, + "'new' keyword should not be necessary to instantiate Warning" + ); +} )(); + + +/** + * Warning message should be taken from the exception passed to it + */ +( function testCanWarningMessageIsSetFromWrappedException() +{ + var err = Error( 'oshit' ), + warning = Warning( err ); + + assert.equal( warning.message, err.message, + "Warning message should be taken from wrapped exception" + ); +} )(); + + +/** + * The whole point of Warning is to wrap an exception. So, ensure that one is + * wrapped. + */ +( function testThrowsExceptionIfNoExceptionIsWrapped() +{ + assert.throws( function() + { + Warning( /* nothing provided to wrap */ ); + }, TypeError, "Exception should be thrown if no exception is provided" ); + + assert.throws( function() + { + Warning( 'not an exception' ); + }, TypeError, "Exception should be thrown if given value is not an Error" ); +} )(); + + +/** + * We must provide access to the wrapped exception so that it can be properly + * handled. Warning is only intended to provide additional information so that + * ease.js may handle it differently than other Error instances. + */ +( function testCanRetrieveWrappedException() +{ + var err = Error( 'foo' ), + warning = Warning( err ); + + assert.deepEqual( err, warning.getError(), + "Can retrieve wrapped exception" + ); +} )(); + diff --git a/tools/combine b/tools/combine index e968e5b..ed2bf6f 100755 --- a/tools/combine +++ b/tools/combine @@ -29,7 +29,7 @@ TPL_VAR='/**{CONTENT}**/' RMTRAIL="$PATH_TOOLS/rmtrail" # order matters -CAT_MODULES="prop_parser util propobj member_builder class_builder" +CAT_MODULES="warn prop_parser util propobj member_builder class_builder" CAT_MODULES="$CAT_MODULES class class_final class_abstract interface" ## From 90aaaeb3a39d83adf1fa79d5427f392fbf56d322 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 29 Jun 2011 21:22:57 -0400 Subject: [PATCH 12/17] Added 'log' warning handler --- lib/warn.js | 28 +++++++++ test/test-warn-handlers.js | 118 +++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 test/test-warn-handlers.js diff --git a/lib/warn.js b/lib/warn.js index b9983f8..03409f2 100644 --- a/lib/warn.js +++ b/lib/warn.js @@ -65,3 +65,31 @@ Warning.prototype.getError = function() return this._error; }; + +/** + * Core warning handlers + * @type {Object} + */ +exports.handlers = { + /** + * Logs message to console + * + * Will attempt to log using console.warn(), falling back to console.log() + * if necessary and aborting entirely if neither is available. + * + * This is useful as a default option to bring problems to the developer's + * attention without affecting the control flow of the software. + * + * @param {Warning} warning to log + * + * @return {undefined} + */ + log: function( warning ) + { + var dest; + + console && ( dest = console.warn || console.log ) && + dest( warning.message ); + }, +}; + diff --git a/test/test-warn-handlers.js b/test/test-warn-handlers.js new file mode 100644 index 0000000..66d7a6e --- /dev/null +++ b/test/test-warn-handlers.js @@ -0,0 +1,118 @@ +/** + * Tests core warning handlers + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser 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 Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package test + */ + +var common = require( './common' ), + assert = require( 'assert' ), + warn = common.require( 'warn' ), + Warning = warn.Warning, + + warning = Warning( Error( 'gninraw' ) ) +; + + +/** + * The log warning handler should log warnings to the console + */ +( function testLogWarningHandlerLogsMessageToConsole() +{ + var logged = false, + + // back up console ref + console_ = console + ; + + // mock console + console = { + warn: function( message ) + { + assert.equal( message, warning.message, + "Should log proper message to console" + ); + + logged = true; + }, + }; + + // call handler with the warning + warn.handlers.log( warning ); + + assert.equal( logged, true, + "Message should be logged to console" + ); + + // restore console + console = console_; +} )(); + + +/** + * Some environments may not have a console reference, or they may not have + * console.warn. In this case, we just want to make sure we don't throw an error + * when attempting to invoke undefined, or access a property of undefined. + */ +( function testLogWarningHandlerHandlesMissingConsole() +{ + // back up console + var console_ = console; + + // destroy it + console = undefined; + + // attempt to log + warn.handlers.log( warning ); + + // restore console + console = console_; +} )(); + + +/** + * Furthermore, an environment may implement console.log(), but not + * console.warn(). By default, we use warn(), so let's ensure we can fall back + * to log() if warn() is unavailable. + */ +( function testLogWarningHandlerWillFallBackToLogMethodIfWarnIsMissing() +{ + // back up and overwrite console to contain only log() + var console_ = console, + given = ''; + + console = { + log: function( message ) + { + given = message; + } + }; + + // attempt to log + warn.handlers.log( warning ); + + assert.equal( given, warning.message, + "Should fall back to log() and log proper message" + ); + + // restore console + console = console_; +} )(); + From bdd7df20112fe5660ec5a59017615473e5eaa7a3 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 29 Jun 2011 21:34:33 -0400 Subject: [PATCH 13/17] Added throwError warning handler --- lib/warn.js | 16 ++++++++++++++++ test/test-warn-handlers.js | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/lib/warn.js b/lib/warn.js index 03409f2..9a4116b 100644 --- a/lib/warn.js +++ b/lib/warn.js @@ -91,5 +91,21 @@ exports.handlers = { console && ( dest = console.warn || console.log ) && dest( warning.message ); }, + + + /** + * Throws the error associated with the warning + * + * This handler is useful for development and will ensure that problems are + * brought to the attention of the developer. + * + * @param {Warning} warning to log + * + * @return {undefined} + */ + throwError: function( warning ) + { + throw warning.getError(); + }, }; diff --git a/test/test-warn-handlers.js b/test/test-warn-handlers.js index 66d7a6e..8d509b5 100644 --- a/test/test-warn-handlers.js +++ b/test/test-warn-handlers.js @@ -116,3 +116,25 @@ var common = require( './common' ), console = console_; } )(); + +/** + * The throwError warning handler should throw the wrapped error as an exception + */ +( function testThrowErrorWarningHandlerThrowsWrappedError() +{ + try + { + warn.handlers.throwError( warning ); + } + catch ( e ) + { + assert.deepEqual( e, warning.getError(), + "Wrapped exception should be thrown" + ); + + return; + } + + assert.fail( "Wrapped exception should be thrown" ); +} )(); + From f405f072f2a385222a4a153b3219647d45ad9d7a Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 29 Jun 2011 21:39:50 -0400 Subject: [PATCH 14/17] Added 'dismiss' warning handler - Phew. That was a hard one. --- lib/warn.js | 19 +++++++++++++++++++ test/test-warn-handlers.js | 22 ++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/warn.js b/lib/warn.js index 9a4116b..bb670ef 100644 --- a/lib/warn.js +++ b/lib/warn.js @@ -107,5 +107,24 @@ exports.handlers = { { throw warning.getError(); }, + + + /** + * Ignores warnings + * + * This is useful in a production environment where (a) warnings will affect + * the reputation of the software or (b) warnings may provide too much + * insight into the software. If using this option, you should always + * develop in a separate environment so that the system may bring warnings + * to your attention. + * + * @param {Warning} warning to log + * + * @return {undefined} + */ + dismiss: function( warning ) + { + // do nothing + }, }; diff --git a/test/test-warn-handlers.js b/test/test-warn-handlers.js index 8d509b5..e729cba 100644 --- a/test/test-warn-handlers.js +++ b/test/test-warn-handlers.js @@ -138,3 +138,25 @@ var common = require( './common' ), assert.fail( "Wrapped exception should be thrown" ); } )(); + +/** + * The 'dismiss' error handler is a pretty basic concept. Simply do nothing. We + * don't want to log, we don't want to throw anything, we just want to pretend + * nothing ever happened and move on our merry way. This is intended for use in + * production environments where providing warnings may provide too much insight + * into the software. + */ +( function testDismissWarningHandlerShouldDoNothing() +{ + // destroy the console to ensure nothing is logged + var console_ = console; + console = undefined; + + // don't catch anything, to ensure no errors occur and that no exceptions + // are thrown + warn.handlers.dismiss( warning ); + + // restore console + console = console_; +} )(); + From e7c20e049439db558c76c8c75708b32b576baf24 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 29 Jun 2011 21:42:39 -0400 Subject: [PATCH 15/17] Ensure we don't throw errors for warning handler tests if console is not defined --- test/test-warn-handlers.js | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/test/test-warn-handlers.js b/test/test-warn-handlers.js index e729cba..6c875b0 100644 --- a/test/test-warn-handlers.js +++ b/test/test-warn-handlers.js @@ -31,6 +31,23 @@ var common = require( './common' ), ; +/** + * Return the console object, without throwing errors if it does not exist + * + * @return {Object} console + */ +function backupConsole() +{ + // ensure that we don't throw errors if console is not defined + if ( typeof console !== 'undefined' ) + { + return console; + } + + return undefined; +} + + /** * The log warning handler should log warnings to the console */ @@ -39,7 +56,7 @@ var common = require( './common' ), var logged = false, // back up console ref - console_ = console + console_ = backupConsole() ; // mock console @@ -74,7 +91,7 @@ var common = require( './common' ), ( function testLogWarningHandlerHandlesMissingConsole() { // back up console - var console_ = console; + var console_ = backupConsole(); // destroy it console = undefined; @@ -95,7 +112,7 @@ var common = require( './common' ), ( function testLogWarningHandlerWillFallBackToLogMethodIfWarnIsMissing() { // back up and overwrite console to contain only log() - var console_ = console, + var console_ = backupConsole(), given = ''; console = { @@ -149,7 +166,7 @@ var common = require( './common' ), ( function testDismissWarningHandlerShouldDoNothing() { // destroy the console to ensure nothing is logged - var console_ = console; + var console_ = backupConsole(); console = undefined; // don't catch anything, to ensure no errors occur and that no exceptions From c672a2b5756e56d1ac77dfadcb2182dd9ea34305 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 29 Jun 2011 22:33:57 -0400 Subject: [PATCH 16/17] Added ability to set and trigger the active warning handler --- lib/warn.js | 38 +++++++++++++++++++ test/test-warn-impl.js | 83 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 test/test-warn-impl.js diff --git a/lib/warn.js b/lib/warn.js index bb670ef..af45948 100644 --- a/lib/warn.js +++ b/lib/warn.js @@ -22,6 +22,12 @@ * @package core */ +/** + * Active warning handler + * @type {function()} + */ +var _handler = null; + /** * Permits wrapping an exception as a warning @@ -128,3 +134,35 @@ exports.handlers = { }, }; + +/** + * Sets the active warning handler + * + * You may use any of the predefined warning handlers or pass your own function. + * + * @param {function( Warning )} handler warning handler + * + * @return {undefined} + */ +exports.setHandler = function( handler ) +{ + _handler = handler; +}; + + +/** + * Handles a warning using the active warning handler + * + * @param {Warning} warning warning to handle + * + * @return {undefined} + */ +exports.handle = function( warning ) +{ + _handler( warning ); +} + + +// set the default handler +_handler = exports.handlers.log; + diff --git a/test/test-warn-impl.js b/test/test-warn-impl.js new file mode 100644 index 0000000..7e714ae --- /dev/null +++ b/test/test-warn-impl.js @@ -0,0 +1,83 @@ +/** + * Tests warning system implementation + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser 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 Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package test + */ + +var common = require( './common' ), + assert = require( 'assert' ), + warn = common.require( 'warn' ) +; + + +/** + * The default warning handler should be the 'log' handler. This is a friendly + * compromise that will allow the developer to be warned of potential issues + * without affecting program execution. + */ +( function testDefaultHandlerIsLogger() +{ + // back up console object + var console_ = ( typeof console !== 'undefined' ) ? console : undefined, + called = false; + + // stub it + console = { + warn: function() + { + called = true; + }, + }; + + warn.handle( warn.Warning( Error( 'foo' ) ) ); + + assert.ok( called, + "Default handler will log to console" + ); + + // restore console + console = console_; +} )(); + + +/** + * The warning handler can be altered at runtime. Ensure we can set it and call + * it appropriately. We do not need to use one of the pre-defined handlers. + */ +( function testCanSetAndCallWarningHandler() +{ + var given, + warning = warn.Warning( Error( 'foo' ) ); + + // set a stub warning handler + warn.setHandler( function( warn ) + { + given = warn; + } ); + + // trigger the handler + warn.handle( warning ); + + assert.deepEqual( given, warning, + "Set warning handler should be called with given Warning" + ); +} )(); + From 6906eee77f3cc1a8d2bf77a57e4c5ee7808ba8c9 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 30 Jun 2011 23:03:09 -0400 Subject: [PATCH 17/17] Tests fail on minified files with uglify-js 1.0.3; disallowing for now --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b8e0beb..97ec0cf 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "dependencies": {}, "devDependencies": { - "uglify-js": ">=1.0.2" + "uglify-js": "<=1.0.2" }, "licenses": [