From dad43cbd734b45653779e2e30a1258732e8abf98 Mon Sep 17 00:00:00 2001 From: Idan Attias Date: Mon, 4 Oct 2021 18:19:48 +0300 Subject: [PATCH 1/4] fix(eslint-plugin): [no-shadow] exclude external type declaration merging --- packages/eslint-plugin/src/rules/no-shadow.ts | 60 ++++++++++++- .../tests/rules/no-shadow.test.ts | 87 +++++++++++++++++++ 2 files changed, 145 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index a49afd502cf..cb18ef5d745 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -95,10 +95,10 @@ export default util.createRule({ } function isTypeImport( - definition: Definition, + definition?: Definition, ): definition is ImportBindingDefinition { return ( - definition.type === DefinitionType.ImportBinding && + definition?.type === DefinitionType.ImportBinding && definition.parent.importKind === 'type' ); } @@ -225,6 +225,58 @@ export default util.createRule({ ); } + function isImportDeclaration( + definition: + | TSESTree.ImportDeclaration + | TSESTree.TSImportEqualsDeclaration, + ): definition is TSESTree.ImportDeclaration { + return definition.type === AST_NODE_TYPES.ImportDeclaration; + } + + function isStringLiteral( + node: TSESTree.Node | null, + ): node is TSESTree.StringLiteral { + return ( + !!node && + node.type === AST_NODE_TYPES.Literal && + typeof node.value === 'string' + ); + } + + function isExternalModuleDeclarationWithName( + scope: TSESLint.Scope.Scope, + name: string, + ): boolean { + return ( + scope.type === ScopeType.tsModule && + scope.block.type === AST_NODE_TYPES.TSModuleDeclaration && + scope.block.id.type === AST_NODE_TYPES.Literal && + scope.block.id.value === name + ); + } + + function isExternalDeclerationMerging( + scope: TSESLint.Scope.Scope, + variable: TSESLint.Scope.Variable, + shadowed: TSESLint.Scope.Variable, + ): boolean { + const [firstDefinition] = shadowed.defs; + const [secondDefinition] = variable.defs; + + return ( + isTypeImport(firstDefinition) && + isImportDeclaration(firstDefinition.parent) && + isStringLiteral(firstDefinition.parent.source) && + isExternalModuleDeclarationWithName( + scope, + firstDefinition.parent.source.value, + ) && + secondDefinition.node.type === AST_NODE_TYPES.TSInterfaceDeclaration && + secondDefinition.node.parent?.type === + AST_NODE_TYPES.ExportNamedDeclaration + ); + } + /** * Check if variable name is allowed. * @param variable The variable to check. @@ -404,6 +456,10 @@ export default util.createRule({ continue; } + if (isExternalDeclerationMerging(scope, variable, shadowed)) { + continue; + } + const isESLintGlobal = 'writeable' in shadowed; if ( (shadowed.identifiers.length > 0 || diff --git a/packages/eslint-plugin/tests/rules/no-shadow.test.ts b/packages/eslint-plugin/tests/rules/no-shadow.test.ts index c3e5a09f344..bedb825df70 100644 --- a/packages/eslint-plugin/tests/rules/no-shadow.test.ts +++ b/packages/eslint-plugin/tests/rules/no-shadow.test.ts @@ -51,6 +51,15 @@ class Foo { } interface Foo { prop2: string; +} + `, + ` +import type { Foo } from 'bar'; + +declare module 'bar' { + export interface Foo { + x: string; + } } `, // type value shadowing @@ -1442,5 +1451,83 @@ function doThing(foo: number, bar: number) {} }, ], }, + { + code: ` +interface Foo {} + +declare module 'bar' { + export interface Foo { + x: string; + } +} + `, + errors: [ + { + messageId: 'noShadow', + data: { name: 'Foo' }, + type: AST_NODE_TYPES.Identifier, + line: 5, + column: 20, + }, + ], + }, + { + code: ` +import type { Foo } from 'bar'; + +declare module 'baz' { + export interface Foo { + x: string; + } +} + `, + errors: [ + { + messageId: 'noShadow', + data: { name: 'Foo' }, + type: AST_NODE_TYPES.Identifier, + line: 5, + column: 20, + }, + ], + }, + { + code: ` +import type { Foo } from 'bar'; + +declare module 'bar' { + export type Foo = string; +} + `, + errors: [ + { + messageId: 'noShadow', + data: { name: 'Foo' }, + type: AST_NODE_TYPES.Identifier, + line: 5, + column: 15, + }, + ], + }, + { + code: ` +import type { Foo } from 'bar'; + +declare module 'bar' { + interface Foo { + x: string; + } +} + `, + errors: [ + { + messageId: 'noShadow', + data: { name: 'Foo' }, + type: AST_NODE_TYPES.Identifier, + line: 5, + column: 13, + }, + ], + }, ], }); From 2c97f713d7062ea969d758cf2086cf35a0cde83c Mon Sep 17 00:00:00 2001 From: Idan Attias Date: Tue, 5 Oct 2021 08:48:21 +0300 Subject: [PATCH 2/4] fix(eslint-plugin): [no-shadow] typo --- packages/eslint-plugin/src/rules/no-shadow.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index cb18ef5d745..710975bf557 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -255,7 +255,7 @@ export default util.createRule({ ); } - function isExternalDeclerationMerging( + function isExternalDeclarationMerging( scope: TSESLint.Scope.Scope, variable: TSESLint.Scope.Variable, shadowed: TSESLint.Scope.Variable, @@ -456,7 +456,7 @@ export default util.createRule({ continue; } - if (isExternalDeclerationMerging(scope, variable, shadowed)) { + if (isExternalDeclarationMerging(scope, variable, shadowed)) { continue; } From 2b00d98047023152e83d12ec9011167c5eb12b12 Mon Sep 17 00:00:00 2001 From: Idan Attias Date: Sun, 17 Oct 2021 14:34:23 +0300 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Josh Goldberg --- packages/eslint-plugin/src/rules/no-shadow.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index 710975bf557..1885c0279fa 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -237,8 +237,7 @@ export default util.createRule({ node: TSESTree.Node | null, ): node is TSESTree.StringLiteral { return ( - !!node && - node.type === AST_NODE_TYPES.Literal && + node?.type === AST_NODE_TYPES.Literal && typeof node.value === 'string' ); } @@ -266,7 +265,6 @@ export default util.createRule({ return ( isTypeImport(firstDefinition) && isImportDeclaration(firstDefinition.parent) && - isStringLiteral(firstDefinition.parent.source) && isExternalModuleDeclarationWithName( scope, firstDefinition.parent.source.value, From a57c4a0cb0c4d7125368facf28eb51111b04984a Mon Sep 17 00:00:00 2001 From: Idan Attias Date: Sun, 17 Oct 2021 14:45:11 +0300 Subject: [PATCH 4/4] fix(eslint-plugin): lint - remove isStringLiteral (no longer used) --- packages/eslint-plugin/src/rules/no-shadow.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index 1885c0279fa..b3d8310fb3f 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -233,15 +233,6 @@ export default util.createRule({ return definition.type === AST_NODE_TYPES.ImportDeclaration; } - function isStringLiteral( - node: TSESTree.Node | null, - ): node is TSESTree.StringLiteral { - return ( - node?.type === AST_NODE_TYPES.Literal && - typeof node.value === 'string' - ); - } - function isExternalModuleDeclarationWithName( scope: TSESLint.Scope.Scope, name: string,