From c50168efbcf7dbb6239e442329d657f8c6541a34 Mon Sep 17 00:00:00 2001 From: arka1002 Date: Sat, 20 Jan 2024 11:23:36 +0530 Subject: [PATCH 1/5] feat(eslint-plugin): [no-redundant-type-constituents] incorrectly marks & string as redundant fixes: #7580 --- .../rules/no-redundant-type-constituents.ts | 49 ++++++++++++++++++- .../no-redundant-type-constituents.test.ts | 45 +++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index a9b22e71de1..2e7e9dcfc8a 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -272,6 +272,10 @@ export default createRule({ PrimitiveTypeFlag, TSESTree.TypeNode[] >(); + const seenUnionTypes = new Map< + TSESTree.TypeNode, + TypeFlagsWithName[] + >(); function checkIntersectionBottomAndTopTypes( { typeFlags, typeName }: TypeFlagsWithName, @@ -323,8 +327,51 @@ export default createRule({ } } } - } + typePartFlags.length >= 2 + ? seenUnionTypes.set(typeNode, typePartFlags) + : null; + } + const checkIfUnionsAreAssignable = () => { + let typeFlagsOfUnions: ts.TypeFlags[] = []; + let result = true; + let primitiveUnit: number; + + seenUnionTypes.forEach((value, key) => { + value.forEach(union => { + typeFlagsOfUnions.push(union.typeFlags); + }); + for (const iterator of typeFlagsOfUnions) { + if ( + // @ts-expect-error + seenPrimitiveTypes.has(literalToPrimitiveTypeFlags[iterator]) + ) { + result = true; + // @ts-expect-error + primitiveUnit = literalToPrimitiveTypeFlags[iterator]; + } else { + result = false; + break; + } + } + if (result) { + context.report({ + data: { + literal: value.map(name => name.typeName).join(' | '), + // @ts-expect-error + primitive: primitiveTypeFlagNames[primitiveUnit], + }, + messageId: 'primitiveOverridden', + node: key, + }); + } + typeFlagsOfUnions = []; + }); + }; + if (seenUnionTypes.size > 0) { + checkIfUnionsAreAssignable(); + return; + } // For each primitive type of all the seen primitive types, // if there was a literal type seen that overrides it, // report each of the primitive type's type nodes diff --git a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts index 04ee15735c4..4b1d7407f60 100644 --- a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts +++ b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts @@ -156,6 +156,10 @@ ruleTester.run('no-redundant-type-constituents', rule, { type B = string; type T = B & null; `, + ` + type T = 'a' | 1; + type U = T & string; + `, { code: 'type T = `${string}` & null;', dependencyConstraints: { @@ -804,5 +808,46 @@ ruleTester.run('no-redundant-type-constituents', rule, { }, ], }, + { + code: ` + type T = "a" | "b"; + type U = T & string; + `, + errors: [ + { + column: 18, + data: { + literal: '"a" | "b"', + primitive: 'string', + }, + messageId: 'primitiveOverridden', + }, + ], + }, + { + code: ` + type S = 1 | 2; + type T = "a" | "b"; + type U = S & T & string & number; + `, + errors: [ + { + column: 18, + data: { + literal: '1 | 2', + primitive: 'number', + }, + messageId: 'primitiveOverridden', + }, + { + column: 22, + data: { + literal: '"a" | "b"', + primitive: 'string', + }, + messageId: 'primitiveOverridden', + }, + ], + }, ], }); From 53326d134c6d1121147069b0a286343810cb6098 Mon Sep 17 00:00:00 2001 From: arka1002 Date: Sun, 28 Jan 2024 16:15:10 +0530 Subject: [PATCH 2/5] chore: lint errors --- .../rules/no-redundant-type-constituents.ts | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index 2e7e9dcfc8a..07f5478f0be 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -332,7 +332,7 @@ export default createRule({ ? seenUnionTypes.set(typeNode, typePartFlags) : null; } - const checkIfUnionsAreAssignable = () => { + const checkIfUnionsAreAssignable = (): undefined => { let typeFlagsOfUnions: ts.TypeFlags[] = []; let result = true; let primitiveUnit: number; @@ -343,12 +343,17 @@ export default createRule({ }); for (const iterator of typeFlagsOfUnions) { if ( - // @ts-expect-error - seenPrimitiveTypes.has(literalToPrimitiveTypeFlags[iterator]) + seenPrimitiveTypes.has( + literalToPrimitiveTypeFlags[ + iterator as keyof typeof literalToPrimitiveTypeFlags + ], + ) ) { result = true; - // @ts-expect-error - primitiveUnit = literalToPrimitiveTypeFlags[iterator]; + primitiveUnit = + literalToPrimitiveTypeFlags[ + iterator as keyof typeof literalToPrimitiveTypeFlags + ]; } else { result = false; break; @@ -358,8 +363,10 @@ export default createRule({ context.report({ data: { literal: value.map(name => name.typeName).join(' | '), - // @ts-expect-error - primitive: primitiveTypeFlagNames[primitiveUnit], + primitive: + primitiveTypeFlagNames[ + primitiveUnit as keyof typeof primitiveTypeFlagNames + ], }, messageId: 'primitiveOverridden', node: key, From 30af3a5c0ce657244cb74d06d0514eea06dd4e99 Mon Sep 17 00:00:00 2001 From: arka1002 Date: Sun, 28 Jan 2024 16:26:01 +0530 Subject: [PATCH 3/5] chore: lint errors --- .../tests/rules/no-redundant-type-constituents.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts index 4b1d7407f60..a2b89b1b827 100644 --- a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts +++ b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts @@ -810,7 +810,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { }, { code: ` - type T = "a" | "b"; + type T = 'a' | 'b'; type U = T & string; `, errors: [ @@ -827,7 +827,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { { code: ` type S = 1 | 2; - type T = "a" | "b"; + type T = 'a' | 'b'; type U = S & T & string & number; `, errors: [ From e55363d920911bf7058b28494dd6d13a735943ae Mon Sep 17 00:00:00 2001 From: Arka Pratim Chaudhuri <105232141+arka1002@users.noreply.github.com> Date: Sun, 25 Feb 2024 18:04:57 +0530 Subject: [PATCH 4/5] chore: address style review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Josh Goldberg ✨ --- .../src/rules/no-redundant-type-constituents.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index 07f5478f0be..2ac90bdb671 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -328,9 +328,9 @@ export default createRule({ } } - typePartFlags.length >= 2 - ? seenUnionTypes.set(typeNode, typePartFlags) - : null; + if (typePartFlags.length >= 2) { + seenUnionTypes.set(typeNode, typePartFlags); + } } const checkIfUnionsAreAssignable = (): undefined => { let typeFlagsOfUnions: ts.TypeFlags[] = []; From 52783f0e414d7024e1f642a2036410907c686674 Mon Sep 17 00:00:00 2001 From: arka1002 Date: Mon, 26 Feb 2024 12:33:04 +0530 Subject: [PATCH 5/5] chore: address reviews --- .../rules/no-redundant-type-constituents.ts | 42 +++++++++---------- .../no-redundant-type-constituents.test.ts | 2 +- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index 2ac90bdb671..0c6d6c82612 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -327,53 +327,53 @@ export default createRule({ } } } - + // if any typeNode is TSTypeReference and typePartFlags have more than 1 element, than the referenced type is definitely a union. if (typePartFlags.length >= 2) { seenUnionTypes.set(typeNode, typePartFlags); } } + /** + * @example + * ```ts + * type F = "a"|2|"b"; + * type I = F & string; + * ``` + * This function checks if all the union members of `F` are assignable to the other member of `I`. If every member is assignable, then its reported else not. + */ const checkIfUnionsAreAssignable = (): undefined => { - let typeFlagsOfUnions: ts.TypeFlags[] = []; - let result = true; - let primitiveUnit: number; - - seenUnionTypes.forEach((value, key) => { - value.forEach(union => { - typeFlagsOfUnions.push(union.typeFlags); - }); - for (const iterator of typeFlagsOfUnions) { + for (const [typeRef, typeValues] of seenUnionTypes) { + let primitive: number | undefined = undefined; + for (const { typeFlags } of typeValues) { if ( seenPrimitiveTypes.has( literalToPrimitiveTypeFlags[ - iterator as keyof typeof literalToPrimitiveTypeFlags + typeFlags as keyof typeof literalToPrimitiveTypeFlags ], ) ) { - result = true; - primitiveUnit = + primitive = literalToPrimitiveTypeFlags[ - iterator as keyof typeof literalToPrimitiveTypeFlags + typeFlags as keyof typeof literalToPrimitiveTypeFlags ]; } else { - result = false; + primitive = undefined; break; } } - if (result) { + if (Number.isInteger(primitive)) { context.report({ data: { - literal: value.map(name => name.typeName).join(' | '), + literal: typeValues.map(name => name.typeName).join(' | '), primitive: primitiveTypeFlagNames[ - primitiveUnit as keyof typeof primitiveTypeFlagNames + primitive as keyof typeof primitiveTypeFlagNames ], }, messageId: 'primitiveOverridden', - node: key, + node: typeRef, }); } - typeFlagsOfUnions = []; - }); + } }; if (seenUnionTypes.size > 0) { checkIfUnionsAreAssignable(); diff --git a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts index 9a1fec0a2cd..b0a9ca844da 100644 --- a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts +++ b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts @@ -162,7 +162,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { type T = B & null; `, ` - type T = 'a' | 1; + type T = 'a' | 1 | 'b'; type U = T & string; `, ],