fix(eslint-plugin): [no-unnecessary-type-constraint] change to suggestion fix, fix multiple trailing comma failures

[Josh-Cena]

PR Checklist

• Addresses an existing open issue: fixes [no-unnecessary-type-constraint] false positive for generic type with default type on arrow function in a JSX/TSX file #4062
• That issue was marked as accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

1. This rule has been moved to suggestion, as it can report false-positives (around {} being the actual constraint)
2. The algorithm for adding trailing commas is improved: it now only adds a trailing comma where there's actual ambiguity, preventing some weird syntax errors being generated.

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 0b69748. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 43 targets

• Node 16.x - nx test @typescript-eslint/eslint-plugin-tslint
• Node 16.x - nx test @typescript-eslint/eslint-plugin
• Node 12.x - nx test @typescript-eslint/eslint-plugin-internal
• Node 12.x - nx test @typescript-eslint/eslint-plugin-tslint
• Node 12.x - nx test @typescript-eslint/eslint-plugin
• Node 12.x - nx test @typescript-eslint/eslint-plugin-tslint
• Node 12.x - nx test @typescript-eslint/eslint-plugin
• Node 14.x - nx test @typescript-eslint/eslint-plugin-tslint
• Node 14.x - nx test @typescript-eslint/eslint-plugin
• Node 12.x - nx run-many --target=build --all --parallel --exclude website
• Node 12.x - nx run-many --target=typecheck --all --parallel
• Node 12.x - nx run-many --target=build --all --parallel --exclude website
• Node 16.x - nx test @typescript-eslint/parser
• Node 16.x - nx test @typescript-eslint/type-utils
• Node 16.x - nx test @typescript-eslint/utils
• Node 16.x - nx test @typescript-eslint/scope-manager
• Node 16.x - nx test @typescript-eslint/visitor-keys
• Node 16.x - nx test @typescript-eslint/typescript-estree
• Node 16.x - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 12.x - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 12.x - nx test @typescript-eslint/parser
• Node 12.x - nx test @typescript-eslint/type-utils
• Node 12.x - nx test @typescript-eslint/utils
• Node 12.x - nx test @typescript-eslint/scope-manager --code-coverage
• Node 12.x - nx test @typescript-eslint/visitor-keys
• Node 12.x - nx test @typescript-eslint/typescript-estree
• Node 12.x - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 12.x - nx test @typescript-eslint/parser
• Node 12.x - nx test @typescript-eslint/type-utils
• Node 12.x - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 12.x - nx test @typescript-eslint/utils
• Node 14.x - nx test @typescript-eslint/parser
• Node 12.x - nx test @typescript-eslint/scope-manager
• Node 12.x - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14.x - nx test @typescript-eslint/type-utils
• Node 12.x - nx test @typescript-eslint/visitor-keys
• Node 14.x - nx test @typescript-eslint/utils
• Node 12.x - nx test @typescript-eslint/typescript-estree
• Node 14.x - nx test @typescript-eslint/scope-manager
• Node 12.x - nx run-many --target=build --all --parallel --exclude website --exclude website
• Node 14.x - nx test @typescript-eslint/visitor-keys
• Node 14.x - nx test @typescript-eslint/typescript-estree
• Node 14.x - nx run-many --target=build --all --parallel --exclude website --exclude website

---

Sent with 💌 from NxCloud.

[typescript-eslint]

Thanks for the PR, @Josh-Cena!

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. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

[netlify]

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	0b69748
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/6274c455af90a9000820e5ca
😎 Deploy Preview	https://deploy-preview-4901--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[Josh-Cena]

I don't know a better way to get the presence of trailing comma...

[JoshuaKGoldberg]

(me neither!)

[JoshuaKGoldberg]

Looks great so far! Posted a suggestion about the whitespace shenanigans, and a request for a bit more testing. But the logic looks good!

[JoshuaKGoldberg]

Super, thanks! ✨

[Josh-Cena]

I'm not sure why the docs test is erroring. I've already removed the fixable icon.

[bradzacher]

I'm not sure why the docs test is erroring. I've already removed the fixable icon.

there are two rules lists that need to be updated.
run yarn generate:rules-lists in the plugin package root to update the tables automatically.

[codecov]

Codecov Report

Merging #4901 (0b69748) into main (47f4605) will increase coverage by 2.49%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4901      +/-   ##
==========================================
+ Coverage   91.76%   94.25%   +2.49%     
==========================================
  Files         226      153      -73     
  Lines       10586     8305    -2281     
  Branches     3280     2702     -578     
==========================================
- Hits         9714     7828    -1886     
+ Misses        591      263     -328     
+ Partials      281      214      -67     

Flag	Coverage Δ
unittest	94.25% <100.00%> (+2.49%)	⬆️

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

Impacted Files	Coverage Δ
...s/eslint-plugin/src/rules/no-restricted-imports.ts	96.42% <100.00%> (-0.07%)	⬇️
...plugin/src/rules/no-unnecessary-type-constraint.ts	88.00% <100.00%> (+3.00%)	⬆️
packages/utils/src/ts-eslint/ESLint.ts
packages/utils/src/ts-eslint/RuleTester.ts
packages/utils/src/ts-eslint/SourceCode.ts
packages/utils/src/eslint-utils/applyDefault.ts
packages/utils/src/ts-eslint-scope/ScopeManager.ts
packages/type-utils/src/isTypeReadonly.ts
...ils/src/ast-utils/eslint-utils/ReferenceTracker.ts
packages/type-utils/src/getContextualType.ts
... and 65 more

[bradzacher]

thanks for this!