Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
fix(eslint-plugin): [consistent-indexed-object-style] don't report on indirect circular references#10537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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 onhttps://opencollective.com/typescript-eslint. |
netlifybot commentedDec 21, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site configuration. |
nx-cloudbot commentedDec 21, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
View yourCI Pipeline Execution ↗ for commitc81fe19.
☁️Nx Cloud last updated this comment at |
| visited.add(node); | ||
| switch (node.type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I went over the possibilities and I think I got them all, though it's possible I missed some 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is pretty thorough and for a not-very-common case. I'm never surprised by edge cases popping up in"check all kinds of nodes/types" logic, but would be surprised if many came out of this.
codecovbot commentedDec 21, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #10537 +/- ##==========================================+ Coverage 86.83% 86.86% +0.02%========================================== Files 445 445 Lines 15424 15456 +32 Branches 4497 4503 +6 ==========================================+ Hits 13394 13426 +32 Misses 1675 1675 Partials 355 355
Flags with carried forward coverage won't be shown.Click here to find out more.
|
| // Workaround for infinite type recursion | ||
| // Also, https://github.com/typescript-eslint/typescript-eslint/issues/7863 | ||
| // eslint-disable-next-line @typescript-eslint/consistent-indexed-object-style |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is no longer needed, dog-fooding 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Whoohoo! Glad to finally see this fixed, thanks! 🥳
7eba36e intotypescript-eslint:mainUh oh!
There was an error while loading.Please reload this page.
##### [v8.19.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8190-2024-12-30)##### 🚀 Features- **eslint-plugin:** \[strict-boolean-expressions] check array predicate functions' return statements ([#10106](typescript-eslint/typescript-eslint#10106))##### 🩹 Fixes- **eslint-plugin:** \[member-ordering] ignore method overloading ([#10536](typescript-eslint/typescript-eslint#10536))- **eslint-plugin:** \[consistent-indexed-object-style] don't report on indirect circular references ([#10537](typescript-eslint/typescript-eslint#10537))- **eslint-plugin:** \[array-type] autofix with conditional types needs parentheses ([#10522](typescript-eslint/typescript-eslint#10522))- **eslint-plugin:** add getConstraintInfo to handle generic constraints better ([#10496](typescript-eslint/typescript-eslint#10496))##### ❤️ Thank You- Karl Werner- Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger)- 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.
… indirect circular references (typescript-eslint#10537)* initial implementation (squashed)* revert unrelated changes* remove now redundant eslint-disable comment* slight formatting change* remove unrelated changes* add a test for missing coverage* add indirect union recursive test* remove additional unnecessary comments
| datasource | package | from | to || ---------- | -------------------------------- | ------ | ------ || npm | @typescript-eslint/eslint-plugin | 8.18.2 | 8.19.0 || npm | @typescript-eslint/parser | 8.18.2 | 8.19.0 |## [v8.19.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8190-2024-12-30)##### 🚀 Features- **eslint-plugin:** \[strict-boolean-expressions] check array predicate functions' return statements ([#10106](typescript-eslint/typescript-eslint#10106))##### 🩹 Fixes- **eslint-plugin:** \[member-ordering] ignore method overloading ([#10536](typescript-eslint/typescript-eslint#10536))- **eslint-plugin:** \[consistent-indexed-object-style] don't report on indirect circular references ([#10537](typescript-eslint/typescript-eslint#10537))- **eslint-plugin:** \[array-type] autofix with conditional types needs parentheses ([#10522](typescript-eslint/typescript-eslint#10522))- **eslint-plugin:** add getConstraintInfo to handle generic constraints better ([#10496](typescript-eslint/typescript-eslint#10496))##### ❤️ Thank You- Karl Werner- Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger)- 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.
Uh oh!
There was an error while loading.Please reload this page.
PR Checklist
Overview
This PR tackles#7863 and replaces the current logic for detecting circular references with one that also checks for indirect cycles:
The PR also flags some recursive patterns that are valid as a type reference (and can be fixed into a
Record<...>form), which currently the rule doesn't flag (playground link). There is no open issue for this, but it made sense to me. This means the rule will flag certain recursive patterns that it currently doesn't.