From 5065525f0d908ee846c72999de012a6429808e77 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 14 Jun 2011 22:18:26 -0400 Subject: [PATCH] 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" + ); +} )(); +