Validation warnings now stored in state object

This will allow for additional processing before actually triggering the
warnings. For the sake of this commit, though, we just keep with existing
functionality.

lib/ClassBuilder.js

@@ -451,6 +451,9 @@ exports.prototype.buildMembers = function buildMembers(
         smethods = static_members.methods,
         sprops   = static_members.props,
 
+        // holds member builder state
+        state = {},
+
         _self = this
     ;
 
@@ -527,7 +530,7 @@ exports.prototype.buildMembers = function buildMembers(
 
             var used = _self._memberBuilder.buildMethod(
                 dest, null, name, func, keywords, instLookup,
-                class_id, base
+                class_id, base, state
             );
 
             // do nothing more if we didn't end up using this definition
@@ -556,6 +559,9 @@ exports.prototype.buildMembers = function buildMembers(
             }
         },
     } );
+
+    // process accumulated member state
+    this._memberBuilder.end( state );
 }
 
 

lib/MemberBuilder.js

@@ -95,6 +95,10 @@ exports.initMembers = function( mpublic, mprotected, mprivate )
  * Copies a method to the appropriate member prototype, depending on
  * visibility, and assigns necessary metadata from keywords
  *
+ * The provided ``member run'' state object is required and will be
+ * initialized automatically if it has not been already. For the first
+ * member of a run, the object should be empty.
+ *
  * @param  {__visobj}  members
  * @param  {!Object}   meta     metadata container
  * @param  {string}    name     property name
@@ -108,10 +112,12 @@ exports.initMembers = function( mpublic, mprotected, mprivate )
  * @param  {number}   cid   class id
  * @param  {Object=}  base  optional base object to scan
  *
+ * @param  {Object}  state  member run state object
+ *
  * @return  {undefined}
  */
 exports.buildMethod = function(
-    members, meta, name, value, keywords, instCallback, cid, base
+    members, meta, name, value, keywords, instCallback, cid, base, state
 )
 {
     // TODO: We can improve performance by not scanning each one individually
@@ -125,7 +131,7 @@ exports.buildMethod = function(
     // ensure that the declaration is valid (keywords make sense, argument
     // length, etc)
     this._validate.validateMethod(
-        name, value, keywords, prev_data, prev_keywords
+        name, value, keywords, prev_data, prev_keywords, state
     );
 
     // we might be overriding an existing method
@@ -492,3 +498,19 @@ exports._getVisibilityValue = function( keywords )
     }
 }
 
+
+/**
+ * End member run and perform post-processing on state data
+ *
+ * A ``member run'' should consist of the members required for a particular
+ * object (class/interface/etc). This action will perform validation
+ * post-processing if a validator is available.
+ *
+ * @param  {Object}  state  member run state
+ *
+ * @return  {undefined}
+ */
+exports.end = function( state )
+{
+    this._validate && this._validate.end( state );
+};

lib/MemberBuilderValidator.js

