From 215b9659a53ec70f2d32559c9048112cbf90dd36 Mon Sep 17 00:00:00 2001 From: Vincent Girard Date: Tue, 28 May 2024 14:58:19 -0400 Subject: [PATCH 1/4] fix(eslint-plugin): [no-magic-numbers] fix implementation of the `ignore` option --- .../src/rules/no-magic-numbers.ts | 43 +- .../tests/rules/no-magic-numbers.test.ts | 456 ++++++++++++++++++ .../eslint-plugin/typings/eslint-rules.d.ts | 2 +- 3 files changed, 499 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-magic-numbers.ts b/packages/eslint-plugin/src/rules/no-magic-numbers.ts index bc738eb614b7..199bf6d58db4 100644 --- a/packages/eslint-plugin/src/rules/no-magic-numbers.ts +++ b/packages/eslint-plugin/src/rules/no-magic-numbers.ts @@ -58,11 +58,14 @@ export default createRule({ ignoreNumericLiteralTypes: false, ignoreEnums: false, ignoreReadonlyClassProperties: false, + ignoreTypeIndexes: false, }, ], create(context, [options]) { const rules = baseRule.create(context); + const ignored = new Set((options.ignore ?? []).map(normalizeIgnoreValue)); + return { Literal(node): void { // If it’s not a numeric literal we’re not interested @@ -76,8 +79,12 @@ export default createRule({ // not one of our exception cases, and we’ll fall back to the base rule. let isAllowed: boolean | undefined; + // Check if the node is ignored + if (ignored.has(normalizeLiteralValue(node))) { + isAllowed = true; + } // Check if the node is a TypeScript enum declaration - if (isParentTSEnumDeclaration(node)) { + else if (isParentTSEnumDeclaration(node)) { isAllowed = options.ignoreEnums === true; } // Check TypeScript specific nodes for Numeric Literal @@ -129,6 +136,40 @@ export default createRule({ }, }); +/** + * Convert the value to bigint if it's a string. Otherwise, return the value as-is. + * @param value The value to normalize. + * @returns The normalized value. + */ +function normalizeIgnoreValue( + value: bigint | number | string, +): bigint | number { + if (typeof value === 'string') { + return BigInt(value.slice(0, -1)); + } + + return value; +} + +/** + * Converts the node to its numeric value, handling prefixed numbers (-1 / +1) + * @param node the node to normalize. + */ +function normalizeLiteralValue( + node: TSESTree.BigIntLiteral | TSESTree.NumberLiteral, +): bigint | number { + if ( + node.parent.type === AST_NODE_TYPES.UnaryExpression && + ['-', '+'].includes(node.parent.operator) + ) { + if (node.parent.operator === '-') { + return -(node.value ?? BigInt(node.raw)); + } + } + + return node.value ?? BigInt(node.raw); +} + /** * Gets the true parent of the literal, handling prefixed numbers (-1 / +1) */ diff --git a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts index ccf55f9eaf58..26fecd3bee51 100644 --- a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts +++ b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts @@ -117,6 +117,104 @@ type Foo = { `, options: [{ ignoreTypeIndexes: true }], }, + { + code: ` +type Foo = + | 1 + | -2 + | 3n + | -4n + | 5.6 + | -7.8 + | 0x0a + | -0xbc + | 1e2 + | -3e4 + | 5e-6 + | -7e-8 + | 1.1e2 + | -3.1e4 + | 5.1e-6 + | -7.1e-8; + `, + options: [ + { + ignore: [ + 1, + -2, + '3n', + '-4n', + 5.6, + -7.8, + 0x0a, + -0xbc, + 1e2, + -3e4, + 5e-6, + -7e-8, + 1.1e2, + -3.1e4, + 5.1e-6, + -7.1e-8, + ], + }, + ], + }, + { + code: ` +interface Foo { + bar: 1; +} + `, + options: [{ ignoreNumericLiteralTypes: true, ignore: [1] }], + }, + { + code: ` +enum foo { + SECOND = 1000, + NUM = '0123456789', + NEG = -1, + POS = +2, +} + `, + options: [{ ignoreEnums: false, ignore: [1000, -1, 2] }], + }, + { + code: ` +class Foo { + readonly A = 1; + readonly B = 2; + public static readonly C = 3; + static readonly D = 4; + readonly E = -5; + readonly F = +6; + private readonly G = 100n; + private static readonly H = -2000n; +} + `, + options: [ + { + ignoreReadonlyClassProperties: false, + ignore: [1, 2, 3, 4, -5, 6, '100n', '-2000n'], + }, + ], + }, + { + code: 'type Foo = Bar[0];', + options: [{ ignoreTypeIndexes: false, ignore: [0] }], + }, + { + code: ` +type Other = { + [0]: 3; +}; + +type Foo = { + [K in keyof Other]: \`\${K & number}\`; +}; + `, + options: [{ ignoreTypeIndexes: true, ignore: [0, 3] }], + }, ], invalid: [ @@ -599,5 +697,363 @@ type Foo = { }, ], }, + { + code: ` +type Foo = + | 1 + | -2 + | 3n + | -4n + | 5.6 + | -7.8 + | 0x0a + | -0xbc + | 1e2 + | -3e4 + | 5e-6 + | -7e-8 + | 1.1e2 + | -3.1e4 + | 5.1e-6 + | -7.1e-8; + `, + options: [ + { + ignore: [ + -1, + 2, + '-3n', + '4n', + -5.6, + 7.8, + -0x0a, + 0xbc, + -1e2, + 3e4, + -5e-6, + 7e-8, + -1.1e2, + 3.1e4, + -5.1e-6, + 7.1e-8, + ], + }, + ], + errors: [ + { + messageId: 'noMagic', + data: { + raw: '1', + }, + line: 3, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '-2', + }, + line: 4, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '3n', + }, + line: 5, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '-4n', + }, + line: 6, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '5.6', + }, + line: 7, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '-7.8', + }, + line: 8, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '0x0a', + }, + line: 9, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '-0xbc', + }, + line: 10, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '1e2', + }, + line: 11, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '-3e4', + }, + line: 12, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '5e-6', + }, + line: 13, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '-7e-8', + }, + line: 14, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '1.1e2', + }, + line: 15, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '-3.1e4', + }, + line: 16, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '5.1e-6', + }, + line: 17, + column: 5, + }, + { + messageId: 'noMagic', + data: { + raw: '-7.1e-8', + }, + line: 18, + column: 5, + }, + ], + }, + { + code: ` +interface Foo { + bar: 1; +} + `, + options: [{ ignoreNumericLiteralTypes: true, ignore: [-1] }], + errors: [ + { + messageId: 'noMagic', + data: { + raw: '1', + }, + line: 3, + column: 8, + }, + ], + }, + { + code: ` +enum foo { + SECOND = 1000, + NUM = '0123456789', + NEG = -1, + POS = +2, +} + `, + options: [{ ignoreEnums: false, ignore: [-1000, 1, -2] }], + errors: [ + { + messageId: 'noMagic', + data: { + raw: '1000', + }, + line: 3, + column: 12, + }, + { + messageId: 'noMagic', + data: { + raw: '-1', + }, + line: 5, + column: 9, + }, + { + messageId: 'noMagic', + data: { + raw: '2', + }, + line: 6, + column: 10, + }, + ], + }, + { + code: ` +class Foo { + readonly A = 1; + readonly B = 2; + public static readonly C = 3; + static readonly D = 4; + readonly E = -5; + readonly F = +6; + private readonly G = 100n; + private static readonly H = -2000n; +} + `, + options: [ + { + ignoreReadonlyClassProperties: false, + ignore: [-1, -2, -3, -4, 5, -6, '-100n', '2000n'], + }, + ], + errors: [ + { + messageId: 'noMagic', + data: { + raw: '1', + }, + line: 3, + column: 16, + }, + { + messageId: 'noMagic', + data: { + raw: '2', + }, + line: 4, + column: 16, + }, + { + messageId: 'noMagic', + data: { + raw: '3', + }, + line: 5, + column: 30, + }, + { + messageId: 'noMagic', + data: { + raw: '4', + }, + line: 6, + column: 23, + }, + { + messageId: 'noMagic', + data: { + raw: '-5', + }, + line: 7, + column: 16, + }, + { + messageId: 'noMagic', + data: { + raw: '6', + }, + line: 8, + column: 17, + }, + { + messageId: 'noMagic', + data: { + raw: '100n', + }, + line: 9, + column: 24, + }, + { + messageId: 'noMagic', + data: { + raw: '-2000n', + }, + line: 10, + column: 31, + }, + ], + }, + { + code: 'type Foo = Bar[1];', + options: [{ ignoreTypeIndexes: false, ignore: [-1] }], + errors: [ + { + messageId: 'noMagic', + data: { + raw: '1', + }, + line: 1, + column: 16, + }, + ], + }, + { + code: ` +type Other = { + [1]: 3; +}; + +type Foo = { + [K in keyof Other]: \`\${K & number}\`; +}; + `, + options: [{ ignoreTypeIndexes: true, ignore: [-1, -3] }], + errors: [ + { + messageId: 'noMagic', + data: { + raw: '1', + }, + line: 3, + column: 4, + }, + { + messageId: 'noMagic', + data: { + raw: '3', + }, + line: 3, + column: 8, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index c72e018bc9bb..78b10297d113 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -448,7 +448,7 @@ declare module 'eslint/lib/rules/no-magic-numbers' { 'noMagic', [ { - ignore?: string[]; + ignore?: (number | string)[]; ignoreArrayIndexes?: boolean; enforceConst?: boolean; detectObjects?: boolean; From ef05ffb21b13a2ad0c20fc8a74826cb512ff1a9a Mon Sep 17 00:00:00 2001 From: Vincent Girard Date: Sun, 2 Jun 2024 11:06:23 -0400 Subject: [PATCH 2/4] Passed node's value as parameter --- packages/eslint-plugin/src/rules/no-magic-numbers.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-magic-numbers.ts b/packages/eslint-plugin/src/rules/no-magic-numbers.ts index 199bf6d58db4..5a86056cb90d 100644 --- a/packages/eslint-plugin/src/rules/no-magic-numbers.ts +++ b/packages/eslint-plugin/src/rules/no-magic-numbers.ts @@ -80,7 +80,7 @@ export default createRule({ let isAllowed: boolean | undefined; // Check if the node is ignored - if (ignored.has(normalizeLiteralValue(node))) { + if (ignored.has(normalizeLiteralValue(node, node.value))) { isAllowed = true; } // Check if the node is a TypeScript enum declaration @@ -154,20 +154,22 @@ function normalizeIgnoreValue( /** * Converts the node to its numeric value, handling prefixed numbers (-1 / +1) * @param node the node to normalize. + * @param value the node's value. */ function normalizeLiteralValue( node: TSESTree.BigIntLiteral | TSESTree.NumberLiteral, + value: number | bigint, ): bigint | number { if ( node.parent.type === AST_NODE_TYPES.UnaryExpression && ['-', '+'].includes(node.parent.operator) ) { if (node.parent.operator === '-') { - return -(node.value ?? BigInt(node.raw)); + return -value; } } - return node.value ?? BigInt(node.raw); + return value; } /** From 1abbe89bae81fdb26bde053068e1a1ffa8f369b8 Mon Sep 17 00:00:00 2001 From: Vincent Girard Date: Sun, 2 Jun 2024 13:06:42 -0400 Subject: [PATCH 3/4] Simple tests to check if value was correctly not ignored --- .../tests/rules/no-magic-numbers.test.ts | 348 ++++++------------ 1 file changed, 107 insertions(+), 241 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts index 26fecd3bee51..dc933bb17d67 100644 --- a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts +++ b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts @@ -698,360 +698,226 @@ type Foo = { ], }, { - code: ` -type Foo = - | 1 - | -2 - | 3n - | -4n - | 5.6 - | -7.8 - | 0x0a - | -0xbc - | 1e2 - | -3e4 - | 5e-6 - | -7e-8 - | 1.1e2 - | -3.1e4 - | 5.1e-6 - | -7.1e-8; - `, - options: [ - { - ignore: [ - -1, - 2, - '-3n', - '4n', - -5.6, - 7.8, - -0x0a, - 0xbc, - -1e2, - 3e4, - -5e-6, - 7e-8, - -1.1e2, - 3.1e4, - -5.1e-6, - 7.1e-8, - ], - }, - ], + code: 'type Foo = 1;', + options: [{ ignore: [-1] }], errors: [ { messageId: 'noMagic', data: { raw: '1', }, - line: 3, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = -2;', + options: [{ ignore: [2] }], + errors: [ { messageId: 'noMagic', data: { raw: '-2', }, - line: 4, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = 3n;', + options: [{ ignore: ['-3n'] }], + errors: [ { messageId: 'noMagic', data: { raw: '3n', }, - line: 5, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = -4n;', + options: [{ ignore: ['4n'] }], + errors: [ { messageId: 'noMagic', data: { raw: '-4n', }, - line: 6, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = 5.6;', + options: [{ ignore: [-5.6] }], + errors: [ { messageId: 'noMagic', data: { raw: '5.6', }, - line: 7, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = -7.8;', + options: [{ ignore: [7.8] }], + errors: [ { messageId: 'noMagic', data: { raw: '-7.8', }, - line: 8, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = 0x0a;', + options: [{ ignore: [-0x0a] }], + errors: [ { messageId: 'noMagic', data: { raw: '0x0a', }, - line: 9, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = -0xbc;', + options: [{ ignore: [0xbc] }], + errors: [ { messageId: 'noMagic', data: { raw: '-0xbc', }, - line: 10, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = 1e2;', + options: [{ ignore: [-1e2] }], + errors: [ { messageId: 'noMagic', data: { raw: '1e2', }, - line: 11, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = -3e4;', + options: [{ ignore: [3e4] }], + errors: [ { messageId: 'noMagic', data: { raw: '-3e4', }, - line: 12, - column: 5, + line: 1, + column: 12, }, + ], + }, + { + code: 'type Foo = 5e-6;', + options: [{ ignore: [-5e-6] }], + errors: [ { messageId: 'noMagic', data: { raw: '5e-6', }, - line: 13, - column: 5, - }, - { - messageId: 'noMagic', - data: { - raw: '-7e-8', - }, - line: 14, - column: 5, - }, - { - messageId: 'noMagic', - data: { - raw: '1.1e2', - }, - line: 15, - column: 5, - }, - { - messageId: 'noMagic', - data: { - raw: '-3.1e4', - }, - line: 16, - column: 5, - }, - { - messageId: 'noMagic', - data: { - raw: '5.1e-6', - }, - line: 17, - column: 5, - }, - { - messageId: 'noMagic', - data: { - raw: '-7.1e-8', - }, - line: 18, - column: 5, + line: 1, + column: 12, }, ], }, { - code: ` -interface Foo { - bar: 1; -} - `, - options: [{ ignoreNumericLiteralTypes: true, ignore: [-1] }], + code: 'type Foo = -7e-8;', + options: [{ ignore: [7e-8] }], errors: [ { messageId: 'noMagic', data: { - raw: '1', + raw: '-7e-8', }, - line: 3, - column: 8, + line: 1, + column: 12, }, ], }, { - code: ` -enum foo { - SECOND = 1000, - NUM = '0123456789', - NEG = -1, - POS = +2, -} - `, - options: [{ ignoreEnums: false, ignore: [-1000, 1, -2] }], + code: 'type Foo = 1.1e2;', + options: [{ ignore: [-1.1e2] }], errors: [ { messageId: 'noMagic', data: { - raw: '1000', + raw: '1.1e2', }, - line: 3, + line: 1, column: 12, }, - { - messageId: 'noMagic', - data: { - raw: '-1', - }, - line: 5, - column: 9, - }, - { - messageId: 'noMagic', - data: { - raw: '2', - }, - line: 6, - column: 10, - }, ], }, { - code: ` -class Foo { - readonly A = 1; - readonly B = 2; - public static readonly C = 3; - static readonly D = 4; - readonly E = -5; - readonly F = +6; - private readonly G = 100n; - private static readonly H = -2000n; -} - `, - options: [ - { - ignoreReadonlyClassProperties: false, - ignore: [-1, -2, -3, -4, 5, -6, '-100n', '2000n'], - }, - ], + code: 'type Foo = -3.1e4;', + options: [{ ignore: [3.1e4] }], errors: [ { messageId: 'noMagic', data: { - raw: '1', - }, - line: 3, - column: 16, - }, - { - messageId: 'noMagic', - data: { - raw: '2', - }, - line: 4, - column: 16, - }, - { - messageId: 'noMagic', - data: { - raw: '3', - }, - line: 5, - column: 30, - }, - { - messageId: 'noMagic', - data: { - raw: '4', - }, - line: 6, - column: 23, - }, - { - messageId: 'noMagic', - data: { - raw: '-5', - }, - line: 7, - column: 16, - }, - { - messageId: 'noMagic', - data: { - raw: '6', - }, - line: 8, - column: 17, - }, - { - messageId: 'noMagic', - data: { - raw: '100n', - }, - line: 9, - column: 24, - }, - { - messageId: 'noMagic', - data: { - raw: '-2000n', + raw: '-3.1e4', }, - line: 10, - column: 31, + line: 1, + column: 12, }, ], }, { - code: 'type Foo = Bar[1];', - options: [{ ignoreTypeIndexes: false, ignore: [-1] }], + code: 'type Foo = 5.1e-6;', + options: [{ ignore: [-5.1e-6] }], errors: [ { messageId: 'noMagic', data: { - raw: '1', + raw: '5.1e-6', }, line: 1, - column: 16, + column: 12, }, ], }, { - code: ` -type Other = { - [1]: 3; -}; - -type Foo = { - [K in keyof Other]: \`\${K & number}\`; -}; - `, - options: [{ ignoreTypeIndexes: true, ignore: [-1, -3] }], + code: 'type Foo = -7.1e-8;', + options: [{ ignore: [7.1e-8] }], errors: [ { messageId: 'noMagic', data: { - raw: '1', - }, - line: 3, - column: 4, - }, - { - messageId: 'noMagic', - data: { - raw: '3', + raw: '-7.1e-8', }, - line: 3, - column: 8, + line: 1, + column: 12, }, ], }, From 02d78e9a419e4ebb826ba7768d323e1d0e1164cc Mon Sep 17 00:00:00 2001 From: Vincent Girard Date: Sun, 2 Jun 2024 14:40:40 -0400 Subject: [PATCH 4/4] Simple tests to check if value was correctly ignored --- .../tests/rules/no-magic-numbers.test.ts | 103 +++++++++++------- 1 file changed, 62 insertions(+), 41 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts index dc933bb17d67..fecfd8b45e5b 100644 --- a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts +++ b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts @@ -118,47 +118,68 @@ type Foo = { options: [{ ignoreTypeIndexes: true }], }, { - code: ` -type Foo = - | 1 - | -2 - | 3n - | -4n - | 5.6 - | -7.8 - | 0x0a - | -0xbc - | 1e2 - | -3e4 - | 5e-6 - | -7e-8 - | 1.1e2 - | -3.1e4 - | 5.1e-6 - | -7.1e-8; - `, - options: [ - { - ignore: [ - 1, - -2, - '3n', - '-4n', - 5.6, - -7.8, - 0x0a, - -0xbc, - 1e2, - -3e4, - 5e-6, - -7e-8, - 1.1e2, - -3.1e4, - 5.1e-6, - -7.1e-8, - ], - }, - ], + code: 'type Foo = 1;', + options: [{ ignore: [1] }], + }, + { + code: 'type Foo = -2;', + options: [{ ignore: [-2] }], + }, + { + code: 'type Foo = 3n;', + options: [{ ignore: ['3n'] }], + }, + { + code: 'type Foo = -4n;', + options: [{ ignore: ['-4n'] }], + }, + { + code: 'type Foo = 5.6;', + options: [{ ignore: [5.6] }], + }, + { + code: 'type Foo = -7.8;', + options: [{ ignore: [-7.8] }], + }, + { + code: 'type Foo = 0x0a;', + options: [{ ignore: [0x0a] }], + }, + { + code: 'type Foo = -0xbc;', + options: [{ ignore: [-0xbc] }], + }, + { + code: 'type Foo = 1e2;', + options: [{ ignore: [1e2] }], + }, + { + code: 'type Foo = -3e4;', + options: [{ ignore: [-3e4] }], + }, + { + code: 'type Foo = 5e-6;', + options: [{ ignore: [5e-6] }], + }, + { + code: 'type Foo = -7e-8;', + options: [{ ignore: [-7e-8] }], + }, + { + code: 'type Foo = 1.1e2;', + options: [{ ignore: [1.1e2] }], + }, + { + code: 'type Foo = -3.1e4;', + options: [{ ignore: [-3.1e4] }], + }, + { + code: 'type Foo = 5.1e-6;', + options: [{ ignore: [5.1e-6] }], + }, + { + code: 'type Foo = -7.1e-8;', + options: [{ ignore: [-7.1e-8] }], }, { code: `