Skip to content

Commit 9d0a75d

Browse files
brettz9brettz9
committedFeb 3, 2022
fix(check-types): prevent parent objects from being reported in "typescript" mode even with generic preferredTypes match (unless there is unifyParentAndChildTypeChecks config); fixes #800
·
v50.6.9v37.7.1
1 parent b67e14a commit 9d0a75d

File tree

4 files changed

+229
-19
lines changed

4 files changed

+229
-19
lines changed
 

‎.README/rules/check-types.md

+19-8
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ number
1313
bigint
1414
string
1515
symbol
16-
object
16+
object (For TypeScript's sake, however, using `Object` when specifying child types on it like `Object<string, number>`)
1717
Array
1818
Function
1919
Date
@@ -72,7 +72,7 @@ the `valid-types` rule to report parsing errors.
7272

7373
Why are `boolean`, `number` and `string` exempt from starting with a capital
7474
letter? Let's take `string` as an example. In Javascript, everything is an
75-
object. The string Object has prototypes for string functions such as
75+
object. The `String` object has prototypes for string functions such as
7676
`.toUpperCase()`.
7777

7878
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}
9595
new String('lard') === 'lard' // false
9696
```
9797

98-
To make things more confusing, there are also object literals and object
99-
Objects. But object literals are still static Objects and object Objects are
100-
instantiated Objects. So an object primitive is still an object Object.
98+
To make things more confusing, there are also object literals (like `{}`) and
99+
`Object` objects. But object literals are still static `Object`s and `Object`
100+
objects are instantiated objects. So an object primitive is still an `Object`
101+
object.
101102

102103
However, `Object.create(null)` objects are not `instanceof Object`, however, so
103-
in the case of this Object we lower-case to indicate possible support for
104-
these objects.
104+
in the case of such a plain object we lower-case to indicate possible support
105+
for these objects. Also, nowadays, TypeScript also discourages use of `Object`
106+
as a lone type. However, one additional complexity is that TypeScript allows and
107+
actually [currently requires](https://github.com/microsoft/TypeScript/issues/20555)
108+
`Object` (with the initial upper-case) if used in the syntax
109+
`Object.<keyType, valueType>` or `Object<keyType, valueType`, perhaps to
110+
adhere to what [JSDoc documents](https://jsdoc.app/tags-type.html).
111+
112+
So, for optimal compatibility with TypeScript (especially since TypeScript
113+
tools can be used on plain JavaScript with JSDoc), we are now allowing this
114+
TypeScript approach by default.
105115

106116
Basically, for primitives, we want to define the type as a primitive, because
107117
that's what we use in 99.9% of cases. For everything else, we use the type
108-
rather than the primitive. Otherwise it would all just be `{object}`.
118+
rather than the primitive. Otherwise it would all just be `{object}` (with the
119+
additional exception of the special case of `Object.<>` just mentioned).
109120

110121
In short: It's not about consistency, rather about the 99.9% use case. (And
111122
some functions might not even support the objects if they are checking for

‎README.md

+55-8
Original file line numberDiff line numberDiff line change
@@ -4728,7 +4728,7 @@ number
47284728
bigint
47294729
string
47304730
symbol
4731-
object
4731+
object (For TypeScript's sake, however, using `Object` when specifying child types on it like `Object<string, number>`)
47324732
Array
47334733
Function
47344734
Date
@@ -4789,7 +4789,7 @@ the `valid-types` rule to report parsing errors.
47894789

47904790
Why are `boolean`, `number` and `string` exempt from starting with a capital
47914791
letter? Let's take `string` as an example. In Javascript, everything is an
4792-
object. The string Object has prototypes for string functions such as
4792+
object. The `String` object has prototypes for string functions such as
47934793
`.toUpperCase()`.
47944794

47954795
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}
48124812
new String('lard') === 'lard' // false
48134813
```
48144814

4815-
To make things more confusing, there are also object literals and object
4816-
Objects. But object literals are still static Objects and object Objects are
4817-
instantiated Objects. So an object primitive is still an object Object.
4815+
To make things more confusing, there are also object literals (like `{}`) and
4816+
`Object` objects. But object literals are still static `Object`s and `Object`
4817+
objects are instantiated objects. So an object primitive is still an `Object`
4818+
object.
48184819

