Store commentLineNumber on errors. Fixes #159

cc @jfirebaugh for the review

bin/documentation.js

@@ -6,7 +6,7 @@ var documentation = require('../'),
   streamArray = require('stream-array'),
   fs = require('fs'),
   vfs = require('vinyl-fs'),
-  formatError = require('../lib/error'),
+  formatError = require('../lib/format_error'),
   args = require('../lib/args.js');
 
 var parsedArgs = args(process.argv.slice(2)),

lib/error.js → lib/format_error.js

@@ -13,8 +13,8 @@ var path = require('path');
  */
 module.exports = function (comment, error) {
   var relativePath = path.relative(process.cwd(), comment.context.file),
-    lineNumber = comment.loc.start.line;
+    lineNumber = comment.loc.start.line + (error.commentLineNumber || 0);
 
-  return format.apply(format, ['%s:%d: ' + error, relativePath, lineNumber]
+  return format.apply(format, ['%s:%d: ' + error.message, relativePath, lineNumber]
     .concat(Array.prototype.slice.call(arguments, 3)));
 };

lib/hierarchy.js

@@ -55,10 +55,18 @@ function inferHierarchy(comments) {
       continue;
     }
 
+    var memberOfTag = comment.tags.filter(function (tag) {
+      return tag.title === 'memberof'
+    })[0];
+    var memberOfTagLineNumber = (memberOfTag && memberOfTag.lineNumber) || 0;
+
     var parent = nameIndex[comment.memberof];
 
     if (!parent) {
-      comment.errors.push('memberof reference to ' + comment.memberof + ' not found');
+      comment.errors.push({
+        message: 'memberof reference to ' + comment.memberof + ' not found',
+        commentLineNumber: memberOfTagLineNumber
+      });
       continue;
     }
 
@@ -70,7 +78,10 @@ function inferHierarchy(comments) {
 
     default:
       if (!comment.scope) {
-        parent.errors.push('found memberof but no @scope, @static, or @instance tag');
+        parent.errors.push({
+          message: 'found memberof but no @scope, @static, or @instance tag',
+          commentLineNumber: memberOfTagLineNumber
+        });
         continue;
       }
       parent.members[comment.scope].push(comment);

lib/lint.js

@@ -21,8 +21,10 @@ module.exports = function (comment) {
   comment.tags.forEach(function (tag) {
     function nameInvariant(name) {
       if (CANONICAL[name]) {
-        comment.errors.push(
-          'type ' + name + ' found, ' + CANONICAL[name] + ' is standard');
+        comment.errors.push({
+          message: 'type ' + name + ' found, ' + CANONICAL[name] + ' is standard',
+          commentLineNumber: tag.lineNumber
+        });
       }
     }
 

lib/nest_params.js

@@ -30,8 +30,10 @@ module.exports = function (comment) {
     if (parts.length > 1) {
       var parent = index[parts[0]];
       if (parent === undefined) {
-        result.errors.push(
-          '@param ' + param.name + '\'s parent ' + parts[0] + ' not found');
+        result.errors.push({
+          message: '@param ' + param.name + '\'s parent ' + parts[0] + ' not found',
+          commentLineNumber: param.lineNumber
+        });
         result.params.push(param);
         return;
       }

test/fixture/_external-deps-included.json

@@ -36,7 +36,10 @@
       "code": "/**\n * This function returns the number one.\n * @return {Number} numberone\n */\nmodule.exports = function () {\n  // this returns 1\n  return 1;\n};\n"
     },
     "errors": [
-      "memberof reference to module not found"
+      {
+        "message": "memberof reference to module not found",
+        "commentLineNumber": 0
+      }
     ],
     "returns": [
       {
@@ -99,7 +102,10 @@
       "code": "'use strict';\n\nvar otherDep = require('external2');\n\n/**\n * This function returns the number one.\n * @return {Number} numberone\n */\nmodule.exports = function () {\n  // this returns 1\n  return otherDep() - 1;\n};\n"
     },
     "errors": [
-      "memberof reference to module not found"
+      {
+        "message": "memberof reference to module not found",
+        "commentLineNumber": 0
+      }
     ],
     "returns": [
       {

test/fixture/_multi-file-input.json

@@ -36,7 +36,10 @@
       "code": "/**\n * This function returns the number one.\n * @returns {Number} numberone\n */\nmodule.exports = function () {\n  // this returns 1\n  return 1;\n};\n"
     },
     "errors": [
-      "memberof reference to module not found"
+      {
+        "message": "memberof reference to module not found",
+        "commentLineNumber": 0
+      }
     ],
     "returns": [
       {
@@ -114,7 +117,10 @@
       "code": "/**\n * This function returns the number plus two.\n *\n * @param {Number} a the number\n * @returns {Number} numbertwo\n * @example\n * var result = returnTwo(4);\n * // result is 6\n */\nfunction returnTwo(a) {\n  // this returns a + 2\n  return a + 2;\n}\n"
     },
     "errors": [
-      "type Number found, number is standard"
+      {
+        "message": "type Number found, number is standard",
+        "commentLineNumber": 3
+      }
     ],
     "params": [
       {

test/fixture/factory.output.json

@@ -148,7 +148,10 @@
       "code": "/**\n * an area chart generator\n * @returns {area} chart\n */\nvar area = function() {\n\n  /**\n   * @class area\n   */\n  var chart = function(selection) {\n  };\n\n  /**\n   * Sets the chart data.\n   * @method\n   */\n  chart.data = function(_) {\n  };\n\n  return chart;\n};\n"
     },
     "errors": [
-      "memberof reference to chart not found"
+      {
+        "message": "memberof reference to chart not found",
+        "commentLineNumber": 0
+      }
     ],
     "function": null,
     "name": "data",

test/fixture/inline-link.output.json

@@ -127,7 +127,10 @@
       "code": "/**\n * Adds one to a number\n * @param {number} a the input\n * @returns {number} the output\n */\nfunction addOne(a) {\n  return a + 1;\n}\n\n/**\n * This function returns the number one. Internally, this uses\n * {@link addOne} to do the math.\n * @param {number} a the input\n * @returns {number} numberone\n */\nmodule.exports = function (a) {\n  return addOne(a);\n};\n"
     },
     "errors": [
-      "memberof reference to module not found"
+      {
+        "message": "memberof reference to module not found",
+        "commentLineNumber": 0
+      }
     ],
     "params": [
       {

test/fixture/jsx.output.json

@@ -36,7 +36,10 @@
       "code": "var util = require('util');\n\n/**\n * This function returns the number one.\n * @returns {Number} numberone\n */\nmodule.exports = (<p>hello</p>);\n"
     },
     "errors": [
-      "memberof reference to module not found"
+      {
+        "message": "memberof reference to module not found",
+        "commentLineNumber": 0
+      }
     ],
     "returns": [
       {

test/fixture/memberof-missing.output.json

@@ -55,7 +55,10 @@
       }
     },
     "errors": [
-      "memberof reference to Foo not found"
+      {
+        "message": "memberof reference to Foo not found",
+        "commentLineNumber": 4
+      }
     ],
     "name": "setTime",
     "params": [

test/fixture/memberof-no-scope.output.json

@@ -33,7 +33,10 @@
       }
     },
     "errors": [
-      "found memberof but no @scope, @static, or @instance tag"
+      {
+        "message": "found memberof but no @scope, @static, or @instance tag",
+        "commentLineNumber": 4
+      }
     ],
     "class": {
       "name": "Foo"

test/fixture/multiexample.output.json

@@ -78,7 +78,10 @@
       "code": "/**\n * This function returns the number one.\n * @returns {Number} numberone\n * @example\n * foo(1);\n * @example\n * foo(2);\n * @throws {Error} if you give it something\n * @throws {TypeError} if you give it something else\n * @augments Foo\n * @augments Bar\n */\nmodule.exports = function () {\n  // this returns 1\n  return 1;\n};\n"
     },
     "errors": [
-      "memberof reference to module not found"
+      {
+        "message": "memberof reference to module not found",
+        "commentLineNumber": 0
+      }
     ],
     "returns": [
       {

test/fixture/newline-in-description.output.json

@@ -36,7 +36,10 @@
       }
     },
     "errors": [
-      "type Number found, number is standard"
+      {
+        "message": "type Number found, number is standard",
+        "commentLineNumber": 2
+      }
     ],
     "params": [
       {

test/fixture/simple-hashbang.output.json

@@ -36,7 +36,10 @@
       "code": "//#!/usr/bin/env node\n\n/**\n * This function returns the number one.\n * @returns {Number} numberone\n */\nmodule.exports = function () {\n  // this returns 1\n  return 1;\n};\n"
     },
     "errors": [
-      "memberof reference to module not found"
+      {
+        "message": "memberof reference to module not found",
+        "commentLineNumber": 0
+      }
     ],
     "returns": [
       {

test/fixture/simple-two.output.json

@@ -51,7 +51,10 @@
       "code": "/**\n * This function returns the number plus two.\n *\n * @param {Number} a the number\n * @returns {Number} numbertwo\n * @example\n * var result = returnTwo(4);\n * // result is 6\n */\nfunction returnTwo(a) {\n  // this returns a + 2\n  return a + 2;\n}\n"
     },
     "errors": [
-      "type Number found, number is standard"
+      {
+        "message": "type Number found, number is standard",
+        "commentLineNumber": 3
+      }
     ],
     "params": [
       {

test/fixture/simple.output.github.json

@@ -38,7 +38,10 @@
       "github": "[github]"
     },
     "errors": [
-      "memberof reference to module not found"
+      {
+        "message": "memberof reference to module not found",
+        "commentLineNumber": 0
+      }
     ],
     "returns": [
       {

test/fixture/simple.output.json

@@ -36,7 +36,10 @@
       "code": "/**\n * This function returns the number one.\n * @returns {Number} numberone\n */\nmodule.exports = function () {\n  // this returns 1\n  return 1;\n};\n"
     },
     "errors": [
-      "memberof reference to module not found"
+      {
+        "message": "memberof reference to module not found",
+        "commentLineNumber": 0
+      }
     ],
     "returns": [
       {

test/fixture/throws.output.json

@@ -60,7 +60,10 @@
       "code": "/**\n * This function returns the number plus two.\n *\n * @param {Number} a the number\n * @returns {Number} numbertwo\n * @throws {Error} if number is 3\n * @example\n * var result = returnTwo(4);\n * // result is 6\n */\nfunction returnTwo(a) {\n  if (a === 3) throw new Error('cannot be 3');\n  // this returns a + 2\n  return a + 2;\n}\n"
     },
     "errors": [
-      "type Number found, number is standard"
+      {
+        "message": "type Number found, number is standard",
+        "commentLineNumber": 3
+      }
     ],
     "params": [
       {

test/lib/error.js

@@ -2,9 +2,9 @@
 
 var test = require('tap').test,
   path = require('path'),
-  error = require('../../lib/error');
+  formatError = require('../../lib/format_error');
 
-test('error', function (t) {
+test('formatError', function (t) {
   var comment = {
     context: {
       file: path.join(process.cwd(), 'file.js')
@@ -16,7 +16,14 @@ test('error', function (t) {
     }
   };
 
-  t.deepEqual(error(comment, 'test'), 'file.js:1: test');
+  t.deepEqual(formatError(comment, {
+    message: 'test'
+  }), 'file.js:1: test');
+
+  t.deepEqual(formatError(comment, {
+    message: 'test',
+    commentLineNumber: 1
+  }), 'file.js:2: test');
 
   t.end();
 });

test/lib/hierarchy.js

@@ -94,6 +94,9 @@ test('hierarchy - missing memberof', function (t) {
   });
 
   t.equal(result.length, 1);
-  t.equal(result[0].errors[0], 'memberof reference to DoesNotExist not found', 'correct error message');
+  t.deepEqual(result[0].errors[0], {
+    message: 'memberof reference to DoesNotExist not found',
+    commentLineNumber: 2
+  }, 'correct error message');
   t.end();
 });

test/lib/lint.js

@@ -2,6 +2,7 @@
 
 var test = require('tap').test,
   parse = require('../../lib/parsers/javascript'),
+  formatError = require('../../lib/format_error'),
   lint = require('../../lib/lint');
 
 function toComment(fn, filename) {
@@ -23,10 +24,25 @@ test('lint', function (t) {
      */
     return 0;
   }).errors, [
-    'type String found, string is standard',
-    'type array found, Array is standard'],
+    { commentLineNumber: 1, message: 'type String found, string is standard' },
+    { commentLineNumber: 2, message: 'type array found, Array is standard' }],
     'non-canonical');
 
+  var comment = evaluate(function () {/**
+     * @param {String} foo
+     * @param {array} bar
+     */
+    return 0;
+  });
+
+  t.equal(formatError(comment, comment.errors[0]),
+    'input.js:2: type String found, string is standard',
+    'formatError');
+
+  t.equal(formatError(comment, comment.errors[1]),
+    'input.js:3: type array found, Array is standard',
+    'formatError');
+
   t.deepEqual(evaluate(function () {
     /**
      * @param {string} foo

test/lib/nest_params.js

@@ -48,13 +48,14 @@ test('nestParams - basic', function (t) {
 });
 
 test('nestParams - missing parent', function (t) {
-  var result = toComment(function () {
-    /** @param {string} foo.bar */
+  var result = toComment(function () {/** @param {string} foo.bar */
     return 0;
   });
   t.equal(result[0].params.length, 1);
-  t.equal(result[0].errors[0], '@param foo.bar\'s parent foo not found',
-    'correct error message');
+  t.deepEqual(result[0].errors[0], {
+    message: '@param foo.bar\'s parent foo not found',
+    commentLineNumber: 0
+  }, 'correct error message');
   t.equal(result[0].params[0].name, 'foo.bar');
   t.end();
 });