Merge branch 'master' into visibility/master

README.md

@@ -52,6 +52,15 @@ class. The constructor is provided as the `__construct()` method (influenced by
         },
     });
 
+The above creates an anonymous class and stores it in the variable ``Foo``. You
+have the option of naming class in order to provide more useful error messages
+and toString() output:
+
+    var Foo = Class( 'Foo',
+    {
+        // ...
+    });
+
 ### Extending Classes
 Classes may inherit from one-another. If the supertype was created using
 `Class.extend()`, a convenience `extend()` method has been added to it. Classes

TODO

@@ -12,8 +12,6 @@ Misc
         functions, will not impact function logic.
     - Should be able to run source file without preprocessing, so C-style macros
       cannot be used (# is not a valid token)
-  - Class/Interface naming
-    - Will be especially helpful for error messages
   - Class module is becoming too large; refactor
 
 Property Keywords

lib/class.js

@@ -263,8 +263,8 @@ function createNamedClass( name, def )
     if ( arguments.length > 2 )
     {
         throw Error(
-            "Expecting two arguments for named Class definition; " +
+            "Expecting two arguments for definition of named Class '" + name +
-                arguments.length + " given."
+                "'; " + arguments.length + " given."
         );
     }
 
@@ -278,7 +278,8 @@ function createNamedClass( name, def )
     else if ( typeof def !== 'object' )
     {
         throw TypeError(
-            "Unexpected value for named class definition; object expected"
+            "Unexpected value for definition of named Class '" + name +
+                "'; object expected"
         );
     }
 
@@ -434,7 +435,10 @@ var extend = ( function( extending )
                 // disallow use of our internal __initProps() method
                 if ( name === '__initProps' )
                 {
-                    throw new Error( "__initProps is a reserved method" );
+                    throw new Error(
+                        ( ( cname ) ? cname + '::' : '' ) +
+                        "__initProps is a reserved method"
+                    );
                 }
             },
 
@@ -497,7 +501,7 @@ var extend = ( function( extending )
         prototype.parent = base.prototype;
 
         // set up the new class
-        var new_class = createCtor( cname, abstract_methods );
+        var new_class = createCtor( cname, abstract_methods, members );
 
         attachPropInit( prototype, prop_init, members );
 
@@ -531,10 +535,11 @@ var extend = ( function( extending )
      *
      * @param  {string}          cname             class name (may be empty)
      * @param  {Array.<string>}  abstract_methods  list of abstract methods
+     * @param  {Object}          members           class members
      *
      * @return  {Function}  constructor
      */
-    function createCtor( cname, abstract_methods )
+    function createCtor( cname, abstract_methods, members )
     {
         // concrete class
         if ( abstract_methods.__length === 0 )
@@ -572,18 +577,24 @@ var extend = ( function( extending )
                 // constructor to ensure they are not overridden)
                 attachInstanceOf( this );
 
-                // provide a more intuitive string representation of the class
+                // Provide a more intuitive string representation of the class
-                // instance
+                // instance. If a toString() method was already supplied for us,
-                this.toString = ( cname )
+                // use that one instead.
-                    ? function()
+                if ( !( Object.prototype.hasOwnProperty.call(
-                    {
+                    members[ 'public' ], 'toString'
-                        return '[object #<' + cname + '>]';
+                ) ) )
-                    }
+                {
-                    : function()
+                    this.toString = ( cname )
-                    {
+                        ? function()
-                        return '[object #<anonymous>]';
+                        {
-                    }
+                            return '[object #<' + cname + '>]';
-                ;
+                        }
+                        : function()
+                        {
+                            return '[object #<anonymous>]';
+                        }
+                    ;
+                }
             };
 
             // provide a more intuitive string representation
@@ -601,7 +612,10 @@ var extend = ( function( extending )
             {
                 if ( !extending )
                 {
-                    throw Error( "Abstract classes cannot be instantiated" );
+                    throw Error(
+                        "Abstract class " + ( cname || '(anonymous)' ) +
+                            " cannot be instantiated"
+                    );
                 }
             };
 

lib/interface.js

@@ -148,8 +148,8 @@ function createNamedInterface( name, def )
     if ( arguments.length > 2 )
     {
         throw Error(
-            "Expecting two arguments for named Interface definition; " +
+            "Expecting two arguments for definition of named Interface '" +
-                arguments.length + " given."
+                name + "'; " + arguments.length + " given."
         );
     }
 
@@ -157,7 +157,8 @@ function createNamedInterface( name, def )
     if ( typeof def !== 'object' )
     {
         throw TypeError(
-            "Unexpected value for named class definition; object expected"
+            "Unexpected value for definition of named Interface '" +
+                name + "'; object expected"
         );
     }
 
@@ -196,28 +197,31 @@ var extend = ( function( extending )
         // sanity check
         inheritCheck( prototype );
 
-        var new_interface = createInterface();
+        var new_interface = createInterface( iname );
 
         util.propParse( props, {
             property: function()
             {
                 throw TypeError(
-                    "Properties are not permitted within Interface " +
+                    "Property not permitted within definition of " +
-                        "definitions (did you forget the 'abstract' keyword?)"
+                        "Interface '" + iname + "' (did you forget the " +
+                        "'abstract' keyword?)"
                 );
             },
 
             getter: function()
             {
                 throw TypeError(
-                    "Getters are not permitted within Interface definitions"
+                    "Getter not permitter within definition of Interface '" +
+                        iname + "'"
                 );
             },
 
             setter: function()
             {
                 throw TypeError(
-                    "Setters are not permitted within Interface definitions"
+                    "Setter within definition of Interface '" +
+                        iname + "'"
                 );
             },
 
@@ -226,8 +230,9 @@ var extend = ( function( extending )
                 if ( !is_abstract )
                 {
                     throw TypeError(
-                        "Only abstract methods are permitted within " +
+                        "Concrete method not permitted in declaration of " +
-                            "Interface definitions"
+                            "Interface '" + iname + "'; please declare as " +
+                            "abstract."
                     );
                 }
 
@@ -256,9 +261,11 @@ var extend = ( function( extending )
     /**
      * Creates a new interface constructor function
      *
+     * @param  {string=}  iname  interface name
+     *
      * @return  {function()}
      */
-    function createInterface()
+    function createInterface( iname )
     {
         return function()
         {
@@ -268,7 +275,10 @@ var extend = ( function( extending )
             {
                 // only called if someone tries to create a new instance of an
                 // interface
-                throw Error( "Interfaces cannot be instantiated" );
+                throw Error(
+                    "Interface" + ( ( iname ) ? ( iname + ' ' ) : '' ) +
+                        " cannot be instantiated"
+                );
             }
         };
     }

test/test-class-extend.js

@@ -284,3 +284,29 @@ for ( var i = 0; i < class_count; i++ )
     }
 } )();
 
+
+/**
+ * We provide a useful default toString() method, but one may wish to override
+ * it
+ */
+( function testCanOverrideToStringMethod()
+{
+    var str    = 'foomookittypoo',
+        result = ''
+    ;
+
+    result = Class( 'Foo',
+    {
+        toString: function()
+        {
+            return str;
+        }
+    })().toString();
+
+    assert.equal(
+        result,
+        str,
+        "Can override default toString() method of class"
+    );
+} )();
+

test/test-class-name.js

@@ -45,15 +45,45 @@ var common = require( './common' ),
             "Class defined with name is returned as a valid class"
         );
     }, Error, "Class accepts name" );
+} )();
 
-    // the second argument must be an object
+
-    assert.throws( function()
+/**
+ * The class definition must be an object, which is equivalent to the class
+ * body
+ */
+( function testNamedClassDefinitionRequiresThatDefinitionBeAnObject()
+{
+    var name = 'Foo';
+
+    try
     {
-        Class( 'Foo', 'Bar' );
+        Class( name, 'Bar' );
-    }, TypeError, "Second argument to named class must be the definition" );
+
+        // if all goes well, we'll never get to this point
+        assert.fail( "Second argument to named class must be the definition" );
+    }
+    catch ( e )
+    {
+        assert.notEqual(
+            e.toString().match( name ),
+            null,
+            "Class definition argument count error string contains class name"
+        );
+    }
+} )();
+
+
+/**
+ * Extraneous arguments likely indicate a misunderstanding of the API
+ */
+( function testNamedClassDefinitionIsStrictOnArgumentCount()
+{
+    var name = 'Foo',
+        args = [ name, {}, 'extra' ]
+    ;
 
     // we should be permitted only two arguments
-    var args = [ 'Foo', {}, 'extra' ];
     try
     {
         Class.apply( null, args );
@@ -66,8 +96,16 @@ var common = require( './common' ),
     }
     catch ( e )
     {
+        var errstr = e.toString();
+
         assert.notEqual(
-            e.toString().match( args.length + ' given' ),
+            errstr.match( name ),
+            null,
+            "Named class error should provide name of class"
+        );
+
+        assert.notEqual(
+            errstr.match( args.length + ' given' ),
             null,
             "Named class error should provide number of given arguments"
         );
@@ -196,3 +234,53 @@ var common = require( './common' ),
     );
 } )();
 
+
+/**
+ * The class name should be provided in the error thrown when attempting to
+ * instantiate an abstract class, if it's available
+ */
+( function testClassNameIsGivenWhenTryingToInstantiateAbstractClass()
+{
+    var name = 'Foo';
+
+    try
+    {
+        Class( name, { 'abstract foo': [] } )();
+
+        // we're not here to test to make sure it is thrown, but if it's not,
+        // then there's likely a problem
+        assert.fail(
+            "Was expecting instantiation error. There's a bug somewhere!"
+        );
+    }
+    catch ( e )
+    {
+        assert.notEqual(
+            e.toString().match( name ),
+            null,
+            "Abstract class instantiation error should contain class name"
+        );
+    }
+
+    // if no name is provided, then (anonymous) should be indicated
+    try
+    {
+        Class( { 'abstract foo': [] } )();
+
+        // we're not here to test to make sure it is thrown, but if it's not,
+        // then there's likely a problem
+        assert.fail(
+            "Was expecting instantiation error. There's a bug somewhere!"
+        );
+    }
+    catch ( e )
+    {
+        assert.notEqual(
+            e.toString().match( '(anonymous)' ),
+            null,
+            "Abstract class instantiation error should recognize that class " +
+                "is anonymous if no name was given"
+        );
+    }
+} )();
+

test/test-interface-name.js

@@ -44,15 +44,48 @@ var common     = require( './common' ),
             "Interface defined with name is returned as a valid interface"
         );
     }, Error, "Interface accepts name" );
