From 7eba36e2a95f1347318c96a520ad64d4b57b422f Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 30 Dec 2024 19:01:01 +0200 Subject: [PATCH] fix(eslint-plugin): [consistent-indexed-object-style] don't report on indirect circular references (#10537) * initial implementation (squashed) * revert unrelated changes * remove now redundant eslint-disable comment * slight formatting change * remove unrelated changes * add a test for missing coverage * add indirect union recursive test * remove additional unnecessary comments --- .../rules/consistent-indexed-object-style.ts | 104 ++++++- .../consistent-indexed-object-style.test.ts | 283 ++++++++++++++++++ packages/utils/src/json-schema.ts | 3 - 3 files changed, 377 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 15ab691b38dd..b4f28e8099c0 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'; @@ -6,6 +7,7 @@ import { AST_NODE_TYPES, ASTUtils } from '@typescript-eslint/utils'; import { createRule, getFixOrSuggest, + isNodeEqual, isParenthesized, nullThrows, } from '../util'; @@ -78,16 +80,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; } } @@ -269,3 +267,89 @@ function findParentDeclaration( } return undefined; } + +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/tests/rules/consistent-indexed-object-style.test.ts b/packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts index 8441a720e326..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 @@ -49,6 +49,132 @@ 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 ExampleUnion = boolean | number; + +type ExampleRoot = ExampleUnion | ExampleObject; + +interface ExampleObject { + [key: string]: ExampleRoot; +} + `, + // Type literal 'type Foo = {};', ` @@ -94,6 +220,7 @@ interface Foo { []; } `, + // 'index-signature' // Unhandled type { @@ -392,6 +519,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 { @@ -618,6 +872,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]; } `, }, diff --git a/packages/utils/src/json-schema.ts b/packages/utils/src/json-schema.ts index 1c94883578c4..bfa890aac372 100644 --- a/packages/utils/src/json-schema.ts +++ b/packages/utils/src/json-schema.ts @@ -32,9 +32,6 @@ export type JSONSchema4TypeExtended = | JSONSchema4Object | JSONSchema4Type; -// 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; }