48194820
However, `Object.create(null)` objects are not `instanceof Object`, however, so
4820-
in the case of this Object we lower-case to indicate possible support for
4821-
these objects.
4821+
in the case of such a plain object we lower-case to indicate possible support
4822+
for these objects. Also, nowadays, TypeScript also discourages use of `Object`
4823+
as a lone type. However, one additional complexity is that TypeScript allows and
4824+
actually [currently requires](https://github.com/microsoft/TypeScript/issues/20555)
4825+
`Object` (with the initial upper-case) if used in the syntax
4826+
`Object.<keyType, valueType>` or `Object<keyType, valueType`, perhaps to
4827+
adhere to what [JSDoc documents](https://jsdoc.app/tags-type.html).
4828+
4829+
So, for optimal compatibility with TypeScript (especially since TypeScript
4830+
tools can be used on plain JavaScript with JSDoc), we are now allowing this
4831+
TypeScript approach by default.
48224832

48234833
Basically, for primitives, we want to define the type as a primitive, because
48244834
that's what we use in 99.9% of cases. For everything else, we use the type
4825-
rather than the primitive. Otherwise it would all just be `{object}`.
4835+
rather than the primitive. Otherwise it would all just be `{object}` (with the
4836+
additional exception of the special case of `Object.<>` just mentioned).
48264837

48274838
In short: It's not about consistency, rather about the 99.9% use case. (And
48284839
some functions might not even support the objects if they are checking for
@@ -5456,6 +5467,30 @@ function quux (foo) {
54565467
}
54575468
// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"object.<>":"Object"}}}
54585469
// Message: Invalid JSDoc @param "foo" type "object"; prefer: "Object".
5470+
5471+
/**
5472+
* @param {object.<string, number>} foo
5473+
*/
5474+
function quux (foo) {
5475+
}
5476+
// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","Object.<>":"Object<>","object<>":"Object<>"}}}
5477+
// Message: Invalid JSDoc @param "foo" type "object"; prefer: "Object<>".
5478+
5479+
/**
5480+
* @param {Object.<string, number>} foo
5481+
*/
5482+
function quux (foo) {
5483+
}
5484+
// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","Object.<>":"Object<>","object<>":"Object<>"}}}
5485+
// Message: Invalid JSDoc @param "foo" type "Object"; prefer: "Object<>".
5486+
5487+
/**
5488+
* @param {object<string, number>} foo
5489+
*/
5490+
function quux (foo) {
5491+
}
5492+
// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","Object.<>":"Object<>","object<>":"Object<>"}}}
5493+
// Message: Invalid JSDoc @param "foo" type "object"; prefer: "Object<>".
54595494
````
54605495

54615496
The following patterns are not considered problems:
@@ -5751,6 +5786,18 @@ function quux (foo) {
57515786

57525787
}
57535788
// Settings: {"jsdoc":{"mode":"typescript"}}
5789+
5790+
/**
5791+
* @typedef {object} foo
5792+
*/
5793+
function a () {}
5794+
// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","object<>":"Object<>"}}}
5795+
5796+
/**
5797+
* @typedef {Object<string, number>} foo
5798+
*/
5799+
function a () {}
5800+
// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","object<>":"Object<>"}}}
57545801
````
57555802

57565803

‎src/rules/checkTypes.js

+20-3
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ export default iterateJsdoc(({
156156

157157
const tagName = jsdocTag.tag;
158158

159+
// eslint-disable-next-line complexity -- To refactor
159160
traverse(typeAst, (node, parentNode, property) => {
160161
const {
161162
type,
@@ -221,12 +222,28 @@ export default iterateJsdoc(({
221222
]);
222223
} else if (!noDefaults && type === 'JsdocTypeName') {
223224
for (const strictNativeType of strictNativeTypes) {
224-
if (strictNativeType === 'object' && mode === 'typescript' && !preferredTypes?.[nodeName]) {
225+
if (
226+
// Todo: Avoid typescript condition if moving to default typescript
227+
strictNativeType === 'object' && mode === 'typescript' &&
228+
(
229+
// This is not set to remap with exact type match (e.g.,
230+
// `object: 'Object'`), so can ignore (including if circular)
231+
!preferredTypes?.[nodeName] ||
232+
// Although present on `preferredTypes` for remapping, this is a
233+
// parent object without a parent match (and not
234+
// `unifyParentAndChildTypeChecks`) and we don't want
235+
// `object<>` given TypeScript issue https://github.com/microsoft/TypeScript/issues/20555
236+
parentNode?.elements.length && (
237+
parentNode?.left.type === 'JsdocTypeName' &&
238+
parentNode?.left.value === 'Object'
239+
)
240+
)
241+
) {
225242
continue;
226243
}
227244

228-
if (strictNativeType.toLowerCase() === nodeName.toLowerCase() &&
229-
strictNativeType !== nodeName &&
245+
if (strictNativeType !== nodeName &&
246+
strictNativeType.toLowerCase() === nodeName.toLowerCase() &&
230247

231248
// Don't report if user has own map for a strict native type
232249
(!preferredTypes || preferredTypes?.[strictNativeType] === undefined)

‎test/rules/assertions/checkTypes.js

+135
Original file line numberDiff line numberDiff line change
@@ -2193,6 +2193,105 @@ export default {
21932193
},
21942194
},
21952195
},
2196+
{
2197+
code: `
2198+
/**
2199+
* @param {object.<string, number>} foo
2200+
*/
2201+
function quux (foo) {
2202+
}
2203+
`,
2204+
errors: [
2205+
{
2206+
line: 3,
2207+
message: 'Invalid JSDoc @param "foo" type "object"; prefer: "Object<>".',
2208+
},
2209+
],
2210+
output: `
2211+
/**
2212+
* @param {Object<string, number>} foo
2213+
*/
2214+
function quux (foo) {
2215+
}
2216+
`,
2217+
settings: {
2218+
jsdoc: {
2219+
mode: 'typescript',
2220+
preferredTypes: {
2221+
Object: 'object',
2222+
'object.<>': 'Object<>',
2223+
'Object.<>': 'Object<>',
2224+
'object<>': 'Object<>',
2225+
},
2226+
},
2227+
},
2228+
},
2229+
{
2230+
code: `
2231+
/**
2232+
* @param {Object.<string, number>} foo
2233+
*/
2234+
function quux (foo) {
2235+
}
2236+
`,
2237+
errors: [
2238+
{
2239+
line: 3,
2240+
message: 'Invalid JSDoc @param "foo" type "Object"; prefer: "Object<>".',
2241+
},
2242+
],
2243+
output: `
2244+
/**
2245+
* @param {Object<string, number>} foo
2246+
*/
2247+
function quux (foo) {
2248+
}
2249+
`,
2250+
settings: {
2251+
jsdoc: {
2252+
mode: 'typescript',
2253+
preferredTypes: {
2254+
Object: 'object',
2255+
'object.<>': 'Object<>',
2256+
'Object.<>': 'Object<>',
2257+
'object<>': 'Object<>',
2258+
},
2259+
},
2260+
},
2261+
},
2262+
{
2263+
code: `
2264+
/**
2265+
* @param {object<string, number>} foo
2266+
*/
2267+
function quux (foo) {
2268+
}
2269+
`,
2270+
errors: [
2271+
{
2272+
line: 3,
2273+
message: 'Invalid JSDoc @param "foo" type "object"; prefer: "Object<>".',
2274+
},
2275+
],
2276+
output: `
2277+
/**
2278+
* @param {Object<string, number>} foo
2279+
*/
2280+
function quux (foo) {
2281+
}
2282+
`,
2283+
settings: {
2284+
jsdoc: {
2285+
mode: 'typescript',
2286+
preferredTypes: {
2287+
Object: 'object',
2288+
'object.<>': 'Object<>',
2289+
'Object.<>': 'Object<>',
2290+
'object<>': 'Object<>',
2291+
},
2292+
},
2293+
},
2294+
},
21962295
],
21972296
valid: [
21982297
{
@@ -2804,5 +2903,41 @@ export default {
28042903
},
28052904
},
28062905
},
2906+
{
2907+
code: `
2908+
/**
2909+
* @typedef {object} foo
2910+
*/
2911+
function a () {}
2912+
`,
2913+
settings: {
2914+
jsdoc: {
2915+
mode: 'typescript',
2916+
preferredTypes: {
2917+
Object: 'object',
2918+
'object.<>': 'Object<>',
2919+
'object<>': 'Object<>',
2920+
},
2921+
},
2922+
},
2923+
},
2924+
{
2925+
code: `
2926+
/**
2927+
* @typedef {Object<string, number>} foo
2928+
*/
2929+
function a () {}
2930+
`,
2931+
settings: {
2932+
jsdoc: {
2933+
mode: 'typescript',
2934+
preferredTypes: {
2935+
Object: 'object',
2936+
'object.<>': 'Object<>',
2937+
'object<>': 'Object<>',
2938+
},
2939+
},
2940+
},
2941+
},
28072942
],
28082943
};

0 commit comments

Comments
 (0)
Please sign in to comment.