From 625f62bbf1691055c4181e4f6a4f2fb196b8dd42 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 23 Oct 2011 01:14:13 -0400 Subject: [PATCH] [#25] Moved MemberBuilderValidator property tests into new test case --- lib/MemberBuilderValidator.js | 5 +- test/MemberBuilderValidator/PropertyTest.js | 132 ++++++++++++++++++ test/test-member_builder-prop.js | 140 -------------------- 3 files changed, 135 insertions(+), 142 deletions(-) create mode 100644 test/MemberBuilderValidator/PropertyTest.js diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index b11676d..20b8d5d 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -179,7 +179,7 @@ exports.prototype.validateProperty = function( var prev = ( prev_data ) ? prev_data.member : null; // disallow overriding methods with properties - if ( prev instanceof Function ) + if ( typeof prev === 'function' ) { throw new TypeError( "Cannot override method '" + name + "' with property" @@ -188,7 +188,7 @@ exports.prototype.validateProperty = function( // do not allow overriding getters/setters if ( prev_data && ( prev_data.get || prev_data.set ) ) - { + { throw TypeError( "Cannot override getter/setter '" + name + "' with property" ); @@ -214,6 +214,7 @@ exports.prototype.validateProperty = function( ); } + // constants are static if ( keywords[ 'static' ] && keywords[ 'const' ] ) { throw TypeError( diff --git a/test/MemberBuilderValidator/PropertyTest.js b/test/MemberBuilderValidator/PropertyTest.js new file mode 100644 index 0000000..312e0e8 --- /dev/null +++ b/test/MemberBuilderValidator/PropertyTest.js @@ -0,0 +1,132 @@ +/** + * Tests member builder validation rules + * + * 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 shared = require( __dirname + '/inc-common' ); + + +require( 'common' ).testCase( +{ + caseSetUp: function() + { + this.quickFailureTest = shared.quickFailureTest; + + this.quickKeywordPropTest = function( keywords, identifier, prev ) + { + shared.quickKeywordTest.call( this, + 'validateProperty', keywords, identifier, prev + ); + }; + }, + + + setUp: function() + { + this.sut = this.require( 'MemberBuilderValidator' )(); + }, + + + /** + * Clearly, overriding a method with a property will have terrible + * polymorphic consequences on the resulting interface. + */ + 'Cannot override method with property': function() + { + var name = 'foo', + _self = this; + + this.quickFailureTest( name, 'property', function() + { + // attempt to override a method + _self.sut.validateProperty( + name, 'bar', {}, + { member: function() {} }, + {} + ); + } ); + }, + + + /** + * The concept of an abstract property does not make sense, as properties + * simply represent placeholders for values. Whether or not they actually + * hold a value is irrelevant. + */ + 'Cannot declare abstract property': function() + { + this.quickKeywordPropTest( [ 'abstract' ], 'abstract' ); + }, + + + /** + * Properties, unlike methods, are virtual by default. If a property's value + * can be reassigned, why would a subclass not be able to reassign it? If + * one wishes to prevent a property's value from changing, they should use + * the visibility modifiers or declare the property as a constant. + */ + 'Cannot declare virtual property': function() + { + this.quickKeywordPropTest( [ 'virtual' ], 'virtual' ); + }, + + + /** + * Declaring a constant as static would be redundant. + */ + 'Cannot declare static const property': function() + { + this.quickKeywordPropTest( [ 'static', 'const' ], 'Static' ); + }, + + + /* + * While getters act as properties, it doesn't make sense to override + * getters/setters with properties because they are fundamentally different. + */ + 'Cannot override getter/setter with property': function() + { + var name = 'foo', + _self = this; + + // test getter + this.quickFailureTest( name, 'getter/setter', function() + { + _self.sut.validateProperty( + name, 'bar', {}, + { get: function() {} }, + {} + ); + } ); + + // test setter + this.quickFailureTest( name, 'getter/setter', function() + { + _self.sut.validateProperty( + name, 'bar', {}, + { set: function() {} }, + {} + ); + } ); + }, +} ); + diff --git a/test/test-member_builder-prop.js b/test/test-member_builder-prop.js index 4ca47db..5c34ff3 100644 --- a/test/test-member_builder-prop.js +++ b/test/test-member_builder-prop.js @@ -51,143 +51,3 @@ var builder_method = mb_common.buildMember = function() // do assertions common to all member builders mb_common.assertCommon(); - -( function testCannotOverrideMethodWithProperty() -{ - // add a method - mb_common.buildMember = builder_method; - mb_common.value = function() {}; - mb_common.buildMemberQuick(); - - assert.throws( function() - { - // reset - mb_common.buildMember = builder.buildProp; - - // attempt to override with property - mb_common.value = 'foo'; - mb_common.buildMemberQuick( {}, true ); - }, TypeError, "Cannot override method with property" ); -} )(); - - -/** - * Abstract properties do not make sense. Simple as that. - */ -( function testCannotDeclareAbstractProperty() -{ - assert.throws( function() - { - mb_common.buildMemberQuick( { 'abstract': true } ); - }, TypeError, "Cannot declare abstract property" ); -} )(); - - -/** - * Properties, unlike methods, are virtual by default. If a property's value can - * be reassigned, why would a subclass not be able to reassign it? If one wishes - * to prevent a property's value from changing, they should use the visibility - * modifiers or declare the property as a constant. - */ -( function testCannotDeclareVirtualProperty() -{ - try - { - mb_common.buildMember = builder_method; - mb_common.buildMemberQuick( { 'virtual': true } ); - } - catch ( e ) - { - assert.ok( - e.message.search( mb_common.name ) !== -1, - "Virtual property error message should contain property name" - ); - - return; - } - - assert.fail( "Should not be permitted to declare virtual properties" ); -} )(); - - -/* - * While getters act as properties, it doesn't make sense to override - * getters/setters with properties because they are fundamentally different. - */ -( function testCannotOverrideGetters() -{ - if ( util.definePropertyFallback() ) - { - return; - } - - 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() -{ - if ( util.definePropertyFallback() ) - { - return; - } - - 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" - ); -} )(); -