feat(eslint-plugin): [no-redundant-type-constituents] use assignability checking for redundancy checks #10744

mdm317 · 2025-01-29T16:14:20Z

PR Checklist

Addresses an existing open issue: fixes Enhancement: [no-redundant-type-constituents] Use assignability checking for redundancy checks #7742
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

even cases where they're deeply equal, but declared as different object types.

This feature seems to overlap with this rule no-duplicate-type-constituents so I'm not sure whether to include it.
For now, I'll remove it and proceed.

deeply equal

type ShallowEqual = number | number;
type DeepEqual = { a: string } | { a: string };

Union
If x is assignable to y, then in the union x∣y, x is redundant and can be removed.
However, due to TypeScript's Excess Property Checks feature, this logic did not work as expected.

For example, consider the union type { a: 1 } | { a: 1, b: 1 }. Since { a: 1, b: 1 } is assignable to { a: 1 }
So { a: 1, b: 1 } becomes redundant and can be removed.

However, if { a: 1, b: 1 } is removed, the remaining type { a: 1 } exhibits different behavior when declaring values.
For example, { a: 1 } alone can only be assigned to variables of type { a: 1 }, whereas { a: 1 } | { a: 1, b: 1 } allows the declaration of both { a: 1 } and { a: 1, b: 1 }

Example code

type Foo = { a: 1 } | { a: 1, b: 1 }
const foo: Foo = { a: 1 }
const bar: Foo = { a: 1, b: 1 }

type Foo2 = { a: 1 } 
const foo2: Foo2 = { a: 1 }
const bar2: Foo2 = { a: 1, b: 1 } //error

So, before checking whether one type is assignable to another, I first verified that both objects have the same set of keys. Only then did I proceed with the assignability check.
For A | B, i checks if A and B have the same keys. If they do, it then checks assignability.

Union Types with Intersection Types

Union with a Single Intersection:
- If a union type contains an intersection, like A | (B & C & D), the intersection (B & C & D) is treated as a single object type.
Union within an Intersection:
- If an intersection includes a union, like A | (B & (C | D)), TypeScript simplifies the intersection into a union of intersections. B & (C | D) is equivalent to B & C | B & D,
- This means the assignability check becomes: Is A assignable to (B & C) | (B & D)

Intersection
Unlike union types, intersection types did not need excess property checks.
A & B in this case if A is assignable to B B is rebundant.

If there is no union type within the intersection, I proceed the assignability check. A & B
If there is union type, I check if every type in the union is compatible with the intersection and follows the same rules.
A & ( B | C)

A >= B, A >= C  Then  A >= (B | C)
B >= A, C >= A  Then   (B | C) >= A

typescript-eslint · 2025-01-29T16:14:24Z

Thanks for the PR, @mdm317!

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 · 2025-01-29T16:14:54Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`e240759`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/67fe8a91cd0d740008558ccf
😎 Deploy Preview	https://deploy-preview-10744--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 3 from production) Accessibility: 100 (no change from production) Best Practices: 100 (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 · 2025-01-29T16:15:19Z

View your CI Pipeline Execution ↗ for commit e240759.

Command	Status	Duration	Result
`nx typecheck ast-spec`	✅ Succeeded	<1s	View ↗
`nx run-many --target=build --exclude website --...`	✅ Succeeded	3s	View ↗
`nx run-many --target=clean`	✅ Succeeded	11s	View ↗

☁️ Nx Cloud last updated this comment at 2025-04-15 16:54:10 UTC

codecov · 2025-01-29T16:31:30Z

Codecov Report

Attention: Patch coverage is 97.05215% with 13 lines in your changes missing coverage. Please review.

Project coverage is 90.81%. Comparing base (be558e5) to head (e240759).
Report is 137 commits behind head on main.

Files with missing lines	Patch %	Lines
...plugin/src/rules/no-redundant-type-constituents.ts	97.48%	10 Missing and 1 partial ⚠️
...escript-estree/src/semantic-or-syntactic-errors.ts	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10744      +/-   ##
==========================================
- Coverage   90.82%   90.81%   -0.01%     
==========================================
  Files         497      497              
  Lines       50204    50368     +164     
  Branches     8274     8306      +32     
==========================================
+ Hits        45600    45744     +144     
- Misses       4589     4608      +19     
- Partials       15       16       +1

Flag	Coverage Δ
unittest	`90.81% <97.05%> (-0.01%)`	⬇️

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

