fix(eslint-plugin): [unified-signatures] does not differentiate truly private methods #10806

y-hsgw · 2025-02-06T14:52:24Z

PR Checklist

Addresses an existing open issue: fixes Bug: [unified-signatures] Does not differentiate truly private methods. #10800
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

Fixed the bug described in the title.

🐛

typescript-eslint · 2025-02-06T14:52:27Z

Thanks for the PR, @y-hsgw!

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.

netlify · 2025-02-06T14:52:41Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`15f802f`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/67c256321e565600080d1e3c
😎 Deploy Preview	https://deploy-preview-10806--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 1 from production) Accessibility: 100 (no change from production) Best Practices: 100 (🟢 up 8 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.

nx-cloud · 2025-02-06T14:53:13Z

View your CI Pipeline Execution ↗ for commit 15f802f.

Command	Status	Duration	Result
`nx run-many --target=build --exclude website --...`	✅ Succeeded	5s	View ↗
`nx run-many --target=clean`	✅ Succeeded	10s	View ↗

☁️ Nx Cloud last updated this comment at 2025-03-01 22:25:58 UTC

codecov · 2025-02-06T15:09:01Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.43%. Comparing base (89f0c61) to head (15f802f).
Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #10806   +/-   ##
=======================================
  Coverage   87.43%   87.43%           
=======================================
  Files         468      468           
  Lines       16027    16032    +5     
  Branches     4640     4641    +1     
=======================================
+ Hits        14013    14018    +5     
  Misses       1658     1658           
  Partials      356      356

Flag	Coverage Δ
unittest	`87.43% <100.00%> (+<0.01%)`	⬆️

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

Files with missing lines	Coverage Δ
...ages/eslint-plugin/src/rules/unified-signatures.ts	`86.25% <100.00%> (+0.44%)`	⬆️

ronami

Nice, thanks for pushing this! 👍👍

Requesting changes regarding testing and a possible bug with private and non-private methods with the same name.

packages/eslint-plugin/tests/rules/unified-signatures.test.ts

packages/eslint-plugin/src/rules/unified-signatures.ts

ronami

I apologize it took me a while to re-review this, it looks great, just asking for tiny nit changes 🚀🚀

ronami · 2025-02-28T19:44:27Z

packages/eslint-plugin/src/rules/unified-signatures.ts

