1
0
Fork 0

MemberBuilderValidator will now throw a warning if 'override' keyword is used without a super method

closure/master
Mike Gerwitz 2011-11-05 11:56:28 -04:00
parent 566f0af57e
commit 8bcd55dbbb
4 changed files with 73 additions and 5 deletions

View File

@ -122,7 +122,6 @@ exports.buildMethod = function(
if ( prev ) if ( prev )
{ {
// TODO: warning if no super method when override keyword provided
if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] ) if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] )
{ {
// override the method // override the method

View File

@ -23,13 +23,15 @@
*/ */
module.exports = exports = function MemberBuilderValidator() module.exports = exports = function MemberBuilderValidator( warn_handler )
{ {
// permit omitting 'new' keyword // permit omitting 'new' keyword
if ( !( this instanceof module.exports ) ) 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"
) );
}
}; };

View File

@ -65,7 +65,17 @@ require( 'common' ).testCase(
setUp: function() 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' ] ); 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'
);
},
} ); } );

View File

@ -23,6 +23,13 @@
*/ */
/**
* Member name to be used in tests
* @type {string}
*/
exports.testName = 'fooBar';
/** /**
* Quickly tests for validation failures * Quickly tests for validation failures
* *
@ -81,7 +88,7 @@ exports.quickKeywordTest = function( type, keywords, identifier, prev )
var keyword_obj = {}, var keyword_obj = {},
prev_obj = {}, prev_obj = {},
prev_data = {}, prev_data = {},
name = 'fooBar', name = exports.testName,
_self = this; _self = this;
// convert our convenient array into a keyword obj // convert our convenient array into a keyword obj