fix(eslint-plugin): [no-shadow] report correctly on parameters of functions declared with the `declare` keyword #10543

ronami · 2024-12-23T19:28:54Z

PR Checklist

Addresses an existing open issue: fixes Bug: [no-shadow] inconsistent reports on function parameter names inside ambient declarations clashing with real variable declarations #10539
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

This PR addresses #10539 and adjusts the rule to include the following ways of defining functions (and their parameters) behind the ignoreFunctionTypeParameterNameValueShadow flag:

const arg1 = 0;
declare function test1(arg1: string): typeof arg1;

const arg2 = 0;
declare class Test3 {
  p2(arg2: string): typeof arg2;
}

typescript-eslint · 2024-12-23T19:28:58Z

Thanks for the PR, @ronami!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

nx-cloud · 2024-12-23T19:29:46Z

View your CI Pipeline Execution ↗ for commit a11528b.

Command	Status	Duration	Result
`nx test eslint-plugin --coverage=false`	✅ Succeeded	7m 17s	View ↗
`nx test eslint-plugin`	✅ Succeeded	5m 56s	View ↗
`nx test type-utils --coverage=false`	✅ Succeeded	25s	View ↗
`nx test visitor-keys --coverage=false`	✅ Succeeded	3s	View ↗
`nx run types:build`	✅ Succeeded	<1s	View ↗
`nx test utils --coverage=false`	✅ Succeeded	6s	View ↗
`nx test typescript-estree --coverage=false`	✅ Succeeded	1m 12s	View ↗
`nx run-many --target=build --exclude website --...`	✅ Succeeded	1m 15s	View ↗
`Additional runs (24)`	✅ Succeeded	...	View ↗

☁️ Nx Cloud last updated this comment at 2024-12-30 17:20:15 UTC

netlify · 2024-12-23T19:30:13Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`a11528b`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/6772d1da78251f0008cb1ad4
😎 Deploy Preview	https://deploy-preview-10543--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (🟢 up 26 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov · 2024-12-23T19:45:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.90%. Comparing base (5a39e0c) to head (a11528b).
Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10543      +/-   ##
==========================================
+ Coverage   86.84%   86.90%   +0.05%     
==========================================
  Files         445      446       +1     
  Lines       15427    15497      +70     
  Branches     4497     4514      +17     
==========================================
+ Hits        13397    13467      +70     
  Misses       1675     1675              
  Partials      355      355

