[#25] Added visibility de-escalation and escalation tests to MemberBuilderValidator for getters/setters

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(

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_gs = ( ( prev_data.get || prev_data.set ) ? true : false )
-        prev_keywords = ( prev && prev.___$$keywords$$ )
-            ? prev.___$$keywords$$
-            : {}
     ;
 
-    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 + "'"
+            );
+        }
     }
 }
 

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
+                    );
+                }
+            );
+        };
     },
 
 
@@ -76,4 +92,29 @@ require( 'common' ).testCase(
             );
         } );
     },
+
+
+    /**
+     * 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 );
+        } );
+    },
 } );