From a46ee96b12a94d6f818832c556b887d93cbcadd3 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 23 Dec 2024 19:59:04 +0200 Subject: [PATCH 1/7] initial implementation --- packages/eslint-plugin/src/rules/no-shadow.ts | 15 ++++ .../tests/rules/no-shadow/no-shadow.test.ts | 68 +++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index 6a3605e79b8e..b3fc2222497e 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -274,6 +274,16 @@ export default createRule({ ); } + function isParameterOfFunctionDeclaration( + variable: TSESLint.Scope.Variable, + ): boolean { + return ( + variable.scope.block.type === AST_NODE_TYPES.TSDeclareFunction || + variable.scope.block.type === + AST_NODE_TYPES.TSEmptyBodyFunctionExpression + ); + } + /** * Check if variable name is allowed. * @param variable The variable to check. @@ -564,6 +574,11 @@ export default createRule({ continue; } + // ignore variables of function declarations or function/method overloads + if (isParameterOfFunctionDeclaration(variable)) { + continue; + } + // ignore variables of a class name in the class scope of ClassDeclaration if (isDuplicatedClassNameVariable(variable)) { continue; 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 8b68d4dd08f4..7b91337fcddf 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 @@ -459,6 +459,50 @@ function foo any>(fn: T, args: any[]) {} }, ], }, + { + code: ` +const methodParam = 1; + +class SomeClass { + someMethod(): number; + someMethod(methodParam: boolean): boolean; + someMethod(methodParam?: boolean): boolean | number { + return 10; + } +} + `, + errors: [ + { + data: { + name: 'methodParam', + shadowedColumn: 7, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + }, + { + code: ` +const methodParam = 1; + +function someFunction(): number; +function someFunction(methodParam: boolean): boolean; +function someFunction(methodParam?: boolean): boolean | number { + return 10; +} + `, + errors: [ + { + data: { + name: 'methodParam', + shadowedColumn: 7, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + }, ], valid: [ 'function foo any>(arg: T) {}', @@ -859,5 +903,29 @@ const person = { options: [{ ignoreOnInitialization: true }], }, { code: 'const [x = y => y] = [].map(y => y);' }, + { + code: ` +const functionParam = 1; +declare function someFunction(functionParam: any): void; + `, + }, + { + code: ` +const constructorParam = 1; + +declare class SomeClass { + constructor(constructorParam: number); +} + `, + }, + { + code: ` +const functionParam = 1; + +declare namespace myLib { + function someFunction(functionParam: string): string; +} + `, + }, ], }); From a94f1a702eaeba6765b82b641769c3b7d58edd80 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 23 Dec 2024 21:54:41 +0200 Subject: [PATCH 2/7] filter out only parameters --- packages/eslint-plugin/src/rules/no-shadow.ts | 11 ++--- .../tests/rules/no-shadow/no-shadow.test.ts | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index b3fc2222497e..ce6a1d81afe7 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -274,13 +274,14 @@ export default createRule({ ); } - function isParameterOfFunctionDeclaration( + function isParameterOfTypeOnlyFunctionDeclaration( variable: TSESLint.Scope.Variable, ): boolean { return ( - variable.scope.block.type === AST_NODE_TYPES.TSDeclareFunction || - variable.scope.block.type === - AST_NODE_TYPES.TSEmptyBodyFunctionExpression + (variable.scope.block.type === AST_NODE_TYPES.TSDeclareFunction || + variable.scope.block.type === + AST_NODE_TYPES.TSEmptyBodyFunctionExpression) && + variable.defs[0].type === DefinitionType.Parameter ); } @@ -575,7 +576,7 @@ export default createRule({ } // ignore variables of function declarations or function/method overloads - if (isParameterOfFunctionDeclaration(variable)) { + if (isParameterOfTypeOnlyFunctionDeclaration(variable)) { continue; } 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 7b91337fcddf..1b0b41c033ef 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 @@ -503,6 +503,48 @@ function someFunction(methodParam?: boolean): boolean | number { }, ], }, + { + code: ` +const methodParam = 1; + +function someFunction(): number; +function someFunction([methodParam]: [boolean]): boolean; +function someFunction(methodParam?: [boolean]): boolean | number { + return 10; +} + `, + errors: [ + { + data: { + name: 'methodParam', + shadowedColumn: 7, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + }, + { + code: ` +type T = 1; + +function someFunction(): number; +function someFunction(methodParam: boolean): T extends true ? 1 : 2; +function someFunction(methodParam?: boolean): boolean | number { + return 10; +} + `, + errors: [ + { + data: { + name: 'T', + shadowedColumn: 6, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + }, ], valid: [ 'function foo any>(arg: T) {}', From d1b63203b07a7c361fa5f347945973a19c0640e3 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 23 Dec 2024 22:05:29 +0200 Subject: [PATCH 3/7] remove redundant test --- .../tests/rules/no-shadow/no-shadow.test.ts | 21 ------------------- 1 file changed, 21 deletions(-) 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 1b0b41c033ef..8073e840475a 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 @@ -505,27 +505,6 @@ function someFunction(methodParam?: boolean): boolean | number { }, { code: ` -const methodParam = 1; - -function someFunction(): number; -function someFunction([methodParam]: [boolean]): boolean; -function someFunction(methodParam?: [boolean]): boolean | number { - return 10; -} - `, - errors: [ - { - data: { - name: 'methodParam', - shadowedColumn: 7, - shadowedLine: 2, - }, - messageId: 'noShadow', - }, - ], - }, - { - code: ` type T = 1; function someFunction(): number; From dca1dc4eafb361d6346bb708504316a8d51f95d6 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 23 Dec 2024 22:06:14 +0200 Subject: [PATCH 4/7] simplify test --- packages/eslint-plugin/tests/rules/no-shadow/no-shadow.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8073e840475a..fa62ae142e43 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 @@ -508,7 +508,7 @@ function someFunction(methodParam?: boolean): boolean | number { type T = 1; function someFunction(): number; -function someFunction(methodParam: boolean): T extends true ? 1 : 2; +function someFunction(methodParam: boolean): T; function someFunction(methodParam?: boolean): boolean | number { return 10; } From ccc5073a91ead9d4e01be7cf641cd0375073d32e Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Tue, 24 Dec 2024 17:32:28 +0200 Subject: [PATCH 5/7] fix the implementation to respect the 'ignoreFunctionTypeParameterNameValueShadow' flag --- packages/eslint-plugin/src/rules/no-shadow.ts | 18 +- .../tests/rules/no-shadow/no-shadow.test.ts | 201 ++++++++++-------- 2 files changed, 114 insertions(+), 105 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index ce6a1d81afe7..e35fb6fdb67b 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -22,6 +22,8 @@ const allowedFunctionVariableDefTypes = new Set([ AST_NODE_TYPES.TSCallSignatureDeclaration, AST_NODE_TYPES.TSFunctionType, AST_NODE_TYPES.TSMethodSignature, + AST_NODE_TYPES.TSEmptyBodyFunctionExpression, + AST_NODE_TYPES.TSDeclareFunction, ]); export default createRule({ @@ -274,17 +276,6 @@ export default createRule({ ); } - function isParameterOfTypeOnlyFunctionDeclaration( - variable: TSESLint.Scope.Variable, - ): boolean { - return ( - (variable.scope.block.type === AST_NODE_TYPES.TSDeclareFunction || - variable.scope.block.type === - AST_NODE_TYPES.TSEmptyBodyFunctionExpression) && - variable.defs[0].type === DefinitionType.Parameter - ); - } - /** * Check if variable name is allowed. * @param variable The variable to check. @@ -575,11 +566,6 @@ export default createRule({ continue; } - // ignore variables of function declarations or function/method overloads - if (isParameterOfTypeOnlyFunctionDeclaration(variable)) { - continue; - } - // ignore variables of a class name in the class scope of ClassDeclaration if (isDuplicatedClassNameVariable(variable)) { continue; 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 fa62ae142e43..b571fa9b8003 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 @@ -203,6 +203,82 @@ interface Test { }, { code: ` +const arg = 0; + +declare function test(arg: string): typeof arg; + `, + errors: [ + { + data: { + name: 'arg', + shadowedColumn: 7, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + options: [{ ignoreFunctionTypeParameterNameValueShadow: false }], + }, + { + code: ` +const arg = 0; + +declare const test: (arg: string) => typeof arg; + `, + errors: [ + { + data: { + name: 'arg', + shadowedColumn: 7, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + options: [{ ignoreFunctionTypeParameterNameValueShadow: false }], + }, + { + code: ` +const arg = 0; + +declare class Test { + p1(arg: string): typeof arg; +} + `, + errors: [ + { + data: { + name: 'arg', + shadowedColumn: 7, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + options: [{ ignoreFunctionTypeParameterNameValueShadow: false }], + }, + { + code: ` +const arg = 0; + +declare namespace Lib { + function test(arg: string): typeof arg; +} + `, + errors: [ + { + data: { + name: 'arg', + shadowedColumn: 7, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + options: [{ ignoreFunctionTypeParameterNameValueShadow: false }], + }, + { + code: ` import type { foo } from './foo'; function doThing(foo: number) {} `, @@ -459,71 +535,6 @@ function foo any>(fn: T, args: any[]) {} }, ], }, - { - code: ` -const methodParam = 1; - -class SomeClass { - someMethod(): number; - someMethod(methodParam: boolean): boolean; - someMethod(methodParam?: boolean): boolean | number { - return 10; - } -} - `, - errors: [ - { - data: { - name: 'methodParam', - shadowedColumn: 7, - shadowedLine: 2, - }, - messageId: 'noShadow', - }, - ], - }, - { - code: ` -const methodParam = 1; - -function someFunction(): number; -function someFunction(methodParam: boolean): boolean; -function someFunction(methodParam?: boolean): boolean | number { - return 10; -} - `, - errors: [ - { - data: { - name: 'methodParam', - shadowedColumn: 7, - shadowedLine: 2, - }, - messageId: 'noShadow', - }, - ], - }, - { - code: ` -type T = 1; - -function someFunction(): number; -function someFunction(methodParam: boolean): T; -function someFunction(methodParam?: boolean): boolean | number { - return 10; -} - `, - errors: [ - { - data: { - name: 'T', - shadowedColumn: 6, - shadowedLine: 2, - }, - messageId: 'noShadow', - }, - ], - }, ], valid: [ 'function foo any>(arg: T) {}', @@ -682,6 +693,42 @@ const arg = 0; interface Test { p1(arg: string): typeof arg; +} + `, + options: [{ ignoreFunctionTypeParameterNameValueShadow: true }], + }, + { + code: ` +const arg = 0; + +declare function test(arg: string): typeof arg; + `, + options: [{ ignoreFunctionTypeParameterNameValueShadow: true }], + }, + { + code: ` +const arg = 0; + +declare const test: (arg: string) => typeof arg; + `, + options: [{ ignoreFunctionTypeParameterNameValueShadow: true }], + }, + { + code: ` +const arg = 0; + +declare class Test { + p1(arg: string): typeof arg; +} + `, + options: [{ ignoreFunctionTypeParameterNameValueShadow: true }], + }, + { + code: ` +const arg = 0; + +declare namespace Lib { + function test(arg: string): typeof arg; } `, options: [{ ignoreFunctionTypeParameterNameValueShadow: true }], @@ -924,29 +971,5 @@ const person = { options: [{ ignoreOnInitialization: true }], }, { code: 'const [x = y => y] = [].map(y => y);' }, - { - code: ` -const functionParam = 1; -declare function someFunction(functionParam: any): void; - `, - }, - { - code: ` -const constructorParam = 1; - -declare class SomeClass { - constructor(constructorParam: number); -} - `, - }, - { - code: ` -const functionParam = 1; - -declare namespace myLib { - function someFunction(functionParam: string): string; -} - `, - }, ], }); From dba147b546c56c10f68cad2d30901e96f2b53244 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 30 Dec 2024 18:46:22 +0200 Subject: [PATCH 6/7] add TSConstructSignatureDeclaration --- packages/eslint-plugin/src/rules/no-shadow.ts | 1 + .../tests/rules/no-shadow/no-shadow.test.ts | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index e35fb6fdb67b..f7baa0064038 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -24,6 +24,7 @@ const allowedFunctionVariableDefTypes = new Set([ AST_NODE_TYPES.TSMethodSignature, AST_NODE_TYPES.TSEmptyBodyFunctionExpression, AST_NODE_TYPES.TSDeclareFunction, + AST_NODE_TYPES.TSConstructSignatureDeclaration, ]); export default createRule({ 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 b571fa9b8003..83da3489e658 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 @@ -261,6 +261,26 @@ declare class Test { code: ` const arg = 0; +declare const Test: { + new (arg: string): typeof arg; +}; + `, + errors: [ + { + data: { + name: 'arg', + shadowedColumn: 7, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + options: [{ ignoreFunctionTypeParameterNameValueShadow: false }], + }, + { + code: ` +const arg = 0; + declare namespace Lib { function test(arg: string): typeof arg; } @@ -727,6 +747,16 @@ declare class Test { code: ` const arg = 0; +declare const Test: { + new (arg: string): typeof arg; +}; + `, + options: [{ ignoreFunctionTypeParameterNameValueShadow: true }], + }, + { + code: ` +const arg = 0; + declare namespace Lib { function test(arg: string): typeof arg; } From a11528bbd9b2cb90fdafe86bf856bfd6e8a1accb Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 30 Dec 2024 19:01:05 +0200 Subject: [PATCH 7/7] also TSConstructorType --- packages/eslint-plugin/src/rules/no-shadow.ts | 1 + .../tests/rules/no-shadow/no-shadow.test.ts | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts index f7baa0064038..0e1f87149608 100644 --- a/packages/eslint-plugin/src/rules/no-shadow.ts +++ b/packages/eslint-plugin/src/rules/no-shadow.ts @@ -25,6 +25,7 @@ const allowedFunctionVariableDefTypes = new Set([ AST_NODE_TYPES.TSEmptyBodyFunctionExpression, AST_NODE_TYPES.TSDeclareFunction, AST_NODE_TYPES.TSConstructSignatureDeclaration, + AST_NODE_TYPES.TSConstructorType, ]); export default createRule({ 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 83da3489e658..6dd82ef35747 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 @@ -281,6 +281,24 @@ declare const Test: { code: ` const arg = 0; +type Bar = new (arg: number) => typeof arg; + `, + errors: [ + { + data: { + name: 'arg', + shadowedColumn: 7, + shadowedLine: 2, + }, + messageId: 'noShadow', + }, + ], + options: [{ ignoreFunctionTypeParameterNameValueShadow: false }], + }, + { + code: ` +const arg = 0; + declare namespace Lib { function test(arg: string): typeof arg; } @@ -757,6 +775,14 @@ declare const Test: { code: ` const arg = 0; +type Bar = new (arg: number) => typeof arg; + `, + options: [{ ignoreFunctionTypeParameterNameValueShadow: true }], + }, + { + code: ` +const arg = 0; + declare namespace Lib { function test(arg: string): typeof arg; }