Flag	Coverage Δ
unittest	`86.90% <ø> (+0.05%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/eslint-plugin/src/rules/no-shadow.ts	`80.45% <ø> (ø)`

... and 9 files with indirect coverage changes

packages/eslint-plugin/src/rules/no-shadow.ts

…eValueShadow' flag

…declarations

kirkwaiblinger

Looks good! 🎉🎄

yeonjuan · 2024-12-30T16:08:39Z

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,


Hi @ronami IMO, TSConstructSignatureDeclaration and TSConstructorType should also be added, what do you think?

playground on PR

// "ignoreFunctionTypeParameterNameValueShadow": true const arg = 0; declare const Foo: { foo: (arg: number) => void; new(arg: number): void; // 'arg' is already declared in the upper scope on line 2 column 7. } type Bar = new (arg: number) => void; // 'arg' is already declared in the upper scope on line 2 column 7.

Oh, definitely. Thank you @yeonjuan for catching this 👍

@ronami oops. I just edited the comment. I think we should add TSConstructorType also

Oh, I just went over the list of nodes to make sure I didn't miss anything 🙈

Thanks!

LGTM! Thanks! 👍

Thanks @yeonjuan for the catch!

JoshuaKGoldberg

🆒

@y-hsgw

##### [v8.19.1](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8191-2025-01-06) ##### 🩹 Fixes - **eslint-plugin:** \[no-unnecessary-type-assertion] handle literal types ([#10523](typescript-eslint/typescript-eslint#10523)) - **eslint-plugin:** \[no-deprecated] doesn't report on shorthand property in an object expression ([#10550](typescript-eslint/typescript-eslint#10550)) - **eslint-plugin:** \[strict-boolean-expressions] remove remaining (unsafe) autofixes ([#10548](typescript-eslint/typescript-eslint#10548)) - **eslint-plugin:** \[no-shadow] report correctly on parameters of functions declared with the `declare` keyword ([#10543](typescript-eslint/typescript-eslint#10543)) - **eslint-plugin:** \[no-base-to-string] check array generic type ([#10437](typescript-eslint/typescript-eslint#10437)) ##### ❤️ Thank You - Kirk Waiblinger - mdm317 - Ronen Amiel - Yukihiro Hasegawa [@y-hsgw](https://github.com/y-hsgw) You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.

@y-hsgw

| datasource | package | from | to | | ---------- | -------------------------------- | ------ | ------ | | npm | @typescript-eslint/eslint-plugin | 8.19.0 | 8.19.1 | | npm | @typescript-eslint/parser | 8.19.0 | 8.19.1 | ## [v8.19.1](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8191-2025-01-06) ##### 🩹 Fixes - **eslint-plugin:** \[no-unnecessary-type-assertion] handle literal types ([#10523](typescript-eslint/typescript-eslint#10523)) - **eslint-plugin:** \[no-deprecated] doesn't report on shorthand property in an object expression ([#10550](typescript-eslint/typescript-eslint#10550)) - **eslint-plugin:** \[strict-boolean-expressions] remove remaining (unsafe) autofixes ([#10548](typescript-eslint/typescript-eslint#10548)) - **eslint-plugin:** \[no-shadow] report correctly on parameters of functions declared with the `declare` keyword ([#10543](typescript-eslint/typescript-eslint#10543)) - **eslint-plugin:** \[no-base-to-string] check array generic type ([#10437](typescript-eslint/typescript-eslint#10437)) ##### ❤️ Thank You - Kirk Waiblinger - mdm317 - Ronen Amiel - Yukihiro Hasegawa [@y-hsgw](https://github.com/y-hsgw) You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.

initial implementation

a46ee96

ronami changed the title ~~fix(eslint-plugin): [no-shadow] don't report on parameters of function declaration or function/method overloads~~ fix(eslint-plugin): [no-shadow] don't report on parameters of function declaration or function/method overload declarations Dec 23, 2024

filter out only parameters

a94f1a7

ronami changed the title ~~fix(eslint-plugin): [no-shadow] don't report on parameters of function declaration or function/method overload declarations~~ fix(eslint-plugin): [no-shadow] don't report on parameters of type-only function declarations Dec 23, 2024

ronami added 2 commits December 23, 2024 22:05

remove redundant test

d1b6320

simplify test

dca1dc4

ronami marked this pull request as ready for review December 23, 2024 20:24

kirkwaiblinger reviewed Dec 23, 2024

View reviewed changes

packages/eslint-plugin/src/rules/no-shadow.ts Outdated Show resolved Hide resolved

ronami added 2 commits December 24, 2024 17:32

fix the implementation to respect the 'ignoreFunctionTypeParameterNam…

ccc5073

…eValueShadow' flag

Merge branch 'main' into no-shadow-dont-report-parameter-of-function-…

185bd8f

…declarations

ronami requested a review from kirkwaiblinger December 24, 2024 16:38

ronami changed the title ~~fix(eslint-plugin): [no-shadow] don't report on parameters of type-only function declarations~~ fix(eslint-plugin): [no-shadow] report correctly on parameters of functions declared with the declare keyword Dec 24, 2024

kirkwaiblinger previously approved these changes Dec 25, 2024

View reviewed changes

kirkwaiblinger added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Dec 25, 2024

yeonjuan reviewed Dec 30, 2024

View reviewed changes

add TSConstructSignatureDeclaration

dba147b

ronami dismissed kirkwaiblinger’s stale review via dba147b December 30, 2024 16:47

also TSConstructorType

a11528b

kirkwaiblinger approved these changes Dec 30, 2024

View reviewed changes

JoshuaKGoldberg approved these changes Dec 31, 2024

View reviewed changes

JoshuaKGoldberg merged commit cde2f97 into typescript-eslint:main Dec 31, 2024
62 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Jan 8, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(eslint-plugin): [no-shadow] report correctly on parameters of functions declared with the `declare` keyword #10543

fix(eslint-plugin): [no-shadow] report correctly on parameters of functions declared with the `declare` keyword #10543

ronami commented Dec 23, 2024 •

edited

Loading

typescript-eslint bot commented Dec 23, 2024

nx-cloud bot commented Dec 23, 2024 •

edited

Loading

netlify bot commented Dec 23, 2024 •

edited

Loading

codecov bot commented Dec 23, 2024 •

edited

Loading

kirkwaiblinger left a comment •

edited

Loading

yeonjuan Dec 30, 2024 •

edited

Loading

ronami Dec 30, 2024 •

edited

Loading

yeonjuan Dec 30, 2024 •

edited

Loading

ronami Dec 30, 2024

yeonjuan Dec 30, 2024

kirkwaiblinger Dec 30, 2024

JoshuaKGoldberg left a comment

fix(eslint-plugin): [no-shadow] report correctly on parameters of functions declared with the declare keyword #10543

fix(eslint-plugin): [no-shadow] report correctly on parameters of functions declared with the declare keyword #10543

Conversation

ronami commented Dec 23, 2024 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Dec 23, 2024

nx-cloud bot commented Dec 23, 2024 • edited Loading

netlify bot commented Dec 23, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

codecov bot commented Dec 23, 2024 • edited Loading

Codecov Report

kirkwaiblinger left a comment • edited Loading

Choose a reason for hiding this comment

yeonjuan Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

ronami Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

yeonjuan Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

ronami Dec 30, 2024

Choose a reason for hiding this comment

yeonjuan Dec 30, 2024

Choose a reason for hiding this comment

kirkwaiblinger Dec 30, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

fix(eslint-plugin): [no-shadow] report correctly on parameters of functions declared with the `declare` keyword #10543

fix(eslint-plugin): [no-shadow] report correctly on parameters of functions declared with the `declare` keyword #10543

ronami commented Dec 23, 2024 •

edited

Loading

nx-cloud bot commented Dec 23, 2024 •

edited

Loading

netlify bot commented Dec 23, 2024 •

edited

Loading

codecov bot commented Dec 23, 2024 •

edited

Loading

kirkwaiblinger left a comment •

edited

Loading

yeonjuan Dec 30, 2024 •

edited

Loading

ronami Dec 30, 2024 •

edited

Loading

yeonjuan Dec 30, 2024 •

edited

Loading