feat(eslint-plugin): [consistent-indexed-object-style] report mapped types #10160

kirkwaiblinger · 2024-10-16T22:40:32Z

PR Checklist

Addresses an existing open issue: fixes Enhancement: [consistent-indexed-object-style] forbidden from using the "in" keyword in index signature if Record is preferred #6598
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

This reports on mapped types that can be converted to Records. However, note that not all mapped types can be converted to Records, so the rule only reports when they can be converted.

// can't be converted to record, because `K` is used in the computed type. Not reported.
type Foo1 = {
  [K in T]: K
};

// Can be converted to record. Reported.
type Foo2 = {
  [K in T]: T
}
// fixes to 
type Foo2 = Record<T, T>

// Can be converted to record. Reported.
type WithModifiers = {
  +readonly [key in T]+?: T
}
// fixes to 
type WithModifiers = Readonly<Partial<Record<T, T>>>


// WEIRD CASE: Cannot be converted to Record due to keyof (mapped type syntax preserves readonly modifiers in the keys of T)
type WithKeyof = {
  [K in keyof T]: T
}

// WEIRD CASE: Can be converted to Record due to keyof being parenthesized
type WithKeyof = {
  [K in (keyof T)]: T
}

For more detail on the "weird case", study the following example. (Thanks to @Retsam for the helpful discord conversation in https://discord.com/channels/508357248330760243/508357638677856287/1296364542498177046!)

type MyObject = {
  required: string,
  optional?: number,
}

type BooleanValuesNoParens = {
  [key in keyof MyObject]: boolean
}

// valid
const bv: BooleanValuesNoParens = {
  required: true,
}

type BooleanValuesWithParens = {
  [key in (keyof MyObject)]: boolean
}

// TS ERROR: Property 'optional' is missing
const bv: BooleanValuesWithParens = {
  required: true,
}

This actually has practical application inside our repo!

typescript-eslint/packages/utils/src/ts-eslint/Parser.ts

Lines 38 to 41 in 6250dab

    
           ): { 
        
             // intentionally not using a Record to preserve optionals 
        
             [k in keyof ParseResult]: unknown; 
        
           };

There is one open question: should we report on cases where the -readonly modifier is used? I've gone with the approach for now that we should report it even though we can't provide a fix. Open to differing opinions on this, though.

type PlusReadonly = {
  +readonly [K in T]: T
}
// equivalent to 
type PlusReadonly = Readonly<Record<T, T>>

type PlusOptional = {
  [K in T]+?: T
}
// equivalent to
type PlusOptional = Partial<Record<T, T>>

type MinusOptional = {
  [K in T]-?: T
}
// equivalent to
type MinusOptional = Required<Record<T, T>>


type MinusReadonly = {
  -readonly [K in T]: T
}
// no built-in equivalent :( You can write your own easily though.
type Mutable<T> = {
  -readonly [K in keyof T]: T[K]
}
// then,
type MinusReadonly = Mutable<Record<T, T>>

typescript-eslint · 2024-10-16T22:40:35Z

Thanks for the PR, @kirkwaiblinger!

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-10-16T22:40:52Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`90b9c25`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/67116ce26227610007f7daf8
😎 Deploy Preview	https://deploy-preview-10160--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 98 (🔴 down 1 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 90 (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-10-16T22:41:27Z

☁️ Nx Cloud Report

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

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

codecov · 2024-10-17T08:07:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.17%. Comparing base (7effdea) to head (90b9c25).
Report is 17 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10160      +/-   ##
==========================================
+ Coverage   86.15%   86.17%   +0.02%     
==========================================
  Files         429      429              
  Lines       15005    15029      +24     
  Branches     4353     4360       +7     
==========================================
+ Hits        12927    12951      +24     
  Misses       1729     1729              
  Partials      349      349

