From 2ab571e4e594623dfb9da38f010b51e14113e24b Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sat, 11 Sep 2021 21:03:39 -0300 Subject: [PATCH 1/2] fix(eslint-plugin): [no-shadow] ignore type-only imports properly --- packages/eslint-plugin/src/rules/no-shadow.ts | 10 ++++--- .../tests/rules/no-shadow.test.ts | 30 ++++++++++++++++++- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index 6eb2ef38f7c..4c15d9e0f78 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -1,9 +1,9 @@ import { - TSESTree, - TSESLint, AST_NODE_TYPES, + TSESLint, + TSESTree, } from '@typescript-eslint/experimental-utils'; -import { ScopeType } from '@typescript-eslint/scope-manager'; +import { DefinitionType, ScopeType } from '@typescript-eslint/scope-manager'; import * as util from '../util'; type MessageIds = 'noShadow'; @@ -99,7 +99,9 @@ export default util.createRule({ } const isShadowedValue = - 'isValueVariable' in shadowed ? shadowed.isValueVariable : true; + !('isValueVariable' in shadowed) || + (shadowed.defs[0]?.type !== DefinitionType.ImportBinding && + shadowed.isValueVariable); return variable.isValueVariable !== isShadowedValue; } diff --git a/packages/eslint-plugin/tests/rules/no-shadow.test.ts b/packages/eslint-plugin/tests/rules/no-shadow.test.ts index f48adfc1b57..16c06cf89b3 100644 --- a/packages/eslint-plugin/tests/rules/no-shadow.test.ts +++ b/packages/eslint-plugin/tests/rules/no-shadow.test.ts @@ -1,6 +1,6 @@ +import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; import rule from '../../src/rules/no-shadow'; import { RuleTester } from '../RuleTester'; -import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; const ruleTester = new RuleTester({ parserOptions: { @@ -172,6 +172,18 @@ export class Wrapper { } } `, + { + // https://github.com/typescript-eslint/typescript-eslint/issues/3862 + code: ` +import type { foo } from './foo'; +type bar = number; + +// 'foo' is already declared in the upper scope +// 'bar' is fine +function doThing(foo: number, bar: number) {} + `, + options: [{ ignoreTypeValueShadow: true }], + }, ], invalid: [ { @@ -1398,5 +1410,21 @@ function foo(cb) { }, ], }, + { + code: ` +import type { foo } from './foo'; +function doThing(foo: number, bar: number) {} + `, + options: [{ ignoreTypeValueShadow: false }], + errors: [ + { + messageId: 'noShadow', + data: { name: 'foo' }, + type: AST_NODE_TYPES.Identifier, + line: 3, + column: 18, + }, + ], + }, ], }); From ebea51a10dc6fc485cef9028b62972b708244492 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Mon, 20 Sep 2021 21:34:41 -0300 Subject: [PATCH 2/2] fixup! fix(eslint-plugin): [no-shadow] ignore type-only imports properly --- packages/eslint-plugin/src/rules/no-shadow.ts | 21 ++++++++++++++++--- .../tests/rules/no-shadow.test.ts | 16 ++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index 4c15d9e0f78..ea27f87e083 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -3,7 +3,12 @@ import { TSESLint, TSESTree, } from '@typescript-eslint/experimental-utils'; -import { DefinitionType, ScopeType } from '@typescript-eslint/scope-manager'; +import { + Definition, + DefinitionType, + ImportBindingDefinition, + ScopeType, +} from '@typescript-eslint/scope-manager'; import * as util from '../util'; type MessageIds = 'noShadow'; @@ -85,6 +90,15 @@ export default util.createRule({ return variable.defs[0].type === 'Parameter' && variable.name === 'this'; } + function isTypeImport( + definition: Definition, + ): definition is ImportBindingDefinition { + return ( + definition.type === DefinitionType.ImportBinding && + definition.parent.importKind === 'type' + ); + } + function isTypeValueShadow( variable: TSESLint.Scope.Variable, shadowed: TSESLint.Scope.Variable, @@ -98,10 +112,11 @@ export default util.createRule({ return false; } + const [firstDefinition] = shadowed.defs; const isShadowedValue = !('isValueVariable' in shadowed) || - (shadowed.defs[0]?.type !== DefinitionType.ImportBinding && - shadowed.isValueVariable); + !firstDefinition || + (!isTypeImport(firstDefinition) && shadowed.isValueVariable); return variable.isValueVariable !== isShadowedValue; } diff --git a/packages/eslint-plugin/tests/rules/no-shadow.test.ts b/packages/eslint-plugin/tests/rules/no-shadow.test.ts index 16c06cf89b3..c3e5a09f344 100644 --- a/packages/eslint-plugin/tests/rules/no-shadow.test.ts +++ b/packages/eslint-plugin/tests/rules/no-shadow.test.ts @@ -1426,5 +1426,21 @@ function doThing(foo: number, bar: number) {} }, ], }, + { + code: ` +import { foo } from './foo'; +function doThing(foo: number, bar: number) {} + `, + options: [{ ignoreTypeValueShadow: true }], + errors: [ + { + messageId: 'noShadow', + data: { name: 'foo' }, + type: AST_NODE_TYPES.Identifier, + line: 3, + column: 18, + }, + ], + }, ], });