diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index a61dba1..1e16384 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -208,10 +208,12 @@ exports.buildGetterSetter = function( members, meta, name, get, set, keywords, base ) { - var prev_data = scanMembers( members, name, base ); + var prev_data = scanMembers( members, name, base ), + prev_keywords = {} + ; this._validate.validateGetterSetter( - name, keywords, prev_data + name, keywords, prev_data, prev_keywords ); Object.defineProperty( diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index 3682220..e060492 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -242,28 +242,35 @@ exports.prototype.validateProperty = function( * @return {undefined} */ exports.prototype.validateGetterSetter = function( - name, keywords, prev_data + name, keywords, prev_data, prev_keywords ) { - var prev = ( prev_data ) ? prev_data.member : null, - - prev_keywords = ( prev && prev.___$$keywords$$ ) - ? prev.___$$keywords$$ - : {} + var prev = ( prev_data ) ? prev_data.member : null, + prev_gs = ( ( prev_data.get || prev_data.set ) ? true : false ) ; - if ( prev ) + if ( prev || prev_gs ) { // 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 ) ) + if ( !( prev_gs ) ) { throw TypeError( "Cannot override method or property '" + name + "' with getter/setter" ); } + + // do not permit visibility de-escalation + if ( this._getVisibilityValue( prev_keywords ) + < this._getVisibilityValue( keywords ) + ) + { + throw TypeError( + "Cannot de-escalate visibility of getter/setter '" + name + "'" + ); + } } } diff --git a/test/MemberBuilderValidator/GetterSetterTest.js b/test/MemberBuilderValidator/GetterSetterTest.js index 943447c..a57404b 100644 --- a/test/MemberBuilderValidator/GetterSetterTest.js +++ b/test/MemberBuilderValidator/GetterSetterTest.js @@ -31,7 +31,23 @@ require( 'common' ).testCase( { caseSetUp: function() { + var _self = this; + this.quickFailureTest = shared.quickFailureTest; + + this.quickVisChangeTest = function( start, override, failtest ) + { + shared.quickVisChangeTest.call( _self, start, override, failtest, + function( name, startobj, overrideobj ) + { + _self.sut.validateGetterSetter( + name, overrideobj, + { get: function() {}, set: function() {} }, + startobj + ); + } + ); + }; }, @@ -75,5 +91,30 @@ require( 'common' ).testCase( name, {}, { member: 'foo' } ); } ); - }, + }, + + + /** + * De-escalating the visibility of any member would alter the interface of a + * subtype, which would not be polymorphic. + */ + 'Getters/setters do not support visibility de-escalation': function() + { + this.quickVisChangeTest( 'public', 'protected', true ); + this.quickVisChangeTest( 'protected', 'private', true ); + }, + + + /** + * Contrary to the above test, we have no such problem with visibility + * escalation. + */ + 'Getters/setters support visibility escalation and equality': function() + { + var _self = this; + shared.visEscalationTest( function( cur ) + { + _self.quickVisChangeTest( cur[ 0 ], cur[ 1 ], false ); + } ); + }, } );