1
0
Fork 0

[#25] Throwing error instead of method hiding; will implement in future

closure/master
Mike Gerwitz 2011-10-22 13:57:17 -04:00
parent 6e7e031ff9
commit a5e2a507f2
4 changed files with 73 additions and 17 deletions

View File

@ -438,6 +438,12 @@ function hideMethod( super_method, new_method, instCallback, cid )
// TODO: This function is currently unimplemented. It exists at present to
// provide a placeholder and ensure that the override keyword is required to
// override a parent method.
//
// We should never get to this point if the default validation rule set is
// used to prevent omission of the 'override' keyword.
throw Error(
'Method hiding not yet implemented (we should never get here; bug).'
);
}

View File

@ -142,20 +142,15 @@ exports.prototype.validateMethod = function(
);
}
// if redefining a method that has already been implemented in the
// supertype, the default behavior is to "hide" the method of the
// supertype, unless otherwise specified
//
// IMPORTANT: do this last, to ensure we throw errors before warnings
if ( !( keywords[ 'override' ] )
// Disallow overriding method without override keyword (unless parent
// method is abstract). In the future, this will provide a warning to
// default to method hiding.
if ( !( keywords[ 'override' ] || prev_keywords[ 'abstract' ] ) )
{
if ( !( prev_keywords[ 'abstract' ] ) )
{
throw Warning( Error(
"Hiding method '" + name + "'; " +
"use 'new' if intended, or 'override' to override instead"
) );
}
throw TypeError(
"Attempting to override method '" + name +
"' without 'override' keyword"
);
}
}
};

View File

@ -34,10 +34,15 @@ require( 'common' ).testCase(
* Tests to ensure that a method with the given keywords fails
* validation with an error message partially matching the provided
* identifier
*
* To test overrides, specify keywords for 'prev'. To test for success
* instead of failure, set identifier to null.
*/
this.quickKeywordMethodTest = function( keywords, identifier )
this.quickKeywordMethodTest = function( keywords, identifier, prev )
{
var keyword_obj = {},
prev_obj = {},
prev_data = {},
name = 'fooBar';
// convert our convenient array into a keyword obj
@ -46,12 +51,34 @@ require( 'common' ).testCase(
keyword_obj[ keywords[ i ] ] = true;
}
_self.quickFailureTest( name, identifier, function()
// if prev keywords were given, do the same thing with those to
// generate our keyword obj
if ( prev !== undefined )
{
for ( var i = 0, len = prev.length; i < len; i++ )
{
prev_obj[ prev[ i ] ] = true;
}
// define a dummy previous method value
prev_data = { member: function() {} };
}
var testfunc = function()
{
_self.sut.validateMethod(
name, function() {}, keyword_obj, {}, {}
name, function() {}, keyword_obj, prev_data, prev_obj
);
} );
};
if ( identifier )
{
_self.quickFailureTest( name, identifier, testfunc );
}
else
{
_self.assertDoesNotThrow( testfunc, Error );
}
};
@ -343,5 +370,30 @@ require( 'common' ).testCase(
this.quickVisChangeTest( cur[ 0 ], cur[ 1 ], false );
}
},
/**
* If a parent method is defined and the 'override' keyword is not provided,
* regardless of whether or not it is declared as virtual, we need to
* provide an error.
*
* Note: In the future, this will be replaced with the method hiding
* implementation.
*/
'Must provide "override" keyword when overriding methods': function()
{
this.quickKeywordMethodTest( [], 'override', [] );
},
/**
* Building off of the previous test - we should be able to omit the
* 'override' keyword if we are providing a concrete method for an abstract
* method. In terms of ease.js, this is still "overriding".
*/
'Can provide abstract method impl. without override keyword': function()
{
this.quickKeywordMethodTest( [], null, [ 'abstract' ] );
},
} );

View File

@ -41,6 +41,9 @@ var common = require( './common' ),
)
;
// XXX: Disabled until we begin re-development of this feature
return;
/**
* Restores warning handler to the default