Skip to content

Commit

Permalink
fix(check-types): stop regression to unify *all* Object types inclu…
Browse files Browse the repository at this point in the history 
…ding parent objects unless there is `unifyParentAndChildTypeChecks` config (should only unify with arrays); fixes gajus#800

Also updates devDeps.
  • Loading branch information
@brettz9
brettz9 committed Feb 3, 2022
1 parent 5530e07 commit 7d082a2
Show file tree
Hide file tree
Showing 5 changed files with 281 additions and 27 deletions.
49 changes: 41 additions & 8 deletions .README/rules/check-types.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, number>`)
Array
Function
Date
Expand Down Expand Up @@ -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.
Expand All @@ -95,17 +95,50 @@ 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.<keyType, valueType>` or `Object<keyType, valueType`, perhaps to
adhere to what [JSDoc documents](https://jsdoc.app/tags-type.html).

So, for optimal compatibility with TypeScript (especially since TypeScript
tools can be used on plain JavaScript with JSDoc), we are now enforcing this
TypeScript approach as the default (without the dot) as well as disallowing
`object.<>` or `object<>`, styles which TypeScript doesn't support in favor
of `Object<>`, and disallowing plain `Object`—which [it discourages](https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#general-types)
([for](https://www.typescriptlang.org/docs/handbook/typescript-in-5-minutes-func.html#unions) [reasons](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-2.html#object-type)
[similar](https://www.typescriptlang.org/docs/handbook/2/functions.html#object)
to ours here)—in favor of `object`. (You might wish to use `preferredTypes` to
prevent `Object<>` too, whether for simplicity and/or because of a general
preference of the object shorthand TypeScript allows (e.g., `{prop: number}`).)
Although earlier versions of TypeScript only worked with the dotted `Object.<>`
form, and although the TypeScript docs currently use this on its [JSDoc page](https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#type)
as did [JSDoc](https://jsdoc.app/tags-type.html),
the dot-less form has nevertheless been supported for some time in both
environments, and seems to be favored by the community, so we are enforcing
that now.

"preferredTypes": {
// Use 'object' in typescript mode, see TypeScript's Do's and Dont's
"Object": "object",
"object.<>": "Object<>", // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
"Object.<>": "Object<>",
"object<>": "Object<>", // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
},


Basically, for primitives, we want to define the type as a primitive, because
that's what we use in 99.9% of cases. For everything else, we use the type
rather than the primitive. Otherwise it would all just be `{object}`.
rather than the primitive. Otherwise it would all just be `{object}` (with the
additional exception of the special case of `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
Expand Down
85 changes: 77 additions & 8 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, number>`)
Array
Function
Date
Expand Down Expand Up @@ -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.
Expand All @@ -4812,17 +4812,50 @@ 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.<keyType, valueType>` or `Object<keyType, valueType`, perhaps to
adhere to what [JSDoc documents](https://jsdoc.app/tags-type.html).

So, for optimal compatibility with TypeScript (especially since TypeScript
tools can be used on plain JavaScript with JSDoc), we are now enforcing this
TypeScript approach as the default (without the dot) as well as disallowing
`object.<>` or `object<>`, styles which TypeScript doesn't support in favor
of `Object<>`, and disallowing plain `Object`—which [it discourages](https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#general-types)
([for](https://www.typescriptlang.org/docs/handbook/typescript-in-5-minutes-func.html#unions) [reasons](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-2.html#object-type)
[similar](https://www.typescriptlang.org/docs/handbook/2/functions.html#object)
to ours here)—in favor of `object`. (You might wish to use `preferredTypes` to
prevent `Object<>` too, whether for simplicity and/or because of a general
preference of the object shorthand TypeScript allows (e.g., `{prop: number}`).)
Although earlier versions of TypeScript only worked with the dotted `Object.<>`
form, and although the TypeScript docs currently use this on its [JSDoc page](https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#type)
as did [JSDoc](https://jsdoc.app/tags-type.html),
the dot-less form has nevertheless been supported for some time in both
environments, and seems to be favored by the community, so we are enforcing
that now.

"preferredTypes": {
// Use 'object' in typescript mode, see TypeScript's Do's and Dont's
"Object": "object",
"object.<>": "Object<>", // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
"Object.<>": "Object<>",
"object<>": "Object<>", // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
},


Basically, for primitives, we want to define the type as a primitive, because
that's what we use in 99.9% of cases. For everything else, we use the type
rather than the primitive. Otherwise it would all just be `{object}`.
rather than the primitive. Otherwise it would all just be `{object}` (with the
additional exception of the special case of `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
Expand Down Expand Up @@ -5456,6 +5489,30 @@ function quux (foo) {
}
// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"object.<>":"Object"}}}
// Message: Invalid JSDoc @param "foo" type "object"; prefer: "Object".

/**
* @param {object.<string, number>} 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.<string, number>} 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<string, number>} 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:
Expand Down Expand Up @@ -5751,6 +5808,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<string, number>} foo
*/
function a () {}
// Settings: {"jsdoc":{"mode":"typescript","preferredTypes":{"Object":"object","object.<>":"Object<>","object<>":"Object<>"}}}
````


Expand Down
16 changes: 8 additions & 8 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,28 @@
},
"description": "JSDoc linting rules for ESLint.",
"devDependencies": {
"@babel/cli": "^7.16.8",
"@babel/core": "^7.16.12",
"@babel/eslint-parser": "^7.16.5",
"@babel/cli": "^7.17.0",
"@babel/core": "^7.17.0",
"@babel/eslint-parser": "^7.17.0",
"@babel/node": "^7.16.8",
"@babel/plugin-syntax-class-properties": "^7.12.13",
"@babel/plugin-transform-flow-strip-types": "^7.16.7",
"@babel/preset-env": "^7.16.11",
"@babel/register": "^7.16.9",
"@babel/register": "^7.17.0",
"@hkdobrev/run-if-changed": "^0.3.1",
"@typescript-eslint/parser": "^5.10.1",
"@typescript-eslint/parser": "^5.10.2",
"babel-plugin-add-module-exports": "^1.0.4",
"babel-plugin-istanbul": "^6.1.1",
"camelcase": "^6.3.0",
"chai": "^4.3.5",
"chai": "^4.3.6",
"cross-env": "^7.0.3",
"decamelize": "^5.0.1",
"eslint": "^8.7.0",
"eslint": "^8.8.0",
"eslint-config-canonical": "^33.0.1",
"gitdown": "^3.1.4",
"glob": "^7.2.0",
"husky": "^7.0.4",
"lint-staged": "^12.3.1",
"lint-staged": "^12.3.3",
"lodash.defaultsdeep": "^4.6.1",
"mocha": "^9.2.0",
"nyc": "^15.1.0",
Expand Down
23 changes: 20 additions & 3 deletions src/rules/checkTypes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 7d082a2

Please sign in to comment.