@@ -32,10 +32,71 @@ module.exports = exports = function MemberBuilderValidator( warn_handler )
 
 
 /**
- * Validates a method declaration, ensuring that keywords are valid, overrides
+ * Initialize validation state if not already done
- * make sense, etc.
  *
- * Throws exception on validation failure
+ * @param  {Object}  state  validation state
+ *
+ * @return  {Object}  provided state object STATE
+ */
+exports.prototype._initState = function( state )
+{
+    if ( state.__vready ) return state;
+
+    state.warn = {};
+    state.__vready = true;
+    return state;
+};
+
+
+/**
+ * Perform post-processing on and invalidate validation state
+ *
+ * All queued warnings will be triggered.
+ *
+ * @param  {Object}  state  validation state
+ *
+ * @return  {undefined}
+ */
+exports.prototype.end = function( state )
+{
+    // trigger warnings
+    for ( var f in state.warn )
+    {
+        var warns = state.warn[ f ];
+        for ( var id in warns )
+        {
+            this._warningHandler( warns[ id ] );
+        }
+    }
+
+    state.__vready = false;
+};
+
+
+/**
+ * Enqueue warning within validation state
+ *
+ * @param  {Object}   state   validation state
+ * @param  {string}   member  member name
+ * @param  {string}   id      warning identifier
+ * @param  {Warning}  warn    warning
+ *
+ * @return  {undefined}
+ */
+function _addWarn( state, member, id, warn )
+{
+    ( state.warn[ member ] = state.warn[ member ] || {} )[ id ] = warn;
+}
+
+
+/**
+ * Validates a method declaration, ensuring that keywords are valid,
+ * overrides make sense, etc.
+ *
+ * Throws exception on validation failure. Warnings are stored in the state
+ * object for later processing. The state object will be initialized if it
+ * has not been already; for the initial validation, the state object should
+ * be empty.
  *
  * @param  {string}  name  method name
  * @param  {*}       value method value
@@ -45,12 +106,16 @@ module.exports = exports = function MemberBuilderValidator( warn_handler )
  * @param  {Object}  prev_data      data of member being overridden
  * @param  {Object}  prev_keywords  keywords of member being overridden
  *
+ * @param  {Object}  state  pre-initialized state object
+ *
  * @return {undefined}
  */
 exports.prototype.validateMethod = function(
-    name, value, keywords, prev_data, prev_keywords
+    name, value, keywords, prev_data, prev_keywords, state
 )
 {
+    this._initState( state );
+
     var prev = ( prev_data ) ? prev_data.member : null;
 
     if ( keywords[ 'abstract' ] )
@@ -202,7 +267,7 @@ exports.prototype.validateMethod = function(
         // but it shouldn't stop the class definition (it doesn't adversely
         // affect the functionality of the class, unless of course the method
         // attempts to reference a supertype)
-        this._warningHandler( Error(
+        _addWarn( state, name, 'no', Error(
             "Method '" + name +
             "' using 'override' keyword without super method"
         ) );

lib/interface.js

@@ -190,6 +190,9 @@ var extend = ( function( extending )
             prototype = new base(),
             iname     = '',
 
+            // holds validation state
+            vstate = {},
+
             members = member_builder.initMembers(
                 prototype, prototype, prototype
             )
@@ -235,7 +238,8 @@ var extend = ( function( extending )
                     }
 
                     member_builder.buildMethod(
-                        members, null, name, value, keywords
+                        members, null, name, value, keywords,
+                        null, 0, {}, vstate
                     );
                 },
             } );

test/ClassBuilder/MemberRestrictionTest.js

@@ -38,16 +38,6 @@ require( 'common' ).testCase(
     },
 
 
-    setUp: function()
-    {
-        this.builder = this.Sut(
-            this.require( 'MemberBuilder' )(),
-            this.require( 'VisibilityObjectFactoryFactory' )
-                .fromEnvironment()
-        );
-    },
-
-
     /**
      * It's always useful to be able to quickly reference a list of reserved
      * members so that an implementer can programatically handle runtime
@@ -288,4 +278,32 @@ require( 'common' ).testCase(
             _self.AbstractClass( dfn );
         } );
     },
+
+
+    /**
+     * During the course of processing, certain data are accumulated into
+     * the member builder state; this state must be post-processed to
+     * complete anything that may be pending.
+     */
+    'Member builder state is ended after processing': function()
+    {
+        var _self = this,
+            build = this.require( 'MemberBuilder' )();
+
+        var sut = this.Sut(
+            build,
+            this.require( 'VisibilityObjectFactoryFactory' )
+                .fromEnvironment()
+        );
+
+        // TODO: test that we're passed the right state
+        var called = false;
+        build.end = function( state )
+        {
+            called = true;
+        };
+
+        sut.build( {} );
+        this.assertOk( called );
+    },
 } );

test/MemberBuilder/GetterSetterTest.js

@@ -30,9 +30,10 @@ require( 'common' ).testCase(
     {
         var _self = this;
 
-        this.testArgs = function( args, name, value, keywords )
+        this.testArgs = function( args, name, value, keywords, state )
         {
-            shared.testArgs( _self, args, name, value, keywords, function(
+            shared.testArgs( _self, args, name, value, keywords, state,
+                function(
                     prev_default, pval_given, pkey_given
                 )
                 {
@@ -53,7 +54,8 @@ require( 'common' ).testCase(
                             given:    pkey_given,
                         },
                     };
-            } );
+                }
+            );
         };
     },
 

