fix(eslint-plugin): [sort-type-constituents, sort-type-union-intersection-members] handle some required parentheses cases in the fixer #6118
Conversation
|
Thanks for the PR, @cherryblossom000! 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. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6118 +/- ##
==========================================
- Coverage 91.27% 91.26% -0.01%
==========================================
Files 366 366
Lines 12417 12416 -1
Branches 3631 3632 +1
==========================================
- Hits 11333 11332 -1
Misses 774 774
Partials 310 310
Flags with carried forward coverage won't be shown. Click here to find out more.
|
packages/eslint-plugin/src/rules/sort-type-union-intersection-members.ts
Outdated
Show resolved
Hide resolved
| @@ -183,7 +191,7 @@ export default util.createRule<Options, MessageIds>({ | |||
| function checkSorting( | |||
| node: TSESTree.TSIntersectionType | TSESTree.TSUnionType, | |||
| ): void { | |||
| const sourceOrder = node.types.map(type => { | |||
| const sourceOrder = node.types.map((type): Constituent => { | |||
There was a problem hiding this comment.
Nit: any particular reason to add the return type explicitly, here and in the other file?
There was a problem hiding this comment.
I wanted to make it clear what the Constituent was and where it came from. I ended up getting rid of Constituent when I extracted requiresParentheses into a utility function.
9caa9eb to
d7f2588
Compare
|
The force push is because I rebased with main due to a merge conflict. |
|
I also realised there’s another bug with this rule: type T = (B | C) & Agets fixed to type T = C | A & BThis is because |
| or &There was a problem hiding this comment.
I wonder if we want to leverage TS's utilities here for completeness?
https://github.com/microsoft/TypeScript/blob/1b75edcec60415dfa77be2e04aeafce0efa2222e/src/compiler/factory/parenthesizerRules.ts
We probably can't consume them directly (cos I don't think the util existed in old versions of TS) - but we could probably fork it? Maybe something to do in a separate PR?
PR Checklist
Overview
requiresParenthesesnow returnstruefor union types starting with|and intersection types starting with&.