From a0cdea1d012f7d342990beb0ef987ea10c192187 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 21 Dec 2024 14:47:48 +0200 Subject: [PATCH 1/8] initial implementation (squashed) --- .../rules/consistent-indexed-object-style.ts | 122 ++++++++- .../eslint-plugin/src/util/isNodeEqual.ts | 6 + .../consistent-indexed-object-style.test.ts | 244 ++++++++++++++++++ 3 files changed, 358 insertions(+), 14 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts b/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts index 15ab691b38dd..2ac74354ace5 100644 --- a/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts +++ b/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts @@ -1,3 +1,4 @@ +import type { ScopeVariable } from '@typescript-eslint/scope-manager'; import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import type { ReportFixFunction } from '@typescript-eslint/utils/ts-eslint'; @@ -5,8 +6,9 @@ import { AST_NODE_TYPES, ASTUtils } from '@typescript-eslint/utils'; import { createRule, - getFixOrSuggest, + isNodeEqual, isParenthesized, + getFixOrSuggest, nullThrows, } from '../util'; @@ -44,13 +46,14 @@ export default createRule({ defaultOptions: ['record'], create(context, [mode]) { function checkMembers( - members: TSESTree.TypeElement[], node: TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeLiteral, parentId: TSESTree.Identifier | undefined, prefix: string, postfix: string, safeFix = true, ): void { + const members = getMembers(node); + if (members.length !== 1) { return; } @@ -78,16 +81,12 @@ export default createRule({ if (parentId) { const scope = context.sourceCode.getScope(parentId); const superVar = ASTUtils.findVariable(scope, parentId.name); - if (superVar) { - const isCircular = superVar.references.some( - item => - item.isTypeReference && - node.range[0] <= item.identifier.range[0] && - node.range[1] >= item.identifier.range[1], - ); - if (isCircular) { - return; - } + + if ( + superVar && + isDeeplyReferencingType(node, superVar, new Set([parentId])) + ) { + return; } } @@ -160,7 +159,6 @@ export default createRule({ } checkMembers( - node.body.body, node, node.id, `type ${node.id.name}${genericTypes} = `, @@ -251,7 +249,7 @@ export default createRule({ }, TSTypeLiteral(node): void { const parent = findParentDeclaration(node); - checkMembers(node.members, node, parent?.id, '', ''); + checkMembers(node, parent?.id, '', ''); }, }), }; @@ -269,3 +267,99 @@ function findParentDeclaration( } return undefined; } + +function getMembers( + node: TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeLiteral, +): TSESTree.TypeElement[] { + if (node.type === AST_NODE_TYPES.TSInterfaceDeclaration) { + return node.body.body; + } + + return node.members; +} + +function isDeeplyReferencingType( + node: TSESTree.Node, + superVar: ScopeVariable, + visited: Set, +): boolean { + if (visited.has(node)) { + // something on the chain is circular but it's not the reference being checked + return false; + } + + visited.add(node); + + switch (node.type) { + case AST_NODE_TYPES.TSTypeLiteral: + return node.members.some(member => + isDeeplyReferencingType(member, superVar, visited), + ); + case AST_NODE_TYPES.TSTypeAliasDeclaration: + return isDeeplyReferencingType(node.typeAnnotation, superVar, visited); + case AST_NODE_TYPES.TSIndexedAccessType: + return [node.indexType, node.objectType].some(type => + isDeeplyReferencingType(type, superVar, visited), + ); + case AST_NODE_TYPES.TSConditionalType: + return [ + node.checkType, + node.extendsType, + node.falseType, + node.trueType, + ].some(type => isDeeplyReferencingType(type, superVar, visited)); + case AST_NODE_TYPES.TSUnionType: + case AST_NODE_TYPES.TSIntersectionType: + return node.types.some(type => + isDeeplyReferencingType(type, superVar, visited), + ); + case AST_NODE_TYPES.TSInterfaceDeclaration: + return node.body.body.some(type => + isDeeplyReferencingType(type, superVar, visited), + ); + case AST_NODE_TYPES.TSTypeAnnotation: + return isDeeplyReferencingType(node.typeAnnotation, superVar, visited); + case AST_NODE_TYPES.TSIndexSignature: { + if (node.typeAnnotation) { + return isDeeplyReferencingType(node.typeAnnotation, superVar, visited); + } + break; + } + case AST_NODE_TYPES.TSTypeParameterInstantiation: { + return node.params.some(param => + isDeeplyReferencingType(param, superVar, visited), + ); + } + case AST_NODE_TYPES.TSTypeReference: { + if (isDeeplyReferencingType(node.typeName, superVar, visited)) { + return true; + } + + if ( + node.typeArguments && + isDeeplyReferencingType(node.typeArguments, superVar, visited) + ) { + return true; + } + + break; + } + case AST_NODE_TYPES.Identifier: { + // check if the identifier is a reference of the type being checked + if (superVar.references.some(ref => isNodeEqual(ref.identifier, node))) { + return true; + } + + // otherwise, follow its definition(s) + const refVar = ASTUtils.findVariable(superVar.scope, node.name); + + if (refVar) { + return refVar.defs.some(def => + isDeeplyReferencingType(def.node, superVar, visited), + ); + } + } + } + + return false; +} diff --git a/packages/eslint-plugin/src/util/isNodeEqual.ts b/packages/eslint-plugin/src/util/isNodeEqual.ts index 729b38b58888..f2e0f377911e 100644 --- a/packages/eslint-plugin/src/util/isNodeEqual.ts +++ b/packages/eslint-plugin/src/util/isNodeEqual.ts @@ -21,6 +21,12 @@ export function isNodeEqual(a: TSESTree.Node, b: TSESTree.Node): boolean { ) { return a.name === b.name; } + if ( + a.type === AST_NODE_TYPES.TSTypeReference && + b.type === AST_NODE_TYPES.TSTypeReference + ) { + return isNodeEqual(a.typeName, b.typeName); + } if ( a.type === AST_NODE_TYPES.MemberExpression && b.type === AST_NODE_TYPES.MemberExpression diff --git a/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts b/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts index 8441a720e326..fa50a2a3b88f 100644 --- a/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts @@ -49,6 +49,123 @@ interface Foo { [key: string]: Foo | string; } `, + ` +interface Foo { + [s: string]: Foo & {}; +} + `, + ` +interface Foo { + [s: string]: Foo | string; +} + `, + ` +interface Foo { + [s: string]: Foo extends T ? string : number; +} + `, + ` +interface Foo { + [s: string]: T extends Foo ? string : number; +} + `, + ` +interface Foo { + [s: string]: T extends true ? Foo : number; +} + `, + ` +interface Foo { + [s: string]: T extends true ? string : Foo; +} + `, + ` +interface Foo { + [s: string]: Foo[number]; +} + `, + ` +interface Foo { + [s: string]: {}[Foo]; +} + `, + + // circular (indirect) + ` +interface Foo1 { + [key: string]: Foo2; +} + +interface Foo2 { + [key: string]: Foo1; +} + `, + ` +interface Foo1 { + [key: string]: Foo2; +} + +interface Foo2 { + [key: string]: Foo3; +} + +interface Foo3 { + [key: string]: Foo1; +} + `, + ` +interface Foo1 { + [key: string]: Foo2; +} + +interface Foo2 { + [key: string]: Foo3; +} + +interface Foo3 { + [key: string]: Record; +} + `, + ` +type Foo1 = { + [key: string]: Foo2; +}; + +type Foo2 = { + [key: string]: Foo3; +}; + +type Foo3 = { + [key: string]: Foo1; +}; + `, + ` +interface Foo1 { + [key: string]: Foo2; +} + +type Foo2 = { + [key: string]: Foo3; +}; + +interface Foo3 { + [key: string]: Foo1; +} + `, + ` +type Foo1 = { + [key: string]: Foo2; +}; + +interface Foo2 { + [key: string]: Foo3; +} + +interface Foo3 { + [key: string]: Foo1; +} + `, + // Type literal 'type Foo = {};', ` @@ -392,6 +509,133 @@ interface Foo { } `, }, + { + code: ` +interface Foo { + [key: string]: { foo: Foo }; +} + `, + errors: [{ column: 1, line: 2, messageId: 'preferRecord' }], + output: ` +type Foo = Record; + `, + }, + { + code: ` +interface Foo { + [key: string]: Foo[]; +} + `, + errors: [{ column: 1, line: 2, messageId: 'preferRecord' }], + output: ` +type Foo = Record; + `, + }, + { + code: ` +interface Foo { + [key: string]: () => Foo; +} + `, + errors: [{ column: 1, line: 2, messageId: 'preferRecord' }], + output: ` +type Foo = Record Foo>; + `, + }, + { + code: ` +interface Foo { + [s: string]: [Foo]; +} + `, + errors: [{ column: 1, line: 2, messageId: 'preferRecord' }], + output: ` +type Foo = Record; + `, + }, + + // Circular (indirect) + { + code: ` +interface Foo1 { + [key: string]: Foo2; +} + +interface Foo2 { + [key: string]: Foo3; +} + +interface Foo3 { + [key: string]: Foo2; +} + `, + errors: [{ column: 1, line: 2, messageId: 'preferRecord' }], + output: ` +type Foo1 = Record; + +interface Foo2 { + [key: string]: Foo3; +} + +interface Foo3 { + [key: string]: Foo2; +} + `, + }, + { + code: ` +interface Foo1 { + [key: string]: Record; +} + +interface Foo2 { + [key: string]: Foo3; +} + +interface Foo3 { + [key: string]: Foo2; +} + `, + errors: [{ column: 1, line: 2, messageId: 'preferRecord' }], + output: ` +type Foo1 = Record>; + +interface Foo2 { + [key: string]: Foo3; +} + +interface Foo3 { + [key: string]: Foo2; +} + `, + }, + { + code: ` +type Foo1 = { + [key: string]: { foo2: Foo2 }; +}; + +type Foo2 = { + [key: string]: Foo3; +}; + +type Foo3 = { + [key: string]: Record; +}; + `, + errors: [ + { column: 13, line: 2, messageId: 'preferRecord' }, + { column: 13, line: 6, messageId: 'preferRecord' }, + { column: 13, line: 10, messageId: 'preferRecord' }, + ], + output: ` +type Foo1 = Record; + +type Foo2 = Record; + +type Foo3 = Record>; + `, + }, // Generic { From 43464cf7c23b00bde05c889c16b773c44efc44d3 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 21 Dec 2024 23:13:36 +0200 Subject: [PATCH 2/8] revert unrelated changes --- .../src/rules/consistent-indexed-object-style.ts | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts b/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts index 2ac74354ace5..bcac1c69eb5f 100644 --- a/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts +++ b/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts @@ -46,14 +46,13 @@ export default createRule({ defaultOptions: ['record'], create(context, [mode]) { function checkMembers( + members: TSESTree.TypeElement[], node: TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeLiteral, parentId: TSESTree.Identifier | undefined, prefix: string, postfix: string, safeFix = true, ): void { - const members = getMembers(node); - if (members.length !== 1) { return; } @@ -159,6 +158,7 @@ export default createRule({ } checkMembers( + node.body.body, node, node.id, `type ${node.id.name}${genericTypes} = `, @@ -249,7 +249,7 @@ export default createRule({ }, TSTypeLiteral(node): void { const parent = findParentDeclaration(node); - checkMembers(node, parent?.id, '', ''); + checkMembers(node.members, node, parent?.id, '', ''); }, }), }; @@ -268,16 +268,6 @@ function findParentDeclaration( return undefined; } -function getMembers( - node: TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeLiteral, -): TSESTree.TypeElement[] { - if (node.type === AST_NODE_TYPES.TSInterfaceDeclaration) { - return node.body.body; - } - - return node.members; -} - function isDeeplyReferencingType( node: TSESTree.Node, superVar: ScopeVariable, From 474df6b5bc466f51413ca263eaa7ed9f864dc747 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 21 Dec 2024 23:27:13 +0200 Subject: [PATCH 3/8] remove now redundant eslint-disable comment --- packages/utils/src/json-schema.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/utils/src/json-schema.ts b/packages/utils/src/json-schema.ts index 1c94883578c4..f2620b871675 100644 --- a/packages/utils/src/json-schema.ts +++ b/packages/utils/src/json-schema.ts @@ -34,7 +34,6 @@ export type JSONSchema4TypeExtended = // Workaround for infinite type recursion // Also, https://github.com/typescript-eslint/typescript-eslint/issues/7863 -// eslint-disable-next-line @typescript-eslint/consistent-indexed-object-style export interface JSONSchema4Object { [key: string]: JSONSchema4TypeExtended; } From 59a8383e9c7a3c503f0310c648612ef4c5c5c0bb Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 21 Dec 2024 23:37:37 +0200 Subject: [PATCH 4/8] slight formatting change --- .../eslint-plugin/src/rules/consistent-indexed-object-style.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts b/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts index bcac1c69eb5f..b4f28e8099c0 100644 --- a/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts +++ b/packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts @@ -6,9 +6,9 @@ import { AST_NODE_TYPES, ASTUtils } from '@typescript-eslint/utils'; import { createRule, + getFixOrSuggest, isNodeEqual, isParenthesized, - getFixOrSuggest, nullThrows, } from '../util'; From 9bc7d99a748cd8485c65caf86c76057f1b0c91b9 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 21 Dec 2024 23:41:04 +0200 Subject: [PATCH 5/8] remove unrelated changes --- packages/eslint-plugin/src/util/isNodeEqual.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/eslint-plugin/src/util/isNodeEqual.ts b/packages/eslint-plugin/src/util/isNodeEqual.ts index f2e0f377911e..729b38b58888 100644 --- a/packages/eslint-plugin/src/util/isNodeEqual.ts +++ b/packages/eslint-plugin/src/util/isNodeEqual.ts @@ -21,12 +21,6 @@ export function isNodeEqual(a: TSESTree.Node, b: TSESTree.Node): boolean { ) { return a.name === b.name; } - if ( - a.type === AST_NODE_TYPES.TSTypeReference && - b.type === AST_NODE_TYPES.TSTypeReference - ) { - return isNodeEqual(a.typeName, b.typeName); - } if ( a.type === AST_NODE_TYPES.MemberExpression && b.type === AST_NODE_TYPES.MemberExpression From 185b38d911509508f9763ddbda35657a87430b0b Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sun, 22 Dec 2024 00:13:26 +0200 Subject: [PATCH 6/8] add a test for missing coverage --- .../consistent-indexed-object-style.test.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts b/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts index fa50a2a3b88f..65128a90fa45 100644 --- a/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts @@ -862,6 +862,35 @@ function f(): { output: ` function f(): Record { return {}; +} + `, + }, + + // missing index signature type annotation while checking for a recursive type + { + code: ` +interface Foo { + [key: string]: Bar; +} + +interface Bar { + [key: string]; +} + `, + errors: [ + { + column: 1, + endColumn: 2, + endLine: 4, + line: 2, + messageId: 'preferRecord', + }, + ], + output: ` +type Foo = Record; + +interface Bar { + [key: string]; } `, }, From 762fb84342e390ee902941b8419187848c7886be Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sun, 22 Dec 2024 00:32:34 +0200 Subject: [PATCH 7/8] add indirect union recursive test --- .../rules/consistent-indexed-object-style.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts b/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts index 65128a90fa45..5ecceba510a3 100644 --- a/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts @@ -165,6 +165,15 @@ interface Foo3 { [key: string]: Foo1; } `, + ` +type ExampleUnion = boolean | number; + +type ExampleRoot = ExampleUnion | ExampleObject; + +interface ExampleObject { + [key: string]: ExampleRoot; +} + `, // Type literal 'type Foo = {};', @@ -211,6 +220,7 @@ interface Foo { []; } `, + // 'index-signature' // Unhandled type { From c81fe195c9be32a85c5d66a45fba1c1271cfd8fd Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sun, 22 Dec 2024 00:51:03 +0200 Subject: [PATCH 8/8] remove additional unnecessary comments --- packages/utils/src/json-schema.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/utils/src/json-schema.ts b/packages/utils/src/json-schema.ts index f2620b871675..bfa890aac372 100644 --- a/packages/utils/src/json-schema.ts +++ b/packages/utils/src/json-schema.ts @@ -32,8 +32,6 @@ export type JSONSchema4TypeExtended = | JSONSchema4Object | JSONSchema4Type; -// Workaround for infinite type recursion -// Also, https://github.com/typescript-eslint/typescript-eslint/issues/7863 export interface JSONSchema4Object { [key: string]: JSONSchema4TypeExtended; }