test/MemberBuilder/MethodTest.js

@@ -27,9 +27,10 @@ require( 'common' ).testCase(
     {
         var _self = this;
 
-        this.testArgs = function( args, name, value, keywords )
+        this.testArgs = function( args, name, value, keywords, state )
         {
-            shared.testArgs( _self, args, name, value, keywords, function(
+            shared.testArgs( _self, args, name, value, keywords, state,
+                function(
                     prev_default, pval_given, pkey_given
                 )
                 {
@@ -50,7 +51,8 @@ require( 'common' ).testCase(
                             given:    pkey_given,
                         },
                     };
-            } );
+                }
+            );
         };
 
         // simply intended to execute test two two perspectives
@@ -100,17 +102,19 @@ require( 'common' ).testCase(
 
             name     = 'foo',
             value    = function() {},
+            state    = {},
             keywords = {}
         ;
 
         this.mockValidate.validateMethod = function()
         {
             called = true;
-            _self.testArgs( arguments, name, value, keywords );
+            _self.testArgs( arguments, name, value, keywords, state );
         };
 
         this.assertOk( this.sut.buildMethod(
-            this.members, {}, name, value, keywords, function() {}, 1, {}
+            this.members, {}, name, value, keywords, function() {}, 1, {},
+            state
         ) );
 
         this.assertEqual( true, called, 'validateMethod() was not called' );

test/MemberBuilder/PropTest.js

@@ -27,9 +27,10 @@ require( 'common' ).testCase(
     {
         var _self = this;
 
-        this.testArgs = function( args, name, value, keywords )
+        this.testArgs = function( args, name, value, keywords, state )
         {
-            shared.testArgs( _self, args, name, value, keywords, function(
+            shared.testArgs( _self, args, name, value, keywords, state,
+                function(
                     prev_default, pval_given, pkey_given
                 )
                 {
@@ -50,7 +51,8 @@ require( 'common' ).testCase(
                             given:    pkey_given,
                         },
                     };
-            } );
+                }
+            );
         };
     },
 

test/MemberBuilder/inc-common.js

@@ -27,11 +27,14 @@
  * @param {string}     name       member name
  * @param {*}          value      expected value
  * @param {Object}     keywords   expected keywords
- * @param {function()} prevLookup function to use to look up prev member data
+ * @param {Object}     state      validation state
+ * @param {function()} prevLookup function to look up prev member data
  *
  * @return {undefined}
  */
-exports.testArgs = function( testcase, args, name, value, keywords, prevLookup )
+exports.testArgs = function(
+    testcase, args, name, value, keywords, state, prevLookup
+)
 {
     var prev = {
         value:    { expected: null, given: args[ 3 ] },
@@ -41,24 +44,28 @@ exports.testArgs = function( testcase, args, name, value, keywords, prevLookup )
     prev = prevLookup( prev, prev.value.given, prev.keywords.given );
 
     testcase.assertEqual( name, args[ 0 ],
-        'Incorrect name passed to validator'
+        "Incorrect name passed to validator"
     );
 
     testcase.assertDeepEqual( value, args[ 1 ],
-        'Incorrect value passed to validator'
+        "Incorrect value passed to validator"
     );
 
     testcase.assertStrictEqual( keywords, args[ 2 ],
-        'Incorrect keywords passed to validator'
+        "Incorrect keywords passed to validator"
     );
 
     testcase.assertStrictEqual( prev.value.expected, prev.value.given,
-        'Previous data should contain prev value if overriding, ' +
+        "Previous data should contain prev value if overriding, " +
-        'otherwise null'
+        "otherwise null"
     );
 
     testcase.assertDeepEqual( prev.keywords.expected, prev.keywords.given,
-        'Previous keywords should contain prev keyword if ' +
+        "Previous keywords should contain prev keyword if " +
-        'overriding, otherwise null'
+        "overriding, otherwise null"
+    );
+
+    testcase.assertStrictEqual( state, args[ 5 ],
+        "State object was not passed to validator"
     );
 };

test/MemberBuilderValidator/MethodTest.js

@@ -51,13 +51,18 @@ require( 'common' ).testCase(
                     startobj.virtual     = true;
                     overrideobj.override = true;
 
+                    var state = {};
+
                     _self.sut.validateMethod(
                         name,
                         function() {},
                         overrideobj,
                         { member: function() {} },
-                        startobj
+                        startobj,
+                        state
                     );
+
+                    _self.sut.end( state );
                 },
                 failstr
             );
@@ -128,7 +133,7 @@ require( 'common' ).testCase(
             _self.sut.validateMethod(
                 name, function() {}, {},
                 { get: function() {} },
-                {}
+                {}, {}
             );
         } );
 
@@ -138,7 +143,7 @@ require( 'common' ).testCase(
             _self.sut.validateMethod(
                 name, function() {}, {},
                 { set: function() {} },
-                {}
+                {}, {}
             );
         } );
     },