Flag	Coverage Δ
unittest	`86.17% <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	`97.59% <100.00%> (+0.98%)`	⬆️
...-plugin/src/rules/restrict-template-expressions.ts	`100.00% <ø> (ø)`
packages/eslint-plugin/src/rules/typedef.ts	`98.61% <ø> (ø)`
...ckages/scope-manager/src/referencer/VisitorBase.ts	`88.23% <ø> (ø)`

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

JoshuaKGoldberg

Looks great to me, thanks! 🚀

Excited to have this extra bit of consistency.

JoshuaKGoldberg · 2024-10-21T14:52:23Z

packages/eslint-plugin/docs/rules/consistent-indexed-object-style.mdx

-TypeScript supports defining arbitrary object keys using an index signature. TypeScript also has a builtin type named `Record` to create an empty object defining only an index signature. For example, the following types are equal:
+TypeScript supports defining arbitrary object keys using an index signature or mapped type.
+TypeScript also has a builtin type named `Record` to create an empty object defining only an index signature.
+For example, the following types are equal:


[Praise] 😄 I like the .\n splitting

Did it just for you 😁

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

@developer-bandi

##### [v8.12.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8120-2024-10-28) ##### 🚀 Features - **eslint-plugin:** \[no-base-to-string] handle String() ([#10005](typescript-eslint/typescript-eslint#10005)) - **eslint-plugin:** \[switch-exhaustiveness-check] add allowDefaultCaseMatchUnionMember option ([#9954](typescript-eslint/typescript-eslint#9954)) - **eslint-plugin:** \[consistent-indexed-object-style] report mapped types ([#10160](typescript-eslint/typescript-eslint#10160)) - **eslint-plugin:** \[prefer-nullish-coalescing] add support for assignment expressions ([#10152](typescript-eslint/typescript-eslint#10152)) ##### ❤️ Thank You - Abraham Guo - Kim Sang Du [@developer-bandi](https://github.com/developer-bandi) - Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger) - 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.

@kirkwaiblinger

| datasource | package | from | to | | ---------- | -------------------------------- | ------ | ------ | | npm | @typescript-eslint/eslint-plugin | 8.11.0 | 8.12.2 | | npm | @typescript-eslint/parser | 8.11.0 | 8.12.2 | ## [v8.12.2](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8122-2024-10-29) ##### 🩹 Fixes - **eslint-plugin:** \[switch-exhaustiveness-check] invert `considerDefaultExhaustiveForUnions` ([#10223](typescript-eslint/typescript-eslint#10223)) ##### ❤️ Thank You - Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger) 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. ## [v8.12.1](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8121-2024-10-28) This was a version bump only for eslint-plugin to align it with other projects, there were no code changes. 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. ## [v8.12.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8120-2024-10-28) ##### 🚀 Features - **eslint-plugin:** \[no-base-to-string] handle String() ([#10005](typescript-eslint/typescript-eslint#10005)) - **eslint-plugin:** \[switch-exhaustiveness-check] add allowDefaultCaseMatchUnionMember option ([#9954](typescript-eslint/typescript-eslint#9954)) - **eslint-plugin:** \[consistent-indexed-object-style] report mapped types ([#10160](typescript-eslint/typescript-eslint#10160)) - **eslint-plugin:** \[prefer-nullish-coalescing] add support for assignment expressions ([#10152](typescript-eslint/typescript-eslint#10152)) ##### ❤️ Thank You - Abraham Guo - Kim Sang Du [@developer-bandi](https://github.com/developer-bandi) - Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger) - 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.

@kirkwaiblinger

| datasource | package | from | to | | ---------- | -------------------------------- | ------ | ------ | | npm | @typescript-eslint/eslint-plugin | 8.11.0 | 8.12.2 | | npm | @typescript-eslint/parser | 8.11.0 | 8.12.2 | ## [v8.12.2](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8122-2024-10-29) ##### 🩹 Fixes - **eslint-plugin:** \[switch-exhaustiveness-check] invert `considerDefaultExhaustiveForUnions` ([#10223](typescript-eslint/typescript-eslint#10223)) ##### ❤️ Thank You - Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger) 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. ## [v8.12.1](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8121-2024-10-28) This was a version bump only for eslint-plugin to align it with other projects, there were no code changes. 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. ## [v8.12.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8120-2024-10-28) ##### 🚀 Features - **eslint-plugin:** \[no-base-to-string] handle String() ([#10005](typescript-eslint/typescript-eslint#10005)) - **eslint-plugin:** \[switch-exhaustiveness-check] add allowDefaultCaseMatchUnionMember option ([#9954](typescript-eslint/typescript-eslint#9954)) - **eslint-plugin:** \[consistent-indexed-object-style] report mapped types ([#10160](typescript-eslint/typescript-eslint#10160)) - **eslint-plugin:** \[prefer-nullish-coalescing] add support for assignment expressions ([#10152](typescript-eslint/typescript-eslint#10152)) ##### ❤️ Thank You - Abraham Guo - Kim Sang Du [@developer-bandi](https://github.com/developer-bandi) - Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger) - 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.

@kirkwaiblinger

| datasource | package | from | to | | ---------- | -------------------------------- | ------ | ------ | | npm | @typescript-eslint/eslint-plugin | 8.11.0 | 8.12.2 | | npm | @typescript-eslint/parser | 8.11.0 | 8.12.2 | ## [v8.12.2](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8122-2024-10-29) ##### 🩹 Fixes - **eslint-plugin:** \[switch-exhaustiveness-check] invert `considerDefaultExhaustiveForUnions` ([#10223](typescript-eslint/typescript-eslint#10223)) ##### ❤️ Thank You - Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger) 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. ## [v8.12.1](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8121-2024-10-28) This was a version bump only for eslint-plugin to align it with other projects, there were no code changes. 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. ## [v8.12.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8120-2024-10-28) ##### 🚀 Features - **eslint-plugin:** \[no-base-to-string] handle String() ([#10005](typescript-eslint/typescript-eslint#10005)) - **eslint-plugin:** \[switch-exhaustiveness-check] add allowDefaultCaseMatchUnionMember option ([#9954](typescript-eslint/typescript-eslint#9954)) - **eslint-plugin:** \[consistent-indexed-object-style] report mapped types ([#10160](typescript-eslint/typescript-eslint#10160)) - **eslint-plugin:** \[prefer-nullish-coalescing] add support for assignment expressions ([#10152](typescript-eslint/typescript-eslint#10152)) ##### ❤️ Thank You - Abraham Guo - Kim Sang Du [@developer-bandi](https://github.com/developer-bandi) - Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger) - 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.

This comment was marked as outdated.

Sign in to view

kirkwaiblinger added 2 commits October 17, 2024 01:40

implementation

d48e76d

fix internal violations

2b090a9

kirkwaiblinger force-pushed the prefer-record-over-in branch from e5a2453 to 2b090a9 Compare October 17, 2024 07:50

kirkwaiblinger marked this pull request as ready for review October 17, 2024 19:55

Merge branch 'main' into prefer-record-over-in

90b9c25

kirkwaiblinger commented Oct 17, 2024

View reviewed changes

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

JoshuaKGoldberg approved these changes Oct 21, 2024

View reviewed changes

JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Oct 21, 2024

JoshuaKGoldberg merged commit 9c956ee into typescript-eslint:main Oct 23, 2024
64 checks passed

kirkwaiblinger deleted the prefer-record-over-in branch October 23, 2024 22:48

Thompson1985 approved these changes Oct 24, 2024

View reviewed changes

JavaScriptBach mentioned this pull request Oct 29, 2024

Bug: [consistent-indexed-object-style] Unsafe autofix with regards to circular references [8.12.1 regression] #10224

Closed

4 tasks

github-actions bot locked as resolved and limited conversation to collaborators Nov 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eslint-plugin): [consistent-indexed-object-style] report mapped types #10160

feat(eslint-plugin): [consistent-indexed-object-style] report mapped types #10160

kirkwaiblinger commented Oct 16, 2024 •

edited

Loading

typescript-eslint bot commented Oct 16, 2024

netlify bot commented Oct 16, 2024 •

edited

Loading

nx-cloud bot commented Oct 16, 2024 •

edited

Loading

This comment was marked as outdated.

codecov bot commented Oct 17, 2024 •

edited

Loading

JoshuaKGoldberg left a comment

JoshuaKGoldberg Oct 21, 2024

kirkwaiblinger Oct 21, 2024

	): {
	// intentionally not using a Record to preserve optionals
	[k in keyof ParseResult]: unknown;
	};

feat(eslint-plugin): [consistent-indexed-object-style] report mapped types #10160

feat(eslint-plugin): [consistent-indexed-object-style] report mapped types #10160

Conversation

kirkwaiblinger commented Oct 16, 2024 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Oct 16, 2024

netlify bot commented Oct 16, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Oct 16, 2024 • edited Loading

☁️ Nx Cloud Report

This comment was marked as outdated.

codecov bot commented Oct 17, 2024 • edited Loading

Codecov Report

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg Oct 21, 2024

Choose a reason for hiding this comment

kirkwaiblinger Oct 21, 2024

Choose a reason for hiding this comment

kirkwaiblinger commented Oct 16, 2024 •

edited

Loading

netlify bot commented Oct 16, 2024 •

edited

Loading

nx-cloud bot commented Oct 16, 2024 •

edited

Loading

codecov bot commented Oct 17, 2024 •

edited

Loading