From e3561a492f9d298f6150e13e1dea6d80308e67b4 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 1 Mar 2011 09:17:24 -0500 Subject: [PATCH 1/2] Modified interface property message to be a bit more helpful in a likely common scenerio --- lib/interface.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/interface.js b/lib/interface.js index c6855cc..7958c80 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -73,7 +73,7 @@ var extend = ( function( extending ) { throw TypeError( "Properties are not permitted within Interface " + - "definitions" + "definitions (did you forget the 'abstract' keyword?)" ); }, From e239352fc05a85b3a1c68df7773fa5668087a9cc Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 1 Mar 2011 12:11:36 -0500 Subject: [PATCH 2/2] Resolved bug that was causing the system to think that Object prototype members were part of the abstract member list when attempting to define a method with the same name --- lib/class.js | 6 ++++-- test/test-class-abstract.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/lib/class.js b/lib/class.js index f1ba4e9..593c661 100644 --- a/lib/class.js +++ b/lib/class.js @@ -210,7 +210,9 @@ var extend = ( function( extending ) var args = Array.prototype.slice.call( arguments ), props = args.pop() || {}, base = args.pop() || Class, - prototype = new base(); + prototype = new base(), + + hasOwn = Array.prototype.hasOwnProperty; var properties = {}, members = member_builder.initMembers( @@ -266,7 +268,7 @@ var extend = ( function( extending ) abstract_methods[ name ] = true; abstract_methods.__length++; } - else if ( ( abstract_methods[ name ] ) + else if ( ( hasOwn.call( abstract_methods, name ) ) && ( is_abstract === false ) ) { diff --git a/test/test-class-abstract.js b/test/test-class-abstract.js index ab5883e..4ce1a05 100644 --- a/test/test-class-abstract.js +++ b/test/test-class-abstract.js @@ -192,3 +192,32 @@ assert.throws( function() ); } )(); + +/** + * There was an issue where the object holding the abstract methods list was not + * checking for methods by using hasOwnProperty(). Therefore, if a method such + * as toString() was defined, it would be matched in the abstract methods list. + * As such, the abstract methods count would be decreased, even though it was + * not an abstract method to begin with (nor was it removed from the list, + * because it was never defined in the first place outside of the prototype). + * + * This negative number !== 0, which causes a problem when checking to ensure + * that there are 0 abstract methods. We check explicitly for 0 for two reasons: + * (a) it's faster than <, and (b - most importantly) if it's non-zero, then + * it's either abstract or something is wrong. Negative is especially wrong. It + * should never be negative! + */ +( function testDoesNotRecognizeObjectPrototypeMembersAsAbstractWhenDefining() +{ + assert.doesNotThrow( function() + { + SubAbstractFoo.extend( { + // concrete, so the result would otherwise not be abstract + 'method': function( one, two, three ) {}, + + // the problem + 'toString': function() {}, + })(); + }, Error, "Should not throw error if overriding a prototype method" ); +} )(); +