fix(eslint-plugin): [consistent-indexed-object-style] don't report on indirect circular references #10537

ronami · 2024-12-21T21:10:45Z

PR Checklist

Addresses an existing open issue: fixes Bug: [consistent-indexed-object-style] Interface with recursive index signature autofixed to Record instead of mapped type #7863
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

This PR tackles #7863 and replaces the current logic for detecting circular references with one that also checks for indirect cycles:

export type ExampleUnion = boolean | number;

export type ExampleRoot = ExampleUnion | ExampleObject;

export interface ExampleObject {
  [key: string]: ExampleRoot;
}

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.

typescript-eslint · 2024-12-21T21:10:48Z

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 on https://opencollective.com/typescript-eslint.

netlify · 2024-12-21T21:11:11Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`c81fe19`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/67674663168e3f00082603ef
😎 Deploy Preview	https://deploy-preview-10537--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 21 from production) Accessibility: 100 (no change from production) Best Practices: 92 (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 · 2024-12-21T21:11:39Z

View your CI Pipeline Execution ↗ for commit c81fe19.

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

☁️ Nx Cloud last updated this comment at 2024-12-22 10:51:08 UTC

ronami · 2024-12-21T21:11:49Z

packages/eslint-plugin/src/rules/consistent-indexed-object-style.ts

+
+  visited.add(node);
+
+  switch (node.type) {


I went over the possibilities and I think I got them all, though it's possible I missed some 🙈

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.

codecov · 2024-12-21T21:29:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.86%. Comparing base (4747299) to head (c81fe19).
Report is 14 commits behind head on main.

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

Flag	Coverage Δ
unittest	`86.86% <100.00%> (+0.02%)`	⬆️

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

Files with missing lines	Coverage Δ
...lugin/src/rules/consistent-indexed-object-style.ts	`98.42% <100.00%> (+0.53%)`	⬆️

ronami · 2024-12-21T22:51:43Z

packages/utils/src/json-schema.ts

-// 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


This is no longer needed, dog-fooding 🎉

JoshuaKGoldberg

Whoohoo! Glad to finally see this fixed, thanks! 🥳

@kirkwaiblinger

##### [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

@kirkwaiblinger

| 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.

initial implementation (squashed)

a0cdea1

ronami commented Dec 21, 2024

View reviewed changes

revert unrelated changes

43464cf

ronami closed this Dec 21, 2024

ronami deleted the consistent-indexed-object-style-recursive branch December 21, 2024 21:24

remove now redundant eslint-disable comment

474df6b

slight formatting change

59a8383

ronami reopened this Dec 21, 2024

ronami added 4 commits December 21, 2024 23:41

remove unrelated changes

9bc7d99

add a test for missing coverage

185b38d

add indirect union recursive test

762fb84

remove additional unnecessary comments

c81fe19

ronami commented Dec 21, 2024

View reviewed changes

ronami marked this pull request as ready for review December 21, 2024 23:06

JoshuaKGoldberg approved these changes Dec 30, 2024

View reviewed changes

JoshuaKGoldberg merged commit 7eba36e into typescript-eslint:main Dec 30, 2024
69 of 71 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Jan 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(eslint-plugin): [consistent-indexed-object-style] don't report on indirect circular references #10537

fix(eslint-plugin): [consistent-indexed-object-style] don't report on indirect circular references #10537

ronami commented Dec 21, 2024 •

edited

Loading

typescript-eslint bot commented Dec 21, 2024

netlify bot commented Dec 21, 2024 •

edited

Loading

nx-cloud bot commented Dec 21, 2024 •

edited

Loading

ronami Dec 21, 2024

JoshuaKGoldberg Dec 30, 2024

codecov bot commented Dec 21, 2024 •

edited

Loading

ronami Dec 21, 2024

JoshuaKGoldberg left a comment

fix(eslint-plugin): [consistent-indexed-object-style] don't report on indirect circular references #10537

fix(eslint-plugin): [consistent-indexed-object-style] don't report on indirect circular references #10537

Conversation

ronami commented Dec 21, 2024 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Dec 21, 2024

netlify bot commented Dec 21, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Dec 21, 2024 • edited Loading

ronami Dec 21, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg Dec 30, 2024

Choose a reason for hiding this comment

codecov bot commented Dec 21, 2024 • edited Loading

Codecov Report

ronami Dec 21, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

ronami commented Dec 21, 2024 •

edited

Loading

netlify bot commented Dec 21, 2024 •

edited

Loading

nx-cloud bot commented Dec 21, 2024 •

edited

Loading

codecov bot commented Dec 21, 2024 •

edited

Loading