From ed5c2a04a435569b2af3553f2084b70e7b4ae7fa Mon Sep 17 00:00:00 2001 From: Tom Macwright Date: Sat, 29 Apr 2017 14:25:07 -0400 Subject: [PATCH] feat(lint): Identify explicit tags that don't match inference in lint stage This will flag cases where people explicitly document things in JSDoc that don't reflect the code reality - in many cases, misnamed parameter names in documentation tags. Fixes https://github.com/documentationjs/documentation/issues/575 --- declarations/comment.js | 16 +------ lib/index.js | 19 ++++---- lib/infer/membership.js | 1 - lib/infer/params.js | 50 ++++++++++++--------- lib/input/shallow.js | 2 +- lib/nest.js | 6 +-- lib/output/html.js | 4 +- lib/output/util/formatters.js | 1 - lib/parse.js | 11 +++++ lib/parsers/polyglot.js | 2 +- test/fixture/lint/lint.input.js | 6 +++ test/fixture/lint/lint.output | 17 +++++--- test/fixture/params.output.json | 19 +++++++- test/lib/infer/params.js | 77 +++++++++++++++++++-------------- 14 files changed, 135 insertions(+), 96 deletions(-) diff --git a/declarations/comment.js b/declarations/comment.js index c6747408e..b1c7744a9 100644 --- a/declarations/comment.js +++ b/declarations/comment.js @@ -59,21 +59,7 @@ type CommentContextGitHub = { url: string }; -type CommentTagBase = { - title: string -}; - -type CommentTag = CommentTagBase & { - name?: string, - title: string, - description?: Object, - default?: any, - lineNumber?: number, - type?: DoctrineType, - properties?: Array -}; - -type CommentTagNamed = CommentTag & { +type CommentTag = { name?: string, title: string, description?: Object, diff --git a/lib/index.js b/lib/index.js index db6c9b103..653cb43d2 100644 --- a/lib/index.js +++ b/lib/index.js @@ -28,17 +28,16 @@ var fs = require('fs'), /** * Build a pipeline of comment handlers. - * @param {...Function|null} args - Pipeline elements. Each is a function that accepts + * @param {Array} fns - Pipeline elements. Each is a function that accepts * a comment and can return a comment or undefined (to drop that comment). * @returns {Function} pipeline * @private */ -function pipeline() { - var elements = arguments; +function pipeline(fns) { return comment => { - for (var i = 0; comment && i < elements.length; i++) { - if (elements[i]) { - comment = elements[i](comment); + for (var i = 0; comment && i < fns.length; i++) { + if (fns[i]) { + comment = fns[i](comment); } } return comment; @@ -95,7 +94,7 @@ function buildInternal(inputsAndConfig) { var parseFn = config.polyglot ? polyglot : parseJavaScript; - var buildPipeline = pipeline( + var buildPipeline = pipeline([ inferName, inferAccess(config.inferPrivate), inferAugments, @@ -108,7 +107,7 @@ function buildInternal(inputsAndConfig) { inferType, config.github && github, garbageCollect - ); + ]); let extractedComments = _.flatMap(inputs, function(sourceFile) { if (!sourceFile.source) { @@ -130,7 +129,7 @@ function lintInternal(inputsAndConfig) { let parseFn = config.polyglot ? polyglot : parseJavaScript; - let lintPipeline = pipeline( + let lintPipeline = pipeline([ lintComments, inferName, inferAccess(config.inferPrivate), @@ -142,7 +141,7 @@ function lintInternal(inputsAndConfig) { inferMembership(), inferType, nest - ); + ]); let extractedComments = _.flatMap(inputs, sourceFile => { if (!sourceFile.source) { diff --git a/lib/infer/membership.js b/lib/infer/membership.js index 17cdeb7bf..ecf558374 100644 --- a/lib/infer/membership.js +++ b/lib/infer/membership.js @@ -187,7 +187,6 @@ function normalizeMemberof(comment /*: Comment*/) /*: Comment */ { * annotations within a file * * @private - * @param {Object} comment parsed comment * @returns {Object} comment with membership inferred */ module.exports = function() { diff --git a/lib/infer/params.js b/lib/infer/params.js index 9a9a2b492..9e48ca64e 100644 --- a/lib/infer/params.js +++ b/lib/infer/params.js @@ -7,7 +7,6 @@ const _ = require('lodash'); const findTarget = require('./finders').findTarget; const flowDoctrine = require('../flow_doctrine'); const util = require('util'); -const debuglog = util.debuglog('documentation'); /** * Infers param tags by reading function parameter names @@ -45,17 +44,22 @@ function inferParams(comment /*: Comment */) { function inferAndCombineParams(params, comment) { var inferredParams = params.map((param, i) => paramToDoc(param, '', i)); - var mergedParams = mergeTrees(inferredParams, comment.params); + var mergedParamsAndErrors = mergeTrees(inferredParams, comment.params); // Then merge the trees. This is the hard part. return _.assign(comment, { - params: mergedParams + params: mergedParamsAndErrors.mergedParams, + errors: comment.errors.concat(mergedParamsAndErrors.errors) }); } // Utility methods ============================================================ // const PATH_SPLIT_CAPTURING = /(\[])?(\.)/g; +const PATH_SPLIT = /(?:\[])?\./g; +function tagDepth(tag /*: CommentTag */) /*: number */ { + return (tag.name || '').split(PATH_SPLIT).length; +} /** * Index tags by their `name` property into an ES6 map. @@ -199,7 +203,7 @@ function paramToDoc( }; default: // (a) - var newParam /*: CommentTagNamed */ = { + var newParam /*: CommentTag*/ = { title: 'param', name: prefix ? prefixedName : param.name, lineNumber: param.loc.start.line @@ -261,25 +265,29 @@ function mergeTrees(inferred, explicit) { function mergeTopNodes(inferred, explicit) { const mapExplicit = mapTags(explicit); const inferredNames = new Set(inferred.map(tag => tag.name)); - const explicitTagsWithoutInference = explicit.filter( - tag => !inferredNames.has(tag.name) - ); + const explicitTagsWithoutInference = explicit.filter(tag => { + return tagDepth(tag) === 1 && !inferredNames.has(tag.name); + }); - if (explicitTagsWithoutInference.length) { - debuglog( - `${explicitTagsWithoutInference.length} tags were specified but didn't match ` + - `inferred information ${explicitTagsWithoutInference - .map(t => t.name) - .join(', ')}` - ); - } + var errors = explicitTagsWithoutInference.map(tag => { + return { + message: `An explicit parameter named ${tag.name || ''} was specified but didn't match ` + + `inferred information ${Array.from(inferredNames).join(', ')}`, + commentLineNumber: tag.lineNumber + }; + }); - return inferred - .map(inferredTag => { - const explicitTag = mapExplicit.get(inferredTag.name); - return explicitTag ? combineTags(inferredTag, explicitTag) : inferredTag; - }) - .concat(explicitTagsWithoutInference); + return { + errors, + mergedParams: inferred + .map(inferredTag => { + const explicitTag = mapExplicit.get(inferredTag.name); + return explicitTag + ? combineTags(inferredTag, explicitTag) + : inferredTag; + }) + .concat(explicitTagsWithoutInference) + }; } // This method is used for _non-root_ properties only - we use mergeTopNodes diff --git a/lib/input/shallow.js b/lib/input/shallow.js index b459a0e84..ed42fbe30 100644 --- a/lib/input/shallow.js +++ b/lib/input/shallow.js @@ -16,7 +16,7 @@ var smartGlob = require('../smart_glob.js'); * or without fs access. * * @param indexes entry points - * @param options parsing options + * @param config parsing options * @return promise with parsed files */ module.exports = function( diff --git a/lib/nest.js b/lib/nest.js index 5dd91d317..b776e93c7 100644 --- a/lib/nest.js +++ b/lib/nest.js @@ -7,7 +7,7 @@ const PATH_SPLIT = /(?:\[])?\./g; function removeUnnamedTags( tags /*: Array */ -) /*: Array */ { +) /*: Array */ { return tags.filter(tag => typeof tag.name === 'string'); } @@ -32,9 +32,7 @@ var tagDepth = tag => tag.name.split(PATH_SPLIT).length; * \-> [].baz * * @private - * @param {Object} comment a comment with tags - * @param {string} tagTitle the tag to nest - * @param {string} target the tag to nest into + * @param {Array} tags a list of tags * @returns {Object} nested comment */ var nestTag = ( diff --git a/lib/output/html.js b/lib/output/html.js index 715200334..f4ca68228 100644 --- a/lib/output/html.js +++ b/lib/output/html.js @@ -8,8 +8,8 @@ var mergeConfig = require('../merge_config'); * Formats documentation as HTML. * * @param comments parsed comments - * @param {Object} args Options that can customize the output - * @param {string} [args.theme='default_theme'] Name of a module used for an HTML theme. + * @param {Object} config Options that can customize the output + * @param {string} [config.theme='default_theme'] Name of a module used for an HTML theme. * @returns {Promise>} Promise with results * @name formats.html * @public diff --git a/lib/output/util/formatters.js b/lib/output/util/formatters.js index 861df2114..f371894a1 100644 --- a/lib/output/util/formatters.js +++ b/lib/output/util/formatters.js @@ -72,7 +72,6 @@ module.exports = function(getHref /*: Function*/) { /** * Link text to this page or to a central resource. * @param {string} text inner text of the link - * @param {string} description link text override * @returns {string} potentially linked HTML */ formatters.autolink = function(text /*: string*/) { diff --git a/lib/parse.js b/lib/parse.js index 60203fca6..442e7149d 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -600,6 +600,17 @@ function parseJSDoc( result.description = parseMarkdown(result.description); } + // Reject parameter tags without a parameter name + result.tags.filter(function(tag) { + if (tag.title === 'param' && tag.name === undefined) { + result.errors.push({ + message: 'A @param tag without a parameter name was rejected' + }); + return false; + } + return true; + }); + result.tags.forEach(function(tag) { if (tag.errors) { for (var j = 0; j < tag.errors.length; j++) { diff --git a/lib/parsers/polyglot.js b/lib/parsers/polyglot.js index dccb9c070..8c55cfde7 100644 --- a/lib/parsers/polyglot.js +++ b/lib/parsers/polyglot.js @@ -10,7 +10,7 @@ var getComments = require('get-comments'), * Documentation stream parser: this receives a module-dep item, * reads the file, parses the JavaScript, parses the JSDoc, and * emits parsed comments. - * @param {Object} data a chunk of data provided by module-deps + * @param sourceFile a chunk of data provided by module-deps * @return {Array} adds to memo */ function parsePolyglot(sourceFile /*: SourceFile*/) { diff --git a/test/fixture/lint/lint.input.js b/test/fixture/lint/lint.input.js index 35b26fe4d..181de48c9 100644 --- a/test/fixture/lint/lint.input.js +++ b/test/fixture/lint/lint.input.js @@ -3,6 +3,7 @@ * @param {Array} foo bar * @param {Array} foo bar * @param {Array|Number} foo bar + * @param {boolean} * @returns {object} bad object return type * @type {Array} bad object type * @memberof notfound @@ -13,3 +14,8 @@ * @property {String} bad property * @private */ + +/** + * @param {number} c explicit but not found + */ +function add(a, b) {} diff --git a/test/fixture/lint/lint.output b/test/fixture/lint/lint.output index 4725962b4..6bbabba2c 100644 --- a/test/fixture/lint/lint.output +++ b/test/fixture/lint/lint.output @@ -1,12 +1,17 @@ - 2:1 warning type String found, string is standard + 2:1 warning An explicit parameter named foo was specified but didn't match inferred information a, b 3:1 warning type Number found, number is standard + 3:1 warning An explicit parameter named foo was specified but didn't match inferred information a, b 4:1 warning type Number found, number is standard + 4:1 warning An explicit parameter named foo was specified but didn't match inferred information a, b 5:1 warning type Number found, number is standard - 6:1 warning type object found, Object is standard + 5:1 warning An explicit parameter named foo was specified but didn't match inferred information a, b + 6:1 warning An explicit parameter named boolean was specified but didn't match inferred information a, b 7:1 warning type object found, Object is standard - 8:1 warning @memberof reference to notfound not found - 11:1 warning could not determine @name for hierarchy - 12:1 warning type String found, string is standard + 8:1 warning type object found, Object is standard + 9:1 warning @memberof reference to notfound not found 13:1 warning type String found, string is standard + 13:1 warning An explicit parameter named baz was specified but didn't match inferred information a, b + 14:1 warning type String found, string is standard + 19:1 warning An explicit parameter named c was specified but didn't match inferred information a, b -⚠ 11 warnings +⚠ 16 warnings diff --git a/test/fixture/params.output.json b/test/fixture/params.output.json index 76bba4dd5..5d298de82 100644 --- a/test/fixture/params.output.json +++ b/test/fixture/params.output.json @@ -1212,7 +1212,24 @@ } }, "augments": [], - "errors": [], + "errors": [ + { + "message": "An explicit parameter named address was specified but didn't match inferred information ", + "commentLineNumber": 5 + }, + { + "message": "An explicit parameter named groups was specified but didn't match inferred information ", + "commentLineNumber": 6 + }, + { + "message": "An explicit parameter named third was specified but didn't match inferred information ", + "commentLineNumber": 7 + }, + { + "message": "An explicit parameter named foo was specified but didn't match inferred information ", + "commentLineNumber": 8 + } + ], "examples": [ { "description": "var address = new Address6('2001::/32');" diff --git a/test/lib/infer/params.js b/test/lib/infer/params.js index 0a45b4b0d..65edac6bf 100644 --- a/test/lib/infer/params.js +++ b/test/lib/infer/params.js @@ -34,17 +34,25 @@ test('mergeTrees', function(t) { } ] ), - [ - { - title: 'param', - description: 'First arg!', - name: 'a', - type: { - type: 'NameExpression', - name: 'string' + { + errors: [ + { + commentLineNumber: null, + message: "An explicit parameter named a was specified but didn't match inferred information " } - } - ] + ], + mergedParams: [ + { + title: 'param', + description: 'First arg!', + name: 'a', + type: { + type: 'NameExpression', + name: 'string' + } + } + ] + } ); t.deepEqual( @@ -85,29 +93,32 @@ test('mergeTrees', function(t) { } ] ), - [ - { - title: 'param', - description: 'First arg!', - name: 'a', - type: { - type: 'NameExpression', - name: 'object' - }, - properties: [ - { - title: 'param', - name: 'a.a', - parameterIndex: 0, - type: { - type: 'NameExpression', - name: 'string' - }, - properties: [] - } - ] - } - ] + { + errors: [], + mergedParams: [ + { + title: 'param', + description: 'First arg!', + name: 'a', + type: { + type: 'NameExpression', + name: 'object' + }, + properties: [ + { + title: 'param', + name: 'a.a', + parameterIndex: 0, + type: { + type: 'NameExpression', + name: 'string' + }, + properties: [] + } + ] + } + ] + } ); t.end();