From cbcc61817453d6e0aeca257a2063a46a030964f5 Mon Sep 17 00:00:00 2001 From: Daniil Dubrava Date: Fri, 6 Nov 2020 00:17:38 +0300 Subject: [PATCH 1/5] fix(eslint-plugin): [consistent-type-definitions] ignore TSInterfaceDeclaration within TSModuleDeclaration (#2707) --- .../src/rules/consistent-type-definitions.ts | 4 +++- .../tests/rules/consistent-type-definitions.test.ts | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts index 794e380e389d..7cf6a53039bc 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts @@ -68,7 +68,9 @@ export default util.createRule({ }); } }, - TSInterfaceDeclaration(node): void { + ':not(TSModuleDeclaration > TSModuleBlock) > TSInterfaceDeclaration'( + node: TSESTree.TSInterfaceDeclaration, + ): void { if (option === 'type') { context.report({ node: node.id, diff --git a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts index 78e5d3ceb018..8d9c84ea199c 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts @@ -59,6 +59,16 @@ export type W = { `, options: ['type'], }, + { + code: ` +declare global { + interface Array { + foo(x: (x: number) => T): T[]; + } +} + `, + options: ['type'], + }, ], invalid: [ { From 867868b073c7ea2810e81a383a831e3869a436f8 Mon Sep 17 00:00:00 2001 From: Daniil Dubrava Date: Mon, 9 Nov 2020 01:40:36 +0300 Subject: [PATCH 2/5] fix(eslint-plugin): [consistent-type-definitions] remove fixer when the interface is within a global module declaration --- .../src/rules/consistent-type-definitions.ts | 71 ++++++++++++------- .../rules/consistent-type-definitions.test.ts | 28 +++++--- 2 files changed, 63 insertions(+), 36 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts index 7cf6a53039bc..d9a63ec64815 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts @@ -1,4 +1,5 @@ import { + AST_NODE_TYPES, AST_TOKEN_TYPES, TSESLint, TSESTree, @@ -31,6 +32,21 @@ export default util.createRule({ create(context, [option]) { const sourceCode = context.getSourceCode(); + function isNodeGrandparentGlobalModuleDeclaration( + node: TSESTree.Node, + ): boolean { + if ( + node.parent?.type === AST_NODE_TYPES.TSModuleBlock && + node.parent.parent?.type === AST_NODE_TYPES.TSModuleDeclaration && + node.parent.parent?.declare && + node.parent.parent?.global + ) { + return true; + } + + return false; + } + return { "TSTypeAliasDeclaration[typeAnnotation.type='TSTypeLiteral']"( node: TSESTree.TSTypeAliasDeclaration, @@ -68,39 +84,42 @@ export default util.createRule({ }); } }, - ':not(TSModuleDeclaration > TSModuleBlock) > TSInterfaceDeclaration'( - node: TSESTree.TSInterfaceDeclaration, - ): void { + TSInterfaceDeclaration(node): void { if (option === 'type') { context.report({ node: node.id, messageId: 'typeOverInterface', - fix(fixer) { - const typeNode = node.typeParameters ?? node.id; - const fixes: TSESLint.RuleFix[] = []; + fix: isNodeGrandparentGlobalModuleDeclaration(node) + ? null + : (fixer): TSESLint.RuleFix[] => { + const typeNode = node.typeParameters ?? node.id; + const fixes: TSESLint.RuleFix[] = []; - const firstToken = sourceCode.getFirstToken(node); - if (firstToken) { - fixes.push(fixer.replaceText(firstToken, 'type')); - fixes.push( - fixer.replaceTextRange( - [typeNode.range[1], node.body.range[0]], - ' = ', - ), - ); - } + const firstToken = sourceCode.getFirstToken(node); + if (firstToken) { + fixes.push(fixer.replaceText(firstToken, 'type')); + fixes.push( + fixer.replaceTextRange( + [typeNode.range[1], node.body.range[0]], + ' = ', + ), + ); + } - if (node.extends) { - node.extends.forEach(heritage => { - const typeIdentifier = sourceCode.getText(heritage); - fixes.push( - fixer.insertTextAfter(node.body, ` & ${typeIdentifier}`), - ); - }); - } + if (node.extends) { + node.extends.forEach(heritage => { + const typeIdentifier = sourceCode.getText(heritage); + fixes.push( + fixer.insertTextAfter( + node.body, + ` & ${typeIdentifier}`, + ), + ); + }); + } - return fixes; - }, + return fixes; + }, }); } }, diff --git a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts index 8d9c84ea199c..057b8b286195 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts @@ -59,16 +59,6 @@ export type W = { `, options: ['type'], }, - { - code: ` -declare global { - interface Array { - foo(x: (x: number) => T): T[]; - } -} - `, - options: ['type'], - }, ], invalid: [ { @@ -207,5 +197,23 @@ export type W = { }, ], }, + { + code: ` +declare global { + interface Array { + foo(x: (x: number) => T): T[]; + } +} + `, + output: null, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 3, + column: 13, + }, + ], + }, ], }); From 6d10e0508e5656720a5a96e3a6f49edfb97a77df Mon Sep 17 00:00:00 2001 From: Daniil Dubrava Date: Mon, 9 Nov 2020 02:27:53 +0300 Subject: [PATCH 3/5] refactor(eslint-plugun): [consistent-type-definitions] mention the issue for removing the fixer --- .../eslint-plugin/src/rules/consistent-type-definitions.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts index d9a63ec64815..7f503394f896 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts @@ -89,6 +89,10 @@ export default util.createRule({ context.report({ node: node.id, messageId: 'typeOverInterface', + /** + * remove automatically fix when the interface is within a declare global + * @see {@link https://github.com/typescript-eslint/typescript-eslint/issues/2707} + */ fix: isNodeGrandparentGlobalModuleDeclaration(node) ? null : (fixer): TSESLint.RuleFix[] => { From 945b193f412fd32febccca71e1a0886028ae5ea9 Mon Sep 17 00:00:00 2001 From: Daniil Dubrava Date: Mon, 9 Nov 2020 02:36:20 +0300 Subject: [PATCH 4/5] test(eslint-plugin): [consistent-type-definitions] add cases when TSModuleDeclaration without declare or global --- .../rules/consistent-type-definitions.test.ts | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts index 057b8b286195..343fc781e7d5 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts @@ -199,6 +199,54 @@ export type W = { }, { code: ` +namespace JSX { + interface Array { + foo(x: (x: number) => T): T[]; + } +} + `, + output: noFormat` +namespace JSX { + type Array = { + foo(x: (x: number) => T): T[]; + } +} + `, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 3, + column: 13, + }, + ], + }, + { + code: ` +global { + interface Array { + foo(x: (x: number) => T): T[]; + } +} + `, + output: noFormat` +global { + type Array = { + foo(x: (x: number) => T): T[]; + } +} + `, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 3, + column: 13, + }, + ], + }, + { + code: ` declare global { interface Array { foo(x: (x: number) => T): T[]; From 4d14d686876a992bf3017b18a440a4178b7e97b4 Mon Sep 17 00:00:00 2001 From: Daniil Dubrava Date: Tue, 10 Nov 2020 21:31:24 +0300 Subject: [PATCH 5/5] refactor(eslint-plugin): [consistent-type-definitions] replace looking at the grandparent by looking at all the ancestors --- .../src/rules/consistent-type-definitions.ts | 28 +++++++++---------- .../rules/consistent-type-definitions.test.ts | 18 ++++++++++++ 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts index 7f503394f896..f5b2c48a44f4 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts @@ -32,19 +32,19 @@ export default util.createRule({ create(context, [option]) { const sourceCode = context.getSourceCode(); - function isNodeGrandparentGlobalModuleDeclaration( - node: TSESTree.Node, - ): boolean { - if ( - node.parent?.type === AST_NODE_TYPES.TSModuleBlock && - node.parent.parent?.type === AST_NODE_TYPES.TSModuleDeclaration && - node.parent.parent?.declare && - node.parent.parent?.global - ) { - return true; - } - - return false; + /** + * Iterates from the highest parent to the currently traversed node + * to determine whether any node in tree is globally declared module declaration + */ + function isCurrentlyTraversedNodeWithinModuleDeclaration(): boolean { + return context + .getAncestors() + .some( + node => + node.type === AST_NODE_TYPES.TSModuleDeclaration && + node.declare && + node.global, + ); } return { @@ -93,7 +93,7 @@ export default util.createRule({ * remove automatically fix when the interface is within a declare global * @see {@link https://github.com/typescript-eslint/typescript-eslint/issues/2707} */ - fix: isNodeGrandparentGlobalModuleDeclaration(node) + fix: isCurrentlyTraversedNodeWithinModuleDeclaration() ? null : (fixer): TSESLint.RuleFix[] => { const typeNode = node.typeParameters ?? node.id; diff --git a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts index 343fc781e7d5..105c25724c23 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts @@ -263,5 +263,23 @@ declare global { }, ], }, + { + code: ` +declare global { + namespace Foo { + interface Bar {} + } +} + `, + output: null, + options: ['type'], + errors: [ + { + messageId: 'typeOverInterface', + line: 4, + column: 15, + }, + ], + }, ], });