From 1e8667082a937843b5109e747e431d6cf20b6736 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 6 Feb 2024 08:55:27 -0700 Subject: [PATCH 1/2] fix #8386: Stop throwing type errors when converting symbols to numbers --- .../eslint-plugin/src/rules/prefer-find.ts | 29 ++++++++++++------- .../tests/rules/prefer-find.test.ts | 18 ++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-find.ts b/packages/eslint-plugin/src/rules/prefer-find.ts index babee93bab8..f1440dcd826 100644 --- a/packages/eslint-plugin/src/rules/prefer-find.ts +++ b/packages/eslint-plugin/src/rules/prefer-find.ts @@ -125,7 +125,7 @@ export default createRule({ return isAtLeastOneArrayishComponent; } - function getObjectIfArrayAtExpression( + function getObjectIfArrayAtZeroExpression( node: TSESTree.CallExpression, ): TSESTree.Expression | undefined { // .at() should take exactly one argument. @@ -133,14 +133,14 @@ export default createRule({ return undefined; } - const atArgument = getStaticValue(node.arguments[0], globalScope); - if (atArgument != null && isTreatedAsZeroByArrayAt(atArgument.value)) { - const callee = node.callee; - if ( - callee.type === AST_NODE_TYPES.MemberExpression && - !callee.optional && - isStaticMemberAccessOfValue(callee, 'at', globalScope) - ) { + const callee = node.callee; + if ( + callee.type === AST_NODE_TYPES.MemberExpression && + !callee.optional && + isStaticMemberAccessOfValue(callee, 'at', globalScope) + ) { + const atArgument = getStaticValue(node.arguments[0], globalScope); + if (atArgument != null && isTreatedAsZeroByArrayAt(atArgument.value)) { return callee.object; } } @@ -153,7 +153,14 @@ export default createRule({ * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/at#parameters */ function isTreatedAsZeroByArrayAt(value: unknown): boolean { - const asNumber = Number(value); + let asNumber: number; + + try { + asNumber = Number(value); + } catch (e) { + // This will happen if trying to convert a symbol. + return false; + } if (isNaN(asNumber)) { return true; @@ -215,7 +222,7 @@ export default createRule({ return { // This query will be used to find things like `filteredResults.at(0)`. CallExpression(node): void { - const object = getObjectIfArrayAtExpression(node); + const object = getObjectIfArrayAtZeroExpression(node); if (object) { const filterExpression = parseIfArrayFilterExpression(object); if (filterExpression) { diff --git a/packages/eslint-plugin/tests/rules/prefer-find.test.ts b/packages/eslint-plugin/tests/rules/prefer-find.test.ts index 6b303ea9aed..59f6d9a07be 100644 --- a/packages/eslint-plugin/tests/rules/prefer-find.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-find.test.ts @@ -56,6 +56,24 @@ ruleTester.run('prefer-find', rule, { "['Just', 'a', 'find'].find(x => x.length > 4);", 'undefined.filter(x => x)[0];', 'null?.filter(x => x)[0];', + // Should not throw. See https://github.com/typescript-eslint/typescript-eslint/issues/8386 + ` + declare function foo(param: any): any; + foo(Symbol.for('foo')); + `, + // Specifically need to test Symbol.for(), not just Symbol(), since only + // Symbol.for() creates a static value that the rule inspects. + ` + declare const arr: string[]; + const s = Symbol.for("Don't throw!"); + arr.filter(item => item === 'aha').at(s); + `, + ` + [1, 2, 3].filter(x => x)[Symbol('0')]; + `, + ` + [1, 2, 3].filter(x => x)[Symbol.for('0')]; + `, ], invalid: [ From a8d2c210242cddac1a7f674d6e763a518640c921 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 6 Feb 2024 10:22:51 -0700 Subject: [PATCH 2/2] Address PR feedback --- packages/eslint-plugin/src/rules/prefer-find.ts | 11 +++++------ .../eslint-plugin/tests/rules/prefer-find.test.ts | 8 ++------ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-find.ts b/packages/eslint-plugin/src/rules/prefer-find.ts index f1440dcd826..154d27d1a86 100644 --- a/packages/eslint-plugin/src/rules/prefer-find.ts +++ b/packages/eslint-plugin/src/rules/prefer-find.ts @@ -153,15 +153,14 @@ export default createRule({ * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/at#parameters */ function isTreatedAsZeroByArrayAt(value: unknown): boolean { - let asNumber: number; - - try { - asNumber = Number(value); - } catch (e) { - // This will happen if trying to convert a symbol. + // This would cause the number constructor coercion to throw. Other static + // values are safe. + if (typeof value === 'symbol') { return false; } + const asNumber = Number(value); + if (isNaN(asNumber)) { return true; } diff --git a/packages/eslint-plugin/tests/rules/prefer-find.test.ts b/packages/eslint-plugin/tests/rules/prefer-find.test.ts index 59f6d9a07be..dddb56f14d3 100644 --- a/packages/eslint-plugin/tests/rules/prefer-find.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-find.test.ts @@ -68,12 +68,8 @@ ruleTester.run('prefer-find', rule, { const s = Symbol.for("Don't throw!"); arr.filter(item => item === 'aha').at(s); `, - ` - [1, 2, 3].filter(x => x)[Symbol('0')]; - `, - ` - [1, 2, 3].filter(x => x)[Symbol.for('0')]; - `, + "[1, 2, 3].filter(x => x)[Symbol('0')];", + "[1, 2, 3].filter(x => x)[Symbol.for('0')];", ], invalid: [