diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index 20f032a..1719a86 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -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).' + ); } diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index 1932b50..b18cfbc 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -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" + ); } } }; diff --git a/test/MemberBuilderValidator/MethodTest.js b/test/MemberBuilderValidator/MethodTest.js index d4413a7..e58551b 100644 --- a/test/MemberBuilderValidator/MethodTest.js +++ b/test/MemberBuilderValidator/MethodTest.js @@ -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' ] ); + }, } ); diff --git a/test/test-member_builder-method-hiding.js b/test/test-member_builder-method-hiding.js index eb6c4a7..e96448f 100644 --- a/test/test-member_builder-method-hiding.js +++ b/test/test-member_builder-method-hiding.js @@ -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