From e438ed3106ed2bac9140e780b30bccda8ddc6714 Mon Sep 17 00:00:00 2001 From: Michal Date: Wed, 1 Jul 2020 16:56:31 -0500 Subject: [PATCH 1/5] fix: Object destructuring in function parameters if key is string | number --- src/jsdocUtils.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/jsdocUtils.js b/src/jsdocUtils.js index 2aa365591..254865c28 100644 --- a/src/jsdocUtils.js +++ b/src/jsdocUtils.js @@ -133,6 +133,11 @@ const getFunctionParameterNames = (functionNode : Object) : Array => { if (param.key.type === 'Identifier') { return param.key.name; } + + // The key of an object could also be a string or number + if (param.key.type === 'Literal') { + return param.key.value; + } } if (param.type === 'ArrayPattern' || param.left?.type === 'ArrayPattern') { From 582bc9db9d03322aa74fb10c4a20726d56437602 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Wed, 1 Jul 2020 18:15:10 -0500 Subject: [PATCH 2/5] test: Add a test case for destructuring function params if key is string --- src/jsdocUtils.js | 3 ++- test/rules/assertions/checkParamNames.js | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/jsdocUtils.js b/src/jsdocUtils.js index 254865c28..43dd2fcf0 100644 --- a/src/jsdocUtils.js +++ b/src/jsdocUtils.js @@ -133,8 +133,9 @@ const getFunctionParameterNames = (functionNode : Object) : Array => { if (param.key.type === 'Identifier') { return param.key.name; } - + // The key of an object could also be a string or number + /* istanbul ignore else */ if (param.key.type === 'Literal') { return param.key.value; } diff --git a/test/rules/assertions/checkParamNames.js b/test/rules/assertions/checkParamNames.js index 9a599df32..4efe4b247 100644 --- a/test/rules/assertions/checkParamNames.js +++ b/test/rules/assertions/checkParamNames.js @@ -925,6 +925,18 @@ export default { } `, }, + { + code: ` + /** + * @param foo + * @param foo.a + * @param foo.b + */ + function quux ({"a": A, b}) { + + } + `, + }, { code: ` /** From 60bf251f22a29f1c67a3ab372dbf4b306b64c858 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Sat, 4 Jul 2020 13:36:46 -0500 Subject: [PATCH 3/5] fix: Add test case with a hyphen & update readme --- README.md | 18 ++++++ test/rules/assertions/checkParamNames.js | 77 +++++++++++++++--------- 2 files changed, 65 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 257481950..ba37f4d53 100644 --- a/README.md +++ b/README.md @@ -2092,6 +2092,24 @@ function quux ({a, b}) { } +/** + * @param foo + * @param foo.a + * @param foo.b + */ +function quux ({"a": A, b}) { + +} + +/** + * @param foo + * @param foo.a-b + * @param foo.b + */ +function quux ({"a-b": A, b}) { + +} + /** * @param foo * @param foo.bar diff --git a/test/rules/assertions/checkParamNames.js b/test/rules/assertions/checkParamNames.js index 4efe4b247..50229cf86 100644 --- a/test/rules/assertions/checkParamNames.js +++ b/test/rules/assertions/checkParamNames.js @@ -40,7 +40,7 @@ export default { settings: { jsdoc: { tagNamePreference: { - param: 'arg', + param: "arg", }, }, }, @@ -73,7 +73,8 @@ export default { errors: [ { line: 3, - message: '@param path declaration ("Foo.Bar") appears before any real parameter.', + message: + '@param path declaration ("Foo.Bar") appears before any real parameter.', }, ], }, @@ -90,7 +91,8 @@ export default { errors: [ { line: 4, - message: '@param path declaration ("Foo.Bar") root node name ("Foo") does not match previous real parameter name ("foo").', + message: + '@param path declaration ("Foo.Bar") root node name ("Foo") does not match previous real parameter name ("foo").', }, ], }, @@ -108,7 +110,8 @@ export default { errors: [ { line: 4, - message: '@param path declaration ("employees[].name") appears before any real parameter.', + message: + '@param path declaration ("employees[].name") appears before any real parameter.', }, ], }, @@ -175,7 +178,8 @@ export default { errors: [ { line: 4, - message: '@param "bar" does not match an existing function parameter.', + message: + '@param "bar" does not match an existing function parameter.', }, ], }, @@ -540,9 +544,9 @@ export default { message: 'Expected @param names to be "property". Got "prop".', }, ], - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), parserOptions: { - sourceType: 'module', + sourceType: "module", }, }, { @@ -561,9 +565,9 @@ export default { message: 'Missing @param "prop.bar"', }, ], - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), parserOptions: { - sourceType: 'module', + sourceType: "module", }, }, { @@ -599,9 +603,9 @@ export default { message: '@param "prop.bar" does not exist on prop', }, ], - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), parserOptions: { - sourceType: 'module', + sourceType: "module", }, }, { @@ -621,9 +625,9 @@ export default { message: '@param "options.bar" does not exist on options', }, ], - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), parserOptions: { - sourceType: 'module', + sourceType: "module", }, }, { @@ -638,7 +642,7 @@ export default { errors: [ { line: 3, - message: 'Unexpected tag `@param`', + message: "Unexpected tag `@param`", }, ], settings: { @@ -661,7 +665,8 @@ export default { errors: [ { line: 4, - message: 'Expected @param names to be "error, cde". Got "error, code".', + message: + 'Expected @param names to be "error, cde". Got "error, code".', }, ], }, @@ -762,7 +767,7 @@ export default { ], options: [ { - checkTypesPattern: 'SVGRect', + checkTypesPattern: "SVGRect", }, ], }, @@ -815,7 +820,7 @@ export default { checkRestProperty: true, }, ], - parser: require.resolve('babel-eslint'), + parser: require.resolve("babel-eslint"), }, { code: ` @@ -845,7 +850,7 @@ export default { message: '@param "options.four" does not exist on options', }, ], - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), }, ], valid: [ @@ -937,6 +942,18 @@ export default { } `, }, + { + code: ` + /** + * @param foo + * @param foo.a-b + * @param foo.b + */ + function quux ({"a-b": A, b}) { + + } + `, + }, { code: ` /** @@ -972,9 +989,9 @@ export default { constructor(private property: string) {} } `, - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), parserOptions: { - sourceType: 'module', + sourceType: "module", }, }, { @@ -988,9 +1005,9 @@ export default { constructor(options: { foo: string, bar: string }) {} } `, - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), parserOptions: { - sourceType: 'module', + sourceType: "module", }, }, { @@ -1004,9 +1021,9 @@ export default { constructor({ foo, bar }: { foo: string, bar: string }) {} } `, - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), parserOptions: { - sourceType: 'module', + sourceType: "module", }, }, { @@ -1020,9 +1037,9 @@ export default { constructor({ foo, bar }: { foo: string, bar: string }) {} } `, - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), parserOptions: { - sourceType: 'module', + sourceType: "module", }, }, { @@ -1146,7 +1163,7 @@ export default { `, options: [ { - checkTypesPattern: 'SVGRect', + checkTypesPattern: "SVGRect", }, ], }, @@ -1162,7 +1179,7 @@ export default { } } `, - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), }, { code: ` @@ -1177,7 +1194,7 @@ export default { input; } `, - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), }, { code: ` @@ -1192,7 +1209,7 @@ export default { } } `, - parser: require.resolve('@typescript-eslint/parser'), + parser: require.resolve("@typescript-eslint/parser"), }, ], }; From b13f099d3b3bb7a71432955641f4e31edfc6bf6b Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Thu, 9 Jul 2020 13:39:35 +0800 Subject: [PATCH 4/5] - Revert quote changes --- test/rules/assertions/checkParamNames.js | 50 ++++++++++++------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/test/rules/assertions/checkParamNames.js b/test/rules/assertions/checkParamNames.js index 50229cf86..4f41802bd 100644 --- a/test/rules/assertions/checkParamNames.js +++ b/test/rules/assertions/checkParamNames.js @@ -40,7 +40,7 @@ export default { settings: { jsdoc: { tagNamePreference: { - param: "arg", + param: 'arg', }, }, }, @@ -544,9 +544,9 @@ export default { message: 'Expected @param names to be "property". Got "prop".', }, ], - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { - sourceType: "module", + sourceType: 'module', }, }, { @@ -565,9 +565,9 @@ export default { message: 'Missing @param "prop.bar"', }, ], - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { - sourceType: "module", + sourceType: 'module', }, }, { @@ -603,9 +603,9 @@ export default { message: '@param "prop.bar" does not exist on prop', }, ], - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { - sourceType: "module", + sourceType: 'module', }, }, { @@ -625,9 +625,9 @@ export default { message: '@param "options.bar" does not exist on options', }, ], - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { - sourceType: "module", + sourceType: 'module', }, }, { @@ -642,7 +642,7 @@ export default { errors: [ { line: 3, - message: "Unexpected tag `@param`", + message: 'Unexpected tag `@param`', }, ], settings: { @@ -767,7 +767,7 @@ export default { ], options: [ { - checkTypesPattern: "SVGRect", + checkTypesPattern: 'SVGRect', }, ], }, @@ -820,7 +820,7 @@ export default { checkRestProperty: true, }, ], - parser: require.resolve("babel-eslint"), + parser: require.resolve('babel-eslint'), }, { code: ` @@ -850,7 +850,7 @@ export default { message: '@param "options.four" does not exist on options', }, ], - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), }, ], valid: [ @@ -989,9 +989,9 @@ export default { constructor(private property: string) {} } `, - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { - sourceType: "module", + sourceType: 'module', }, }, { @@ -1005,9 +1005,9 @@ export default { constructor(options: { foo: string, bar: string }) {} } `, - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { - sourceType: "module", + sourceType: 'module', }, }, { @@ -1021,9 +1021,9 @@ export default { constructor({ foo, bar }: { foo: string, bar: string }) {} } `, - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { - sourceType: "module", + sourceType: 'module', }, }, { @@ -1037,9 +1037,9 @@ export default { constructor({ foo, bar }: { foo: string, bar: string }) {} } `, - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), parserOptions: { - sourceType: "module", + sourceType: 'module', }, }, { @@ -1163,7 +1163,7 @@ export default { `, options: [ { - checkTypesPattern: "SVGRect", + checkTypesPattern: 'SVGRect', }, ], }, @@ -1179,7 +1179,7 @@ export default { } } `, - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), }, { code: ` @@ -1194,7 +1194,7 @@ export default { input; } `, - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), }, { code: ` @@ -1209,7 +1209,7 @@ export default { } } `, - parser: require.resolve("@typescript-eslint/parser"), + parser: require.resolve('@typescript-eslint/parser'), }, ], }; From 95647026afcaba27d74bcf7419c8e6470e2ec84b Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Thu, 9 Jul 2020 14:30:35 +0800 Subject: [PATCH 5/5] - Ensure comparing without quotes (since either param or function may include (for identifiers, numerics) or omit) Also return the raw value in `getFunctionParameterNames` so as to preserve original quotes when reporting. --- README.md | 11 ++++++++++- src/jsdocUtils.js | 4 +++- src/rules/checkParamNames.js | 25 ++++++++++++++++++++++-- test/rules/assertions/checkParamNames.js | 14 ++++++++++++- 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index ba37f4d53..38b4a54a0 100644 --- a/README.md +++ b/README.md @@ -2103,7 +2103,16 @@ function quux ({"a": A, b}) { /** * @param foo - * @param foo.a-b + * @param foo."a" + * @param foo.b + */ +function quux ({a: A, b}) { + +} + +/** + * @param foo + * @param foo."a-b" * @param foo.b */ function quux ({"a-b": A, b}) { diff --git a/src/jsdocUtils.js b/src/jsdocUtils.js index 43dd2fcf0..61b5dac67 100644 --- a/src/jsdocUtils.js +++ b/src/jsdocUtils.js @@ -137,7 +137,9 @@ const getFunctionParameterNames = (functionNode : Object) : Array => { // The key of an object could also be a string or number /* istanbul ignore else */ if (param.key.type === 'Literal') { - return param.key.value; + return param.key.raw || + // istanbul ignore next -- `raw` may not be present in all parsers + param.key.value; } } diff --git a/src/rules/checkParamNames.js b/src/rules/checkParamNames.js index d0aa9aa5c..0189aa5cc 100644 --- a/src/rules/checkParamNames.js +++ b/src/rules/checkParamNames.js @@ -1,5 +1,25 @@ import iterateJsdoc from '../iterateJsdoc'; +/** + * Since path segments may be unquoted (if matching a reserved word, + * identifier or numeric literal) or single or double quoted, in either + * the `@param` or in source, we need to strip the quotes to give a fair + * comparison. + * + * @param {string} str + * @returns {string} + */ +const dropPathSegmentQuotes = (str) => { + return str.replace(/\.(['"])(.*)\1/gu, '.$2'); +}; + +const comparePaths = (name) => { + return (otherPathName) => { + return otherPathName === name || + dropPathSegmentQuotes(otherPathName) === dropPathSegmentQuotes(name); + }; +}; + const validateParameterNames = ( targetTagName : string, allowExtraTrailingParamDocs: boolean, @@ -76,8 +96,9 @@ const validateParameterNames = ( }); const missingProperties = []; + expectedNames.forEach((name, idx) => { - if (!actualNames.includes(name)) { + if (!actualNames.some(comparePaths(name))) { if (!checkRestProperty && rests[idx]) { return; } @@ -89,7 +110,7 @@ const validateParameterNames = ( if (!hasPropertyRest || checkRestProperty) { actualNames.forEach((name, idx) => { const match = name.startsWith(tag.name.trim() + '.'); - if (match && !expectedNames.includes(name) && name !== tag.name) { + if (match && !expectedNames.some(comparePaths(name)) && name !== tag.name) { extraProperties.push([name, paramTags[idx][1]]); } }); diff --git a/test/rules/assertions/checkParamNames.js b/test/rules/assertions/checkParamNames.js index 4f41802bd..f70caaa96 100644 --- a/test/rules/assertions/checkParamNames.js +++ b/test/rules/assertions/checkParamNames.js @@ -946,7 +946,19 @@ export default { code: ` /** * @param foo - * @param foo.a-b + * @param foo."a" + * @param foo.b + */ + function quux ({a: A, b}) { + + } + `, + }, + { + code: ` + /** + * @param foo + * @param foo."a-b" * @param foo.b */ function quux ({"a-b": A, b}) {