+} )();
 
-    // the second argument must be an object
+
-    assert.throws( function()
+/**
+ * The interface definition, which equates to the body of the interface, must be
+ * an object
+ */
+( function testNamedInterfaceDefinitionRequiresThatDefinitionBeAnObject()
+{
+    var name = 'Foo';
+
+    try
     {
-        Interface( 'Foo', 'Bar' );
+        Interface( name, 'Bar' );
-    }, TypeError, "Second argument to named interface must be the definition" );
+
+        // if all goes well, we'll never get to this point
+        assert.fail(
+            "Second argument to named interface must be the definition"
+        );
+    }
+    catch ( e )
+    {
+        assert.notEqual(
+            e.toString().match( name ),
+            null,
+            "Interface definition argument count error string contains " +
+                "interface name"
+        );
+    }
+} )();
+
+
+/**
+ * Extraneous arguments likely indicate a misunderstanding of the API
+ */
+( function testNamedInterfaceDefinitionIsStrictOnArgumentCount()
+{
+    var name = 'Foo',
+        args = [ name, {}, 'extra' ]
+    ;
 
     // we should be permitted only two arguments
-    var args = [ 'Foo', {}, 'extra' ];
     try
     {
         Interface.apply( null, args );
@@ -65,8 +98,16 @@ var common     = require( './common' ),
     }
     catch ( e )
     {
+        var errstr = e.toString();
+
         assert.notEqual(
-            e.toString().match( args.length + ' given' ),
+            errstr.match( name ),
+            null,
+            "Named interface error should provide interface name"
+        );
+
+        assert.notEqual(
+            errstr.match( args.length + ' given' ),
             null,
             "Named interface error should provide number of given arguments"
         );
@@ -104,3 +145,99 @@ var common     = require( './common' ),
     );
 } )();
 