Files with missing lines	Coverage Δ
...t-plugin/src/rules/adjacent-overload-signatures.ts	`100.00% <ø> (ø)`
...ages/eslint-plugin/src/rules/no-unsafe-argument.ts	`93.24% <100.00%> (ø)`
...nt-plugin/src/rules/related-getter-setter-pairs.ts	`100.00% <ø> (ø)`
...escript-estree/src/semantic-or-syntactic-errors.ts	`2.32% <0.00%> (ø)`
...plugin/src/rules/no-redundant-type-constituents.ts	`97.45% <97.48%> (-0.20%)`	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

we can not know typename {a : 2 } using typetostring type B = {a : 2}; type T = { a: 1 } | B

…ame keys.

JoshuaKGoldberg · 2025-03-03T13:43:15Z

👋 Ping @mdm317, are you planning on un-draft ing this soon? We'd prefer not to keep drafts around for too long in case someone else would want to send one too.

mdm317 · 2025-03-03T15:28:34Z

Sorry for the delay. This issue was more challenging than I expected.
I'll try to finish it within two days. If it takes longer, I'll close the PR and reopen it once it's resolved.
Moving forward, if I anticipate a delay, I'll close it first and reopen it when it's ready.

packages/eslint-plugin/src/rules/related-getter-setter-pairs.ts

packages/scope-manager/tests/test-utils/serializers/TSESTreeNode.ts

JoshuaKGoldberg

OK! Sorry for the delay in reviewing. This is a really tricky one 😅.

I think the feature request is a lot trickier than the test cases here currently capture. It'll have to handle a lot of cases, including optional properties, interfaces extending each other, and so on. I suspect the core implementation will have to adjust a bunch - so left a few suggestions on how to reduce the amount of work done. Hope it's helpful, and let me know if you have questions! 🙌

packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts

JoshuaKGoldberg · 2025-03-17T15:47:01Z

packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts

[Testing] There are going to be quite a few variants of types that could break the rule:

Conditional types

Generic types & their type parameters

Mapped types

Optional properties

Interfaces

Interfaces extending other interfaces

keyof and typeof types

ReturnType, Omit, etc. types

Symbols

Etc. - I haven't though super deeply beyond that. Could you think on what other test cases should happen and fill it all out?

The current logic works well for union types, but contains errors when handling intersection types.

Some mapped type has 0 property

Type is a generic type parameter that hasn't been specified yet type U<Type> = {[Property in keyof Type]: 0;};

An object type with potentially unlimited string keys, Record<string, number>

object type 0 property 0> property

0 property check assignable non redundant

0 > property non redundant check assignable

optional property
In typescript type with only the required property a:1 is assignable to type with the same required property plus an optional property b?:1
type T = { a : 1 } & { a : 1, b? : 1}
I've added logic that determines: when type A is assignable to type B and B contains no extra optional properties beyond A, then B is considered redundant.

packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts

mdm317 · 2025-03-20T15:32:29Z

Thank you for your review!
I'll try implementing it according to your suggestion.

mdm317 · 2025-04-05T15:53:43Z

@JoshuaKGoldberg

Hello I have a question about applying rules to our code.
It seems that in our codebase, we are intentionally using overridden types, like in the code below.

typescript-eslint/packages/ast-spec/src/declaration/ClassDeclaration/spec.ts

Lines 15 to 31 in 052b7f9

export interface ClassDeclarationWithName extends ClassDeclarationBase {

id: Identifier;

}

/**

* Default-exported class declarations have optional names:

* ```

* export default class {}

* ```

*/

export interface ClassDeclarationWithOptionalName extends ClassDeclarationBase {

id: Identifier | null;

}

export type ClassDeclaration =

| ClassDeclarationWithName

| ClassDeclarationWithOptionalName;

ClassDeclarationWithName is ovveridden by ClassDeclarationWithOptionalName in this union type

typescript-eslint/packages/ast-spec/src/unions/ExportDeclaration.ts

Lines 23 to 24 in 052b7f9

| FunctionDeclarationWithName

| FunctionDeclarationWithOptionalName

FunctionDeclarationWithOptionalName is ovveridden by FunctionDeclarationWithName in this union type

In this case, should I remove them, use eslint-disable, or modify the rule itself?

JoshuaKGoldberg · 2025-04-07T11:32:46Z

Ha, that's really interesting. Nice find. My instinct is that we should modify the spec.ts files because the union types really are violating the rule - cc @typescript-eslint/triage-team

prev: Record<string,number> & { a : 1} {a:1 } is rebundant now :type T= Record<string,number> & { a : 1} is valid

This reverts commit 35d90d0 because in commit d240c5f.

mdm317

This change was made to applying lint to code.
Let me know if you have any concerns about this adjustment

mdm317 · 2025-04-13T16:47:43Z

packages/ast-spec/src/declaration/FunctionDeclaration/spec.ts

