From db84c6fc6e113f2dc3fec11aa18dfb34b2da8a27 Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike@mikegerwitz.com>
Date: Thu, 22 Dec 2011 23:10:01 -0500
Subject: [PATCH] Added virtual and override restrictions to getters/setters

---
 lib/MemberBuilderValidator.js                 | 15 ++++++++
 .../GetterSetterTest.js                       | 35 ++++++++++++++++++-
 test/MemberBuilderValidator/inc-common.js     |  8 +++--
 test/test-class-gettersetter.js               |  4 +--
 4 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js
index fe33c5a..c0d4f3e 100644
--- a/lib/MemberBuilderValidator.js
+++ b/lib/MemberBuilderValidator.js
@@ -321,6 +321,21 @@ exports.prototype.validateGetterSetter = function(
             );
         }
 
+        if ( !( prev_keywords[ 'virtual' ] ) )
+        {
+            throw TypeError(
+                "Cannot override non-virtual getter/setter '" + name + "'"
+            );
+        }
+
+        if ( !( keywords[ 'override' ] ) )
+        {
+            throw TypeError(
+                "Attempting to override getter/setter '" + name +
+                "' without 'override' keyword"
+            );
+        }
+
         // do not permit visibility de-escalation
         if ( this._getVisibilityValue( prev_keywords || {} )
             < this._getVisibilityValue( keywords )
diff --git a/test/MemberBuilderValidator/GetterSetterTest.js b/test/MemberBuilderValidator/GetterSetterTest.js
index 4e37046..a0e5b6f 100644
--- a/test/MemberBuilderValidator/GetterSetterTest.js
+++ b/test/MemberBuilderValidator/GetterSetterTest.js
@@ -40,7 +40,8 @@ require( 'common' ).testCase(
         this.quickKeywordTest = function( keywords, identifier, prev )
         {
             shared.quickKeywordTest.call( this,
-                'validateGetterSetter', keywords, identifier, prev
+                'validateGetterSetter', keywords, identifier, prev,
+                { get: function() {}, set: function() {} }
             );
         };
 
@@ -49,6 +50,9 @@ require( 'common' ).testCase(
             shared.quickVisChangeTest.call( _self, start, override, failtest,
                 function( name, startobj, overrideobj )
                 {
+                    startobj.virtual     = true;
+                    overrideobj.override = true;
+
                     _self.sut.validateGetterSetter(
                         name, {}, overrideobj,
                         { get: function() {}, set: function() {} },
@@ -165,4 +169,33 @@ require( 'common' ).testCase(
     {
         this.quickKeywordTest( [ 'const' ], 'const' );
     },
+
+
+    /**
+     * Getters/setters can be overridden just like methods, so long as they
+     * follow the same keyword restrictions.
+     */
+    'Can override virtual getter/setter with override keyword': function()
+    {
+        this.quickKeywordTest( [ 'override' ], null, [ 'virtual' ] );
+    },
+
+
+    /**
+     * The 'override' keyword must be provided if a member is being overridden.
+     */
+    'Must provide override keyword when overriding getter/setter': function()
+    {
+        this.quickKeywordTest( [], 'override', [ 'virtual' ] );
+    },
+
+
+    /**
+     * Just like methods, getters/setters may only be overridden if they have
+     * been declared 'virtual'.
+     */
+    'Cannot override non-virtual getter/setter': function()
+    {
+        this.quickKeywordTest( [ 'override' ], 'non-virtual', [] );
+    },
 } );
diff --git a/test/MemberBuilderValidator/inc-common.js b/test/MemberBuilderValidator/inc-common.js
index caf4514..dd67252 100644
--- a/test/MemberBuilderValidator/inc-common.js
+++ b/test/MemberBuilderValidator/inc-common.js
@@ -82,11 +82,13 @@ exports.quickFailureTest = function( name, identifier, action )
  * To test overrides, specify keywords for 'prev'. To test for success instead
  * of failure, set identifier to null.
  */
-exports.quickKeywordTest = function( type, keywords, identifier, prev )
+exports.quickKeywordTest = function(
+    type, keywords, identifier, prev, prev_data
+)
 {
     var keyword_obj = {},
         prev_obj    = {},
-        prev_data   = {},
+        prev_data   = prev_data || {},
         name        = exports.testName,
         _self       = this;
 
@@ -106,7 +108,7 @@ exports.quickKeywordTest = function( type, keywords, identifier, prev )
         }
 
         // define a dummy previous method value
-        prev_data = { member: function() {} };
+        prev_data.member = function() {};
     }
 
     var testfunc = function()
diff --git a/test/test-class-gettersetter.js b/test/test-class-gettersetter.js
index a3d1d57..2e83001 100644
--- a/test/test-class-gettersetter.js
+++ b/test/test-class-gettersetter.js
@@ -52,7 +52,7 @@ Object.defineProperty( foo_def, 'foo', {
     enumerable: true,
 } );
 
-Object.defineProperty( foo_def, 'bar', {
+Object.defineProperty( foo_def, 'virtual bar', {
     get: function ()
     {
         return 'durp';
@@ -64,7 +64,7 @@ Object.defineProperty( foo_def, 'bar', {
     enumerable: true,
 } );
 
-Object.defineProperty( sub_foo_def, 'bar', {
+Object.defineProperty( sub_foo_def, 'override bar', {
     get: function ()
     {
         return this.bar2;