From af0daf818206da68e3182e701899ff4cf0e4736e Mon Sep 17 00:00:00 2001 From: Tom Macwright Date: Tue, 18 Apr 2017 20:18:59 -0400 Subject: [PATCH] Simplify param inference, update test fixtures. This is focused around Array destructuring: documenting destructured array elements with indices instead of names, because the names are purely internal details --- lib/infer/params.js | 248 ++++++++++++++--------------- test/fixture/es6.output-toc.md | 8 +- test/fixture/es6.output.json | 8 +- test/fixture/es6.output.md | 8 +- test/fixture/es6.output.md.json | 8 +- test/fixture/params.output.json | 5 + test/fixture/params.output.md | 1 + test/fixture/params.output.md.json | 22 +++ test/lib/infer/params.js | 28 ++++ 9 files changed, 189 insertions(+), 147 deletions(-) diff --git a/lib/infer/params.js b/lib/infer/params.js index ed18f93a8..2861bee70 100644 --- a/lib/infer/params.js +++ b/lib/infer/params.js @@ -28,22 +28,19 @@ function inferParams(comment /*: Comment */) { return comment; } + var inferredParams = path.node.params.map((param, i) => + paramToDoc(param, '', i)); + + var mergedParams = mergeTrees(inferredParams, comment.params); + // Then merge the trees. This is the hard part. return _.assign(comment, { - params: mergeTrees( - path.node.params.map((param, i) => paramToDoc(param, i, '')), - comment.params - ) + params: mergedParams }); } // Utility methods ============================================================ // -function addPrefix(doc /*: CommentTagNamed */, prefix) { - return _.assign(doc, { - name: prefix + doc.name - }); -} const PATH_SPLIT_CAPTURING = /(\[])?(\.)/g; /** @@ -57,114 +54,6 @@ function mapTags(tags) { ); } -// ___toDoc methods ============================================================ -// -// These methods take Babel AST nodes and output equivalent JSDoc parameter tags. -function destructuringObjectParamToDoc(param, i, prefix) /*: CommentTag */ { - return { - title: 'param', - name: '$' + String(i), - anonymous: true, - type: (param.typeAnnotation && flowDoctrine(param)) || { - type: 'NameExpression', - name: 'Object' - }, - properties: param.properties.map(prop => - destructuringPropertyToDoc(prop, i, prefix)) - }; -} - -function destructuringPropertyToDoc( - property, - i /*: number */, - prefix /*: string */ -) /*: CommentTag */ { - switch (property.type) { - case 'ObjectProperty': - // Object properties can rename their arguments, like - // function f({ x: y }) - // We want to document that as x, not y: y is the internal name. - // So we use the key. In the case where they don't rename, - // key and value are the same. - return paramToDoc(property, i, prefix + '$' + String(i) + '.'); - case 'Identifier': - // if the destructuring type is an array, the elements - // in it are identifiers - return paramToDoc(property, i, prefix + '$' + String(i) + '.'); - case 'RestProperty': - case 'RestElement': - case 'ObjectPattern': - return paramToDoc(property, i, prefix + '$' + String(i) + '.'); - default: - throw new Error(`Unknown property encountered: ${property.type}`); - } -} - -function destructuringObjectPropertyToDoc( - param, - i /*: number */, - prefix /*: string */ -) /*: CommentTag */ { - return _.assign(paramToDoc(param.value, i, prefix), { - name: prefix + param.key.name - }); -} - -function destructuringArrayParamToDoc( - param, - i /*: number */, - prefix /*: string */ -) /*: CommentTag */ { - return { - title: 'param', - name: '$' + String(i), - anonymous: true, - type: (param.typeAnnotation && flowDoctrine(param)) || { - type: 'NameExpression', - name: 'Array' - }, - properties: param.elements.map(element => - destructuringPropertyToDoc(element, i, prefix)) - }; -} - -/** - * Given a parameter like - * - * function a(b = 1) - * - * Format it as an optional parameter in JSDoc land - * - * @param {Object} param ESTree node - * @returns {Object} JSDoc param - */ -function paramWithDefaultToDoc(param, i) /*: CommentTag */ { - const newParam = paramToDoc(param.left, i, ''); - - return _.assign(newParam, { - default: generate(param.right).code, - type: { - type: 'OptionalType', - expression: newParam.type - } - }); -} - -function restParamToDoc(param) /*: CommentTag */ { - let type /*: DoctrineType */ = { - type: 'RestType' - }; - if (param.typeAnnotation) { - type.expression = flowDoctrine(param.typeAnnotation.typeAnnotation); - } - return { - title: 'param', - name: param.argument.name, - lineNumber: param.loc.start.line, - type - }; -} - /** * Babel parses JavaScript source code and produces an abstract syntax * tree that includes methods and their arguments. This function takes @@ -184,27 +73,124 @@ function restParamToDoc(param) /*: CommentTag */ { */ function paramToDoc( param, - i /*: number */, - prefix /*: string */ -) /*: CommentTag */ { - // ES6 default + prefix /*: string */, + i /*: ?number */ +) /*: CommentTag|Array */ { + const autoName = '$' + String(i); + const prefixedName = prefix + '.' + param.name; + switch (param.type) { case 'AssignmentPattern': // (a = b) - return addPrefix(paramWithDefaultToDoc(param, i), prefix); + const newAssignmentParam = paramToDoc(param.left, '', i); + + if (Array.isArray(newAssignmentParam)) { + throw new Error('Encountered an unexpected parameter type'); + } + + return _.assign(newAssignmentParam, { + default: generate(param.right, { + compact: true + }).code, + type: { + type: 'OptionalType', + expression: newAssignmentParam.type + } + }); + // ObjectPattern case 'ObjectPattern': // { a } - return destructuringObjectParamToDoc(param, i, prefix); - case 'ArrayPattern': - return destructuringArrayParamToDoc(param, i, prefix); - // TODO: do we need both? + if (prefix === '') { + // If this is a root-level param, like f({ x }), then we need to name + // it, like $0 or $1, depending on its position. + return { + title: 'param', + name: autoName, + anonymous: true, + type: (param.typeAnnotation && flowDoctrine(param)) || { + type: 'NameExpression', + name: 'Object' + }, + properties: _.flatMap(param.properties, prop => { + return paramToDoc(prop, prefix + autoName); + }) + }; + } else if (param.indexed) { + // Likewise, if this object pattern sits inside of an ArrayPattern, + // like [{ foo }], it shouldn't just look like $0.foo, but like $0.0.foo, + // so make sure it isn't indexed first. + return { + title: 'param', + name: prefixedName, + anonymous: true, + type: (param.typeAnnotation && flowDoctrine(param)) || { + type: 'NameExpression', + name: 'Object' + }, + properties: _.flatMap(param.properties, prop => { + return paramToDoc(prop, prefixedName); + }) + }; + } + // If, otherwise, this is nested, we don't really represent it as + // a parameter in and of itself - we just want its children, and + // it will be the . in obj.prop + return _.flatMap(param.properties, prop => { + return paramToDoc(prop, prefix); + }); + // ArrayPattern + case 'ArrayPattern': // ([a, b, { c }]) + if (prefix === '') { + return { + title: 'param', + name: autoName, + anonymous: true, + type: (param.typeAnnotation && flowDoctrine(param)) || { + type: 'NameExpression', + name: 'Array' + }, + // Array destructuring lets you name the elements in the array, + // but those names don't really make sense within the JSDoc + // indexing tradition, or have any external meaning. So + // instead we're going to (immutably) rename the parameters to their + // indices + properties: _.flatMap(param.elements, (element, idx) => { + var indexedElement = _.assign({}, element, { + name: String(idx), + indexed: true + }); + return paramToDoc(indexedElement, autoName); + }) + }; + } + return _.flatMap(param.elements, (element, idx) => { + var indexedElement = _.assign({}, element, { + name: String(idx) + }); + return paramToDoc(indexedElement, prefix); + }); case 'ObjectProperty': - return destructuringObjectPropertyToDoc(param, i, prefix); - case 'RestProperty': + return _.assign(paramToDoc(param.value, prefix + '.' + param.key.name), { + name: prefix + '.' + param.key.name + }); + case 'RestProperty': // (a, ...b) case 'RestElement': - return addPrefix(restParamToDoc(param), prefix); + let type /*: DoctrineType */ = { + type: 'RestType' + }; + if (param.typeAnnotation) { + type.expression = flowDoctrine(param.typeAnnotation.typeAnnotation); + } + return { + title: 'param', + name: param.argument.name, + name: prefix ? `${prefix}.${param.argument.name}` : param.argument.name, + lineNumber: param.loc.start.line, + type + }; default: + // (a) var newParam /*: CommentTagNamed */ = { title: 'param', - name: param.name, + name: prefix ? prefixedName : param.name, lineNumber: param.loc.start.line }; @@ -213,7 +199,7 @@ function paramToDoc( newParam.type = flowDoctrine(param.typeAnnotation.typeAnnotation); } - return addPrefix(newParam, prefix); + return newParam; } } diff --git a/test/fixture/es6.output-toc.md b/test/fixture/es6.output-toc.md index 7e731557a..4060592c0 100644 --- a/test/fixture/es6.output-toc.md +++ b/test/fixture/es6.output-toc.md @@ -19,9 +19,9 @@ Similar, but with an array **Parameters** - `$0` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)** - - `$0.a` - - `$0.b` - - `$0.c` + - `$0.0` + - `$0.1` + - `$0.2` **Examples** @@ -135,6 +135,6 @@ Regression check for #498 - `array1` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)<T>** - `array2` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)<T>** -- `compareFunction` **function (a: T, b: T): [boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?** (optional, default `(a: T, b: T): boolean => a === b`) +- `compareFunction` **function (a: T, b: T): [boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?** (optional, default `(a:T,b:T):boolean=>a===b`) Returns **[boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)** diff --git a/test/fixture/es6.output.json b/test/fixture/es6.output.json index 596e4a656..2f66ac9b6 100644 --- a/test/fixture/es6.output.json +++ b/test/fixture/es6.output.json @@ -248,17 +248,17 @@ "properties": [ { "title": "param", - "name": "$0.a", + "name": "$0.0", "lineNumber": 14 }, { "title": "param", - "name": "$0.b", + "name": "$0.1", "lineNumber": 14 }, { "title": "param", - "name": "$0.c", + "name": "$0.2", "lineNumber": 14 } ] @@ -2809,7 +2809,7 @@ } } }, - "default": "(a: T, b: T): boolean => a === b" + "default": "(a:T,b:T):boolean=>a===b" } ], "properties": [], diff --git a/test/fixture/es6.output.md b/test/fixture/es6.output.md index 84c9e963e..416e00c6a 100644 --- a/test/fixture/es6.output.md +++ b/test/fixture/es6.output.md @@ -42,9 +42,9 @@ Similar, but with an array **Parameters** - `$0` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)** - - `$0.a` - - `$0.b` - - `$0.c` + - `$0.0` + - `$0.1` + - `$0.2` **Examples** @@ -158,6 +158,6 @@ Regression check for #498 - `array1` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)<T>** - `array2` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)<T>** -- `compareFunction` **function (a: T, b: T): [boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?** (optional, default `(a: T, b: T): boolean => a === b`) +- `compareFunction` **function (a: T, b: T): [boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?** (optional, default `(a:T,b:T):boolean=>a===b`) Returns **[boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)** diff --git a/test/fixture/es6.output.md.json b/test/fixture/es6.output.md.json index 74617b58a..3c6e7eb2b 100644 --- a/test/fixture/es6.output.md.json +++ b/test/fixture/es6.output.md.json @@ -379,7 +379,7 @@ "children": [ { "type": "inlineCode", - "value": "$0.a" + "value": "$0.0" }, { "type": "text", @@ -401,7 +401,7 @@ "children": [ { "type": "inlineCode", - "value": "$0.b" + "value": "$0.1" }, { "type": "text", @@ -423,7 +423,7 @@ "children": [ { "type": "inlineCode", - "value": "$0.c" + "value": "$0.2" }, { "type": "text", @@ -2229,7 +2229,7 @@ }, { "type": "inlineCode", - "value": "(a: T, b: T): boolean => a === b" + "value": "(a:T,b:T):boolean=>a===b" }, { "type": "text", diff --git a/test/fixture/params.output.json b/test/fixture/params.output.json index 2f16910d8..095ba443a 100644 --- a/test/fixture/params.output.json +++ b/test/fixture/params.output.json @@ -2695,6 +2695,11 @@ "name": "any" } }, + { + "title": "param", + "name": "input.0", + "lineNumber": 109 + }, { "title": "param", "name": "input.xs", diff --git a/test/fixture/params.output.md b/test/fixture/params.output.md index 66737b81b..13a12b411 100644 --- a/test/fixture/params.output.md +++ b/test/fixture/params.output.md @@ -134,6 +134,7 @@ iterator destructure (RestElement) - `input` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)** - `input.x` **any** head of iterator + - `input.0` - `input.xs` **...any** Returns **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)<any>** rotated such that the last element was the first diff --git a/test/fixture/params.output.md.json b/test/fixture/params.output.md.json index e0512ec3b..07ed8d7f5 100644 --- a/test/fixture/params.output.md.json +++ b/test/fixture/params.output.md.json @@ -2399,6 +2399,28 @@ } ] }, + { + "type": "listItem", + "children": [ + { + "type": "paragraph", + "children": [ + { + "type": "inlineCode", + "value": "input.0" + }, + { + "type": "text", + "value": " " + }, + { + "type": "text", + "value": " " + } + ] + } + ] + }, { "type": "listItem", "children": [ diff --git a/test/lib/infer/params.js b/test/lib/infer/params.js index a2f05646c..8049ee6de 100644 --- a/test/lib/infer/params.js +++ b/test/lib/infer/params.js @@ -334,6 +334,34 @@ test('inferParams', function(t) { 'renaming' ); + t.deepEqual( + evaluate( + ` + /** Test */ + function f({ x: { y: { z } } }) {} + ` + ).params, + [ + { + anonymous: true, + name: '$0', + properties: [ + { + lineNumber: 3, + name: '$0.x.y.z', + title: 'param' + } + ], + title: 'param', + type: { + name: 'Object', + type: 'NameExpression' + } + } + ], + 'renaming' + ); + t.deepEqual(evaluate('/** Test */ export function f(x) {}').params, [ { lineNumber: 1, name: 'x', title: 'param' } ]);