-  | FunctionDeclarationWithName
-  | FunctionDeclarationWithOptionalName;
+  // eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents
+  FunctionDeclarationWithName | FunctionDeclarationWithOptionalName;


Due to Merging Interfaces, it should not be considered as redundant types.
So I added a lint disable comment for the line.

typescript-eslint/packages/types/src/ts-estree.ts

Lines 92 to 96 in fa265a9

interface FunctionDeclarationWithName {

parent:

| TSESTree.BlockStatement

| TSESTree.ExportDefaultDeclaration

| TSESTree.ExportNamedDeclaration

mdm317 · 2025-04-13T16:52:22Z

packages/ast-spec/src/unions/ExportDeclaration.ts

@@ -47,4 +43,5 @@ export type NamedExportDeclarations =
 // TODO - breaking change remove this in the next major
 export type ExportDeclaration =
  | DefaultExportDeclarations
+  // eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents
  | NamedExportDeclarations;


As in the comment above, It is because of Merging Interfaces FunctionDeclarationWithName

mdm317 · 2025-04-13T17:08:14Z

packages/typescript-estree/src/parser-options.ts

@@ -244,6 +244,7 @@ export interface ParserServicesBase {
 }
 export interface ParserServicesNodeMaps {
  esTreeNodeToTSNodeMap: ParserWeakMapESTreeToTSNode;
+  // eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents
  tsNodeToESTreeNodeMap: ParserWeakMap<TSNode | TSToken, TSESTree.Node>;


In TSNode 's union types, the code below is overridden. I think modifying it too complex, so I added a lint
disable code.

typescript-eslint/packages/typescript-estree/src/ts-estree/ts-nodes.ts

Line 141 in fa265a9

| ts.Modifier

mdm317 · 2025-04-13T17:48:52Z

packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts

@@ -49,6 +49,7 @@ export default createRule({
     * @returns the name and attribute of the member or null if it's a member not relevant to the rule.
     */
    function getMemberMethod(
+      // eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents
      member: Member | MemberDeclaration,


Member -> ProgramStatement -> Statement -> ClassDeclarationWithName
MemberDeclaration -> DefaultExportDeclarations -> ClassDeclarationWithOptionalName
ClassDeclarationWithName is overriden by ClassDeclarationWithOptionalName
I think modifying it too complex, so I added a lint disable code.

…tersection

…ent types

… to redundant

…com/mdm317/typescript-eslint into 7742-no-redundant-type-constituents

mdm317

I'd appreciate your feedback

mdm317 · 2025-04-15T16:35:28Z

packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-redundant-type-constituents.shot

@@ -22,11 +22,11 @@ type IntersectionNever = string | never;
                                  ~~~~~ 'never' is overridden by other types in this union type.

 type IntersectionBooleanLiteral = boolean & false;
-                                  ~~~~~~~ boolean is overridden by the false in this intersection type.
+                                  ~~~~~~~ boolean is overridden by false in this intersection type.


Can we remove the?

mdm317 · 2025-04-15T16:38:26Z

packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts

-    isFunctionOrFunctionType(node.parent.parent)
-  );
+function hasTargetOnlyOptionalProps(
+  sourceType: ts.ObjectType,


Checkint target type contains no extra optional properties beyond sourceType

mdm317 · 2025-04-15T16:41:24Z

packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts

+      (sourceTypeProperties.length === 0) !==
+      (targetTypeProperties.length === 0)
+    ) {
+      return false;


target type is non redundant

object type 0 property 0> property

0 property check assignable non redundant

0 > property non redundant check assignable

mdm317 added 2 commits January 30, 2025 01:05

feat : work for object type in union types

e9c19a6

test : add test for object type in union types

6207864

mdm317 and others added 7 commits January 30, 2025 23:08

test : add more case

ac85ecb

feat : intersection type exists within a union type

11cc737

feat : intersection case

98ac978

fix : using getText instead of typetostring

f4939b7

we can not know typename {a : 2 } using typetostring type B = {a : 2}; type T = { a: 1 } | B

refactor

a6c822a

fix : Emit an error for a union type only when the objects have the s…

91b9cc5

…ame keys.

test: add test for union

81264c0

mdm317 added 7 commits March 4, 2025 01:56

feat : intersection case where objects have different keys

686e5a0

reafactor

6fa4e1b

fix : lint err

567e062

fix: improve test coverage

a81a06e

fix : lint err

7a8d574

Merge branch 'main' into 7742-no-redundant-type-constituents

ba0dd2d

fix : infinite loop at no unused var

70fbe0c

mdm317 commented Mar 5, 2025

View reviewed changes

packages/eslint-plugin/src/rules/related-getter-setter-pairs.ts Show resolved Hide resolved

mdm317 commented Mar 5, 2025

View reviewed changes

packages/scope-manager/tests/test-utils/serializers/TSESTreeNode.ts Show resolved Hide resolved

mdm317 marked this pull request as ready for review March 5, 2025 19:03

JoshuaKGoldberg requested changes Mar 17, 2025

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Mar 17, 2025

mdm317 added 2 commits March 30, 2025 17:25

refactor : use typeToString instead of getText

e2ded16

refactor: integrate message IDs and error reports function

2b9d3b0

mdm317 marked this pull request as draft April 1, 2025 16:56

mdm317 added 2 commits April 2, 2025 03:19

chore : change error message

db67764

feat: optional property in intersection

f2aa34c

mdm317 marked this pull request as ready for review April 5, 2025 15:52

refactor: remove redundunt type

35d90d0

mdm317 marked this pull request as draft April 9, 2025 17:10

mdm317 added 10 commits April 11, 2025 00:20

fix : optional property in intersection

17bf29a

test : add interface case

200f6f0

refactor : hasTargetOnlyOptionalProps function

5c68891

fix :nested object type in union

23d8780

fix : intersection where the objects have no properties.

d240c5f

prev: Record<string,number> & { a : 1} {a:1 } is rebundant now :type T= Record<string,number> & { a : 1} is valid

Revert (Partial) "refactor: remove redundunt type"

c01e485

This reverts commit 35d90d0 because in commit d240c5f.

fix: infinite loop when compare property

5c0de98

refactor : fix lint error

b97cb46

refactor:redundant type detection in intersection types

04d71c1

Merge branch 'main' into 7742-no-redundant-type-constituents

28c64b4

mdm317 commented Apr 13, 2025

View reviewed changes

mdm317 added 7 commits April 14, 2025 17:45

fix: check assignability when both objects have zero properties in in…

f89b9a6

…tersection

fix: error message when super types have the same name but are differ…

3ef7b27

…ent types

refactor: integrate error reporting functionality and refactor assign…

f2eaf07

… to redundant

Merge branch '7742-no-redundant-type-constituents' of https://github.…

1457723

…com/mdm317/typescript-eslint into 7742-no-redundant-type-constituents

Merge branch 'main' into 7742-no-redundant-type-constituents

8a00f2b

fix : snapshot error

729a6fc

test : add test case

e240759

mdm317 commented Apr 15, 2025

View reviewed changes

mdm317 marked this pull request as ready for review April 15, 2025 16:55

mdm317 requested a review from JoshuaKGoldberg April 15, 2025 16:55

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Apr 15, 2025

object type	0 property	0> property
0 property	check assignable	non redundant
0 > property	non redundant	check assignable

	interface FunctionDeclarationWithName {
	parent:
	\| TSESTree.BlockStatement
	\| TSESTree.ExportDefaultDeclaration
	\| TSESTree.ExportNamedDeclaration

Uh oh!

feat(eslint-plugin): [no-redundant-type-constituents] use assignability checking for redundancy checks #10744

Are you sure you want to change the base?

feat(eslint-plugin): [no-redundant-type-constituents] use assignability checking for redundancy checks #10744

Conversation

mdm317 commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Overview

Uh oh!

typescript-eslint bot commented Jan 29, 2025

Uh oh!

netlify bot commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for typescript-eslint ready!

Uh oh!

nx-cloud bot commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JoshuaKGoldberg commented Mar 3, 2025

Uh oh!

mdm317 commented Mar 3, 2025

Uh oh!

Uh oh!

Uh oh!

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JoshuaKGoldberg Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

mdm317 Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mdm317 commented Mar 20, 2025

Uh oh!

mdm317 commented Apr 5, 2025

Uh oh!

JoshuaKGoldberg commented Apr 7, 2025

Uh oh!

mdm317 left a comment

Choose a reason for hiding this comment

Uh oh!

mdm317 Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdm317 Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdm317 Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdm317 Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdm317 left a comment

Choose a reason for hiding this comment

Uh oh!

mdm317 Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

mdm317 Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

mdm317 Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mdm317 commented Jan 29, 2025 •

edited

Loading

netlify bot commented Jan 29, 2025 •

edited

Loading

nx-cloud bot commented Jan 29, 2025 •

edited

Loading

codecov bot commented Jan 29, 2025 •

edited

Loading

mdm317 Apr 15, 2025 •

edited

Loading

mdm317 Apr 13, 2025 •

edited

Loading

mdm317 Apr 13, 2025 •

edited

Loading

mdm317 Apr 13, 2025 •

edited

Loading

mdm317 Apr 13, 2025 •

edited

Loading