diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index ccb0a2e..216e0ab 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -122,7 +122,6 @@ exports.buildMethod = function( if ( prev ) { - // TODO: warning if no super method when override keyword provided if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] ) { // override the method diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index b33928a..6bc0829 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -23,13 +23,15 @@ */ -module.exports = exports = function MemberBuilderValidator() +module.exports = exports = function MemberBuilderValidator( warn_handler ) { // permit omitting 'new' keyword if ( !( this instanceof module.exports ) ) { - return new module.exports(); + return new module.exports( warn_handler ); } + + this._warningHandler = warn_handler || function() {}; }; @@ -153,6 +155,17 @@ exports.prototype.validateMethod = function( ); } } + else if ( keywords.override ) + { + // using the override keyword without a super method may indicate a bug, + // 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( + "Method '" + name + + "' using 'override' keyword without super method" + ) ); + } }; diff --git a/test/MemberBuilderValidator/MethodTest.js b/test/MemberBuilderValidator/MethodTest.js index 9db0bc5..fc4b683 100644 --- a/test/MemberBuilderValidator/MethodTest.js +++ b/test/MemberBuilderValidator/MethodTest.js @@ -65,7 +65,17 @@ require( 'common' ).testCase( setUp: function() { - this.sut = this.require( 'MemberBuilderValidator' )(); + var _self = this; + + // can be used to intercept warnings; redefine in test + this.warningHandler = function( warning ) {}; + + this.sut = this.require( 'MemberBuilderValidator' )( + function( warning ) + { + _self.warningHandler( warning ); + } + ); }, @@ -305,5 +315,44 @@ require( 'common' ).testCase( { this.quickKeywordMethodTest( [], null, [ 'abstract' ] ); }, + + + /** + * If a developer uses the 'override' keyword when there is no super method + * to override, this could hint at a number of problems, including: + * - Misunderstanding the keyword + * - Misspelling the method name + * - Forgetting to specify a class to extend from + * + * All of the above possibilities are pretty significant. In order to safe + * developers from themselves (everyone screws up eventually), let's provide + * a warning. Since this only hints at a potential bug but does not affect + * the functionality, there's no use in throwing an error and preventing the + * class from being defined. + */ + 'Throws warning when using override with no super method': function() + { + var given = null; + + this.warningHandler = function( warning ) + { + given = warning; + }; + + // trigger warning (override keyword with no super method) + this.quickKeywordMethodTest( [ 'override' ] ); + + this.assertNotEqual( null, given, + 'No warning was provided' + ); + + this.assertOk( given instanceof Error, + 'Provided warning is not of type Error' + ); + + this.assertOk( ( given.message.search( shared.testName ) > -1 ), + 'Override warning should contain method name' + ); + }, } ); diff --git a/test/MemberBuilderValidator/inc-common.js b/test/MemberBuilderValidator/inc-common.js index ec0e612..240a414 100644 --- a/test/MemberBuilderValidator/inc-common.js +++ b/test/MemberBuilderValidator/inc-common.js @@ -23,6 +23,13 @@ */ +/** + * Member name to be used in tests + * @type {string} + */ +exports.testName = 'fooBar'; + + /** * Quickly tests for validation failures * @@ -81,7 +88,7 @@ exports.quickKeywordTest = function( type, keywords, identifier, prev ) var keyword_obj = {}, prev_obj = {}, prev_data = {}, - name = 'fooBar', + name = exports.testName, _self = this; // convert our convenient array into a keyword obj