From b6cd90eaf802f6094ee98ca6db92da64d64aed99 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 3 Nov 2022 00:32:26 +0900 Subject: [PATCH 1/4] fix(eslint-plugin): [no-empty-interface] disable autofix for declaration merging with class --- .../src/rules/no-empty-interface.ts | 23 +++++++++++++++---- .../tests/rules/no-empty-interface.test.ts | 20 ++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-empty-interface.ts b/packages/eslint-plugin/src/rules/no-empty-interface.ts index 4e037e9cc8a..97cf44c899b 100644 --- a/packages/eslint-plugin/src/rules/no-empty-interface.ts +++ b/packages/eslint-plugin/src/rules/no-empty-interface.ts @@ -1,4 +1,4 @@ -import type { TSESLint } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES, TSESLint } from '@typescript-eslint/utils'; import * as util from '../util'; @@ -76,26 +76,41 @@ export default util.createRule({ // Check if interface is within ambient declaration let useAutoFix = true; + let hasSuggest = true; + const scope = context.getScope(); + if (util.isDefinitionFile(filename)) { - const scope = context.getScope(); if (scope.type === 'tsModule' && scope.block.declare) { useAutoFix = false; } } + // Not suggest if the interface declaration merged with class declarations. + const defs = scope.set.get(node.id.name)?.defs; + if ( + defs?.some( + def => def.node.type === AST_NODE_TYPES.ClassDeclaration, + ) + ) { + useAutoFix = false; + hasSuggest = false; + } + context.report({ node: node.id, messageId: 'noEmptyWithSuper', ...(useAutoFix ? { fix } - : { + : hasSuggest + ? { suggest: [ { messageId: 'noEmptyWithSuper', fix, }, ], - }), + } + : null), }); } } diff --git a/packages/eslint-plugin/tests/rules/no-empty-interface.test.ts b/packages/eslint-plugin/tests/rules/no-empty-interface.test.ts index 0d7f73342b5..601da6bdf3e 100644 --- a/packages/eslint-plugin/tests/rules/no-empty-interface.test.ts +++ b/packages/eslint-plugin/tests/rules/no-empty-interface.test.ts @@ -58,6 +58,26 @@ interface Bar extends Foo {} }, { code: ` +interface Foo { + props: string; +} + +interface Bar extends Foo {} + +class Bar {} + `, + options: [{ allowSingleExtends: false }], + errors: [ + { + messageId: 'noEmptyWithSuper', + line: 6, + column: 11, + }, + ], + output: null, + }, + { + code: ` interface Foo { name: string; } From 25f022a53180f4492cc75e4f778471878db72f57 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 3 Nov 2022 00:36:30 +0900 Subject: [PATCH 2/4] fix comment --- packages/eslint-plugin/src/rules/no-empty-interface.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-empty-interface.ts b/packages/eslint-plugin/src/rules/no-empty-interface.ts index 97cf44c899b..588e5f577ae 100644 --- a/packages/eslint-plugin/src/rules/no-empty-interface.ts +++ b/packages/eslint-plugin/src/rules/no-empty-interface.ts @@ -85,7 +85,7 @@ export default util.createRule({ } } - // Not suggest if the interface declaration merged with class declarations. + // Checks if the interface declaration is merged with class declarations. const defs = scope.set.get(node.id.name)?.defs; if ( defs?.some( From e52a060e261c48ff12870cf3862f4d8d2b57aac1 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 3 Nov 2022 01:18:42 +0900 Subject: [PATCH 3/4] fix lint --- packages/eslint-plugin/src/rules/no-empty-interface.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-empty-interface.ts b/packages/eslint-plugin/src/rules/no-empty-interface.ts index 588e5f577ae..c1cfd498a7f 100644 --- a/packages/eslint-plugin/src/rules/no-empty-interface.ts +++ b/packages/eslint-plugin/src/rules/no-empty-interface.ts @@ -1,4 +1,5 @@ -import { AST_NODE_TYPES, TSESLint } from '@typescript-eslint/utils'; +import type { TSESLint } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as util from '../util'; From 3840352c5db51b9a8a044f90db2abca757026415 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sat, 19 Nov 2022 00:44:50 +0900 Subject: [PATCH 4/4] Apply review --- .../src/rules/no-empty-interface.ts | 35 ++++------ .../tests/rules/no-empty-interface.test.ts | 68 +++++++++++++++++++ 2 files changed, 83 insertions(+), 20 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-empty-interface.ts b/packages/eslint-plugin/src/rules/no-empty-interface.ts index c1cfd498a7f..d74034114bb 100644 --- a/packages/eslint-plugin/src/rules/no-empty-interface.ts +++ b/packages/eslint-plugin/src/rules/no-empty-interface.ts @@ -74,35 +74,30 @@ export default util.createRule({ )}${typeParam} = ${sourceCode.getText(extend[0])}`, ); }; - - // Check if interface is within ambient declaration - let useAutoFix = true; - let hasSuggest = true; const scope = context.getScope(); - if (util.isDefinitionFile(filename)) { - if (scope.type === 'tsModule' && scope.block.declare) { - useAutoFix = false; - } - } - - // Checks if the interface declaration is merged with class declarations. - const defs = scope.set.get(node.id.name)?.defs; - if ( - defs?.some( + const mergedWithClassDeclaration = scope.set + .get(node.id.name) + ?.defs?.some( def => def.node.type === AST_NODE_TYPES.ClassDeclaration, - ) - ) { - useAutoFix = false; - hasSuggest = false; - } + ); + + const isInAmbientDeclaration = !!( + util.isDefinitionFile(filename) && + scope.type === 'tsModule' && + scope.block.declare + ); + + const useAutoFix = !( + isInAmbientDeclaration || mergedWithClassDeclaration + ); context.report({ node: node.id, messageId: 'noEmptyWithSuper', ...(useAutoFix ? { fix } - : hasSuggest + : !mergedWithClassDeclaration ? { suggest: [ { diff --git a/packages/eslint-plugin/tests/rules/no-empty-interface.test.ts b/packages/eslint-plugin/tests/rules/no-empty-interface.test.ts index 601da6bdf3e..893deaf01d6 100644 --- a/packages/eslint-plugin/tests/rules/no-empty-interface.test.ts +++ b/packages/eslint-plugin/tests/rules/no-empty-interface.test.ts @@ -34,6 +34,18 @@ interface Bar extends Foo {} `, options: [{ allowSingleExtends: true }], }, + { + code: ` +interface Foo { + props: string; +} + +interface Bar extends Foo {} + +class Bar {} + `, + options: [{ allowSingleExtends: true }], + }, ], invalid: [ { @@ -64,6 +76,34 @@ interface Foo { interface Bar extends Foo {} +class Baz {} + `, + output: ` +interface Foo { + props: string; +} + +type Bar = Foo + +class Baz {} + `, + options: [{ allowSingleExtends: false }], + errors: [ + { + messageId: 'noEmptyWithSuper', + line: 6, + column: 11, + }, + ], + }, + { + code: ` +interface Foo { + props: string; +} + +interface Bar extends Foo {} + class Bar {} `, options: [{ allowSingleExtends: false }], @@ -78,6 +118,34 @@ class Bar {} }, { code: ` +interface Foo { + props: string; +} + +interface Bar extends Foo {} + +const bar = class Bar {}; + `, + output: ` +interface Foo { + props: string; +} + +type Bar = Foo + +const bar = class Bar {}; + `, + options: [{ allowSingleExtends: false }], + errors: [ + { + messageId: 'noEmptyWithSuper', + line: 6, + column: 11, + }, + ], + }, + { + code: ` interface Foo { name: string; }