improve test coverage; remove unreachable code; other cleanup

lib/jsdoc/src/astbuilder.js

@@ -69,23 +69,11 @@ function isBefore(beforeRange, afterRange) {
     return beforeRange[1] <= afterRange[0];
 }
 
-// TODO: docs
-function isBetween(middleRange, beforeRange, afterRange) {
-    return isBefore(middleRange, afterRange) && (beforeRange ?
-        isBefore(beforeRange, middleRange) : true);
-}
-
 // TODO: docs
 function isWithin(innerRange, outerRange) {
     return innerRange[0] >= outerRange[0] && innerRange[1] <= outerRange[1];
 }
 
-// TODO: docs
-function isLeadingComment(comment, before, after) {
-    return !!before && !!after && !!canAcceptComment(after) &&
-        isBefore(before.range, after.range) && isBetween(comment.range, before.range, after.range);
-}
-
 // TODO: docs
 function isJsdocComment(comment) {
     return comment && (comment.type === 'Block') && (comment.value[0] === '*');
@@ -315,11 +303,6 @@ CommentAttacher.prototype._isEligible = function(node) {
     return isEligible;
 };
 
-// TODO: docs
-CommentAttacher.prototype._shouldSkipNode = function(node) {
-    return !isWithin(node.range, this._pendingCommentRange);
-};
-
 // TODO: docs
 // TODO: do we ever get multiple candidate nodes?
 CommentAttacher.prototype.visit = function(node) {

lib/jsdoc/tag/dictionary.js

@@ -38,7 +38,7 @@ dictionary = {
         // all the other dictionary functions use normalised names; we should too.
         _tags[def.title] = def;
 
-        if (opts.isNamespace) {
+        if (opts && opts.isNamespace) {
             _namespaces.push(def.title);
         }
 

lib/jsdoc/tag/inline.js

@@ -66,12 +66,7 @@ function regExpFactory(tagName, prefix, suffix) {
  * cases.
  */
 exports.isInlineTag = function(string, tagName) {
-    try {
+    return regExpFactory(tagName, '^', '$').test(string);
-        return regExpFactory(tagName, '^', '$').test(string);
-    }
-    catch(e) {
-        return false;
-    }
 };
 
 /**

lib/jsdoc/tag/type.js

@@ -37,8 +37,6 @@ function unescapeBraces(text) {
  * @return {module:jsdoc/tag/type.TypeExpressionInfo} The type expression and updated tag text.
  */
  function extractTypeExpression(string) {
-    string = string || '';
-
     var completeExpression;
     var count = 0;
     var position = 0;
@@ -110,7 +108,7 @@ function getTagInfo(tagValue, canHaveName, canHaveType) {
     if (canHaveType) {
         typeOverride = jsdoc.tag.inline.extractInlineTag(text, 'type');
         if (typeOverride.tags && typeOverride.tags[0]) {
-            typeExpression = typeOverride.tags[0].text || typeExpression;
+            typeExpression = typeOverride.tags[0].text;
         }
         text = typeOverride.newString;
     }
@@ -258,20 +256,18 @@ function parseTypeExpression(tagInfo) {
             e.message) );
     }
 
-    if (parsedType) {
+    tagInfo.type = tagInfo.type.concat( getTypeStrings(parsedType, true) );
-        tagInfo.type = tagInfo.type.concat( getTypeStrings(parsedType, true) );
 
-        // Catharsis and JSDoc use the same names for 'optional' and 'nullable'...
+    // Catharsis and JSDoc use the same names for 'optional' and 'nullable'...
-        ['optional', 'nullable'].forEach(function(key) {
+    ['optional', 'nullable'].forEach(function(key) {
-            if (parsedType[key] !== null && parsedType[key] !== undefined) {
+        if (parsedType[key] !== null && parsedType[key] !== undefined) {
-                tagInfo[key] = parsedType[key];
+            tagInfo[key] = parsedType[key];
-            }
-        });
-
-        // ...but not 'variable'.
-        if (parsedType.repeatable !== null && parsedType.repeatable !== undefined) {
-            tagInfo.variable = parsedType.repeatable;
         }
+    });
+
+    // ...but not 'variable'.
+    if (parsedType.repeatable !== null && parsedType.repeatable !== undefined) {
+        tagInfo.variable = parsedType.repeatable;
     }
 
     return tagInfo;

test/specs/jsdoc/tag/dictionary.js

@@ -1,6 +1,17 @@
+/*global describe, expect, it */
+'use strict';
+
 describe('jsdoc/tag/dictionary', function() {
     var dictionary = require('jsdoc/tag/dictionary');
 
+    var tagOptions = {
+        canHaveValue: true,
+        isNamespace: true
+    };
+    var tagTitle = '!!!testTag!!!';
+    var tagSynonym = '!!!testTagSynonym!!!';
+    var tagDef = dictionary.defineTag(tagTitle, tagOptions).synonym(tagSynonym);
+
     it('should exist', function() {
         expect(dictionary).toBeDefined();
         expect(typeof dictionary).toBe('object');
@@ -26,73 +37,72 @@ describe('jsdoc/tag/dictionary', function() {
         expect(typeof dictionary.normalise).toBe('function');
     });
 
-    // TODO: should really remove this tag from the dictionary after, but how?
+    describe('defineTag', function() {
-    var tagOptions = {
+        it('returns an object with the correct "title" property', function() {
-        canHaveValue: true,
+            expect(typeof tagDef).toBe('object');
-        isNamespace: true
+            expect(tagDef.title).toBeDefined();
-    },
+            expect(typeof tagDef.title).toBe('string');
-        tagTitle = 'testTag',
+            expect(tagDef.title).toBe(dictionary.normalise(tagTitle));
-        tagSynonym = 'testTag2';
-    var def = dictionary.defineTag(tagTitle, tagOptions).synonym(tagSynonym);
-    // Should really test TagDefinition but they are private.
-    // Instead, we'll just tests all the properties we expect of it.
-    describe("defineTag", function() {
-
-        // Since TagDefinition is private, I'll just test for its properties here.
-        it("returns an object with 'title' property", function() {
-            expect(typeof def).toBe('object');
-            // how to test?
-            expect(def.title).toBeDefined();
-            expect(typeof def.title).toBe('string');
-            expect(def.title).toBe(dictionary.normalise(tagTitle));
         });
 
-        it("returned object has all the tag properties copied over", function() {
+        it('returns an object that contains all of the tag properties', function() {
-            for (var prop in tagOptions) {
+            Object.keys(tagOptions).forEach(function(opt) {
-                if (tagOptions.hasOwnProperty(prop)) {
+                expect(tagDef[opt]).toBe(tagOptions[opt]);
-                    expect(def[prop]).toBe(tagOptions[prop]);
+            });
-                }
+        });
+
+        it('works correctly without an options object', function() {
+            var title = '!!!testTagNoOptions!!!';
+
+            function makeTag() {
+                return dictionary.defineTag(title);
             }
+
+            expect(makeTag).not.toThrow();
+            expect(makeTag().title).toBe(dictionary.normalise(title));
         });
     });
 
-    describe("lookUp", function() {
+    describe('lookUp', function() {
-        it("retrieves definition when using the tag's canonical name", function() {
+        it("retrieves the definition using the tag's canonical name", function() {
-            expect(dictionary.lookUp(tagTitle)).toBe(def);
+            expect(dictionary.lookUp(tagTitle)).toBe(tagDef);
         });
 
-        it("retrieves definition when using a synonym", function() {
+        it('retrieves the definition using a synonym for the tag', function() {
-            expect(dictionary.lookUp(tagSynonym)).toBe(def);
+            expect(dictionary.lookUp(tagSynonym)).toBe(tagDef);
         });
 
-        it("returns FALSE when a tag is not found", function() {
+        it('returns `false` when a tag is not found', function() {
             expect(dictionary.lookUp('lkjas1l24jk')).toBe(false);
         });
     });
 
-    describe("isNamespace", function() {
+    describe('isNamespace', function() {
-        it("returns whether a tag is a namespace when using its canonical name", function() {
+        it("returns whether a tag is a namespace using the tag's canonical name", function() {
             expect(dictionary.isNamespace(tagTitle)).toBe(true);
         });
 
-        it("returns whether a tag is a namespace when using its synonym", function() {
+        it('returns whether a tag is a namespace when using a synonym for the tag', function() {
             expect(dictionary.isNamespace(tagSynonym)).toBe(true);
         });
 
-        it("non-existent tags or non-namespace tags should return false", function() {
+        it('returns `false` for nonexistent tags', function() {
-            expect(dictionary.isNamespace('see')).toBe(false);
             expect(dictionary.isNamespace('lkjasd90034')).toBe(false);
         });
+
+        it('returns `false` for non-namespace tags', function() {
+            expect(dictionary.isNamespace('see')).toBe(false);
+        });
     });
 
-    describe("normalise", function() {
+    describe('normalise', function() {
         it("should return the tag's title if it is not a synonym", function() {
             expect(dictionary.normalise('FooBar')).toBe('foobar');
-            expect(dictionary.normalise(tagTitle)).toBe(def.title);
+            expect(dictionary.normalise(tagTitle)).toBe(tagDef.title);
         });
 
-        it("should return the canonical name of a tag if the synonym is normalised", function() {
+        it('should return the canonical name of a tag if the synonym is normalised', function() {
-            expect(dictionary.normalise(tagSynonym)).toBe(def.title);
+            expect(dictionary.normalise(tagSynonym)).toBe(tagDef.title);
         });
     });
 });

test/specs/jsdoc/tag/inline.js

@@ -1,4 +1,5 @@
 /*global describe: true, expect: true, it: true, jasmine: true */
+'use strict';
 
 describe('jsdoc/tag/inline', function() {
     var jsdoc = {
@@ -56,7 +57,7 @@ describe('jsdoc/tag/inline', function() {
 
         it('should return false (rather than throwing) with invalid input', function() {
             function badInput() {
-                return isInlineTag({});
+                return isInlineTag();
             }
 
             expect(badInput).not.toThrow();

test/specs/jsdoc/tag/type.js

@@ -1,4 +1,5 @@
-/*global describe: true, expect: true, it: true */
+/*global describe, expect, it */
+'use strict';
 
 function buildText(type, name, desc) {
     var text = '';

test/specs/jsdoc/tag/validator.js

@@ -1,4 +1,6 @@
 /*global afterEach, beforeEach, describe, env, expect, it, spyOn */
+'use strict';
+
 describe('jsdoc/tag/validator', function() {
     var doop = require('jsdoc/util/doop');
     var logger = require('jsdoc/util/logger');
@@ -21,7 +23,11 @@ describe('jsdoc/tag/validator', function() {
         var allowUnknown = !!env.conf.tags.allowUnknownTags;
         var badTag = { title: 'lkjasdlkjfb' };
         var badTag2 = new tag.Tag('type', '{string} I am a string!');
-        var meta = { filename: 'asdf.js', lineno: 1 };
+        var meta = {
+            filename: 'asdf.js',
+            lineno: 1,
+            comment: 'Better luck next time.'
+        };
         var goodTag = new tag.Tag('name', 'MyDocletName', meta); // mustHaveValue
         var goodTag2 = new tag.Tag('ignore', '', meta); // mustNotHaveValue
 
@@ -38,28 +44,28 @@ describe('jsdoc/tag/validator', function() {
             env.conf.tags.allowUnknownTags = allowUnknown;
         });
 
-        it("logs an error if the tag is not in the dictionary and conf.tags.allowUnknownTags is false", function() {
+        it('logs an error if the tag is not in the dictionary and conf.tags.allowUnknownTags is false', function() {
             env.conf.tags.allowUnknownTags = false;
             validateTag(badTag);
 
             expect(logger.error).toHaveBeenCalled();
         });
 
-        it("doesn't log an error if the tag is not in the dictionary and conf.tags.allowUnknownTags is true", function() {
+        it('does not log an error if the tag is not in the dictionary and conf.tags.allowUnknownTags is true', function() {
             env.conf.tags.allowUnknownTags = true;
             validateTag(badTag);
 
             expect(logger.error).not.toHaveBeenCalled();
         });
 
-        it("logs no error for valid tags", function() {
+        it('does not log an error for valid tags', function() {
             validateTag(goodTag);
             validateTag(goodTag2);
 
             expect(logger.error).not.toHaveBeenCalled();
         });
 
-        it("logs an error if the tag has no text but .mustHaveValue is true", function() {
+        it('logs an error if the tag has no text but mustHaveValue is true', function() {
             var missingName = doop(goodTag);
             missingName.text = null;
             validateTag(missingName);
@@ -67,7 +73,7 @@ describe('jsdoc/tag/validator', function() {
             expect(logger.error).toHaveBeenCalled();
         });
 
-        it("logs a warning if the tag has text but .mustNotHaveValue is true", function() {
+        it('logs a warning if the tag has text but mustNotHaveValue is true', function() {
             var missingText = doop(goodTag2);
             missingText.mustNotHaveValue = true;
             missingText.text = missingText.text || 'asdf';
@@ -76,10 +82,17 @@ describe('jsdoc/tag/validator', function() {
             expect(logger.warn).toHaveBeenCalled();
         });
 
-        it("logs a warning if the tag has a description but .mustNotHaveDescription is true", function() {
+        it('logs a warning if the tag has a description but mustNotHaveDescription is true', function() {
             validateTag(badTag2);
 
             expect(logger.warn).toHaveBeenCalled();
         });
+
+        it('logs meta.comment when present', function() {
+            env.conf.tags.allowUnknownTags = false;
+            validateTag(badTag);
+
+            expect(logger.error.mostRecentCall.args[0]).toContain(meta.comment);
+        });
     });
 });

test/specs/jsdoc/util/error.js

@@ -1,20 +1,22 @@
 /*global beforeEach, describe, expect, it, spyOn */
-describe("jsdoc/util/error", function() {
+'use strict';
+
+describe('jsdoc/util/error', function() {
     var error = require('jsdoc/util/error');
     var handle = error.handle;
     var logger = require('jsdoc/util/logger');
 
-    it("should exist", function() {
+    it('should exist', function() {
         expect(error).toBeDefined();
-        expect(typeof error).toEqual("object");
+        expect(typeof error).toBe('object');
     });
 
-    it("should export a 'handle' function", function() {
+    it('should export a "handle" function', function() {
         expect(handle).toBeDefined();
-        expect(typeof handle).toEqual("function");
+        expect(typeof handle).toBe('function');
     });
 
-    describe("handle", function() {
+    describe('handle', function() {
         it('should not throw', function() {
             expect(handle).not.toThrow();
         });
@@ -25,5 +27,12 @@ describe("jsdoc/util/error", function() {
 
             expect(logger.error).toHaveBeenCalled();
         });
+
+        it('should use special formatting for Error instances', function() {
+            spyOn(logger, 'error');
+            handle( new Error('Oh no!') );
+
+            expect(logger.error).toHaveBeenCalledWith('Error: Oh no!');
+        });
     });
 });

test/specs/jsdoc/util/markdown.js

@@ -1,4 +1,6 @@
-/*global describe: true, env: true, expect: true, it: true, xit: true */
+/*global describe, env, expect, it, spyOn */
+'use strict';
+
 describe('jsdoc/util/markdown', function() {
     var markdown = require('jsdoc/util/markdown');
 
@@ -91,5 +93,18 @@ describe('jsdoc/util/markdown', function() {
             expect(parser('Visit {@link https://google.com}.'))
                 .toBe('<p>Visit {@link https://google.com}.</p>');
         });
+
+        it('should log an error if an unrecognized Markdown parser is requested', function() {
+            var logger = require('jsdoc/util/logger');
+            var parser;
+            var storage = setMarkdownConf({parser: 'not-a-real-markdown-parser'});
+
+            spyOn(logger, 'error');
+
+            parser = markdown.getParser();
+            restoreMarkdownConf(storage);
+
+            expect(logger.error).toHaveBeenCalled();
+        });
     });
 });