@@ -161,7 +166,7 @@ require( 'common' ).testCase(
             _self.sut.validateMethod(
                 name, function() {}, {},
                 { member: 'immaprop' },
-                {}
+                {}, {}
             );
         } );
     },
@@ -246,7 +251,8 @@ require( 'common' ).testCase(
                 // this function returns each of its arguments, otherwise
                 // they'll be optimized away by Closure Compiler.
                 { member: function( a, b, c ) { return [a,b,c]; } },
-                { 'virtual': true }
+                { 'virtual': true },
+                {}
             );
         } );
 
@@ -262,7 +268,8 @@ require( 'common' ).testCase(
                 function() {},
                 { 'override': true },
                 { member: parent_method },
-                { 'virtual': true }
+                { 'virtual': true },
+                {}
             );
         } );
 
@@ -277,7 +284,8 @@ require( 'common' ).testCase(
                 method,
                 { 'override': true },
                 { member: function( a, b, c ) {} },
-                { 'virtual': true }
+                { 'virtual': true },
+                {}
             );
         }, Error );
     },
@@ -304,7 +312,8 @@ require( 'common' ).testCase(
                 function() {},
                 {},
                 { member: amethod },
-                { 'weak': true, 'abstract': true }
+                { 'weak': true, 'abstract': true },
+                {}
             );
         } );
 
@@ -316,7 +325,7 @@ require( 'common' ).testCase(
                 amethod,
                 { 'weak': true, 'abstract': true },
                 { member: function() {} },
-                {}
+                {}, {}
             );
         } );
     },
@@ -448,7 +457,7 @@ require( 'common' ).testCase(
         {
             // provide function instead of string
             _self.sut.validateMethod(
-                name, function() {}, { 'proxy': true }, {}, {}
+                name, function() {}, { 'proxy': true }, {}, {}, {}
             );
         } );
     },
@@ -464,7 +473,7 @@ require( 'common' ).testCase(
         this.assertDoesNotThrow( function()
         {
             _self.sut.validateMethod(
-                'foo', 'dest', { 'proxy': true }, {}, {}
+                'foo', 'dest', { 'proxy': true }, {}, {}, {}
             );
         }, TypeError );
     },

test/MemberBuilderValidator/inc-common.js

@@ -87,6 +87,7 @@ exports.quickKeywordTest = function(
         prev_obj    = {},
         prev_data   = prev_data || {},
         name        = exports.testName,
+        state       = {},
         _self       = this;
 
     // convert our convenient array into a keyword obj
@@ -114,7 +115,7 @@ exports.quickKeywordTest = function(
         var val = ( keyword_obj[ 'proxy' ] ) ? 'proxyDest': function() {};
 
         _self.sut[ type ](
-            name, val, keyword_obj, prev_data, prev_obj
+            name, val, keyword_obj, prev_data, prev_obj, state
         );
     };
 
@@ -126,6 +127,8 @@ exports.quickKeywordTest = function(
     {
         this.assertDoesNotThrow( testfunc );
     }
+
+    this.sut.end( state );
 };