fix(eslint-plugin): [no-unnecessary-type-parameters] should parenthesize type in suggestion fixer if necessary

[y-hsgw]

PR Checklist

• Addresses an existing open issue: fixes Bug: [no-unnecessary-type-parameters] should parenthesize type in suggestion fixer if necessary #10894
• That issue was marked as accepting prs
• Steps in Contributing were taken

Overview

Fixed the bug described in the title.

Please note that in addition to the error examples in the #10894, I have also addressed several related cases.

🐛

[typescript-eslint]

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]

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	4cb399a
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/67cef2577e601c0008f7caee
😎 Deploy Preview	https://deploy-preview-10907--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 100 (🟢 up 2 from production) Accessibility: 100 (no change from production) Best Practices: 100 (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.

[nx-cloud]

View your CI Pipeline Execution ↗ for commit 4cb399a.

Command	Status	Duration	Result
nx run eslint-plugin:test -- --coverage	✅ Succeeded	6m 14s	View ↗
nx test eslint-plugin	✅ Succeeded	6m 14s	View ↗
nx test eslint-plugin --coverage=false	✅ Succeeded	6m 29s	View ↗
nx run utils:test -- --coverage	✅ Succeeded	<1s	View ↗
nx run visitor-keys:test -- --coverage	✅ Succeeded	<1s	View ↗
nx run types:build	✅ Succeeded	<1s	View ↗
nx run-many --target=build --exclude website --...	✅ Succeeded	18s	View ↗
nx run type-utils:test -- --coverage	✅ Succeeded	<1s	View ↗
Additional runs (25)	✅ Succeeded	...	View ↗

---

☁️ Nx Cloud last updated this comment at 2025-03-10 14:39:15 UTC

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.59%. Comparing base (3c7b8ff) to head (4cb399a).
Report is 22 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10907      +/-   ##
==========================================
+ Coverage   87.43%   87.59%   +0.15%     
==========================================
  Files         468      470       +2     
  Lines       16040    16101      +61     
  Branches     4649     4671      +22     
==========================================
+ Hits        14025    14103      +78     
+ Misses       1658     1642      -16     
+ Partials      357      356       -1     

Flag	Coverage Δ
unittest	87.59% <100.00%> (+0.15%)	⬆️

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

Files with missing lines	Coverage Δ
...plugin/src/rules/no-unnecessary-type-parameters.ts	92.44% <100.00%> (+0.27%)	⬆️

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[JoshuaKGoldberg]

Thanks! 👍

Wait, was too optimistic - taking a closer look..

[JoshuaKGoldberg]

Good stuff! I think just a bit of extra testing & code flow trimming are needed now. 🔥

[JoshuaKGoldberg]

There's a missing code coverage point here, as reported by Codecov. Removing the ? doesn't fail the test.

It looks to me like another example of #6255 / #10682 where the AST types for parent nodes aren't precise enough to know the grandparent of an identifier is defined. If that's the case we'll want a ! type assertion and a followup issue. Or, if not, there's a missing unit test for a case that could be hit. Could you please take a look?

[y-hsgw]

Based on my investigation, it seems likely that when reference.isTypeReference is true, the type of reference.identifier.parent is TSESTree.TSTypeReference.
I agree that a follow-up might be necessary, as you suggested.
I have added the ! type assertion as well. → 91dd50d

This is just a quick idea, but it seems like we might need some form of type narrowing:

  public isTypeReference(): this is Reference & {
    identifier: (TSESTree.Identifier | TSESTree.JSXIdentifier) & {
      parent: TSESTree.TSTypeReference;
    };
  } {
    return (this.#referenceType & ReferenceTypeFlag.Type) !== 0;
  }

If there's no misunderstanding regarding the above, should I create an issue?

[JoshuaKGoldberg]

[Testing] Just to be thorough, could you add a few tests around:

• Combination | union and & intersection types
• Types with unnecessary parenthesis around them (which might need noFormat)

They all worked when I tried them out, so this is just for future-proofing.

[y-hsgw]

I added in 4cb399a!