diff --git a/lib/jsdoc/doclet.js b/lib/jsdoc/doclet.js index 1fb94fd2..8a862ec6 100644 --- a/lib/jsdoc/doclet.js +++ b/lib/jsdoc/doclet.js @@ -209,9 +209,8 @@ function dooper(source, target, properties) { } /** - * Combine two doclets into a target doclet, using properties from the secondary doclet only when - * those properties do not exist on the primary doclet, and ignoring properties that should be - * excluded. + * Copy all but a list of excluded properties from one of two doclets onto a target doclet. Prefers + * the primary doclet over the secondary doclet. * * @private * @param {module:jsdoc/doclet.Doclet} primary - The primary doclet. @@ -219,7 +218,7 @@ function dooper(source, target, properties) { * @param {module:jsdoc/doclet.Doclet} target - The doclet to which properties will be copied. * @param {Array.} exclude - The names of properties to exclude from copying. */ -function combine(primary, secondary, target, exclude) { +function copyMostProperties(primary, secondary, target, exclude) { const primaryProperties = _.difference(Object.getOwnPropertyNames(primary), exclude); const secondaryProperties = _.difference(Object.getOwnPropertyNames(secondary), exclude.concat(primaryProperties)); @@ -229,8 +228,9 @@ function combine(primary, secondary, target, exclude) { } /** - * Combine specified properties from two doclets into a target doclet, using the properties of the - * primary doclet unless the properties of the secondary doclet appear to be a better fit. + * Copy specific properties from one of two doclets onto a target doclet, as long as the property + * has a non-falsy value and a length greater than 0. Prefers the primary doclet over the secondary + * doclet. * * @private * @param {module:jsdoc/doclet.Doclet} primary - The primary doclet. @@ -238,25 +238,14 @@ function combine(primary, secondary, target, exclude) { * @param {module:jsdoc/doclet.Doclet} target - The doclet to which properties will be copied. * @param {Array.} include - The names of properties to copy. */ -function combineWithLogic(primary, secondary, target, include) { +function copySpecificProperties(primary, secondary, target, include) { include.forEach(property => { - let shouldUsePrimary = false; - - if ({}.hasOwnProperty.call(primary, property)) { - // use the primary property if the secondary property is missing or empty - if (!secondary[property] || !secondary[property].length) { - shouldUsePrimary = true; - } - // use the source property if it's not empty - else if (primary[property].length) { - shouldUsePrimary = true; - } - } - - if (shouldUsePrimary) { + if ({}.hasOwnProperty.call(primary, property) && primary[property] && + primary[property].length) { target[property] = jsdoc.util.doop(primary[property]); } - else if ({}.hasOwnProperty.call(secondary, property)) { + else if ({}.hasOwnProperty.call(secondary, property) && secondary[property] && + secondary[property].length) { target[property] = jsdoc.util.doop(secondary[property]); } }); @@ -548,16 +537,26 @@ exports.Doclet = Doclet; * @param {module:jsdoc/doclet.Doclet} primary - The doclet whose properties will be used. * @param {module:jsdoc/doclet.Doclet} secondary - The doclet to use as a fallback for properties * that the primary doclet does not have. + * @returns {module:jsdoc/doclet.Doclet} A new doclet that combines the primary and secondary + * doclets. */ exports.combine = (primary, secondary) => { - const specialCase = [ + const copyMostPropertiesExclude = [ + 'params', + 'properties', + 'undocumented' + ]; + const copySpecificPropertiesInclude = [ 'params', 'properties' ]; const target = new Doclet(''); - combine(primary, secondary, target, specialCase); - combineWithLogic(primary, secondary, target, specialCase); + // First, copy most properties to the target doclet. + copyMostProperties(primary, secondary, target, copyMostPropertiesExclude); + // Then copy a few specific properties to the target doclet, as long as they're not falsy and + // have a length greater than 0. + copySpecificProperties(primary, secondary, target, copySpecificPropertiesInclude); return target; }; diff --git a/package-lock.json b/package-lock.json index 94d6fc65..5936dda4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1313,7 +1313,8 @@ "esprima": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/esprima/-/esprima-4.0.1.tgz", - "integrity": "sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==" + "integrity": "sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==", + "dev": true }, "esquery": { "version": "1.0.1", @@ -1743,7 +1744,8 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "aproba": { "version": "1.2.0", @@ -1764,12 +1766,14 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -1784,17 +1788,20 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -1911,7 +1918,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -1923,6 +1931,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -1937,6 +1946,7 @@ "version": "3.0.4", "bundled": true, "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -1944,12 +1954,14 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -1968,6 +1980,7 @@ "version": "0.5.1", "bundled": true, "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -2048,7 +2061,8 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -2060,6 +2074,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -2145,7 +2160,8 @@ "safe-buffer": { "version": "5.1.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "safer-buffer": { "version": "2.1.2", @@ -2181,6 +2197,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -2200,6 +2217,7 @@ "version": "3.0.1", "bundled": true, "dev": true, + "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -2243,12 +2261,14 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "yallist": { "version": "3.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true } } }, @@ -2829,7 +2849,7 @@ "is-plain-object": { "version": "2.0.4", "resolved": "https://registry.npmjs.org/is-plain-object/-/is-plain-object-2.0.4.tgz", - "integrity": "sha1-LBY7P6+xtgbZ0Xko8FwqHDjgdnc=", + "integrity": "sha512-h5PpgXkWitc38BBMYawTYMWJHFZJVnBquFE57xFpjB8pJFiF6gZ+bU+WyI/yqXiFR5mdLsgYNaPe8uao6Uv9Og==", "dev": true, "requires": { "isobject": "^3.0.1" @@ -3029,6 +3049,7 @@ "version": "3.13.1", "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.13.1.tgz", "integrity": "sha512-YfbcO7jXDdyj0DGxYVSlSeQNHbD7XPWvrVWeVUujrQEoZzWJIRrCPoyk6kL6IAjAG2IolMK4T0hNUe0HOUs5Jw==", + "dev": true, "requires": { "argparse": "^1.0.7", "esprima": "^4.0.0" diff --git a/test/specs/jsdoc/doclet.js b/test/specs/jsdoc/doclet.js index 4aa885cc..4a4ef791 100644 --- a/test/specs/jsdoc/doclet.js +++ b/test/specs/jsdoc/doclet.js @@ -48,10 +48,19 @@ describe('jsdoc/doclet', () => { }); }); + // TODO(hegemonic): More tests. describe('combine', () => { - it('should override most properties of the secondary doclet', () => { - const primaryDoclet = new Doclet('/** New and improved!\n@version 2.0.0 */'); - const secondaryDoclet = new Doclet('/** Hello!\n@version 1.0.0 */'); + it('overrides most properties of the secondary doclet', () => { + const primaryDoclet = new Doclet(` + /** + * New and improved! + * @version 2.0.0 + */`); + const secondaryDoclet = new Doclet(` + /** + * Hello! + * @version 1.0.0 + */`); const newDoclet = jsdoc.doclet.combine(primaryDoclet, secondaryDoclet); Object.getOwnPropertyNames(newDoclet).forEach(property => { @@ -59,29 +68,65 @@ describe('jsdoc/doclet', () => { }); }); - it('should add properties that are missing from the secondary doclet', () => { - const primaryDoclet = new Doclet('/** Hello!\n@version 2.0.0 */'); - const secondaryDoclet = new Doclet('/** Hello! */'); + it('adds properties that are missing from the secondary doclet', () => { + const primaryDoclet = new Doclet(` + /** + * Hello! + * @version 2.0.0 + */`); + const secondaryDoclet = new Doclet(` + /** + * Hello! + */`); const newDoclet = jsdoc.doclet.combine(primaryDoclet, secondaryDoclet); expect(newDoclet.version).toBe('2.0.0'); }); + it('ignores the property `undocumented`', () => { + const primaryDoclet = new Doclet('/** Hello! */'); + const secondaryDoclet = new Doclet('/** Hello again! */'); + let newDoclet; + + primaryDoclet.undocumented = true; + secondaryDoclet.undocumented = true; + + newDoclet = jsdoc.doclet.combine(primaryDoclet, secondaryDoclet); + + expect(newDoclet.undocumented).not.toBeDefined(); + }); + describe('params and properties', () => { const properties = [ 'params', 'properties' ]; - it('should use the secondary doclet\'s params and properties if the primary doclet ' + - 'had none', () => { + it('uses the primary doclet\'s params and properties if present', () => { + const primaryDoclet = new Doclet(` + /** + * @param {number} baz - The baz. + * @property {string} qux - The qux. + */`); + const secondaryDoclet = new Doclet(` + /** + * @param {string} foo - The foo. + * @property {number} bar - The bar. + */`); + const newDoclet = jsdoc.doclet.combine(primaryDoclet, secondaryDoclet); + + properties.forEach(property => { + expect(newDoclet[property]).toEqual(primaryDoclet[property]); + }); + }); + + it('uses the secondary doclet\'s params and properties if necessary', () => { const primaryDoclet = new Doclet('/** Hello! */'); - const secondaryComment = [ - '/**', - ' * @param {string} foo - The foo.', - ' * @property {number} bar - The bar.', - ' */' - ].join('\n'); + const secondaryComment = ` + /** + * @param {string} foo - The foo. + * @property {number} bar - The bar. + */`; const secondaryDoclet = new Doclet(secondaryComment); const newDoclet = jsdoc.doclet.combine(primaryDoclet, secondaryDoclet); @@ -89,29 +134,6 @@ describe('jsdoc/doclet', () => { expect(newDoclet[property]).toEqual(secondaryDoclet[property]); }); }); - - it('should use the primary doclet\'s params and properties if the primary doclet has ' + - 'some', () => { - const primaryComment = [ - '/**', - ' * @param {number} baz - The baz.', - ' * @property {string} qux - The qux.', - ' */' - ].join('\n'); - const primaryDoclet = new Doclet(primaryComment); - const secondaryComment = [ - '/**', - ' * @param {string} foo - The foo.', - ' * @property {number} bar - The bar.', - ' */' - ].join('\n'); - const secondaryDoclet = new Doclet(secondaryComment); - const newDoclet = jsdoc.doclet.combine(primaryDoclet, secondaryDoclet); - - properties.forEach(property => { - expect(newDoclet[property]).toEqual(primaryDoclet[property]); - }); - }); }); }); });