Merge pull request #178 from hegemonic/type-refactor

Refactor type parsing. Closes #118.

rhino_modules/jsdoc/src/handlers.js

@@ -178,9 +178,11 @@ exports.attachTo = function(parser) {
             for (var i = 0, len = newDoclet.properties.length; i < len; i++) {
                 var property = newDoclet.properties[i];
 
-                var parts = jsdoc.name.splitName(property.description);
-                property.name = parts.name;
-                property.description = parts.description;
+                if (property.description) {
+                    var parts = jsdoc.name.splitName(property.description);
+                    property.name = parts.name;
+                    property.description = parts.description;
+                }
             }
         }
     }

rhino_modules/jsdoc/tag.js

@@ -13,7 +13,6 @@
     @requires jsdoc/tag/type
  */
 
-
 var jsdoc = {
     tag: {
         dictionary: require('jsdoc/tag/dictionary'),
@@ -33,34 +32,6 @@ function trim(text, newlines) {
     }
 }
 
-/**
-    Parse the parameter name and parameter desc from the tag text.
-    @inner
-    @method parseParamText
-    @memberof module:jsdoc/tag
-    @param {string} tagText
-    @returns {Array.<string, string, boolean, boolean>} [pname, pdesc, poptional, pdefault].
- */
-function parseParamText(tagText) {
-    var pname, pdesc, poptional, pdefault;
-
-    // like: pname, pname pdesc, or name - pdesc
-    tagText.match(/^(\[[^\]]+\]|\S+)((?:\s*\-\s*|\s+)(\S[\s\S]*))?$/);
-    pname = RegExp.$1;
-    pdesc = RegExp.$3;
-
-    if ( /^\[\s*(.+?)\s*\]$/.test(pname) ) {
-        pname = RegExp.$1;
-        poptional = true;
-        
-        if ( /^(.+?)\s*=\s*(.+)$/.test(pname) ) {
-            pname = RegExp.$1;
-            pdefault = RegExp.$2;
-        }
-    }
-    return { name: pname, desc: pdesc, optional: poptional, default: pdefault };
-}
-
 /**
     Constructs a new tag object. Calls the tag validator.
     @class
@@ -87,38 +58,30 @@ exports.Tag = function(tagTitle, tagBody, meta) {
             this.text = tagDef.onTagText(this.text);
         }
         
-        if (tagDef.canHaveType) {
+        if (tagDef.canHaveType || tagDef.canHaveName) {
         
             /** The value property represents the result of parsing the tag text. */
             this.value = {};
             
-            var tagType = jsdoc.tag.type.parse(this.text);
+            var tagType = jsdoc.tag.type.parse(this.text, tagDef.canHaveName, tagDef.canHaveType);
 
             if (tagType.type && tagType.type.length) {
                 this.value.type = {
-                    names:    tagType.type,
-                    optional: tagType.optional,
-                    nullable: tagType.nullable,
-                    variable: tagType.variable
+                    names: tagType.type
                 };
+                this.value.optional = tagType.optional;
+                this.value.nullable = tagType.nullable;
+                this.value.variable = tagType.variable;
+                this.value['default'] = tagType['default'];
             }
