Skip to content

Commit

Permalink
fix: for "typescript" mode, require object shorthand or index signatu…
Browse files Browse the repository at this point in the history
…res; fixes gajus#1001

BREAKING CHANGE:

For typescript mode, one must use object shorthand or index signatures, e.g., `{[key: string]: number}` or `{a: string, b: number}`
  • Loading branch information
brettz9 committed Apr 10, 2023
1 parent bd37392 commit 33b0251
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 26 deletions.
13 changes: 10 additions & 3 deletions .README/rules/check-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ object.

However, `Object.create(null)` objects are not `instanceof Object`, however, so
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`
for these objects. Also, nowadays, TypeScript also [discourages](https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#:~:text=%E2%9D%8C%20Don't%20ever%20use,used%20appropriately%20in%20JavaScript%20code.)
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
Expand All @@ -112,8 +113,14 @@ adhere to that which [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 requiring this
TypeScript approach by default (if you set `object` type `preferredTypes` in
TypeScript mode, the defaults will not apply).
TypeScript approach by default in non-"typescript" mode (if you set
`object` type `preferredTypes` in TypeScript mode, the defaults will
not apply).

However, for "typescript" mode, a still better choice exists—using index signatures such as `{[key: string]: string}` or using a more precise
shorthand object syntax (e.g., `{a: string, b: number}`). This is superior
for TypeScript because the likes of `Object<string, number>` is not useable
in native TypeScript syntax, even if it is allowed within JSDoc.

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
Expand Down
19 changes: 13 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5262,7 +5262,8 @@ object.

However, `Object.create(null)` objects are not `instanceof Object`, however, so
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`
for these objects. Also, nowadays, TypeScript also [discourages](https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#:~:text=%E2%9D%8C%20Don't%20ever%20use,used%20appropriately%20in%20JavaScript%20code.)
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
Expand All @@ -5271,8 +5272,14 @@ adhere to that which [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 requiring this
TypeScript approach by default (if you set `object` type `preferredTypes` in
TypeScript mode, the defaults will not apply).
TypeScript approach by default in non-"typescript" mode (if you set
`object` type `preferredTypes` in TypeScript mode, the defaults will
not apply).

However, for "typescript" mode, a still better choice exists—using index signatures such as `{[key: string]: string}` or using a more precise
shorthand object syntax (e.g., `{a: string, b: number}`). This is superior
for TypeScript because the likes of `Object<string, number>` is not useable
in native TypeScript syntax, even if it is allowed within JSDoc.

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
Expand Down Expand Up @@ -5952,7 +5959,7 @@ function quux (foo) {

}
// Settings: {"jsdoc":{"mode":"typescript"}}
// Message: Invalid JSDoc @param "foo" type "object"; prefer: "Object<>".
// Message: Use object shorthand or index signatures instead of `object`, e.g., `{[key: string]: string}`

/**
*
Expand Down Expand Up @@ -6234,7 +6241,7 @@ function b () {}
function a () {}

/**
* @typedef {Object<string>} foo
* @typedef {{[key: string]: number}} foo
*/
function b () {}
// Settings: {"jsdoc":{"mode":"typescript"}}
Expand Down Expand Up @@ -6263,7 +6270,7 @@ function quux (foo) {
}

/**
* @param {Object<string>} foo
* @param {{[key: string]: number}} foo
*/
function quux (foo) {

Expand Down
31 changes: 25 additions & 6 deletions src/rules/checkTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,32 @@ export default iterateJsdoc(({
'Object.<>' in preferredTypesOriginal ||
'object<>' in preferredTypesOriginal);

const preferredTypes = {
...injectObjectPreferredTypes ? {
const message = 'Use object shorthand or index signatures instead of ' +
'`object`, e.g., `{[key: string]: string}`';
const info = {
message,
replacement: false,
};

const typeToInject = mode === 'typescript' ?
{
Object: 'object',
'object.<>': info,
'Object.<>': info,
'object<>': info,
'Object<>': info,
} :
{
Object: 'object',
'object.<>': 'Object<>',
'Object.<>': 'Object<>',
'object<>': 'Object<>',
} : {},
};

const preferredTypes = {
...injectObjectPreferredTypes ?
typeToInject :
{},
...preferredTypesOriginal,
};

Expand Down Expand Up @@ -364,7 +383,7 @@ export default iterateJsdoc(({
for (const [
badType,
preferredType = '',
message,
msg,
] of invalidTypes) {
const tagValue = jsdocTag.name ? ` "${jsdocTag.name}"` : '';
if (exemptTagContexts.some(({
Expand All @@ -378,13 +397,13 @@ export default iterateJsdoc(({
}

report(
message ||
msg ||
`Invalid JSDoc @${tagName}${tagValue} type "${badType}"` +
(preferredType ? '; ' : '.') +
(preferredType ? `prefer: ${JSON.stringify(preferredType)}.` : ''),
preferredType ? fix : null,
jsdocTag,
message ? {
msg ? {
tagName,
tagValue,
} : null,
Expand Down
14 changes: 3 additions & 11 deletions test/rules/assertions/checkTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -2339,17 +2339,9 @@ export default {
errors: [
{
line: 3,
message: 'Invalid JSDoc @param "foo" type "object"; prefer: "Object<>".',
message: 'Use object shorthand or index signatures instead of `object`, e.g., `{[key: string]: string}`',
},
],
output: `
/**
* @param {Object<string>} foo
*/
function quux (foo) {
}
`,
settings: {
jsdoc: {
mode: 'typescript',
Expand Down Expand Up @@ -2944,7 +2936,7 @@ export default {
function a () {}
/**
* @typedef {Object<string>} foo
* @typedef {{[key: string]: number}} foo
*/
function b () {}
`,
Expand Down Expand Up @@ -3020,7 +3012,7 @@ export default {
{
code: `
/**
* @param {Object<string>} foo
* @param {{[key: string]: number}} foo
*/
function quux (foo) {
Expand Down

0 comments on commit 33b0251

Please sign in to comment.