+
+( function testDeclarationErrorsProvideInterfaceNameIsAvailable()
+{
+    var name = 'Foo',
+
+        // functions used to cause the various errors
+        tries = [
+            // properties
+            function()
+            {
+                Interface( name, { prop: 'str' } );
+            },
+
+            // methods
+            function()
+            {
+                Interface( name, { method: function() {} } );
+            },
+        ]
+    ;
+
+    // if we have getter/setter support, add those to the tests
+    if ( Object.defineProperty )
+    {
+        // getter
+        tries.push( function()
+        {
+            var obj = {};
+            Object.defineProperty( obj, 'getter', {
+                get:        function() {},
+                enumerable: true,
+            } );
+
+            Interface( name, obj );
+        } );
+
+        // setter
+        tries.push( function()
+        {
+            var obj = {};
+            Object.defineProperty( obj, 'setter', {
+                set:        function() {},
+                enumerable: true,
+            } );
+
+            Interface( name, obj );
+        } );
+    }
+
+    // gather the error strings
+    var i = tries.length;
+    while ( i-- )
+    {
+        try
+        {
+            // cause the error
+            tries[ i ]();
+
+            // we shouldn't get to this point...
+            assert.fail( "Expected error. Something's wrong." );
+        }
+        catch ( e )
+        {
+            // ensure the error string contains the interface name
+            assert.notEqual(
+                e.toString().match( name ),
+                null,
+                "Error contains interface name when available (" + i + ")"
+            );
+        }
+    }
+} )();
+
+
+( function testInterfaceNameIsIncludedInInstantiationError()
+{
+    var name = 'Foo';
+
+    try
+    {
+        // this should throw an exception (cannot instantiate interfaces)
+        Interface( name )();
+
+        // we should never get here
+        assert.fail( "Exception expected. There's a bug somewhere." );
+    }
+    catch ( e )
+    {
+        assert.notEqual(
+            e.toString().match( name ),
+            null,
+            "Interface name is included in instantiation error message"
+        );
+    }
+} )();
+