-
-            var remainingText = tagType.text;
-
-            if (remainingText) {
-                if (tagDef.canHaveName) {
-                    var paramInfo = parseParamText(remainingText);
-                    
-                    // note the dash is a special case: as a param name it means "no name"
-                    if (paramInfo.name && paramInfo.name !== '-') { this.value.name = paramInfo.name; }
-                    
-                    if (paramInfo.desc)     { this.value.description = paramInfo.desc; }
-                    if (paramInfo.optional) { this.value.optional = paramInfo.optional; }
-                    if (paramInfo.default)  { this.value.defaultvalue = paramInfo.default; }
-                }
-                else {
-                    this.value.description = remainingText;
-                }
+            
+            if (tagType.text && tagType.text.length) {
+                this.value.description = tagType.text;
+            }
+            
+            if (tagDef.canHaveName) {
+                // note the dash is a special case: as a param name it means "no name"
+                if (tagType.name && tagType.name !== '-') { this.value.name = tagType.name; }
             }
         }
         else {

rhino_modules/jsdoc/tag/dictionary/definitions.js

@@ -140,7 +140,7 @@ exports.defineTags = function(dictionary) {
         // Allow augments value to be specified as a normal type, e.g. {Type}
         onTagText: function(text) {
             var type = require('jsdoc/tag/type'),
-                tagType = type.getTagType(text);
+                tagType = type.getTagInfo(text, false, true);
             return tagType.type || text;
         },
         onTagged: function(doclet, tag) {
@@ -486,6 +486,7 @@ exports.defineTags = function(dictionary) {
     dictionary.defineTag('property', {
         mustHaveValue: true,
         canHaveType: true,
+        canHaveName: true,
         onTagged: function(doclet, tag) {
             if (!doclet.properties) { doclet.properties = []; }
             doclet.properties.push(tag.value);

rhino_modules/jsdoc/tag/type.js

@@ -2,46 +2,10 @@
     @module jsdoc/tag/type
 
     @author Michael Mathews <micmath@gmail.com>
+    @author Jeff Williams <jeffrey.l.williams@gmail.com>
     @license Apache License 2.0 - See file 'LICENSE.md' in this project.
  */
 
-
-function parseOptional(type) {
-    var optional = null;
-
-    // {sometype=} means optional
-    if ( /(.+)=$/.test(type) ) {
-        type = RegExp.$1;
-        optional = true;
-    }
-
-    return { type: type, optional: optional };
-}
-
-function parseNullable(type) {
-    var nullable = null;
-
-    // {?sometype} means nullable, {!sometype} means not-nullable
-    if ( /^([\?\!])(.+)$/.test(type) ) {
-        type = RegExp.$2;
-        nullable = (RegExp.$1 === '?')? true : false;
-    }
-
-    return { type: type, nullable: nullable };
-}
-
-function parseVariable(type) {
-    var variable = null;
-
-    // {...sometype} means variable number of that type
-    if ( /^(\.\.\.)(.+)$/.test(type) ) {
-        type = RegExp.$2;
-        variable = true;
-    }
-
-    return { type: type, variable: variable };
-}
-
 function parseTypes(type) {
     var types = [];
 
@@ -65,13 +29,14 @@ function trim(text) {
     return text.trim();
 }
 
-function getTagType(tagValue) {
-    var type = '',
-        text = '',
+var getTagInfo = exports.getTagInfo = function(tagValue, canHaveName, canHaveType) {
+    var name = '',
+        type = '',
+        text = tagValue,
         count = 0;
 
     // type expressions start with '{'
-    if (tagValue[0] === '{') {
+    if (canHaveType && tagValue[0] === '{') {
         count++;
 
         // find matching closer '}'
@@ -90,44 +55,40 @@ function getTagType(tagValue) {
             }
         }
     }
-    return { type: type, text: text };
-}
-exports.getTagType = getTagType;
+    
+    if (canHaveName) {
+        // like: name, [name], name text, [name] text, name - text, or [name] - text
+        text.match(/^(\[[^\]]+\]|\S+)((?:\s*\-\s*|\s+)(\S[\s\S]*))?$/);
+        name = RegExp.$1;
+        text = RegExp.$3;
+    }
+    
+    return { name: name, type: type, text: text };
+};
 
 
 /**
     @param {string} tagValue
-    @returns {object} Hash with type, text, optional, nullable, and variable properties
+    @param {boolean} canHaveName
+    @param {boolean} canHaveType
+    @returns {object} Hash with name, type, text, optional, nullable, variable, and default properties
  */
-exports.parse = function(tagValue) {
+exports.parse = function(tagValue, canHaveName, canHaveType) {
     if (typeof tagValue !== 'string') { tagValue = ''; }
-    var type = '',
-        text = '',
-        tagType,
-        optional,
-        nullable,
-        variable;
     
-    tagType = getTagType(tagValue);
-    type = tagType.type;
-    if (tagType.type === '') {
-        text = tagValue;
-    } else {
-        text = tagType.text;
-    }
+    var tagInfo = getTagInfo(tagValue, canHaveName, canHaveType);
     
-    optional = parseOptional(type);
-    nullable = parseNullable(type);
-    variable = parseVariable(type);
-    type = variable.type || nullable.type || optional.type;
-
-    type = parseTypes(type); // make it into an array
+    // extract JSDoc-style type info, then Closure Compiler-style type info
+    tagInfo = require('jsdoc/tag/type/jsdocType').parse(tagInfo);
+    tagInfo = require('jsdoc/tag/type/closureCompilerType').parse(tagInfo);
 
     return {
-        type: type,
-        text: text,
-        optional: optional.optional,
-        nullable: nullable.nullable,
-        variable: variable.variable
+        name: tagInfo.name,
+        type: parseTypes(tagInfo.type), // make it into an array
+        text: tagInfo.text,
+        optional: tagInfo.optional,
+        nullable: tagInfo.nullable,
+        variable: tagInfo.variable,
+        'default': tagInfo['default']
     };
 };

rhino_modules/jsdoc/tag/type/closureCompilerType.js

@@ -0,0 +1,64 @@
+/**
+    @module jsdoc/tag/type/closureCompilerType
+
+    @author Michael Mathews <micmath@gmail.com>
+    @author Jeff Williams <jeffrey.l.williams@gmail.com>
+    @license Apache License 2.0 - See file 'LICENSE.md' in this project.
+ */
+
+function parseOptional(type) {
+    var optional = null;
+
+    // {sometype=} means optional
+    if ( /(.+)=$/.test(type) ) {
+        type = RegExp.$1;
+        optional = true;
+    }
+
+    return { type: type, optional: optional };
+}
+
+function parseNullable(type) {
+    var nullable = null;
+
+    // {?sometype} means nullable, {!sometype} means not-nullable
+    if ( /^([\?\!])(.+)$/.test(type) ) {
+        type = RegExp.$2;
+        nullable = (RegExp.$1 === '?')? true : false;
+    }
+
+    return { type: type, nullable: nullable };
+}
+
+function parseVariable(type) {
+    var variable = null;
+
+    // {...sometype} means variable number of that type
+    if ( /^(\.\.\.)(.+)$/.test(type) ) {
+        type = RegExp.$2;
+        variable = true;
+    }
+
+    return { type: type, variable: variable };
+}
+
+/**
+    Extract Closure Compiler-style type information from the tag info.
+    @param {object} tagInfo Hash with name, type, and text properties.
+    @return {object} Hash with name, type, text, optional, nullable, variable, and default properties.
+ */
+exports.parse = function(tagInfo) {
+    var optional = parseOptional(tagInfo.type),
+        nullable = parseNullable(tagInfo.type),
+        variable = parseVariable(tagInfo.type);
+
+    return {
+        name: tagInfo.name,
+        type: variable.type || nullable.type || optional.type,
+        text: tagInfo.text,
+        optional: tagInfo.optional || optional.optional, // don't override if already true
+        nullable: nullable.nullable,
+        variable: variable.variable,
+        'default': tagInfo['default']
+    };
+};

rhino_modules/jsdoc/tag/type/jsdocType.js

@@ -0,0 +1,40 @@
+/**
+    @module jsdoc/tag/type/jsdocType
+
+    @author Michael Mathews <micmath@gmail.com>
+    @author Jeff Williams <jeffrey.l.williams@gmail.com>
+    @license Apache License 2.0 - See file 'LICENSE.md' in this project.
+ */
+
+ /**
+     Extract JSDoc-style type information from the tag info.
+     @param {object} tagInfo Hash with name, type, text, optional, nullable, variable, and default properties.
+     @return {object} Hash with the same properties as tagInfo.
+  */
+exports.parse = function(tagInfo) {
+    var name = tagInfo.name,
+        optional,
+        tagDefault;
+    
+    // like '[foo]' or '[ foo ]' or '[foo=bar]' or '[ foo=bar ]' or '[ foo = bar ]'
+    if ( /^\[\s*(.+?)\s*\]$/.test(name) ) {
+        name = RegExp.$1;
+        optional = true;
+        
+        // like 'foo=bar' or 'foo = bar'
+        if ( /^(.+?)\s*=\s*(.+)$/.test(name) ) {
+            name = RegExp.$1;
+            tagDefault = RegExp.$2;
+        }
+    }
+    
+    return {
+        name: name,
+        type: tagInfo.type,
+        text: tagInfo.text,
+        optional: optional,
+        nullable: tagInfo.nullable,
+        variable: tagInfo.variable,
+        'default': tagDefault
+    };
+};

test/specs/jsdoc/tag/type.js

@@ -1,3 +1,151 @@
+/*global describe: true, expect: true, it: true */
+
+function buildText(type, name, desc) {
+    var text = '';
+    if (type) {
+        text += "{" + type + "}";
+        if (name || desc) {
+            text += " ";
+        }
+    }
+    
+    if (name) {
+        text += name;
+        if (desc) {
+            text += " ";
+        }
+    }
+    
+    if (desc) {
+        text += desc;
+    }
+    
+    return text;
+}
+
 describe("jsdoc/tag/type", function() {
-    //TODO
-});
+    var jsdoc = {
+        tag: {
+            type: require('jsdoc/tag/type')
+        }
+    };
+    
+    it("should exist", function() {
+        expect(jsdoc.tag.type).toBeDefined();
+        expect(typeof jsdoc.tag.type).toEqual("object");
+    });
+    
+    it("should export a getTagInfo function", function() {
+        expect(jsdoc.tag.type.getTagInfo).toBeDefined();
+        expect(typeof jsdoc.tag.type.getTagInfo).toEqual("function");
+    });
+    
+    it("should export a parse function", function() {
+        expect(jsdoc.tag.type.parse).toBeDefined();
+        expect(typeof jsdoc.tag.type.parse).toEqual("function");
+    });
+    
+    describe("getTagInfo", function() {
+        it("should return an object with name, type, and text properties", function() {
+            var info = jsdoc.tag.type.getTagInfo("");
+            expect(info.name).toBeDefined();
+            expect(info.type).toBeDefined();
+            expect(info.text).toBeDefined();
+        });
+        
+        it("should not extract a name or type if canHaveName and canHaveType are not set", function() {
+            var desc = "{number} foo The foo parameter.";
+            var info = jsdoc.tag.type.getTagInfo(desc);
+            expect(info.type).toEqual('');
+            expect(info.name).toEqual('');
+            expect(info.text).toEqual(desc);
+        });
+        
+        it("should extract a name, but not a type, if canHaveName === true and canHaveType === false", function() {
+            var name = "bar";
+            var desc = "The bar parameter.";
+            var info = jsdoc.tag.type.getTagInfo( buildText(null, name, desc), true, false );
+            expect(info.type).toEqual('');
+            expect(info.name).toEqual(name);
+            expect(info.text).toEqual(desc);
+        });
+        
+        it("should extract a type, but not a name, if canHaveName === false and canHaveType === true", function() {
+            var type = "boolean";
+            var desc = "Set to true on alternate Thursdays.";
+            var info = jsdoc.tag.type.getTagInfo( buildText(type, null, desc), false, true );
+            expect(info.type).toEqual(type);
+            expect(info.name).toEqual('');
+            expect(info.text).toEqual(desc);
+        });
+        
+        it("should extract a name and type if canHaveName and canHaveType are true", function() {
+            var type = "string";
+            var name = "baz";
+            var desc = "The baz parameter.";
+            var info = jsdoc.tag.type.getTagInfo( buildText(type, name, desc), true, true );
+            expect(info.type).toEqual(type);
+            expect(info.name).toEqual(name);
+            expect(info.text).toEqual(desc);
+        });
+        
+        it("should work with JSDoc-style optional parameters", function() {
+            var name = "[qux]";
+            var desc = "The qux parameter.";
+            var info = jsdoc.tag.type.getTagInfo( buildText(null, name, desc), true, false );
+            expect(info.name).toEqual(name);
+            expect(info.text).toEqual(desc);
+            
+            name = "[ qux ]";
+            info = jsdoc.tag.type.getTagInfo( buildText(null, name, desc), true, false );
+            expect(info.name).toEqual(name);
+            expect(info.text).toEqual(desc);
+            
+            name = "[qux=hooray]";
+            info = jsdoc.tag.type.getTagInfo( buildText(null, name, desc), true, false );
+            expect(info.name).toEqual(name);
+            expect(info.text).toEqual(desc);
+            
+            name = "[  qux   =  hooray ]";
+            info = jsdoc.tag.type.getTagInfo( buildText(null, name, desc), true, false );
+            expect(info.name).toEqual(name);
+            expect(info.text).toEqual(desc);
+        });
+    });
+    
+    describe("parse", function() {
+        it("should report optional types correctly no matter which syntax we use", function() {
+            var desc = "{string} [foo]";
+            var info = jsdoc.tag.type.parse(desc, true, true);
+            expect(info.optional).toEqual(true);
+            
+            desc = "{string=} [foo]";
+            info = jsdoc.tag.type.parse(desc, true, true);
+            expect(info.optional).toEqual(true);
+        });
+        
+        it("should return the types as an array", function() {
+            var desc = "{string} foo";
+            var info = jsdoc.tag.type.parse(desc, true, true);
+            expect(info.type).toEqual( ["string"] );
+        });
+        
+        it("should recognize the entire list of possible types", function() {
+            var desc = "{string|number} foo";
+            var info = jsdoc.tag.type.parse(desc, true, true);
+            expect(info.type).toEqual( ["string", "number"] );
+            
+            desc = "{ string | number } foo";
+            info = jsdoc.tag.type.parse(desc, true, true);
+            expect(info.type).toEqual( ["string", "number"] );
+            
+            desc = "{  (   string  | number)} foo";
+            info = jsdoc.tag.type.parse(desc, true, true);
+            expect(info.type).toEqual( ["string", "number"] );
+            
+            desc = "{(string|number|boolean|function)} foo";
+            info = jsdoc.tag.type.parse(desc, true, true);
+            expect(info.type).toEqual( ["string", "number", "boolean", "function"] );
+        });
+    });
+});

test/specs/jsdoc/tag/type/closureCompilerType.js

@@ -0,0 +1,3 @@
+describe("jsdoc/tag/type/closureCompilerType", function() {
+    //TODO
+});

test/specs/jsdoc/tag/type/jsdocType.js

@@ -0,0 +1,68 @@
+/*global describe: true, expect: true, it: true */
+
+var hasOwnProp = Object.prototype.hasOwnProperty;
+
+describe("jsdoc/tag/type/jsdocType", function() {
+    var jsdocType = require("jsdoc/tag/type/jsdocType");
+    
+    it("should exist", function() {
+        expect(jsdocType).toBeDefined();
+        expect(typeof jsdocType).toEqual("object");
+    });
+    
+    it("should export a parse function", function() {
+        expect(jsdocType.parse).toBeDefined();
+        expect(typeof jsdocType.parse).toEqual("function");
+    });
+    
+    describe("parse", function() {
+        it("should recognize optional properties without default values", function() {
+            var info = jsdocType.parse( { name: "[foo]" } );
+            expect(info.name).toEqual("foo");
+            expect(info.optional).toEqual(true);
+            expect( info['default'] ).toEqual(null);
+            
+            info = jsdocType.parse( { name: "[ bar   ]" } );
+            expect(info.name).toEqual("bar");
+            expect(info.optional).toEqual(true);
+            expect( info['default'] ).toEqual(null);
+        });
+        
+        it("should recognize optional properties with default values", function() {
+            var info = jsdocType.parse( { name: "[foo=bar]" } );
+            expect(info.name).toEqual("foo");
+            expect(info.optional).toEqual(true);
+            expect( info['default'] ).toEqual("bar");
+            
+            info = jsdocType.parse( { name: "[     baz  =   qux ]" } );
+            expect(info.name).toEqual("baz");
+            expect(info.optional).toEqual(true);
+            expect( info['default'] ).toEqual("qux");
+        });
+        
+        it("should only change the `name`, `optional`, and `default` properties", function() {
+            var obj = {
+                name: "[foo=bar]",
+                type: "boolean|string",
+                text: "Sample text.",
+                optional: null,
+                nullable: null,
+                variable: null,
+                'default': null
+            };
+            var shouldChange = [ "name", "optional", "default" ];
+            
+            var info = jsdocType.parse(obj);
+            for (var key in info) {
+                if ( hasOwnProp.call(info, key) ) {
+                    if ( shouldChange.indexOf(key) !== -1 ) {
+                        expect( info[key] ).not.toEqual( obj[key] );
+                    }
+                    else {
+                        expect( info[key] ).toEqual( obj[key] );
+                    }
+                }
+            }
+        });
+    });
+});