From a22d6d3c04b8e70625c98ddc023a34d687950623 Mon Sep 17 00:00:00 2001 From: sopa301 <96387349+sopa301@users.noreply.github.com> Date: Sat, 25 Jan 2025 02:36:27 +0800 Subject: [PATCH 1/2] Ignore "declare" in definition files for no-shadow Having "declare"-tagged variables with the same name as global variables will raise an error with no-shadow even though they are types. This is a false positive due to the "declare" variables being interpreted as values during analysis. This also happens most frequently in definition files. Let's change the logic such that "declare"-tagged variables in definition files are ignored under the no-shadow rule. --- packages/eslint-plugin/src/rules/no-shadow.ts | 29 ++- .../tests/rules/no-shadow/no-shadow.test.ts | 171 ++++++++++++++++++ 2 files changed, 199 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index fb4b83b9da9..ff19493953b 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -3,7 +3,7 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { DefinitionType, ScopeType } from '@typescript-eslint/scope-manager'; import { AST_NODE_TYPES, ASTUtils } from '@typescript-eslint/utils'; -import { createRule } from '../util'; +import { createRule, isDefinitionFile } from '../util'; import { isTypeImport } from '../util/isTypeImport'; export type MessageIds = 'noShadow' | 'noShadowGlobal'; @@ -567,6 +567,28 @@ export default createRule({ }; } + /** + * Checks if the initialization of a variable has the declare modifier in a + * definition file. + * @param variable The variable to check. + * @returns Whether or not the initialization of a variable has the declare + * modifier in a definition file. + */ + function isDeclareInDTSFile(variable: TSESLint.Scope.Variable): boolean { + const fileName = context.filename; + if (!isDefinitionFile(fileName)) { + return false; + } + return variable.defs.some(def => { + return ( + (def.type === DefinitionType.Variable && def.parent.declare) || + (def.type === DefinitionType.ClassName && def.node.declare) || + (def.type === DefinitionType.TSEnumName && def.node.declare) || + (def.type === DefinitionType.TSModuleName && def.node.declare) + ); + }); + } + /** * Checks the current context for shadowed variables. * @param scope Fixme @@ -605,6 +627,11 @@ export default createRule({ continue; } + // ignore variables with the declare keyword in .d.ts files + if (isDeclareInDTSFile(variable)) { + continue; + } + // Gets shadowed variable. const shadowed = scope.upper ? ASTUtils.findVariable(scope.upper, variable.name) diff --git a/packages/eslint-plugin/tests/rules/no-shadow/no-shadow.test.ts b/packages/eslint-plugin/tests/rules/no-shadow/no-shadow.test.ts index e4c420092c9..47e6af95b7d 100644 --- a/packages/eslint-plugin/tests/rules/no-shadow/no-shadow.test.ts +++ b/packages/eslint-plugin/tests/rules/no-shadow/no-shadow.test.ts @@ -931,6 +931,48 @@ function foo any>(fn: T, args: any[]) {} }, ], }, + { + code: ` +declare const has = (environment: 'dev' | 'prod' | 'test') => boolean; + `, + errors: [ + { + data: { + name: 'has', + }, + messageId: 'noShadowGlobal', + }, + ], + languageOptions: { + globals: { + has: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +declare const has: (environment: 'dev' | 'prod' | 'test') => boolean; +const fn = (has: string) => {}; + `, + errors: [ + { + data: { + name: 'has', + shadowedColumn: 15, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + filename: 'foo.d.ts', + languageOptions: { + globals: { + has: false, + }, + }, + options: [{ builtinGlobals: true }], + }, ], valid: [ 'function foo any>(arg: T) {}', @@ -1479,5 +1521,134 @@ type A = 1; `, options: [{ hoist: 'functions' }], }, + { + code: ` +declare const foo1: boolean; + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + foo1: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +declare let foo2: boolean; + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + foo2: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +declare var foo3: boolean; + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + foo3: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +function foo4(name: string): void; + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + foo4: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +declare class Foopy1 { + name: string; +} + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + Foopy1: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +declare interface Foopy2 { + name: string; +} + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + Foopy2: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +declare type Foopy3 = { + x: number; +}; + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + Foopy3: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +declare enum Foopy4 { + x, +} + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + Foopy4: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +declare namespace Foopy5 {} + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + Foopy5: false, + }, + }, + options: [{ builtinGlobals: true }], + }, + { + code: ` +declare; +foo5: boolean; + `, + filename: 'baz.d.ts', + languageOptions: { + globals: { + foo5: false, + }, + }, + options: [{ builtinGlobals: true }], + }, ], }); From a237ac2a3d782e27da3368d91a8bca296564c206 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Mon, 3 Feb 2025 10:29:37 -0500 Subject: [PATCH 2/2] Update packages/eslint-plugin/src/rules/no-shadow.ts --- packages/eslint-plugin/src/rules/no-shadow.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index 2275210ce16..fa79c362ec5 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -569,9 +569,6 @@ export default createRule({ /** * Checks if the initialization of a variable has the declare modifier in a * definition file. - * @param variable The variable to check. - * @returns Whether or not the initialization of a variable has the declare - * modifier in a definition file. */ function isDeclareInDTSFile(variable: TSESLint.Scope.Variable): boolean { const fileName = context.filename;