From 16e7de8435338d51e958682a13626c4c785e0207 Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Thu, 3 Feb 2022 15:24:03 +0800 Subject: [PATCH] fix(`check-types`): prevent parent objects from being reported in "typescript" mode even with generic preferredTypes match (unless there is `unifyParentAndChildTypeChecks` config); fixes #800 --- .README/rules/check-types.md | 27 ++++-- README.md | 63 +++++++++++-- src/rules/checkTypes.js | 23 ++++- test/rules/assertions/checkTypes.js | 135 ++++++++++++++++++++++++++++ 4 files changed, 229 insertions(+), 19 deletions(-) diff --git a/.README/rules/check-types.md b/.README/rules/check-types.md index 832396a4c..4e8145be1 100644 --- a/.README/rules/check-types.md +++ b/.README/rules/check-types.md @@ -13,7 +13,7 @@ number bigint string symbol -object +object (For TypeScript's sake, however, using `Object` when specifying child types on it like `Object`) Array Function Date @@ -72,7 +72,7 @@ the `valid-types` rule to report parsing errors. Why are `boolean`, `number` and `string` exempt from starting with a capital letter? Let's take `string` as an example. In Javascript, everything is an -object. The string Object has prototypes for string functions such as +object. The `String` object has prototypes for string functions such as `.toUpperCase()`. Fortunately we don't have to write `new String()` everywhere in our code. @@ -95,17 +95,28 @@ new String('lard') // String {0: "l", 1: "a", 2: "r", 3: "d", length: 4} new String('lard') === 'lard' // false ``` -To make things more confusing, there are also object literals and object -Objects. But object literals are still static Objects and object Objects are -instantiated Objects. So an object primitive is still an object Object. +To make things more confusing, there are also object literals (like `{}`) and +`Object` objects. But object literals are still static `Object`s and `Object` +objects are instantiated objects. So an object primitive is still an `Object` +object. However, `Object.create(null)` objects are not `instanceof Object`, however, so -in the case of this Object we lower-case to indicate possible support for -these objects. +in the case of such a plain object we lower-case to indicate possible support +for these objects. Also, nowadays, TypeScript also discourages use of `Object` +as a lone type. However, one additional complexity is that TypeScript allows and +actually [currently requires](https://github.com/microsoft/TypeScript/issues/20555) +`Object` (with the initial upper-case) if used in the syntax +`Object.` or `Object` just mentioned). In short: It's not about consistency, rather about the 99.9% use case. (And some functions might not even support the objects if they are checking for diff --git a/README.md b/README.md index 448301892..4fb921b61 100644 --- a/README.md +++ b/README.md @@ -4728,7 +4728,7 @@ number bigint string symbol -object +object (For TypeScript's sake, however, using `Object` when specifying child types on it like `Object`) Array Function Date @@ -4789,7 +4789,7 @@ the `valid-types` rule to report parsing errors. Why are `boolean`, `number` and `string` exempt from starting with a capital letter? Let's take `string` as an example. In Javascript, everything is an -object. The string Object has prototypes for string functions such as +object. The `String` object has prototypes for string functions such as `.toUpperCase()`. Fortunately we don't have to write `new String()` everywhere in our code. @@ -4812,17 +4812,28 @@ new String('lard') // String {0: "l", 1: "a", 2: "r", 3: "d", length: 4} new String('lard') === 'lard' // false ``` -To make things more confusing, there are also object literals and object -Objects. But object literals are still static Objects and object Objects are -instantiated Objects. So an object primitive is still an object Object. +To make things more confusing, there are also object literals (like `{}`) and +`Object` objects. But object literals are still static `Object`s and `Object` +objects are instantiated objects. So an object primitive is still an `Object` +object. However, `Object.create(null)` objects are not `instanceof Object`, however, so -in the case of this Object we lower-case to indicate possible support for -these objects. +in the case of such a plain object we lower-case to indicate possible support +for these objects. Also, nowadays, TypeScript also discourages use of `Object` +as a lone type. However, one additional complexity is that TypeScript allows and +actually [currently requires](https://github.com/microsoft/TypeScript/issues/20555) +`Object` (with the initial upper-case) if used in the syntax +`Object.` or `Object` just mentioned). In short: It's not about consistency, rather about the 99.9% use case. (And some functions might not even support the objects if they are checking for @@ -5456,6 +5467,30 @@ function quux (foo) { } // Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"object.<>":"Object"}}} // Message: Invalid JSDoc @param "foo" type "object"; prefer: "Object". + +/** + * @param {object.} foo + */ +function quux (foo) { +} +// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","Object.<>":"Object<>","object<>":"Object<>"}}} +// Message: Invalid JSDoc @param "foo" type "object"; prefer: "Object<>". + +/** + * @param {Object.} foo + */ +function quux (foo) { +} +// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","Object.<>":"Object<>","object<>":"Object<>"}}} +// Message: Invalid JSDoc @param "foo" type "Object"; prefer: "Object<>". + +/** + * @param {object} foo + */ +function quux (foo) { +} +// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","Object.<>":"Object<>","object<>":"Object<>"}}} +// Message: Invalid JSDoc @param "foo" type "object"; prefer: "Object<>". ```` The following patterns are not considered problems: @@ -5751,6 +5786,18 @@ function quux (foo) { } // Settings: {"jsdoc":{"mode":"typescript"}} + +/** + * @typedef {object} foo + */ +function a () {} +// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","object<>":"Object<>"}}} + +/** + * @typedef {Object} foo + */ +function a () {} +// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","object<>":"Object<>"}}} ```` diff --git a/src/rules/checkTypes.js b/src/rules/checkTypes.js index 96f9d0b5f..e39aa22b0 100644 --- a/src/rules/checkTypes.js +++ b/src/rules/checkTypes.js @@ -156,6 +156,7 @@ export default iterateJsdoc(({ const tagName = jsdocTag.tag; + // eslint-disable-next-line complexity -- To refactor traverse(typeAst, (node, parentNode, property) => { const { type, @@ -221,12 +222,28 @@ export default iterateJsdoc(({ ]); } else if (!noDefaults && type === 'JsdocTypeName') { for (const strictNativeType of strictNativeTypes) { - if (strictNativeType === 'object' && mode === 'typescript' && !preferredTypes?.[nodeName]) { + if ( + // Todo: Avoid typescript condition if moving to default typescript + strictNativeType === 'object' && mode === 'typescript' && + ( + // This is not set to remap with exact type match (e.g., + // `object: 'Object'`), so can ignore (including if circular) + !preferredTypes?.[nodeName] || + // Although present on `preferredTypes` for remapping, this is a + // parent object without a parent match (and not + // `unifyParentAndChildTypeChecks`) and we don't want + // `object<>` given TypeScript issue https://github.com/microsoft/TypeScript/issues/20555 + parentNode?.elements.length && ( + parentNode?.left.type === 'JsdocTypeName' && + parentNode?.left.value === 'Object' + ) + ) + ) { continue; } - if (strictNativeType.toLowerCase() === nodeName.toLowerCase() && - strictNativeType !== nodeName && + if (strictNativeType !== nodeName && + strictNativeType.toLowerCase() === nodeName.toLowerCase() && // Don't report if user has own map for a strict native type (!preferredTypes || preferredTypes?.[strictNativeType] === undefined) diff --git a/test/rules/assertions/checkTypes.js b/test/rules/assertions/checkTypes.js index 0fdeff811..c6de12d0d 100644 --- a/test/rules/assertions/checkTypes.js +++ b/test/rules/assertions/checkTypes.js @@ -2193,6 +2193,105 @@ export default { }, }, }, + { + code: ` + /** + * @param {object.} foo + */ + function quux (foo) { + } + `, + errors: [ + { + line: 3, + message: 'Invalid JSDoc @param "foo" type "object"; prefer: "Object<>".', + }, + ], + output: ` + /** + * @param {Object} foo + */ + function quux (foo) { + } + `, + settings: { + jsdoc: { + mode: 'typescript', + preferredTypes: { + Object: 'object', + 'object.<>': 'Object<>', + 'Object.<>': 'Object<>', + 'object<>': 'Object<>', + }, + }, + }, + }, + { + code: ` + /** + * @param {Object.} foo + */ + function quux (foo) { + } + `, + errors: [ + { + line: 3, + message: 'Invalid JSDoc @param "foo" type "Object"; prefer: "Object<>".', + }, + ], + output: ` + /** + * @param {Object} foo + */ + function quux (foo) { + } + `, + settings: { + jsdoc: { + mode: 'typescript', + preferredTypes: { + Object: 'object', + 'object.<>': 'Object<>', + 'Object.<>': 'Object<>', + 'object<>': 'Object<>', + }, + }, + }, + }, + { + code: ` + /** + * @param {object} foo + */ + function quux (foo) { + } + `, + errors: [ + { + line: 3, + message: 'Invalid JSDoc @param "foo" type "object"; prefer: "Object<>".', + }, + ], + output: ` + /** + * @param {Object} foo + */ + function quux (foo) { + } + `, + settings: { + jsdoc: { + mode: 'typescript', + preferredTypes: { + Object: 'object', + 'object.<>': 'Object<>', + 'Object.<>': 'Object<>', + 'object<>': 'Object<>', + }, + }, + }, + }, ], valid: [ { @@ -2804,5 +2903,41 @@ export default { }, }, }, + { + code: ` + /** + * @typedef {object} foo + */ + function a () {} + `, + settings: { + jsdoc: { + mode: 'typescript', + preferredTypes: { + Object: 'object', + 'object.<>': 'Object<>', + 'object<>': 'Object<>', + }, + }, + }, + }, + { + code: ` + /** + * @typedef {Object} foo + */ + function a () {} + `, + settings: { + jsdoc: { + mode: 'typescript', + preferredTypes: { + Object: 'object', + 'object.<>': 'Object<>', + 'object<>': 'Object<>', + }, + }, + }, + }, ], };