@@ -616,7 +616,15 @@ function getOverloadInfo(node: OverloadNode): string {
    default: {
      const { key } = node as MethodDefinition;

-      return isIdentifier(key) ? key.name : (key as TSESTree.Literal).raw;
+      if (isPrivateIdentifier(key)) {
+        return `privateId_${key.name}`;


[Nit]: The returned string mixes camel-case and underscore-case. What do you think about making it consistent (e.g. private_identifier_${key.name})?

I'd personally also use the whole word (identifier) instead of its shorter name (id), but that's just my personal preference, feel free to push me back on this 👍

This is also relevant for the other string returned below.

You're right, it's better to unify and clarify the naming. I've made the corrections with 15f802f.

…to fix/unified-signatures

ronami

Thanks 👍👍

JoshuaKGoldberg

Clever solution 🙂 thanks!

@jfet97

| datasource | package | from | to | | ---------- | -------------------------------- | ------ | ------ | | npm | @typescript-eslint/eslint-plugin | 8.24.0 | 8.26.0 | | npm | @typescript-eslint/parser | 8.24.0 | 8.26.0 | ## [v8.26.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8260-2025-03-03) ##### 🚀 Features - **eslint-plugin:** \[unified-signatures] support ignoring overload signatures with different JSDoc comments ([#10781](typescript-eslint/typescript-eslint#10781)) - **eslint-plugin:** \[explicit-module-boundary-types] add an option to ignore overload implementations ([#10889](typescript-eslint/typescript-eslint#10889)) - **eslint-plugin:** \[no-unused-var] handle implicit exports in declaration files ([#10714](typescript-eslint/typescript-eslint#10714)) - support TypeScript 5.8 ([#10903](typescript-eslint/typescript-eslint#10903)) - **eslint-plugin:** \[no-unnecessary-type-parameters] special case tuples and parameter location arrays as single-use ([#9536](typescript-eslint/typescript-eslint#9536)) ##### 🩹 Fixes - **eslint-plugin:** \[no-unnecessary-type-assertion] handle unknown ([#10875](typescript-eslint/typescript-eslint#10875)) - **eslint-plugin:** \[no-invalid-void-type] report `accessor` properties with an invalid `void` type ([#10864](typescript-eslint/typescript-eslint#10864)) - **eslint-plugin:** \[unified-signatures] does not differentiate truly private methods ([#10806](typescript-eslint/typescript-eslint#10806)) ##### ❤️ Thank You - Andrea Simone Costa [@jfet97](https://github.com/jfet97) - Dirk Luijk [@dirkluijk](https://github.com/dirkluijk) - Ronen Amiel - YeonJuan [@yeonjuan](https://github.com/yeonjuan) - 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. ## [v8.25.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8250-2025-02-24) ##### 🚀 Features - **eslint-plugin:** \[no-misused-spread] add suggestions ([#10719](typescript-eslint/typescript-eslint#10719)) ##### 🩹 Fixes - **eslint-plugin:** \[prefer-nullish-coalescing] report on chain expressions in a ternary ([#10708](typescript-eslint/typescript-eslint#10708)) - **eslint-plugin:** \[no-deprecated] report usage of deprecated private identifiers ([#10844](typescript-eslint/typescript-eslint#10844)) - **eslint-plugin:** \[unified-signatures] handle getter-setter ([#10818](typescript-eslint/typescript-eslint#10818)) ##### ❤️ Thank You - Olivier Zalmanski [@OlivierZal](https://github.com/OlivierZal) - Ronen Amiel - YeonJuan [@yeonjuan](https://github.com/yeonjuan) 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. ## [v8.24.1](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8241-2025-02-17) ##### 🩹 Fixes - **eslint-plugin:** \[class-methods-use-this] check `accessor` methods with a function initializer ([#10796](typescript-eslint/typescript-eslint#10796)) - **eslint-plugin:** \[no-misused-promises] don't report on `static` `accessor` properties ([#10814](typescript-eslint/typescript-eslint#10814)) - **eslint-plugin:** \[no-deprecated] don't report on deprecated `accessor` property declaration ([#10813](typescript-eslint/typescript-eslint#10813)) - **eslint-plugin:** \[explicit-member-accessibility] check `accessor` class properties for missing accessibility modifier ([#10805](typescript-eslint/typescript-eslint#10805)) - **eslint-plugin:** \[explicit-module-boundary-types] check `accessor` class properties with a function initializer ([#10804](typescript-eslint/typescript-eslint#10804)) - **eslint-plugin:** \[prefer-return-this-type] check `accessor` properties with a function initializer ([#10794](typescript-eslint/typescript-eslint#10794)) - **eslint-plugin:** \[consistent-generic-constructors] check `accessor` class properties ([#10789](typescript-eslint/typescript-eslint#10789)) - **eslint-plugin:** \[no-unsafe-assignment] report on an `any` value assigned as an initializer of an `accessor` property ([#10785](typescript-eslint/typescript-eslint#10785)) - **eslint-plugin:** \[no-unnecessary-template-expression] ignore enum and enum members ([#10782](typescript-eslint/typescript-eslint#10782)) - **eslint-plugin:** \[no-inferrable-types] handle accessor ([#10780](typescript-eslint/typescript-eslint#10780)) ##### ❤️ Thank You - Ronen Amiel - YeonJuan 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 added 2 commits February 6, 2025 23:50

fix: support private methods

634960f

test: add test

08e1c24

y-hsgw changed the title ~~fix(eslint-plugin): [unified-signatures] does not differentiate truly private methods.~~ fix(eslint-plugin): [unified-signatures] does not differentiate truly private methods Feb 6, 2025

fix: fix lint

2b7c1b9

ronami requested changes Feb 8, 2025

View reviewed changes

packages/eslint-plugin/tests/rules/unified-signatures.test.ts Outdated Show resolved Hide resolved

packages/eslint-plugin/src/rules/unified-signatures.ts Outdated Show resolved Hide resolved

ronami added the awaiting response Issues waiting for a reply from the OP or another party label Feb 8, 2025

y-hsgw added 2 commits February 9, 2025 14:08

test: update test

d31b107

fix: allow same name for private and non-private methods

bf38dd1

y-hsgw requested a review from ronami February 9, 2025 05:28

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Feb 9, 2025

ronami requested changes Feb 28, 2025

View reviewed changes

ronami added the awaiting response Issues waiting for a reply from the OP or another party label Feb 28, 2025

y-hsgw added 2 commits March 1, 2025 09:32

Merge branch 'main' of https://github.com/y-hsgw/typescript-eslint in…

060154e

…to fix/unified-signatures

fix: update prefix

15f802f

y-hsgw requested a review from ronami March 1, 2025 00:36

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Mar 1, 2025

ronami approved these changes Mar 1, 2025

View reviewed changes

ronami added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Mar 1, 2025

JoshuaKGoldberg approved these changes Mar 2, 2025

View reviewed changes

JoshuaKGoldberg merged commit a6feb59 into typescript-eslint:main Mar 2, 2025
94 checks passed

y-hsgw deleted the fix/unified-signatures branch March 2, 2025 06:04

Morinyo1907 mentioned this pull request Mar 3, 2025

0x287BC6b58d629e143D436a5e94D81fabC59d4b02 Morinyo1907/0x7Bb8B7Eb88EDCd255427094e58ADD138BA52B177#8

Closed

github-actions bot locked as resolved and limited conversation to collaborators Mar 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(eslint-plugin): [unified-signatures] does not differentiate truly private methods #10806

fix(eslint-plugin): [unified-signatures] does not differentiate truly private methods #10806

y-hsgw commented Feb 6, 2025 •

edited

Loading

typescript-eslint bot commented Feb 6, 2025

netlify bot commented Feb 6, 2025 •

edited

Loading

nx-cloud bot commented Feb 6, 2025 •

edited

Loading

codecov bot commented Feb 6, 2025 •

edited

Loading

ronami left a comment

ronami left a comment

ronami Feb 28, 2025

y-hsgw Mar 1, 2025

ronami left a comment

JoshuaKGoldberg left a comment

fix(eslint-plugin): [unified-signatures] does not differentiate truly private methods #10806

fix(eslint-plugin): [unified-signatures] does not differentiate truly private methods #10806

Conversation

y-hsgw commented Feb 6, 2025 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Feb 6, 2025

netlify bot commented Feb 6, 2025 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Feb 6, 2025 • edited Loading

codecov bot commented Feb 6, 2025 • edited Loading

Codecov Report

ronami left a comment

Choose a reason for hiding this comment

ronami left a comment

Choose a reason for hiding this comment

ronami Feb 28, 2025

Choose a reason for hiding this comment

y-hsgw Mar 1, 2025

Choose a reason for hiding this comment

ronami left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

y-hsgw commented Feb 6, 2025 •

edited

Loading

netlify bot commented Feb 6, 2025 •

edited

Loading

nx-cloud bot commented Feb 6, 2025 •

edited

Loading

codecov bot commented Feb 6, 2025 •

edited

Loading