From 762cd475bc41cafeac74cf7b08c36cb1e1d9e3d8 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 20 Oct 2024 13:01:16 -0400 Subject: [PATCH 01/36] add no-unnecessary-coercion rule --- .../docs/rules/no-unnecessary-coercion.mdx | 74 ++++++ packages/eslint-plugin/src/configs/all.ts | 1 + .../src/configs/disable-type-checked.ts | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-unnecessary-coercion.ts | 242 ++++++++++++++++++ .../no-unnecessary-coercion.shot | 57 +++++ .../rules/no-unnecessary-coercion.test.ts | 212 +++++++++++++++ .../no-unnecessary-coercion.shot | 14 + packages/typescript-eslint/src/configs/all.ts | 1 + .../src/configs/disable-type-checked.ts | 1 + 10 files changed, 605 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx create mode 100644 packages/eslint-plugin/src/rules/no-unnecessary-coercion.ts create mode 100644 packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot create mode 100644 packages/eslint-plugin/tests/rules/no-unnecessary-coercion.test.ts create mode 100644 packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx b/packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx new file mode 100644 index 000000000000..aca0aa9fb1d2 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx @@ -0,0 +1,74 @@ +--- +description: 'Disallow conversion idioms when they do not change the type or value of the expression.' +--- + +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/no-unnecessary-coercion** for documentation. + +JavaScript has several idioms used to convert values to certain types. With TypeScript, it is possible to see at build time if the value is already of that type, making the conversion unnecessary. +Performing unnecessary conversions increases visual clutter and harms code readability, so it's generally best practice to remove them if they don't change the type of an expression. +This rule reports when a type conversion idiom is identified which does not change the type of an expression. + +## Examples + + + + +```ts +String('123'); +'123'.toString(); +'' + '123'; +'123' + ''; + +Number(123); ++123; +~~123; + +Boolean(true); +!!true; + +BigInt(BigInt(1)); + +let str = '123'; +str += ''; +``` + + + + +```ts +function foo(bar: string | number) { + String(bar); + bar.toString(); + '' + bar; + bar + ''; + + Number(bar); + +bar; + ~~bar; + + Boolean(bar); + !!bar; + + BigInt(1); + + bar += ''; +} +``` + + + + +## When Not To Use It + +If you don't care about having no-op type conversions in your code, then you can turn off this rule. +If you have types which are not accurate, then this rule might cause you to remove conversions that you actually do need. + +## Related To + +- [no-unnecessary-type-assertion](./no-unnecessary-type-assertion.mdx) +- [no-useless-template-literals](./no-useless-template-literals.mdx) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 107f369260ec..1322a06ab616 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -90,6 +90,7 @@ export = { '@typescript-eslint/no-shadow': 'error', '@typescript-eslint/no-this-alias': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', + '@typescript-eslint/no-unnecessary-coercion': 'error', '@typescript-eslint/no-unnecessary-condition': 'error', '@typescript-eslint/no-unnecessary-parameter-property-assignment': 'error', '@typescript-eslint/no-unnecessary-qualifier': 'error', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 7cf867b382f2..f135889dd5e4 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -28,6 +28,7 @@ export = { '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'off', + '@typescript-eslint/no-unnecessary-coercion': 'off', '@typescript-eslint/no-unnecessary-condition': 'off', '@typescript-eslint/no-unnecessary-qualifier': 'off', '@typescript-eslint/no-unnecessary-template-expression': 'off', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 72c62f3e0122..5ee0de48e8da 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -67,6 +67,7 @@ import noShadow from './no-shadow'; import noThisAlias from './no-this-alias'; import noTypeAlias from './no-type-alias'; import noUnnecessaryBooleanLiteralCompare from './no-unnecessary-boolean-literal-compare'; +import noUnnecessaryCoercion from './no-unnecessary-coercion'; import noUnnecessaryCondition from './no-unnecessary-condition'; import noUnnecessaryParameterPropertyAssignment from './no-unnecessary-parameter-property-assignment'; import noUnnecessaryQualifier from './no-unnecessary-qualifier'; @@ -196,6 +197,7 @@ const rules = { 'no-this-alias': noThisAlias, 'no-type-alias': noTypeAlias, 'no-unnecessary-boolean-literal-compare': noUnnecessaryBooleanLiteralCompare, + 'no-unnecessary-coercion': noUnnecessaryCoercion, 'no-unnecessary-condition': noUnnecessaryCondition, 'no-unnecessary-parameter-property-assignment': noUnnecessaryParameterPropertyAssignment, diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-coercion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-coercion.ts new file mode 100644 index 000000000000..b69c811d8ddc --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unnecessary-coercion.ts @@ -0,0 +1,242 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import type { RuleFix } from '@typescript-eslint/utils/ts-eslint'; +import * as ts from 'typescript'; + +import { + createRule, + getConstrainedTypeAtLocation, + getParserServices, + isTypeFlagSet, +} from '../util'; + +type Options = []; +type MessageIds = 'unnecessaryCoercion'; + +export default createRule({ + name: 'no-unnecessary-coercion', + meta: { + docs: { + description: + 'Disallow conversion idioms when they do not change the type or value of the expression', + requiresTypeChecking: true, + }, + fixable: 'code', + messages: { + unnecessaryCoercion: + '{{violation}} does not change the type or value of the {{type}}.', + }, + schema: [], + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + function doesUnderlyingTypeMatchFlag( + type: ts.Type, + typeFlag: ts.TypeFlags, + ): boolean { + const isString = (t: ts.Type): boolean => { + return isTypeFlagSet(t, typeFlag); + }; + + if (type.isUnion()) { + return type.types.every(isString); + } + + if (type.isIntersection()) { + return type.types.some(isString); + } + + return isString(type); + } + + const services = getParserServices(context); + + return { + CallExpression(node: TSESTree.CallExpression): void { + if ( + node.callee.type === AST_NODE_TYPES.Identifier && + node.arguments.length === 1 + ) { + const type = getConstrainedTypeAtLocation( + services, + node.arguments[0], + ); + + if ( + (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.StringLike) && + // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum + node.callee.name === 'String') || + (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike) && + node.callee.name === 'Number') || + (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.BooleanLike) && + // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum + node.callee.name === 'Boolean') || + (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.BigIntLike) && + node.callee.name === 'BigInt') + ) { + context.report({ + node, + messageId: 'unnecessaryCoercion', + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.range[0], node.arguments[0].range[0]]), + fixer.removeRange([node.arguments[0].range[1], node.range[1]]), + ], + data: { + violation: `Passing a ${node.callee.name.toLowerCase()} to ${node.callee.name}()`, + type: node.callee.name.toLowerCase(), + }, + }); + } + } + }, + 'CallExpression > MemberExpression.callee > Identifier[name = "toString"].property'( + node: TSESTree.Expression, + ): void { + const memberExpr = node.parent as TSESTree.MemberExpression; + const type = getConstrainedTypeAtLocation(services, memberExpr.object); + if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.StringLike)) { + context.report({ + node, + messageId: 'unnecessaryCoercion', + fix: (fixer): RuleFix[] => [ + fixer.removeRange([ + memberExpr.parent.range[0], + memberExpr.object.range[0], + ]), + fixer.removeRange([ + memberExpr.object.range[1], + memberExpr.parent.range[1], + ]), + ], + loc: { + start: memberExpr.object.loc.end, + end: memberExpr.parent.loc.end, + }, + data: { + violation: 'Using .toString() on a string', + type: 'string', + }, + }); + } + }, + 'AssignmentExpression[operator = "+="], BinaryExpression[operator = "+"]'( + node: TSESTree.AssignmentExpression | TSESTree.BinaryExpression, + ): void { + const leftType = services.getTypeAtLocation(node.left); + const rightType = services.getTypeAtLocation(node.right); + if ( + doesUnderlyingTypeMatchFlag(leftType, ts.TypeFlags.StringLike) && + node.right.type === AST_NODE_TYPES.Literal && + node.right.value === '' + ) { + context.report({ + node, + messageId: 'unnecessaryCoercion', + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.range[0], node.left.range[0]]), + fixer.removeRange([node.left.range[1], node.range[1]]), + ], + loc: { + start: node.left.loc.end, + end: node.loc.end, + }, + data: { + violation: "Concatenating a string with ''", + type: 'string', + }, + }); + } + if ( + node.left.type === AST_NODE_TYPES.Literal && + node.left.value === '' && + doesUnderlyingTypeMatchFlag(rightType, ts.TypeFlags.StringLike) + ) { + context.report({ + node, + messageId: 'unnecessaryCoercion', + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.range[0], node.right.range[0]]), + fixer.removeRange([node.right.range[1], node.range[1]]), + ], + loc: { + start: node.loc.start, + end: node.right.loc.start, + }, + data: { + violation: "Concatenating '' with a string", + type: 'string', + }, + }); + } + }, + 'UnaryExpression[operator = "+"]'(node: TSESTree.UnaryExpression): void { + const type = services.getTypeAtLocation(node.argument); + if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike)) { + context.report({ + node, + messageId: 'unnecessaryCoercion', + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.range[0], node.argument.range[0]]), + fixer.removeRange([node.argument.range[1], node.range[1]]), + ], + loc: { + start: node.loc.start, + end: node.argument.loc.start, + }, + data: { + violation: 'Using the unary + operator on a number', + type: 'number', + }, + }); + } + }, + 'UnaryExpression[operator = "!"] > UnaryExpression[operator = "!"]'( + node: TSESTree.UnaryExpression, + ): void { + const type = services.getTypeAtLocation(node.argument); + if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.BooleanLike)) { + context.report({ + node, + messageId: 'unnecessaryCoercion', + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.parent.range[0], node.argument.range[0]]), + fixer.removeRange([node.argument.range[1], node.range[1]]), + ], + loc: { + start: node.parent.loc.start, + end: node.argument.loc.start, + }, + data: { + violation: 'Using !! on a boolean', + type: 'boolean', + }, + }); + } + }, + 'UnaryExpression[operator = "~"] > UnaryExpression[operator = "~"]'( + node: TSESTree.UnaryExpression, + ): void { + const type = services.getTypeAtLocation(node.argument); + if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike)) { + context.report({ + node, + messageId: 'unnecessaryCoercion', + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.parent.range[0], node.argument.range[0]]), + fixer.removeRange([node.argument.range[1], node.range[1]]), + ], + loc: { + start: node.parent.loc.start, + end: node.argument.loc.start, + }, + data: { + violation: 'Using ~~ on a number', + type: 'number', + }, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot new file mode 100644 index 000000000000..e60890002628 --- /dev/null +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot @@ -0,0 +1,57 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Validating rule docs no-unnecessary-coercion.mdx code examples ESLint output 1`] = ` +"Incorrect + +String('123'); +~~~~~~~~~~~~~ Passing a string to String() does not change the type or value of the string. +'123'.toString(); + ~~~~~~~~~~~ Using .toString() on a string does not change the type or value of the string. +'' + '123'; +~~~~~ Concatenating '' with a string does not change the type or value of the string. +'123' + ''; + ~~~~~ Concatenating a string with '' does not change the type or value of the string. + +Number(123); +~~~~~~~~~~~ Passing a number to Number() does not change the type or value of the number. ++123; +~ Using the unary + operator on a number does not change the type or value of the number. +~~123; +~~ Using ~~ on a number does not change the type or value of the number. + +Boolean(true); +~~~~~~~~~~~~~ Passing a boolean to Boolean() does not change the type or value of the boolean. +!!true; +~~ Using !! on a boolean does not change the type or value of the boolean. + +BigInt(BigInt(1)); +~~~~~~~~~~~~~~~~~ Passing a bigint to BigInt() does not change the type or value of the bigint. + +let str = '123'; +str += ''; + ~~~~~~ Concatenating a string with '' does not change the type or value of the string. +" +`; + +exports[`Validating rule docs no-unnecessary-coercion.mdx code examples ESLint output 2`] = ` +"Correct + +function foo(bar: string | number) { + String(bar); + bar.toString(); + '' + bar; + bar + ''; + + Number(bar); + +bar; + ~~bar; + + Boolean(bar); + !!bar; + + BigInt(1); + + bar += ''; +} +" +`; diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-coercion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-coercion.test.ts new file mode 100644 index 000000000000..bb05841b77bd --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-coercion.test.ts @@ -0,0 +1,212 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import rule from '../../src/rules/no-unnecessary-coercion'; +import { getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + languageOptions: { + parserOptions: { + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, + }, +}); + +const messageId = 'unnecessaryCoercion'; + +ruleTester.run('no-unnecessary-coercion', rule, { + valid: [ + ` +String(1); + `, + ` +(1).toString(); + `, + ` +\`\${1}\`; + `, + ` +'' + 1; + `, + ` +1 + ''; + `, + ` +let str = 1; +str += ''; + `, + ` +Number('2'); + `, + ` ++'2'; + `, + ` +~~'2'; + `, + ` +Boolean(0); + `, + ` +!!0; + `, + ` +BigInt(3); + `, + ` +new String('asdf'); + `, + ` +!false; + `, + ` +~256; + `, + ], + + invalid: [ + { + code: ` +String('asdf'); + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.CallExpression, + }, + ], + output: ` +'asdf'; + `, + }, + { + code: ` +'asdf'.toString(); + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.Identifier, + }, + ], + output: ` +'asdf'; + `, + }, + { + code: ` +'' + 'asdf'; + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.BinaryExpression, + }, + ], + output: ` +'asdf'; + `, + }, + { + code: ` +'asdf' + ''; + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.BinaryExpression, + }, + ], + output: ` +'asdf'; + `, + }, + { + code: ` +Number(123); + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.CallExpression, + }, + ], + output: ` +123; + `, + }, + { + code: ` ++123; + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.UnaryExpression, + }, + ], + output: ` +123; + `, + }, + { + code: ` +~~123; + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.UnaryExpression, + }, + ], + output: ` +123; + `, + }, + { + code: ` +Boolean(true); + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.CallExpression, + }, + ], + output: ` +true; + `, + }, + { + code: ` +!!true; + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.UnaryExpression, + }, + ], + output: ` +true; + `, + }, + { + code: ` +BigInt(BigInt(1)); + `, + errors: [ + { + messageId, + type: AST_NODE_TYPES.CallExpression, + }, + ], + output: ` +BigInt(1); + `, + }, + ], +}); diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot b/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot new file mode 100644 index 000000000000..b28f7ab24d5c --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot @@ -0,0 +1,14 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Rule schemas should be convertible to TS types for documentation purposes no-unnecessary-coercion 1`] = ` +" +# SCHEMA: + +[] + + +# TYPES: + +/** No options declared */ +type Options = [];" +`; diff --git a/packages/typescript-eslint/src/configs/all.ts b/packages/typescript-eslint/src/configs/all.ts index 1c177f1943bf..3ea323f797fa 100644 --- a/packages/typescript-eslint/src/configs/all.ts +++ b/packages/typescript-eslint/src/configs/all.ts @@ -103,6 +103,7 @@ export default ( '@typescript-eslint/no-shadow': 'error', '@typescript-eslint/no-this-alias': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', + '@typescript-eslint/no-unnecessary-coercion': 'error', '@typescript-eslint/no-unnecessary-condition': 'error', '@typescript-eslint/no-unnecessary-parameter-property-assignment': 'error', diff --git a/packages/typescript-eslint/src/configs/disable-type-checked.ts b/packages/typescript-eslint/src/configs/disable-type-checked.ts index b4c2afd20ec8..0504a77abbcf 100644 --- a/packages/typescript-eslint/src/configs/disable-type-checked.ts +++ b/packages/typescript-eslint/src/configs/disable-type-checked.ts @@ -35,6 +35,7 @@ export default ( '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'off', + '@typescript-eslint/no-unnecessary-coercion': 'off', '@typescript-eslint/no-unnecessary-condition': 'off', '@typescript-eslint/no-unnecessary-qualifier': 'off', '@typescript-eslint/no-unnecessary-template-expression': 'off', From 3060bffd616dd748e4eb91d714366be0b432da69 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 20 Oct 2024 13:26:36 -0400 Subject: [PATCH 02/36] resolve no-unnecessary-coercion rule problems in repository --- eslint.config.mjs | 1 + .../eslint-plugin/src/rules/adjacent-overload-signatures.ts | 2 +- .../src/rules/explicit-function-return-type.ts | 4 ++-- packages/eslint-plugin/src/rules/member-ordering.ts | 2 +- .../eslint-plugin/src/rules/no-duplicate-enum-values.ts | 4 ++-- packages/eslint-plugin/src/rules/no-empty-interface.ts | 5 ++--- .../src/rules/no-redundant-type-constituents.ts | 2 +- .../src/rules/no-unnecessary-template-expression.ts | 6 ++---- packages/eslint-plugin/src/rules/no-unsafe-assignment.ts | 2 +- packages/eslint-plugin/src/rules/prefer-return-this-type.ts | 2 +- packages/website/src/components/ast/HiddenItem.tsx | 2 +- packages/website/src/components/config/ConfigEditor.tsx | 2 +- 12 files changed, 16 insertions(+), 18 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index d7dcd7586712..ebf2f4401d83 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -129,6 +129,7 @@ export default tseslint.config( '@typescript-eslint/explicit-module-boundary-types': 'error', '@typescript-eslint/no-explicit-any': 'error', 'no-constant-condition': 'off', + '@typescript-eslint/no-unnecessary-coercion': 'error', '@typescript-eslint/no-unnecessary-condition': [ 'error', { allowConstantLoopConditions: true, checkTypePredicates: true }, diff --git a/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts b/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts index 4729da05be7c..1c2b5ad1abaa 100644 --- a/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts +++ b/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts @@ -79,7 +79,7 @@ export default createRule({ return { ...getNameFromMember(member, context.sourceCode), callSignature: false, - static: !!member.static, + static: member.static, }; case AST_NODE_TYPES.TSCallSignatureDeclaration: return { diff --git a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts index 186241cb72a2..dab32a85168f 100644 --- a/packages/eslint-plugin/src/rules/explicit-function-return-type.ts +++ b/packages/eslint-plugin/src/rules/explicit-function-return-type.ts @@ -169,14 +169,14 @@ export default createRule({ } } } - if (!!funcName && !!options.allowedNames.includes(funcName)) { + if (!!funcName && options.allowedNames.includes(funcName)) { return true; } } if ( node.type === AST_NODE_TYPES.FunctionDeclaration && node.id && - !!options.allowedNames.includes(node.id.name) + options.allowedNames.includes(node.id.name) ) { return true; } diff --git a/packages/eslint-plugin/src/rules/member-ordering.ts b/packages/eslint-plugin/src/rules/member-ordering.ts index 83b297152267..569e2287aeb9 100644 --- a/packages/eslint-plugin/src/rules/member-ordering.ts +++ b/packages/eslint-plugin/src/rules/member-ordering.ts @@ -488,7 +488,7 @@ function isMemberOptional(node: Member): boolean { case AST_NODE_TYPES.PropertyDefinition: case AST_NODE_TYPES.TSAbstractMethodDefinition: case AST_NODE_TYPES.MethodDefinition: - return !!node.optional; + return node.optional; } return false; } diff --git a/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts b/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts index 41464609c518..bb344522fd60 100644 --- a/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts +++ b/packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts @@ -48,9 +48,9 @@ export default createRule({ let value: number | string | undefined; if (isStringLiteral(member.initializer)) { - value = String(member.initializer.value); + value = member.initializer.value; } else if (isNumberLiteral(member.initializer)) { - value = Number(member.initializer.value); + value = member.initializer.value; } if (value === undefined) { diff --git a/packages/eslint-plugin/src/rules/no-empty-interface.ts b/packages/eslint-plugin/src/rules/no-empty-interface.ts index 855d228dbd34..f2628bd515bd 100644 --- a/packages/eslint-plugin/src/rules/no-empty-interface.ts +++ b/packages/eslint-plugin/src/rules/no-empty-interface.ts @@ -86,11 +86,10 @@ export default createRule({ def => def.node.type === AST_NODE_TYPES.ClassDeclaration, ); - const isInAmbientDeclaration = !!( + const isInAmbientDeclaration = isDefinitionFile(context.filename) && scope.type === ScopeType.tsModule && - scope.block.declare - ); + scope.block.declare; const useAutoFix = !( isInAmbientDeclaration || mergedWithClassDeclaration 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 b550802df58d..acf73ce9326a 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -172,7 +172,7 @@ function describeLiteralTypeNode(typeNode: TSESTree.TypeNode): string { } function isNodeInsideReturnType(node: TSESTree.TSUnionType): boolean { - return !!( + return ( node.parent.type === AST_NODE_TYPES.TSTypeAnnotation && isFunctionOrFunctionType(node.parent.parent) ); diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts index 9c801cefccaa..f1c098895279 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -21,9 +21,7 @@ const evenNumOfBackslashesRegExp = /(?({ @@ -176,7 +174,7 @@ export default createRule<[], MessageId>({ // \\${ -> \\\${ .replaceAll( new RegExp( - `${String(evenNumOfBackslashesRegExp.source)}(\`|\\\${)`, + `${evenNumOfBackslashesRegExp.source}(\`|\\\${)`, 'g', ), '\\$1', diff --git a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts index a9f1358c0c6d..4a68bd24cc40 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-assignment.ts @@ -200,7 +200,7 @@ export default createRule({ receiverProperty.key.type === AST_NODE_TYPES.TemplateLiteral && receiverProperty.key.quasis.length === 1 ) { - key = String(receiverProperty.key.quasis[0].value.cooked); + key = receiverProperty.key.quasis[0].value.cooked; } else { // can't figure out the name, so skip it continue; diff --git a/packages/eslint-plugin/src/rules/prefer-return-this-type.ts b/packages/eslint-plugin/src/rules/prefer-return-this-type.ts index 304f63dd1027..dd3ba1d7dc49 100644 --- a/packages/eslint-plugin/src/rules/prefer-return-this-type.ts +++ b/packages/eslint-plugin/src/rules/prefer-return-this-type.ts @@ -62,7 +62,7 @@ export default createRule({ function isThisSpecifiedInParameters(originalFunc: FunctionLike): boolean { const firstArg = originalFunc.params.at(0); - return !!( + return ( firstArg?.type === AST_NODE_TYPES.Identifier && firstArg.name === 'this' ); } diff --git a/packages/website/src/components/ast/HiddenItem.tsx b/packages/website/src/components/ast/HiddenItem.tsx index 500f017bf9cf..1ccd52ca92b1 100644 --- a/packages/website/src/components/ast/HiddenItem.tsx +++ b/packages/website/src/components/ast/HiddenItem.tsx @@ -42,7 +42,7 @@ export default function HiddenItem({ value.map(([key], index) => ( {index > 0 && ', '} - {String(key)} + {key} )) )} diff --git a/packages/website/src/components/config/ConfigEditor.tsx b/packages/website/src/components/config/ConfigEditor.tsx index 1c8a572a6e87..827cd8b78060 100644 --- a/packages/website/src/components/config/ConfigEditor.tsx +++ b/packages/website/src/components/config/ConfigEditor.tsx @@ -36,7 +36,7 @@ function filterConfig( .map(group => ({ heading: group.heading, fields: group.fields.filter(item => - String(item.key.toLowerCase()).includes(filter.toLowerCase()), + item.key.toLowerCase().includes(filter.toLowerCase()), ), })) .filter(group => group.fields.length > 0); From 393fb2f0a8634e34160651e668bf0c54a88d5878 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 3 Nov 2024 07:52:11 -0500 Subject: [PATCH 03/36] rename to no-unnecessary-type-conversion --- eslint.config.mjs | 2 +- .../docs/rules/no-unnecessary-coercion.mdx | 2 +- packages/eslint-plugin/src/configs/all.ts | 2 +- .../src/configs/disable-type-checked.ts | 2 +- packages/eslint-plugin/src/rules/index.ts | 4 ++-- ...n.ts => no-unnecessary-type-conversion.ts} | 20 +++++++++---------- .../no-unnecessary-coercion.shot | 4 ++-- ...=> no-unnecessary-type-conversion.test.ts} | 6 +++--- .../no-unnecessary-coercion.shot | 2 +- packages/typescript-eslint/src/configs/all.ts | 2 +- .../src/configs/disable-type-checked.ts | 2 +- 11 files changed, 24 insertions(+), 24 deletions(-) rename packages/eslint-plugin/src/rules/{no-unnecessary-coercion.ts => no-unnecessary-type-conversion.ts} (94%) rename packages/eslint-plugin/tests/rules/{no-unnecessary-coercion.test.ts => no-unnecessary-type-conversion.test.ts} (94%) diff --git a/eslint.config.mjs b/eslint.config.mjs index ebf2f4401d83..a83f81c86e7d 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -129,11 +129,11 @@ export default tseslint.config( '@typescript-eslint/explicit-module-boundary-types': 'error', '@typescript-eslint/no-explicit-any': 'error', 'no-constant-condition': 'off', - '@typescript-eslint/no-unnecessary-coercion': 'error', '@typescript-eslint/no-unnecessary-condition': [ 'error', { allowConstantLoopConditions: true, checkTypePredicates: true }, ], + '@typescript-eslint/no-unnecessary-type-conversion': 'error', '@typescript-eslint/no-unnecessary-type-parameters': 'error', '@typescript-eslint/no-unused-expressions': 'error', '@typescript-eslint/no-var-requires': 'off', diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx b/packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx index aca0aa9fb1d2..af6c0297720f 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx @@ -7,7 +7,7 @@ import TabItem from '@theme/TabItem'; > 🛑 This file is source code, not the primary documentation location! 🛑 > -> See **https://typescript-eslint.io/rules/no-unnecessary-coercion** for documentation. +> See **https://typescript-eslint.io/rules/no-unnecessary-type-conversion** for documentation. JavaScript has several idioms used to convert values to certain types. With TypeScript, it is possible to see at build time if the value is already of that type, making the conversion unnecessary. Performing unnecessary conversions increases visual clutter and harms code readability, so it's generally best practice to remove them if they don't change the type of an expression. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 1322a06ab616..398a789106be 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -90,7 +90,7 @@ export = { '@typescript-eslint/no-shadow': 'error', '@typescript-eslint/no-this-alias': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', - '@typescript-eslint/no-unnecessary-coercion': 'error', + '@typescript-eslint/no-unnecessary-type-conversion': 'error', '@typescript-eslint/no-unnecessary-condition': 'error', '@typescript-eslint/no-unnecessary-parameter-property-assignment': 'error', '@typescript-eslint/no-unnecessary-qualifier': 'error', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index f135889dd5e4..60dc33e34b1e 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -28,7 +28,7 @@ export = { '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'off', - '@typescript-eslint/no-unnecessary-coercion': 'off', + '@typescript-eslint/no-unnecessary-type-conversion': 'off', '@typescript-eslint/no-unnecessary-condition': 'off', '@typescript-eslint/no-unnecessary-qualifier': 'off', '@typescript-eslint/no-unnecessary-template-expression': 'off', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 5ee0de48e8da..b29075ede75b 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -67,7 +67,6 @@ import noShadow from './no-shadow'; import noThisAlias from './no-this-alias'; import noTypeAlias from './no-type-alias'; import noUnnecessaryBooleanLiteralCompare from './no-unnecessary-boolean-literal-compare'; -import noUnnecessaryCoercion from './no-unnecessary-coercion'; import noUnnecessaryCondition from './no-unnecessary-condition'; import noUnnecessaryParameterPropertyAssignment from './no-unnecessary-parameter-property-assignment'; import noUnnecessaryQualifier from './no-unnecessary-qualifier'; @@ -75,6 +74,7 @@ import noUnnecessaryTemplateExpression from './no-unnecessary-template-expressio import noUnnecessaryTypeArguments from './no-unnecessary-type-arguments'; import noUnnecessaryTypeAssertion from './no-unnecessary-type-assertion'; import noUnnecessaryTypeConstraint from './no-unnecessary-type-constraint'; +import noUnnecessaryTypeConversion from './no-unnecessary-type-conversion'; import noUnnecessaryTypeParameters from './no-unnecessary-type-parameters'; import noUnsafeArgument from './no-unsafe-argument'; import noUnsafeAssignment from './no-unsafe-assignment'; @@ -197,7 +197,6 @@ const rules = { 'no-this-alias': noThisAlias, 'no-type-alias': noTypeAlias, 'no-unnecessary-boolean-literal-compare': noUnnecessaryBooleanLiteralCompare, - 'no-unnecessary-coercion': noUnnecessaryCoercion, 'no-unnecessary-condition': noUnnecessaryCondition, 'no-unnecessary-parameter-property-assignment': noUnnecessaryParameterPropertyAssignment, @@ -206,6 +205,7 @@ const rules = { 'no-unnecessary-type-arguments': noUnnecessaryTypeArguments, 'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion, 'no-unnecessary-type-constraint': noUnnecessaryTypeConstraint, + 'no-unnecessary-type-conversion': noUnnecessaryTypeConversion, 'no-unnecessary-type-parameters': noUnnecessaryTypeParameters, 'no-unsafe-argument': noUnsafeArgument, 'no-unsafe-assignment': noUnsafeAssignment, diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-coercion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts similarity index 94% rename from packages/eslint-plugin/src/rules/no-unnecessary-coercion.ts rename to packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index b69c811d8ddc..51cee8d11d4a 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-coercion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -11,10 +11,10 @@ import { } from '../util'; type Options = []; -type MessageIds = 'unnecessaryCoercion'; +type MessageIds = 'unnecessaryTypeConversion'; export default createRule({ - name: 'no-unnecessary-coercion', + name: 'no-unnecessary-type-conversion', meta: { docs: { description: @@ -23,7 +23,7 @@ export default createRule({ }, fixable: 'code', messages: { - unnecessaryCoercion: + unnecessaryTypeConversion: '{{violation}} does not change the type or value of the {{type}}.', }, schema: [], @@ -77,7 +77,7 @@ export default createRule({ ) { context.report({ node, - messageId: 'unnecessaryCoercion', + messageId: 'unnecessaryTypeConversion', fix: (fixer): RuleFix[] => [ fixer.removeRange([node.range[0], node.arguments[0].range[0]]), fixer.removeRange([node.arguments[0].range[1], node.range[1]]), @@ -98,7 +98,7 @@ export default createRule({ if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.StringLike)) { context.report({ node, - messageId: 'unnecessaryCoercion', + messageId: 'unnecessaryTypeConversion', fix: (fixer): RuleFix[] => [ fixer.removeRange([ memberExpr.parent.range[0], @@ -132,7 +132,7 @@ export default createRule({ ) { context.report({ node, - messageId: 'unnecessaryCoercion', + messageId: 'unnecessaryTypeConversion', fix: (fixer): RuleFix[] => [ fixer.removeRange([node.range[0], node.left.range[0]]), fixer.removeRange([node.left.range[1], node.range[1]]), @@ -154,7 +154,7 @@ export default createRule({ ) { context.report({ node, - messageId: 'unnecessaryCoercion', + messageId: 'unnecessaryTypeConversion', fix: (fixer): RuleFix[] => [ fixer.removeRange([node.range[0], node.right.range[0]]), fixer.removeRange([node.right.range[1], node.range[1]]), @@ -175,7 +175,7 @@ export default createRule({ if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike)) { context.report({ node, - messageId: 'unnecessaryCoercion', + messageId: 'unnecessaryTypeConversion', fix: (fixer): RuleFix[] => [ fixer.removeRange([node.range[0], node.argument.range[0]]), fixer.removeRange([node.argument.range[1], node.range[1]]), @@ -198,7 +198,7 @@ export default createRule({ if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.BooleanLike)) { context.report({ node, - messageId: 'unnecessaryCoercion', + messageId: 'unnecessaryTypeConversion', fix: (fixer): RuleFix[] => [ fixer.removeRange([node.parent.range[0], node.argument.range[0]]), fixer.removeRange([node.argument.range[1], node.range[1]]), @@ -221,7 +221,7 @@ export default createRule({ if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike)) { context.report({ node, - messageId: 'unnecessaryCoercion', + messageId: 'unnecessaryTypeConversion', fix: (fixer): RuleFix[] => [ fixer.removeRange([node.parent.range[0], node.argument.range[0]]), fixer.removeRange([node.argument.range[1], node.range[1]]), diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot index e60890002628..bd3623d3c0f6 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Validating rule docs no-unnecessary-coercion.mdx code examples ESLint output 1`] = ` +exports[`Validating rule docs no-unnecessary-type-conversion.mdx code examples ESLint output 1`] = ` "Incorrect String('123'); @@ -33,7 +33,7 @@ str += ''; " `; -exports[`Validating rule docs no-unnecessary-coercion.mdx code examples ESLint output 2`] = ` +exports[`Validating rule docs no-unnecessary-type-conversion.mdx code examples ESLint output 2`] = ` "Correct function foo(bar: string | number) { diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-coercion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts similarity index 94% rename from packages/eslint-plugin/tests/rules/no-unnecessary-coercion.test.ts rename to packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index bb05841b77bd..2e2fcf5f2993 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-coercion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -1,7 +1,7 @@ import { RuleTester } from '@typescript-eslint/rule-tester'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import rule from '../../src/rules/no-unnecessary-coercion'; +import rule from '../../src/rules/no-unnecessary-type-conversion'; import { getFixturesRootDir } from '../RuleTester'; const rootDir = getFixturesRootDir(); @@ -15,9 +15,9 @@ const ruleTester = new RuleTester({ }, }); -const messageId = 'unnecessaryCoercion'; +const messageId = 'unnecessaryTypeConversion'; -ruleTester.run('no-unnecessary-coercion', rule, { +ruleTester.run('no-unnecessary-type-conversion', rule, { valid: [ ` String(1); diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot b/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot index b28f7ab24d5c..d269eed9d351 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Rule schemas should be convertible to TS types for documentation purposes no-unnecessary-coercion 1`] = ` +exports[`Rule schemas should be convertible to TS types for documentation purposes no-unnecessary-type-conversion 1`] = ` " # SCHEMA: diff --git a/packages/typescript-eslint/src/configs/all.ts b/packages/typescript-eslint/src/configs/all.ts index 3ea323f797fa..8e70171fdbdd 100644 --- a/packages/typescript-eslint/src/configs/all.ts +++ b/packages/typescript-eslint/src/configs/all.ts @@ -103,7 +103,7 @@ export default ( '@typescript-eslint/no-shadow': 'error', '@typescript-eslint/no-this-alias': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', - '@typescript-eslint/no-unnecessary-coercion': 'error', + '@typescript-eslint/no-unnecessary-type-conversion': 'error', '@typescript-eslint/no-unnecessary-condition': 'error', '@typescript-eslint/no-unnecessary-parameter-property-assignment': 'error', diff --git a/packages/typescript-eslint/src/configs/disable-type-checked.ts b/packages/typescript-eslint/src/configs/disable-type-checked.ts index 0504a77abbcf..31709c5fd824 100644 --- a/packages/typescript-eslint/src/configs/disable-type-checked.ts +++ b/packages/typescript-eslint/src/configs/disable-type-checked.ts @@ -35,7 +35,7 @@ export default ( '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'off', - '@typescript-eslint/no-unnecessary-coercion': 'off', + '@typescript-eslint/no-unnecessary-type-conversion': 'off', '@typescript-eslint/no-unnecessary-condition': 'off', '@typescript-eslint/no-unnecessary-qualifier': 'off', '@typescript-eslint/no-unnecessary-template-expression': 'off', From 00476f9334106c05484a089b825214632c0d7d73 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 3 Nov 2024 08:49:26 -0500 Subject: [PATCH 04/36] make perfectionist happy --- .../rules/no-unnecessary-type-conversion.ts | 175 +++++++++--------- 1 file changed, 88 insertions(+), 87 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 51cee8d11d4a..bcbc26876da8 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -1,6 +1,7 @@ import type { TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import type { RuleFix } from '@typescript-eslint/utils/ts-eslint'; + +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as ts from 'typescript'; import { @@ -16,6 +17,7 @@ type MessageIds = 'unnecessaryTypeConversion'; export default createRule({ name: 'no-unnecessary-type-conversion', meta: { + type: 'suggestion', docs: { description: 'Disallow conversion idioms when they do not change the type or value of the expression', @@ -27,7 +29,6 @@ export default createRule({ '{{violation}} does not change the type or value of the {{type}}.', }, schema: [], - type: 'suggestion', }, defaultOptions: [], create(context) { @@ -53,6 +54,56 @@ export default createRule({ const services = getParserServices(context); return { + 'AssignmentExpression[operator = "+="], BinaryExpression[operator = "+"]'( + node: TSESTree.AssignmentExpression | TSESTree.BinaryExpression, + ): void { + const leftType = services.getTypeAtLocation(node.left); + const rightType = services.getTypeAtLocation(node.right); + if ( + doesUnderlyingTypeMatchFlag(leftType, ts.TypeFlags.StringLike) && + node.right.type === AST_NODE_TYPES.Literal && + node.right.value === '' + ) { + context.report({ + loc: { + start: node.left.loc.end, + end: node.loc.end, + }, + node, + messageId: 'unnecessaryTypeConversion', + data: { + type: 'string', + violation: "Concatenating a string with ''", + }, + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.range[0], node.left.range[0]]), + fixer.removeRange([node.left.range[1], node.range[1]]), + ], + }); + } + if ( + node.left.type === AST_NODE_TYPES.Literal && + node.left.value === '' && + doesUnderlyingTypeMatchFlag(rightType, ts.TypeFlags.StringLike) + ) { + context.report({ + loc: { + start: node.loc.start, + end: node.right.loc.start, + }, + node, + messageId: 'unnecessaryTypeConversion', + data: { + type: 'string', + violation: "Concatenating '' with a string", + }, + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.range[0], node.right.range[0]]), + fixer.removeRange([node.right.range[1], node.range[1]]), + ], + }); + } + }, CallExpression(node: TSESTree.CallExpression): void { if ( node.callee.type === AST_NODE_TYPES.Identifier && @@ -78,14 +129,14 @@ export default createRule({ context.report({ node, messageId: 'unnecessaryTypeConversion', + data: { + type: node.callee.name.toLowerCase(), + violation: `Passing a ${node.callee.name.toLowerCase()} to ${node.callee.name}()`, + }, fix: (fixer): RuleFix[] => [ fixer.removeRange([node.range[0], node.arguments[0].range[0]]), fixer.removeRange([node.arguments[0].range[1], node.range[1]]), ], - data: { - violation: `Passing a ${node.callee.name.toLowerCase()} to ${node.callee.name}()`, - type: node.callee.name.toLowerCase(), - }, }); } } @@ -97,8 +148,16 @@ export default createRule({ const type = getConstrainedTypeAtLocation(services, memberExpr.object); if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.StringLike)) { context.report({ + loc: { + start: memberExpr.object.loc.end, + end: memberExpr.parent.loc.end, + }, node, messageId: 'unnecessaryTypeConversion', + data: { + type: 'string', + violation: 'Using .toString() on a string', + }, fix: (fixer): RuleFix[] => [ fixer.removeRange([ memberExpr.parent.range[0], @@ -109,64 +168,29 @@ export default createRule({ memberExpr.parent.range[1], ]), ], - loc: { - start: memberExpr.object.loc.end, - end: memberExpr.parent.loc.end, - }, - data: { - violation: 'Using .toString() on a string', - type: 'string', - }, }); } }, - 'AssignmentExpression[operator = "+="], BinaryExpression[operator = "+"]'( - node: TSESTree.AssignmentExpression | TSESTree.BinaryExpression, + 'UnaryExpression[operator = "!"] > UnaryExpression[operator = "!"]'( + node: TSESTree.UnaryExpression, ): void { - const leftType = services.getTypeAtLocation(node.left); - const rightType = services.getTypeAtLocation(node.right); - if ( - doesUnderlyingTypeMatchFlag(leftType, ts.TypeFlags.StringLike) && - node.right.type === AST_NODE_TYPES.Literal && - node.right.value === '' - ) { + const type = services.getTypeAtLocation(node.argument); + if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.BooleanLike)) { context.report({ - node, - messageId: 'unnecessaryTypeConversion', - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.left.range[0]]), - fixer.removeRange([node.left.range[1], node.range[1]]), - ], loc: { - start: node.left.loc.end, - end: node.loc.end, - }, - data: { - violation: "Concatenating a string with ''", - type: 'string', + start: node.parent.loc.start, + end: node.argument.loc.start, }, - }); - } - if ( - node.left.type === AST_NODE_TYPES.Literal && - node.left.value === '' && - doesUnderlyingTypeMatchFlag(rightType, ts.TypeFlags.StringLike) - ) { - context.report({ node, messageId: 'unnecessaryTypeConversion', - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.right.range[0]]), - fixer.removeRange([node.right.range[1], node.range[1]]), - ], - loc: { - start: node.loc.start, - end: node.right.loc.start, - }, data: { - violation: "Concatenating '' with a string", - type: 'string', + type: 'boolean', + violation: 'Using !! on a boolean', }, + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.parent.range[0], node.argument.range[0]]), + fixer.removeRange([node.argument.range[1], node.range[1]]), + ], }); } }, @@ -174,43 +198,20 @@ export default createRule({ const type = services.getTypeAtLocation(node.argument); if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike)) { context.report({ - node, - messageId: 'unnecessaryTypeConversion', - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.argument.range[0]]), - fixer.removeRange([node.argument.range[1], node.range[1]]), - ], loc: { start: node.loc.start, end: node.argument.loc.start, }, + node, + messageId: 'unnecessaryTypeConversion', data: { - violation: 'Using the unary + operator on a number', type: 'number', + violation: 'Using the unary + operator on a number', }, - }); - } - }, - 'UnaryExpression[operator = "!"] > UnaryExpression[operator = "!"]'( - node: TSESTree.UnaryExpression, - ): void { - const type = services.getTypeAtLocation(node.argument); - if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.BooleanLike)) { - context.report({ - node, - messageId: 'unnecessaryTypeConversion', fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.parent.range[0], node.argument.range[0]]), + fixer.removeRange([node.range[0], node.argument.range[0]]), fixer.removeRange([node.argument.range[1], node.range[1]]), ], - loc: { - start: node.parent.loc.start, - end: node.argument.loc.start, - }, - data: { - violation: 'Using !! on a boolean', - type: 'boolean', - }, }); } }, @@ -220,20 +221,20 @@ export default createRule({ const type = services.getTypeAtLocation(node.argument); if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike)) { context.report({ - node, - messageId: 'unnecessaryTypeConversion', - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.parent.range[0], node.argument.range[0]]), - fixer.removeRange([node.argument.range[1], node.range[1]]), - ], loc: { start: node.parent.loc.start, end: node.argument.loc.start, }, + node, + messageId: 'unnecessaryTypeConversion', data: { - violation: 'Using ~~ on a number', type: 'number', + violation: 'Using ~~ on a number', }, + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.parent.range[0], node.argument.range[0]]), + fixer.removeRange([node.argument.range[1], node.range[1]]), + ], }); } }, From 89ecd74d272e3c57d92c0fb35a796a2eeeba98db Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 3 Nov 2024 08:49:51 -0500 Subject: [PATCH 05/36] rename isString to matchesType to accurately reflect that it works with more than just string types --- .../src/rules/no-unnecessary-type-conversion.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index bcbc26876da8..c8ec4bdf9b85 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -36,19 +36,19 @@ export default createRule({ type: ts.Type, typeFlag: ts.TypeFlags, ): boolean { - const isString = (t: ts.Type): boolean => { + const matchesType = (t: ts.Type): boolean => { return isTypeFlagSet(t, typeFlag); }; if (type.isUnion()) { - return type.types.every(isString); + return type.types.every(matchesType); } if (type.isIntersection()) { - return type.types.some(isString); + return type.types.some(matchesType); } - return isString(type); + return matchesType(type); } const services = getParserServices(context); From 879dd41995b028cd5420051bd547ec1664269416 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 3 Nov 2024 08:50:51 -0500 Subject: [PATCH 06/36] don't pass node to context.report if a loc is already being passed --- .../src/rules/no-unnecessary-type-conversion.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index c8ec4bdf9b85..82ff454dad5b 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -69,7 +69,6 @@ export default createRule({ start: node.left.loc.end, end: node.loc.end, }, - node, messageId: 'unnecessaryTypeConversion', data: { type: 'string', @@ -91,7 +90,6 @@ export default createRule({ start: node.loc.start, end: node.right.loc.start, }, - node, messageId: 'unnecessaryTypeConversion', data: { type: 'string', @@ -152,7 +150,6 @@ export default createRule({ start: memberExpr.object.loc.end, end: memberExpr.parent.loc.end, }, - node, messageId: 'unnecessaryTypeConversion', data: { type: 'string', @@ -181,7 +178,6 @@ export default createRule({ start: node.parent.loc.start, end: node.argument.loc.start, }, - node, messageId: 'unnecessaryTypeConversion', data: { type: 'boolean', @@ -202,7 +198,6 @@ export default createRule({ start: node.loc.start, end: node.argument.loc.start, }, - node, messageId: 'unnecessaryTypeConversion', data: { type: 'number', @@ -225,7 +220,6 @@ export default createRule({ start: node.parent.loc.start, end: node.argument.loc.start, }, - node, messageId: 'unnecessaryTypeConversion', data: { type: 'number', From 1638cb33f63302dba2de09001a98c899d562d80f Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 3 Nov 2024 09:09:28 -0500 Subject: [PATCH 07/36] improve loc generation for !!, +, and ~~ operators --- .../src/rules/no-unnecessary-type-conversion.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 82ff454dad5b..2adb12eb8192 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -176,7 +176,10 @@ export default createRule({ context.report({ loc: { start: node.parent.loc.start, - end: node.argument.loc.start, + end: { + column: node.loc.start.column + 1, + line: node.loc.start.line, + }, }, messageId: 'unnecessaryTypeConversion', data: { @@ -196,7 +199,10 @@ export default createRule({ context.report({ loc: { start: node.loc.start, - end: node.argument.loc.start, + end: { + column: node.loc.start.column + 1, + line: node.loc.start.line, + }, }, messageId: 'unnecessaryTypeConversion', data: { @@ -218,7 +224,10 @@ export default createRule({ context.report({ loc: { start: node.parent.loc.start, - end: node.argument.loc.start, + end: { + column: node.loc.start.column + 1, + line: node.loc.start.line, + }, }, messageId: 'unnecessaryTypeConversion', data: { From 0112f0a8bd48f9d971cc672be1ba25e033f6e2a8 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 3 Nov 2024 09:20:17 -0500 Subject: [PATCH 08/36] don't use messageId constant in tests --- .../tests/rules/await-thenable.test.ts | 19 +++-- .../tests/rules/no-array-constructor.test.ts | 16 ++--- .../rules/no-unnecessary-qualifier.test.ts | 20 +++--- .../no-unnecessary-type-conversion.test.ts | 24 +++---- .../rules/promise-function-async.test.ts | 69 +++++++++---------- 5 files changed, 70 insertions(+), 78 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts index c91542819bb8..ad9213f4f410 100644 --- a/packages/eslint-plugin/tests/rules/await-thenable.test.ts +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -4,7 +4,6 @@ import rule from '../../src/rules/await-thenable'; import { getFixturesRootDir } from '../RuleTester'; const rootDir = getFixturesRootDir(); -const messageId = 'await'; const ruleTester = new RuleTester({ languageOptions: { @@ -235,7 +234,7 @@ for await (const s of asyncIter) { errors: [ { line: 1, - messageId, + messageId: 'await', suggestions: [ { messageId: 'removeAwait', @@ -250,7 +249,7 @@ for await (const s of asyncIter) { errors: [ { line: 1, - messageId, + messageId: 'await', suggestions: [ { messageId: 'removeAwait', @@ -265,7 +264,7 @@ for await (const s of asyncIter) { errors: [ { line: 1, - messageId, + messageId: 'await', suggestions: [ { messageId: 'removeAwait', @@ -280,7 +279,7 @@ for await (const s of asyncIter) { errors: [ { line: 1, - messageId, + messageId: 'await', suggestions: [ { messageId: 'removeAwait', @@ -298,7 +297,7 @@ await new NonPromise(); errors: [ { line: 3, - messageId, + messageId: 'await', suggestions: [ { messageId: 'removeAwait', @@ -325,7 +324,7 @@ async function test() { errors: [ { line: 8, - messageId, + messageId: 'await', suggestions: [ { messageId: 'removeAwait', @@ -352,7 +351,7 @@ await callback?.(); errors: [ { line: 3, - messageId, + messageId: 'await', suggestions: [ { messageId: 'removeAwait', @@ -373,7 +372,7 @@ await obj.a?.b?.(); errors: [ { line: 3, - messageId, + messageId: 'await', suggestions: [ { messageId: 'removeAwait', @@ -394,7 +393,7 @@ await obj?.a.b.c?.(); errors: [ { line: 3, - messageId, + messageId: 'await', suggestions: [ { messageId: 'removeAwait', diff --git a/packages/eslint-plugin/tests/rules/no-array-constructor.test.ts b/packages/eslint-plugin/tests/rules/no-array-constructor.test.ts index 4ce184ce984b..50bfc6f299ca 100644 --- a/packages/eslint-plugin/tests/rules/no-array-constructor.test.ts +++ b/packages/eslint-plugin/tests/rules/no-array-constructor.test.ts @@ -5,8 +5,6 @@ import rule from '../../src/rules/no-array-constructor'; const ruleTester = new RuleTester(); -const messageId = 'useLiteral'; - ruleTester.run('no-array-constructor', rule, { valid: [ 'new Array(x);', @@ -42,7 +40,7 @@ ruleTester.run('no-array-constructor', rule, { code: 'new Array();', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.NewExpression, }, ], @@ -52,7 +50,7 @@ ruleTester.run('no-array-constructor', rule, { code: 'Array();', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.CallExpression, }, ], @@ -62,7 +60,7 @@ ruleTester.run('no-array-constructor', rule, { code: 'new Array(x, y);', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.NewExpression, }, ], @@ -72,7 +70,7 @@ ruleTester.run('no-array-constructor', rule, { code: 'Array(x, y);', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.CallExpression, }, ], @@ -82,7 +80,7 @@ ruleTester.run('no-array-constructor', rule, { code: 'new Array(0, 1, 2);', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.NewExpression, }, ], @@ -92,7 +90,7 @@ ruleTester.run('no-array-constructor', rule, { code: 'Array(0, 1, 2);', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.CallExpression, }, ], @@ -104,7 +102,7 @@ new Array(0, 1, 2); `, errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.NewExpression, }, ], diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-qualifier.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-qualifier.test.ts index 80ba657442a2..91a153d7dd33 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-qualifier.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-qualifier.test.ts @@ -15,8 +15,6 @@ const ruleTester = new RuleTester({ }, }); -const messageId = 'unnecessaryQualifier'; - ruleTester.run('no-unnecessary-qualifier', rule, { valid: [ ` @@ -95,7 +93,7 @@ namespace A { `, errors: [ { - messageId, + messageId: 'unnecessaryQualifier', type: AST_NODE_TYPES.Identifier, }, ], @@ -115,7 +113,7 @@ namespace A { `, errors: [ { - messageId, + messageId: 'unnecessaryQualifier', type: AST_NODE_TYPES.Identifier, }, ], @@ -137,7 +135,7 @@ namespace A { `, errors: [ { - messageId, + messageId: 'unnecessaryQualifier', type: AST_NODE_TYPES.Identifier, }, ], @@ -161,7 +159,7 @@ namespace A { `, errors: [ { - messageId, + messageId: 'unnecessaryQualifier', type: AST_NODE_TYPES.TSQualifiedName, }, ], @@ -185,7 +183,7 @@ namespace A { `, errors: [ { - messageId, + messageId: 'unnecessaryQualifier', type: AST_NODE_TYPES.TSQualifiedName, }, ], @@ -209,7 +207,7 @@ namespace A { `, errors: [ { - messageId, + messageId: 'unnecessaryQualifier', type: AST_NODE_TYPES.MemberExpression, }, ], @@ -231,7 +229,7 @@ enum A { `, errors: [ { - messageId, + messageId: 'unnecessaryQualifier', type: AST_NODE_TYPES.Identifier, }, ], @@ -253,7 +251,7 @@ namespace Foo { `, errors: [ { - messageId, + messageId: 'unnecessaryQualifier', type: AST_NODE_TYPES.MemberExpression, }, ], @@ -275,7 +273,7 @@ declare module './foo' { `, errors: [ { - messageId, + messageId: 'unnecessaryQualifier', type: AST_NODE_TYPES.Identifier, }, ], diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index 2e2fcf5f2993..d04f8b879ea9 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -9,14 +9,12 @@ const rootDir = getFixturesRootDir(); const ruleTester = new RuleTester({ languageOptions: { parserOptions: { - tsconfigRootDir: rootDir, project: './tsconfig.json', + tsconfigRootDir: rootDir, }, }, }); -const messageId = 'unnecessaryTypeConversion'; - ruleTester.run('no-unnecessary-type-conversion', rule, { valid: [ ` @@ -74,7 +72,7 @@ String('asdf'); `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.CallExpression, }, ], @@ -88,7 +86,7 @@ String('asdf'); `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.Identifier, }, ], @@ -102,7 +100,7 @@ String('asdf'); `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.BinaryExpression, }, ], @@ -116,7 +114,7 @@ String('asdf'); `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.BinaryExpression, }, ], @@ -130,7 +128,7 @@ Number(123); `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.CallExpression, }, ], @@ -144,7 +142,7 @@ Number(123); `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.UnaryExpression, }, ], @@ -158,7 +156,7 @@ Number(123); `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.UnaryExpression, }, ], @@ -172,7 +170,7 @@ Boolean(true); `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.CallExpression, }, ], @@ -186,7 +184,7 @@ true; `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.UnaryExpression, }, ], @@ -200,7 +198,7 @@ BigInt(BigInt(1)); `, errors: [ { - messageId, + messageId: 'unnecessaryTypeConversion', type: AST_NODE_TYPES.CallExpression, }, ], diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index 51473ffaea00..fc217ea3efcd 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -4,7 +4,6 @@ import rule from '../../src/rules/promise-function-async'; import { getFixturesRootDir } from '../RuleTester'; const rootDir = getFixturesRootDir(); -const messageId = 'missingAsync'; const ruleTester = new RuleTester({ languageOptions: { @@ -192,7 +191,7 @@ function returnsAny(): any { `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], options: [ @@ -210,7 +209,7 @@ function returnsUnknown(): unknown { `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], options: [ @@ -228,7 +227,7 @@ const nonAsyncPromiseFunctionExpressionA = function (p: Promise) { `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -245,7 +244,7 @@ const nonAsyncPromiseFunctionExpressionB = function () { `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -262,7 +261,7 @@ function nonAsyncPromiseFunctionDeclarationA(p: Promise) { `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -279,7 +278,7 @@ function nonAsyncPromiseFunctionDeclarationB() { `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -294,7 +293,7 @@ const nonAsyncPromiseArrowFunctionA = (p: Promise) => p; `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -307,7 +306,7 @@ const nonAsyncPromiseArrowFunctionB = () => new Promise(); `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -325,7 +324,7 @@ const functions = { errors: [ { line: 3, - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -351,11 +350,11 @@ class Test { errors: [ { line: 3, - messageId, + messageId: 'missingAsync', }, { line: 7, - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -391,15 +390,15 @@ class Test { errors: [ { line: 2, - messageId, + messageId: 'missingAsync', }, { line: 6, - messageId, + messageId: 'missingAsync', }, { line: 13, - messageId, + messageId: 'missingAsync', }, ], options: [ @@ -446,15 +445,15 @@ class Test { errors: [ { line: 2, - messageId, + messageId: 'missingAsync', }, { line: 10, - messageId, + messageId: 'missingAsync', }, { line: 13, - messageId, + messageId: 'missingAsync', }, ], options: [ @@ -501,15 +500,15 @@ class Test { errors: [ { line: 6, - messageId, + messageId: 'missingAsync', }, { line: 10, - messageId, + messageId: 'missingAsync', }, { line: 13, - messageId, + messageId: 'missingAsync', }, ], options: [ @@ -556,15 +555,15 @@ class Test { errors: [ { line: 2, - messageId, + messageId: 'missingAsync', }, { line: 6, - messageId, + messageId: 'missingAsync', }, { line: 10, - messageId, + messageId: 'missingAsync', }, ], options: [ @@ -599,7 +598,7 @@ const returnAllowedType = () => new PromiseType(); errors: [ { line: 4, - messageId, + messageId: 'missingAsync', }, ], options: [ @@ -625,7 +624,7 @@ function foo(): Promise | SPromise { errors: [ { line: 3, - messageId, + messageId: 'missingAsync', }, ], options: [ @@ -651,7 +650,7 @@ class Test { } } `, - errors: [{ column: 3, line: 4, messageId }], + errors: [{ column: 3, line: 4, messageId: 'missingAsync' }], output: ` class Test { @decorator @@ -677,9 +676,9 @@ class Test { } `, errors: [ - { column: 3, line: 4, messageId }, - { column: 3, line: 7, messageId }, - { column: 3, line: 10, messageId }, + { column: 3, line: 4, messageId: 'missingAsync' }, + { column: 3, line: 7, messageId: 'missingAsync' }, + { column: 3, line: 10, messageId: 'missingAsync' }, ], output: ` class Test { @@ -718,17 +717,17 @@ class Foo { { column: 3, line: 3, - messageId, + messageId: 'missingAsync', }, { column: 3, line: 7, - messageId, + messageId: 'missingAsync', }, { column: 3, line: 12, - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -760,7 +759,7 @@ const foo = { { column: 3, line: 3, - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -779,7 +778,7 @@ function promiseInUnionWithoutExplicitReturnType(p: boolean) { `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], output: ` From bfd8c5ba472fcfafe44f2ac4c96f841fa7b32204 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 3 Nov 2024 09:40:36 -0500 Subject: [PATCH 09/36] retrieve types only when we need to in order to be more performant --- .../rules/no-unnecessary-type-conversion.ts | 52 +++++++++++-------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 2adb12eb8192..30cd73f425e4 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -57,12 +57,13 @@ export default createRule({ 'AssignmentExpression[operator = "+="], BinaryExpression[operator = "+"]'( node: TSESTree.AssignmentExpression | TSESTree.BinaryExpression, ): void { - const leftType = services.getTypeAtLocation(node.left); - const rightType = services.getTypeAtLocation(node.right); if ( - doesUnderlyingTypeMatchFlag(leftType, ts.TypeFlags.StringLike) && node.right.type === AST_NODE_TYPES.Literal && - node.right.value === '' + node.right.value === '' && + doesUnderlyingTypeMatchFlag( + services.getTypeAtLocation(node.left), + ts.TypeFlags.StringLike, + ) ) { context.report({ loc: { @@ -79,11 +80,13 @@ export default createRule({ fixer.removeRange([node.left.range[1], node.range[1]]), ], }); - } - if ( + } else if ( node.left.type === AST_NODE_TYPES.Literal && node.left.value === '' && - doesUnderlyingTypeMatchFlag(rightType, ts.TypeFlags.StringLike) + doesUnderlyingTypeMatchFlag( + services.getTypeAtLocation(node.right), + ts.TypeFlags.StringLike, + ) ) { context.report({ loc: { @@ -107,22 +110,29 @@ export default createRule({ node.callee.type === AST_NODE_TYPES.Identifier && node.arguments.length === 1 ) { - const type = getConstrainedTypeAtLocation( - services, - node.arguments[0], - ); + const getType = () => + getConstrainedTypeAtLocation(services, node.arguments[0]); if ( - (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.StringLike) && - // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum - node.callee.name === 'String') || - (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike) && - node.callee.name === 'Number') || - (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.BooleanLike) && - // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum - node.callee.name === 'Boolean') || - (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.BigIntLike) && - node.callee.name === 'BigInt') + // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum + (node.callee.name === 'String' && + doesUnderlyingTypeMatchFlag( + getType(), + ts.TypeFlags.StringLike, + )) || + (node.callee.name === 'Number' && + doesUnderlyingTypeMatchFlag( + getType(), + ts.TypeFlags.NumberLike, + )) || + // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum + (node.callee.name === 'Boolean' && + doesUnderlyingTypeMatchFlag( + getType(), + ts.TypeFlags.BooleanLike, + )) || + (node.callee.name === 'BigInt' && + doesUnderlyingTypeMatchFlag(getType(), ts.TypeFlags.BigIntLike)) ) { context.report({ node, From e125bb92b199ccd1e1fa50b6f30ccb51676c80ff Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sat, 25 Jan 2025 17:12:55 -0500 Subject: [PATCH 10/36] PR feedback --- ...mdx => no-unnecessary-type-conversion.mdx} | 0 packages/eslint-plugin/src/configs/all.ts | 2 +- .../src/configs/disable-type-checked.ts | 2 +- .../rules/no-unnecessary-type-conversion.ts | 227 +++++++++------ ...ot => no-unnecessary-type-conversion.shot} | 12 +- .../no-unnecessary-type-conversion.test.ts | 261 ++++++++++-------- .../rules/promise-function-async.test.ts | 6 +- ...ot => no-unnecessary-type-conversion.shot} | 0 packages/typescript-eslint/src/configs/all.ts | 2 +- .../src/configs/disable-type-checked.ts | 2 +- 10 files changed, 308 insertions(+), 206 deletions(-) rename packages/eslint-plugin/docs/rules/{no-unnecessary-coercion.mdx => no-unnecessary-type-conversion.mdx} (100%) rename packages/eslint-plugin/tests/docs-eslint-output-snapshots/{no-unnecessary-coercion.shot => no-unnecessary-type-conversion.shot} (72%) rename packages/eslint-plugin/tests/schema-snapshots/{no-unnecessary-coercion.shot => no-unnecessary-type-conversion.shot} (100%) diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx b/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx similarity index 100% rename from packages/eslint-plugin/docs/rules/no-unnecessary-coercion.mdx rename to packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index f25c48d6d321..bd3d765dc5b0 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -91,7 +91,6 @@ export = { '@typescript-eslint/no-shadow': 'error', '@typescript-eslint/no-this-alias': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', - '@typescript-eslint/no-unnecessary-type-conversion': 'error', '@typescript-eslint/no-unnecessary-condition': 'error', '@typescript-eslint/no-unnecessary-parameter-property-assignment': 'error', '@typescript-eslint/no-unnecessary-qualifier': 'error', @@ -99,6 +98,7 @@ export = { '@typescript-eslint/no-unnecessary-type-arguments': 'error', '@typescript-eslint/no-unnecessary-type-assertion': 'error', '@typescript-eslint/no-unnecessary-type-constraint': 'error', + '@typescript-eslint/no-unnecessary-type-conversion': 'error', '@typescript-eslint/no-unnecessary-type-parameters': 'error', '@typescript-eslint/no-unsafe-argument': 'error', '@typescript-eslint/no-unsafe-assignment': 'error', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 0aa0e5a29867..9dd6c95c929e 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -29,12 +29,12 @@ export = { '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'off', - '@typescript-eslint/no-unnecessary-type-conversion': 'off', '@typescript-eslint/no-unnecessary-condition': 'off', '@typescript-eslint/no-unnecessary-qualifier': 'off', '@typescript-eslint/no-unnecessary-template-expression': 'off', '@typescript-eslint/no-unnecessary-type-arguments': 'off', '@typescript-eslint/no-unnecessary-type-assertion': 'off', + '@typescript-eslint/no-unnecessary-type-conversion': 'off', '@typescript-eslint/no-unnecessary-type-parameters': 'off', '@typescript-eslint/no-unsafe-argument': 'off', '@typescript-eslint/no-unsafe-assignment': 'off', diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 30cd73f425e4..224568582434 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -1,5 +1,9 @@ import type { TSESTree } from '@typescript-eslint/utils'; -import type { RuleFix } from '@typescript-eslint/utils/ts-eslint'; +import type { + ReportDescriptorMessageData, + RuleFix, + RuleFixer, +} from '@typescript-eslint/utils/ts-eslint'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as ts from 'typescript'; @@ -51,11 +55,86 @@ export default createRule({ return matchesType(type); } + function doesTypeRequireParentheses(type: AST_NODE_TYPES) { + return ![ + AST_NODE_TYPES.Literal, + AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.UnaryExpression, + AST_NODE_TYPES.CallExpression, + AST_NODE_TYPES.MemberExpression, + ].includes(type); + } + + const ruleFixFilter = (ruleFix: boolean | RuleFix) => + typeof ruleFix !== 'boolean'; + + function handleUnaryOperator( + node: TSESTree.UnaryExpression, + typeFlag: ts.TypeFlags, + reportDescriptorMessageData: ReportDescriptorMessageData, + isDoubleOperator: boolean, // !! or ~~ + ) { + const type = services.getTypeAtLocation(node.argument); + if (doesUnderlyingTypeMatchFlag(type, typeFlag)) { + const keepParens = doesTypeRequireParentheses(node.argument.type); + context.report({ + loc: { + start: isDoubleOperator ? node.parent.loc.start : node.loc.start, + end: { + column: node.loc.start.column + 1, + line: node.loc.start.line, + }, + }, + messageId: 'unnecessaryTypeConversion', + data: reportDescriptorMessageData, + fix: (fixer: RuleFixer): RuleFix[] => + [ + keepParens && + fixer.insertTextBeforeRange(node.argument.range, '('), + fixer.removeRange([ + isDoubleOperator ? node.parent.range[0] : node.range[0], + node.argument.range[0], + ]), + fixer.removeRange([node.argument.range[1], node.range[1]]), + keepParens && + fixer.insertTextAfterRange(node.argument.range, ')'), + ].filter(ruleFixFilter), + }); + } + } + const services = getParserServices(context); return { - 'AssignmentExpression[operator = "+="], BinaryExpression[operator = "+"]'( - node: TSESTree.AssignmentExpression | TSESTree.BinaryExpression, + 'AssignmentExpression[operator = "+="]'( + node: TSESTree.AssignmentExpression, + ): void { + if ( + node.right.type === AST_NODE_TYPES.Literal && + node.right.value === '' && + doesUnderlyingTypeMatchFlag( + services.getTypeAtLocation(node.left), + ts.TypeFlags.StringLike, + ) + ) { + context.report({ + loc: { + start: node.left.loc.end, + end: node.loc.end, + }, + messageId: 'unnecessaryTypeConversion', + data: { + type: 'string', + violation: "Concatenating a string with ''", + }, + fix: (fixer): RuleFix[] => [ + fixer.removeRange([node.range[0], node.range[1]]), + ], + }); + } + }, + 'BinaryExpression[operator = "+"]'( + node: TSESTree.BinaryExpression, ): void { if ( node.right.type === AST_NODE_TYPES.Literal && @@ -134,17 +213,31 @@ export default createRule({ (node.callee.name === 'BigInt' && doesUnderlyingTypeMatchFlag(getType(), ts.TypeFlags.BigIntLike)) ) { + const keepParens = doesTypeRequireParentheses( + node.arguments[0].type, + ); context.report({ - node, + loc: node.callee.loc, messageId: 'unnecessaryTypeConversion', data: { type: node.callee.name.toLowerCase(), violation: `Passing a ${node.callee.name.toLowerCase()} to ${node.callee.name}()`, }, - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.arguments[0].range[0]]), - fixer.removeRange([node.arguments[0].range[1], node.range[1]]), - ], + fix: (fixer): RuleFix[] => + [ + keepParens && + fixer.insertTextBeforeRange(node.arguments[0].range, '('), + fixer.removeRange([ + node.range[0], + node.arguments[0].range[0], + ]), + fixer.removeRange([ + node.arguments[0].range[1], + node.range[1], + ]), + keepParens && + fixer.insertTextAfterRange(node.arguments[0].range, ')'), + ].filter(ruleFixFilter), }); } } @@ -155,9 +248,10 @@ export default createRule({ const memberExpr = node.parent as TSESTree.MemberExpression; const type = getConstrainedTypeAtLocation(services, memberExpr.object); if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.StringLike)) { + const keepParens = doesTypeRequireParentheses(memberExpr.object.type); context.report({ loc: { - start: memberExpr.object.loc.end, + start: memberExpr.property.loc.start, end: memberExpr.parent.loc.end, }, messageId: 'unnecessaryTypeConversion', @@ -165,91 +259,60 @@ export default createRule({ type: 'string', violation: 'Using .toString() on a string', }, - fix: (fixer): RuleFix[] => [ - fixer.removeRange([ - memberExpr.parent.range[0], - memberExpr.object.range[0], - ]), - fixer.removeRange([ - memberExpr.object.range[1], - memberExpr.parent.range[1], - ]), - ], + fix: (fixer): RuleFix[] => + [ + keepParens && + fixer.insertTextBeforeRange(memberExpr.object.range, '('), + fixer.removeRange([ + memberExpr.parent.range[0], + memberExpr.object.range[0], + ]), + fixer.removeRange([ + memberExpr.object.range[1], + memberExpr.parent.range[1], + ]), + keepParens && + fixer.insertTextAfterRange(memberExpr.object.range, ')'), + ].filter(ruleFixFilter), }); } }, 'UnaryExpression[operator = "!"] > UnaryExpression[operator = "!"]'( node: TSESTree.UnaryExpression, ): void { - const type = services.getTypeAtLocation(node.argument); - if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.BooleanLike)) { - context.report({ - loc: { - start: node.parent.loc.start, - end: { - column: node.loc.start.column + 1, - line: node.loc.start.line, - }, - }, - messageId: 'unnecessaryTypeConversion', - data: { - type: 'boolean', - violation: 'Using !! on a boolean', - }, - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.parent.range[0], node.argument.range[0]]), - fixer.removeRange([node.argument.range[1], node.range[1]]), - ], - }); - } + handleUnaryOperator( + node, + ts.TypeFlags.BooleanLike, + { + type: 'boolean', + violation: 'Using !! on a boolean', + }, + true, + ); }, 'UnaryExpression[operator = "+"]'(node: TSESTree.UnaryExpression): void { - const type = services.getTypeAtLocation(node.argument); - if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike)) { - context.report({ - loc: { - start: node.loc.start, - end: { - column: node.loc.start.column + 1, - line: node.loc.start.line, - }, - }, - messageId: 'unnecessaryTypeConversion', - data: { - type: 'number', - violation: 'Using the unary + operator on a number', - }, - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.argument.range[0]]), - fixer.removeRange([node.argument.range[1], node.range[1]]), - ], - }); - } + handleUnaryOperator( + node, + ts.TypeFlags.NumberLike, + { + type: 'number', + violation: 'Using the unary + operator on a number', + }, + false, + ); }, 'UnaryExpression[operator = "~"] > UnaryExpression[operator = "~"]'( node: TSESTree.UnaryExpression, ): void { - const type = services.getTypeAtLocation(node.argument); - if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.NumberLike)) { - context.report({ - loc: { - start: node.parent.loc.start, - end: { - column: node.loc.start.column + 1, - line: node.loc.start.line, - }, - }, - messageId: 'unnecessaryTypeConversion', - data: { - type: 'number', - violation: 'Using ~~ on a number', - }, - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.parent.range[0], node.argument.range[0]]), - fixer.removeRange([node.argument.range[1], node.range[1]]), - ], - }); - } + handleUnaryOperator( + node, + ts.TypeFlags.NumberLike, + { + type: 'boolean', + violation: 'Using ~~ on a number', + }, + true, + ); }, }; }, diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot similarity index 72% rename from packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot rename to packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot index bd3623d3c0f6..d2a2f2d0ac10 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-coercion.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot @@ -4,28 +4,28 @@ exports[`Validating rule docs no-unnecessary-type-conversion.mdx code examples E "Incorrect String('123'); -~~~~~~~~~~~~~ Passing a string to String() does not change the type or value of the string. +~~~~~~ Passing a string to String() does not change the type or value of the string. '123'.toString(); - ~~~~~~~~~~~ Using .toString() on a string does not change the type or value of the string. + ~~~~~~~~~~ Using .toString() on a string does not change the type or value of the string. '' + '123'; ~~~~~ Concatenating '' with a string does not change the type or value of the string. '123' + ''; ~~~~~ Concatenating a string with '' does not change the type or value of the string. Number(123); -~~~~~~~~~~~ Passing a number to Number() does not change the type or value of the number. +~~~~~~ Passing a number to Number() does not change the type or value of the number. +123; ~ Using the unary + operator on a number does not change the type or value of the number. ~~123; -~~ Using ~~ on a number does not change the type or value of the number. +~~ Using ~~ on a number does not change the type or value of the boolean. Boolean(true); -~~~~~~~~~~~~~ Passing a boolean to Boolean() does not change the type or value of the boolean. +~~~~~~~ Passing a boolean to Boolean() does not change the type or value of the boolean. !!true; ~~ Using !! on a boolean does not change the type or value of the boolean. BigInt(BigInt(1)); -~~~~~~~~~~~~~~~~~ Passing a bigint to BigInt() does not change the type or value of the bigint. +~~~~~~ Passing a bigint to BigInt() does not change the type or value of the bigint. let str = '123'; str += ''; diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index d04f8b879ea9..10d2c19ab851 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -1,5 +1,4 @@ import { RuleTester } from '@typescript-eslint/rule-tester'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import rule from '../../src/rules/no-unnecessary-type-conversion'; import { getFixturesRootDir } from '../RuleTester'; @@ -17,194 +16,234 @@ const ruleTester = new RuleTester({ ruleTester.run('no-unnecessary-type-conversion', rule, { valid: [ + 'String(1);', + '(1).toString();', + '`${1}`;', + "'' + 1;", + "1 + '';", ` -String(1); - `, - ` -(1).toString(); - `, - ` -\`\${1}\`; - `, - ` -'' + 1; - `, - ` -1 + ''; - `, - ` -let str = 1; -str += ''; - `, - ` -Number('2'); - `, - ` -+'2'; - `, - ` -~~'2'; - `, - ` -Boolean(0); - `, - ` -!!0; - `, - ` -BigInt(3); - `, - ` -new String('asdf'); - `, - ` -!false; - `, - ` -~256; + let str = 1; + str += ''; `, + "Number('2');", + "+'2';", + "~~'2';", + 'Boolean(0);', + '!!0;', + 'BigInt(3);', + "new String('asdf');", + '!false;', + '~256;', ], invalid: [ { - code: ` -String('asdf'); - `, + code: "String('asdf');", errors: [ { + column: 1, + endColumn: 7, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.CallExpression, }, ], - output: ` -'asdf'; - `, + output: "'asdf';", }, { - code: ` -'asdf'.toString(); - `, + code: "'asdf'.toString();", errors: [ { + column: 8, + endColumn: 18, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.Identifier, }, ], - output: ` -'asdf'; - `, + output: "'asdf';", }, { - code: ` -'' + 'asdf'; - `, + code: "'' + 'asdf';", errors: [ { + column: 1, + endColumn: 6, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.BinaryExpression, }, ], - output: ` -'asdf'; - `, + output: "'asdf';", }, { - code: ` -'asdf' + ''; - `, + code: "'asdf' + '';", errors: [ { + column: 7, + endColumn: 12, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.BinaryExpression, }, ], - output: ` -'asdf'; - `, + output: "'asdf';", }, { code: ` -Number(123); + let str = 'asdf'; + str += ''; `, errors: [ { + column: 12, + endColumn: 18, + endLine: 3, + line: 3, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.CallExpression, }, ], output: ` -123; + let str = 'asdf'; + ; `, }, { - code: ` -+123; - `, + code: 'Number(123);', errors: [ { + column: 1, + endColumn: 7, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.UnaryExpression, }, ], - output: ` -123; - `, + output: '123;', }, { - code: ` -~~123; - `, + code: '+123;', errors: [ { + column: 1, + endColumn: 2, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.UnaryExpression, }, ], - output: ` -123; - `, + output: '123;', }, { - code: ` -Boolean(true); - `, + code: '~~123;', errors: [ { + column: 1, + endColumn: 3, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.CallExpression, }, ], - output: ` -true; - `, + output: '123;', }, { - code: ` -!!true; - `, + code: 'Boolean(true);', errors: [ { + column: 1, + endColumn: 8, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.UnaryExpression, }, ], - output: ` -true; - `, + output: 'true;', }, { - code: ` -BigInt(BigInt(1)); - `, + code: '!!true;', errors: [ { + column: 1, + endColumn: 3, messageId: 'unnecessaryTypeConversion', - type: AST_NODE_TYPES.CallExpression, }, ], - output: ` -BigInt(1); - `, + output: 'true;', + }, + { + code: 'BigInt(BigInt(1));', + errors: [ + { + column: 1, + endColumn: 7, + messageId: 'unnecessaryTypeConversion', + }, + ], + output: 'BigInt(1);', + }, + + // tests to make sure autofixes preserve parentheses in cases where logic would otherwise break + { + code: "('a' + 'b').toString().length;", + errors: [ + { + column: 13, + endColumn: 23, + messageId: 'unnecessaryTypeConversion', + }, + ], + output: "('a' + 'b').length;", + }, + { + code: '2 * +(2 + 2);', + errors: [ + { + column: 5, + endColumn: 6, + messageId: 'unnecessaryTypeConversion', + }, + ], + output: '2 * (2 + 2);', + }, + { + code: '2 * Number(2 + 2);', + errors: [ + { + column: 5, + endColumn: 11, + messageId: 'unnecessaryTypeConversion', + }, + ], + output: '2 * (2 + 2);', + }, + { + code: '2 * ~~(2 + 2);', + errors: [ + { + column: 5, + endColumn: 7, + messageId: 'unnecessaryTypeConversion', + }, + ], + output: '2 * (2 + 2);', + }, + { + code: 'false && !!(false || true);', + errors: [ + { + column: 10, + endColumn: 12, + messageId: 'unnecessaryTypeConversion', + }, + ], + output: 'false && (false || true);', + }, + { + code: 'false && Boolean(false || true);', + errors: [ + { + column: 10, + endColumn: 17, + messageId: 'unnecessaryTypeConversion', + }, + ], + output: 'false && (false || true);', + }, + { + code: '2n * BigInt(2n + 2n);', + errors: [ + { + column: 6, + endColumn: 12, + messageId: 'unnecessaryTypeConversion', + }, + ], + output: '2n * (2n + 2n);', }, ], }); diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index c67f7dfb8cc2..f2d5760caecc 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -845,7 +845,7 @@ function overloadingThatCanReturnPromise( `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], output: ` @@ -868,7 +868,7 @@ function overloadingThatIncludeAny(a?: boolean): any | number { `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], options: [{ allowAny: false }], @@ -883,7 +883,7 @@ function overloadingThatIncludeUnknown(a?: boolean): unknown | number { `, errors: [ { - messageId, + messageId: 'missingAsync', }, ], options: [{ allowAny: false }], diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot b/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-type-conversion.shot similarity index 100% rename from packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-coercion.shot rename to packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-type-conversion.shot diff --git a/packages/typescript-eslint/src/configs/all.ts b/packages/typescript-eslint/src/configs/all.ts index 1262754cb00c..dc40d391e4fd 100644 --- a/packages/typescript-eslint/src/configs/all.ts +++ b/packages/typescript-eslint/src/configs/all.ts @@ -104,7 +104,6 @@ export default ( '@typescript-eslint/no-shadow': 'error', '@typescript-eslint/no-this-alias': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', - '@typescript-eslint/no-unnecessary-type-conversion': 'error', '@typescript-eslint/no-unnecessary-condition': 'error', '@typescript-eslint/no-unnecessary-parameter-property-assignment': 'error', @@ -113,6 +112,7 @@ export default ( '@typescript-eslint/no-unnecessary-type-arguments': 'error', '@typescript-eslint/no-unnecessary-type-assertion': 'error', '@typescript-eslint/no-unnecessary-type-constraint': 'error', + '@typescript-eslint/no-unnecessary-type-conversion': 'error', '@typescript-eslint/no-unnecessary-type-parameters': 'error', '@typescript-eslint/no-unsafe-argument': 'error', '@typescript-eslint/no-unsafe-assignment': 'error', diff --git a/packages/typescript-eslint/src/configs/disable-type-checked.ts b/packages/typescript-eslint/src/configs/disable-type-checked.ts index 962a71da2f1d..15c5bb0e3dcf 100644 --- a/packages/typescript-eslint/src/configs/disable-type-checked.ts +++ b/packages/typescript-eslint/src/configs/disable-type-checked.ts @@ -36,12 +36,12 @@ export default ( '@typescript-eslint/no-mixed-enums': 'off', '@typescript-eslint/no-redundant-type-constituents': 'off', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'off', - '@typescript-eslint/no-unnecessary-type-conversion': 'off', '@typescript-eslint/no-unnecessary-condition': 'off', '@typescript-eslint/no-unnecessary-qualifier': 'off', '@typescript-eslint/no-unnecessary-template-expression': 'off', '@typescript-eslint/no-unnecessary-type-arguments': 'off', '@typescript-eslint/no-unnecessary-type-assertion': 'off', + '@typescript-eslint/no-unnecessary-type-conversion': 'off', '@typescript-eslint/no-unnecessary-type-parameters': 'off', '@typescript-eslint/no-unsafe-argument': 'off', '@typescript-eslint/no-unsafe-assignment': 'off', From 0a1257b1627c902431406fe36155462cca00649f Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 26 Jan 2025 01:55:58 -0500 Subject: [PATCH 11/36] implement satisfies suggestions --- .../rules/no-unnecessary-type-conversion.ts | 231 +++++++++++++----- .../no-unnecessary-type-conversion.shot | 4 +- .../no-unnecessary-type-conversion.test.ts | 26 +- 3 files changed, 194 insertions(+), 67 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 224568582434..4caaac9a5215 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -16,7 +16,9 @@ import { } from '../util'; type Options = []; -type MessageIds = 'unnecessaryTypeConversion'; +type MessageIds = + | 'unnecessaryTypeConversion' + | 'unnecessaryTypeConversionSuggestion'; export default createRule({ name: 'no-unnecessary-type-conversion', @@ -28,9 +30,12 @@ export default createRule({ requiresTypeChecking: true, }, fixable: 'code', + hasSuggestions: true, messages: { unnecessaryTypeConversion: '{{violation}} does not change the type or value of the {{type}}.', + unnecessaryTypeConversionSuggestion: + 'Instead, assert that the value satisfies type {{type}}.', }, schema: [], }, @@ -65,6 +70,9 @@ export default createRule({ ].includes(type); } + const services = getParserServices(context); + const checker = services.program.getTypeChecker(); + const ruleFixFilter = (ruleFix: boolean | RuleFix) => typeof ruleFix !== 'boolean'; @@ -77,6 +85,18 @@ export default createRule({ const type = services.getTypeAtLocation(node.argument); if (doesUnderlyingTypeMatchFlag(type, typeFlag)) { const keepParens = doesTypeRequireParentheses(node.argument.type); + const fixFunction = (fixer: RuleFixer): RuleFix[] => + [ + keepParens && fixer.insertTextBeforeRange(node.argument.range, '('), + fixer.removeRange([ + isDoubleOperator ? node.parent.range[0] : node.range[0], + node.argument.range[0], + ]), + fixer.removeRange([node.argument.range[1], node.range[1]]), + keepParens && fixer.insertTextAfterRange(node.argument.range, ')'), + ].filter(ruleFixFilter); + const typeString = checker.typeToString(type); + context.report({ loc: { start: isDoubleOperator ? node.parent.loc.start : node.loc.start, @@ -87,24 +107,29 @@ export default createRule({ }, messageId: 'unnecessaryTypeConversion', data: reportDescriptorMessageData, - fix: (fixer: RuleFixer): RuleFix[] => - [ - keepParens && - fixer.insertTextBeforeRange(node.argument.range, '('), - fixer.removeRange([ - isDoubleOperator ? node.parent.range[0] : node.range[0], - node.argument.range[0], - ]), - fixer.removeRange([node.argument.range[1], node.range[1]]), - keepParens && - fixer.insertTextAfterRange(node.argument.range, ')'), - ].filter(ruleFixFilter), + fix: fixFunction, + suggest: + node.argument.type === AST_NODE_TYPES.Identifier + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: reportDescriptorMessageData.type }, + fix(fixer): RuleFix[] { + return [ + ...fixFunction(fixer), + fixer.insertTextAfterRange( + node.argument.range, + ` satisfies ${typeString}`, + ), + ]; + }, + }, + ] + : null, }); } } - const services = getParserServices(context); - return { 'AssignmentExpression[operator = "+="]'( node: TSESTree.AssignmentExpression, @@ -117,19 +142,34 @@ export default createRule({ ts.TypeFlags.StringLike, ) ) { + const fixFunction = (fixer: RuleFixer): RuleFix[] => [ + fixer.removeRange([node.left.range[1], node.range[1]]), + ]; + context.report({ - loc: { - start: node.left.loc.end, - end: node.loc.end, - }, + node, messageId: 'unnecessaryTypeConversion', data: { type: 'string', violation: "Concatenating a string with ''", }, - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.range[1]]), - ], + fix: fixFunction, + suggest: + node.left.type === AST_NODE_TYPES.Identifier + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + ...fixFunction(fixer), + fixer.insertTextAfterRange( + node.range, + ' satisfies string', + ), + ], + }, + ] + : null, }); } }, @@ -144,6 +184,11 @@ export default createRule({ ts.TypeFlags.StringLike, ) ) { + const fixFunction = (fixer: RuleFixer): RuleFix[] => [ + fixer.removeRange([node.range[0], node.left.range[0]]), + fixer.removeRange([node.left.range[1], node.range[1]]), + ]; + context.report({ loc: { start: node.left.loc.end, @@ -154,10 +199,23 @@ export default createRule({ type: 'string', violation: "Concatenating a string with ''", }, - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.left.range[0]]), - fixer.removeRange([node.left.range[1], node.range[1]]), - ], + fix: fixFunction, + suggest: + node.left.type === AST_NODE_TYPES.Identifier + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + ...fixFunction(fixer), + fixer.insertTextAfterRange( + node.range, + ' satisfies string', + ), + ], + }, + ] + : null, }); } else if ( node.left.type === AST_NODE_TYPES.Literal && @@ -167,6 +225,11 @@ export default createRule({ ts.TypeFlags.StringLike, ) ) { + const fixFunction = (fixer: RuleFixer): RuleFix[] => [ + fixer.removeRange([node.range[0], node.right.range[0]]), + fixer.removeRange([node.right.range[1], node.range[1]]), + ]; + context.report({ loc: { start: node.loc.start, @@ -177,10 +240,23 @@ export default createRule({ type: 'string', violation: "Concatenating '' with a string", }, - fix: (fixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.right.range[0]]), - fixer.removeRange([node.right.range[1], node.range[1]]), - ], + fix: fixFunction, + suggest: + node.right.type === AST_NODE_TYPES.Identifier + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + ...fixFunction(fixer), + fixer.insertTextAfterRange( + node.range, + ' satisfies string', + ), + ], + }, + ] + : null, }); } }, @@ -216,28 +292,41 @@ export default createRule({ const keepParens = doesTypeRequireParentheses( node.arguments[0].type, ); + const fixFunction = (fixer: RuleFixer): RuleFix[] => + [ + keepParens && + fixer.insertTextBeforeRange(node.arguments[0].range, '('), + fixer.removeRange([node.range[0], node.arguments[0].range[0]]), + fixer.removeRange([node.arguments[0].range[1], node.range[1]]), + keepParens && + fixer.insertTextAfterRange(node.arguments[0].range, ')'), + ].filter(ruleFixFilter); + const typeString = node.callee.name.toLowerCase(); + context.report({ loc: node.callee.loc, messageId: 'unnecessaryTypeConversion', data: { type: node.callee.name.toLowerCase(), - violation: `Passing a ${node.callee.name.toLowerCase()} to ${node.callee.name}()`, + violation: `Passing a ${typeString} to ${node.callee.name}()`, }, - fix: (fixer): RuleFix[] => - [ - keepParens && - fixer.insertTextBeforeRange(node.arguments[0].range, '('), - fixer.removeRange([ - node.range[0], - node.arguments[0].range[0], - ]), - fixer.removeRange([ - node.arguments[0].range[1], - node.range[1], - ]), - keepParens && - fixer.insertTextAfterRange(node.arguments[0].range, ')'), - ].filter(ruleFixFilter), + fix: fixFunction, + suggest: + node.arguments[0].type === AST_NODE_TYPES.Identifier + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: typeString }, + fix: fixer => [ + ...fixFunction(fixer), + fixer.insertTextAfterRange( + node.range, + ` satisfies ${typeString}`, + ), + ], + }, + ] + : null, }); } } @@ -249,6 +338,22 @@ export default createRule({ const type = getConstrainedTypeAtLocation(services, memberExpr.object); if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.StringLike)) { const keepParens = doesTypeRequireParentheses(memberExpr.object.type); + const fixFunction = (fixer: RuleFixer): RuleFix[] => + [ + keepParens && + fixer.insertTextBeforeRange(memberExpr.object.range, '('), + fixer.removeRange([ + memberExpr.parent.range[0], + memberExpr.object.range[0], + ]), + fixer.removeRange([ + memberExpr.object.range[1], + memberExpr.parent.range[1], + ]), + keepParens && + fixer.insertTextAfterRange(memberExpr.object.range, ')'), + ].filter(ruleFixFilter); + context.report({ loc: { start: memberExpr.property.loc.start, @@ -257,23 +362,25 @@ export default createRule({ messageId: 'unnecessaryTypeConversion', data: { type: 'string', - violation: 'Using .toString() on a string', + violation: "Calling a string's .toString() method", }, - fix: (fixer): RuleFix[] => - [ - keepParens && - fixer.insertTextBeforeRange(memberExpr.object.range, '('), - fixer.removeRange([ - memberExpr.parent.range[0], - memberExpr.object.range[0], - ]), - fixer.removeRange([ - memberExpr.object.range[1], - memberExpr.parent.range[1], - ]), - keepParens && - fixer.insertTextAfterRange(memberExpr.object.range, ')'), - ].filter(ruleFixFilter), + fix: fixFunction, + suggest: + memberExpr.object.type === AST_NODE_TYPES.Identifier + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + ...fixFunction(fixer), + fixer.insertTextAfterRange( + memberExpr.object.range, + ` satisfies string`, + ), + ], + }, + ] + : null, }); } }, diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot index d2a2f2d0ac10..fa6f09e54441 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot @@ -6,7 +6,7 @@ exports[`Validating rule docs no-unnecessary-type-conversion.mdx code examples E String('123'); ~~~~~~ Passing a string to String() does not change the type or value of the string. '123'.toString(); - ~~~~~~~~~~ Using .toString() on a string does not change the type or value of the string. + ~~~~~~~~~~ Calling a string's .toString() method does not change the type or value of the string. '' + '123'; ~~~~~ Concatenating '' with a string does not change the type or value of the string. '123' + ''; @@ -29,7 +29,7 @@ BigInt(BigInt(1)); let str = '123'; str += ''; - ~~~~~~ Concatenating a string with '' does not change the type or value of the string. +~~~~~~~~~ Concatenating a string with '' does not change the type or value of the string. " `; diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index 10d2c19ab851..4e7c521915bc 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -88,16 +88,25 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { `, errors: [ { - column: 12, + column: 9, endColumn: 18, endLine: 3, line: 3, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: ` + let str = 'asdf'; + str satisfies string; + `, + }, + ], }, ], output: ` let str = 'asdf'; - ; + str; `, }, { @@ -167,7 +176,18 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { output: 'BigInt(1);', }, - // tests to make sure autofixes preserve parentheses in cases where logic would otherwise break + // make sure autofixes preserve parentheses in cases where logic would otherwise break + { + code: "String('a' + 'b').length;", + errors: [ + { + column: 1, + endColumn: 7, + messageId: 'unnecessaryTypeConversion', + }, + ], + output: "('a' + 'b').length;", + }, { code: "('a' + 'b').toString().length;", errors: [ From 0dd5d524a564bacd700e7efbe8d88924ed78576b Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 26 Jan 2025 02:31:06 -0500 Subject: [PATCH 12/36] don't trigger rule if type constructor calls are overriden --- .../src/rules/no-unnecessary-type-conversion.ts | 17 +++++++++++++---- .../no-unnecessary-type-conversion.test.ts | 6 ++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 4caaac9a5215..d3679c39dd24 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -268,25 +268,34 @@ export default createRule({ const getType = () => getConstrainedTypeAtLocation(services, node.arguments[0]); + const isBuiltInCall = (name: string) => { + if ((node.callee as TSESTree.Identifier).name === name) { + const scope = context.sourceCode.getScope(node); + const variable = scope.set.get(name); + return !variable?.defs.length; + } + return false; + }; + if ( // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum - (node.callee.name === 'String' && + (isBuiltInCall('String') && doesUnderlyingTypeMatchFlag( getType(), ts.TypeFlags.StringLike, )) || - (node.callee.name === 'Number' && + (isBuiltInCall('Number') && doesUnderlyingTypeMatchFlag( getType(), ts.TypeFlags.NumberLike, )) || // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum - (node.callee.name === 'Boolean' && + (isBuiltInCall('Boolean') && doesUnderlyingTypeMatchFlag( getType(), ts.TypeFlags.BooleanLike, )) || - (node.callee.name === 'BigInt' && + (isBuiltInCall('BigInt') && doesUnderlyingTypeMatchFlag(getType(), ts.TypeFlags.BigIntLike)) ) { const keepParens = doesTypeRequireParentheses( diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index 4e7c521915bc..26fd25fcf0a5 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -34,6 +34,12 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { "new String('asdf');", '!false;', '~256;', + ` + function String(value) { + return value; + } + String('asdf'); + `, ], invalid: [ From b054d606f5f749f10d9588c08cc0c16e57035e85 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 26 Jan 2025 03:33:36 -0500 Subject: [PATCH 13/36] put parentheses around satisfies suggestion autofix when parent is a memberexpression --- .../rules/no-unnecessary-type-conversion.ts | 107 +++++++++++------- .../no-unnecessary-type-conversion.test.ts | 56 +++++++++ 2 files changed, 119 insertions(+), 44 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index d3679c39dd24..c189616d84ff 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -73,8 +73,17 @@ export default createRule({ const services = getParserServices(context); const checker = services.program.getTypeChecker(); - const ruleFixFilter = (ruleFix: boolean | RuleFix) => - typeof ruleFix !== 'boolean'; + const surroundWithParentheses = ( + keepParens: boolean, + fixer: RuleFixer, + range: TSESTree.Range, + ) => + keepParens + ? [ + fixer.insertTextBeforeRange(range, '('), + fixer.insertTextAfterRange(range, ')'), + ] + : []; function handleUnaryOperator( node: TSESTree.UnaryExpression, @@ -85,16 +94,14 @@ export default createRule({ const type = services.getTypeAtLocation(node.argument); if (doesUnderlyingTypeMatchFlag(type, typeFlag)) { const keepParens = doesTypeRequireParentheses(node.argument.type); - const fixFunction = (fixer: RuleFixer): RuleFix[] => - [ - keepParens && fixer.insertTextBeforeRange(node.argument.range, '('), - fixer.removeRange([ - isDoubleOperator ? node.parent.range[0] : node.range[0], - node.argument.range[0], - ]), - fixer.removeRange([node.argument.range[1], node.range[1]]), - keepParens && fixer.insertTextAfterRange(node.argument.range, ')'), - ].filter(ruleFixFilter); + const fixFunction = (fixer: RuleFixer): RuleFix[] => [ + fixer.removeRange([ + isDoubleOperator ? node.parent.range[0] : node.range[0], + node.argument.range[0], + ]), + fixer.removeRange([node.argument.range[1], node.range[1]]), + ...surroundWithParentheses(keepParens, fixer, node.argument.range), + ]; const typeString = checker.typeToString(type); context.report({ @@ -116,11 +123,11 @@ export default createRule({ data: { type: reportDescriptorMessageData.type }, fix(fixer): RuleFix[] { return [ - ...fixFunction(fixer), fixer.insertTextAfterRange( node.argument.range, ` satisfies ${typeString}`, ), + ...fixFunction(fixer), ]; }, }, @@ -161,11 +168,11 @@ export default createRule({ messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, fix: fixer => [ - ...fixFunction(fixer), fixer.insertTextAfterRange( node.range, ' satisfies string', ), + ...fixFunction(fixer), ], }, ] @@ -207,11 +214,11 @@ export default createRule({ messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, fix: fixer => [ - ...fixFunction(fixer), fixer.insertTextAfterRange( node.range, ' satisfies string', ), + ...fixFunction(fixer), ], }, ] @@ -248,11 +255,11 @@ export default createRule({ messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, fix: fixer => [ - ...fixFunction(fixer), fixer.insertTextAfterRange( node.range, ' satisfies string', ), + ...fixFunction(fixer), ], }, ] @@ -301,15 +308,14 @@ export default createRule({ const keepParens = doesTypeRequireParentheses( node.arguments[0].type, ); - const fixFunction = (fixer: RuleFixer): RuleFix[] => - [ - keepParens && - fixer.insertTextBeforeRange(node.arguments[0].range, '('), - fixer.removeRange([node.range[0], node.arguments[0].range[0]]), - fixer.removeRange([node.arguments[0].range[1], node.range[1]]), - keepParens && - fixer.insertTextAfterRange(node.arguments[0].range, ')'), - ].filter(ruleFixFilter); + const fixFunction = ( + fixer: RuleFixer, + keepParens: boolean, + ): RuleFix[] => [ + fixer.removeRange([node.range[0], node.arguments[0].range[0]]), + fixer.removeRange([node.arguments[0].range[1], node.range[1]]), + ...surroundWithParentheses(keepParens, fixer, node.range), + ]; const typeString = node.callee.name.toLowerCase(); context.report({ @@ -319,7 +325,7 @@ export default createRule({ type: node.callee.name.toLowerCase(), violation: `Passing a ${typeString} to ${node.callee.name}()`, }, - fix: fixFunction, + fix: fixer => fixFunction(fixer, keepParens), suggest: node.arguments[0].type === AST_NODE_TYPES.Identifier ? [ @@ -327,11 +333,16 @@ export default createRule({ messageId: 'unnecessaryTypeConversionSuggestion', data: { type: typeString }, fix: fixer => [ - ...fixFunction(fixer), fixer.insertTextAfterRange( node.range, ` satisfies ${typeString}`, ), + ...fixFunction( + fixer, + keepParens || + node.parent.type === + AST_NODE_TYPES.MemberExpression, + ), ], }, ] @@ -347,21 +358,24 @@ export default createRule({ const type = getConstrainedTypeAtLocation(services, memberExpr.object); if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.StringLike)) { const keepParens = doesTypeRequireParentheses(memberExpr.object.type); - const fixFunction = (fixer: RuleFixer): RuleFix[] => - [ - keepParens && - fixer.insertTextBeforeRange(memberExpr.object.range, '('), - fixer.removeRange([ - memberExpr.parent.range[0], - memberExpr.object.range[0], - ]), - fixer.removeRange([ - memberExpr.object.range[1], - memberExpr.parent.range[1], - ]), - keepParens && - fixer.insertTextAfterRange(memberExpr.object.range, ')'), - ].filter(ruleFixFilter); + const fixFunction = ( + fixer: RuleFixer, + keepParens: boolean, + ): RuleFix[] => [ + fixer.removeRange([ + memberExpr.parent.range[0], + memberExpr.object.range[0], + ]), + fixer.removeRange([ + memberExpr.object.range[1], + memberExpr.parent.range[1], + ]), + ...surroundWithParentheses( + keepParens, + fixer, + memberExpr.object.range, + ), + ]; context.report({ loc: { @@ -373,7 +387,7 @@ export default createRule({ type: 'string', violation: "Calling a string's .toString() method", }, - fix: fixFunction, + fix: fixer => fixFunction(fixer, keepParens), suggest: memberExpr.object.type === AST_NODE_TYPES.Identifier ? [ @@ -381,11 +395,16 @@ export default createRule({ messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, fix: fixer => [ - ...fixFunction(fixer), fixer.insertTextAfterRange( memberExpr.object.range, ` satisfies string`, ), + ...fixFunction( + fixer, + keepParens || + node.parent.type === + AST_NODE_TYPES.MemberExpression, + ), ], }, ] diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index 26fd25fcf0a5..4047b133879f 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -271,5 +271,61 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { ], output: '2n * (2n + 2n);', }, + + // make sure suggestions add parentheses in cases where syntax would otherwise break + { + code: ` + let str = 'asdf'; + String(str).length; + `, + errors: [ + { + column: 9, + endColumn: 15, + endLine: 3, + line: 3, + messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: ` + let str = 'asdf'; + (str satisfies string).length; + `, + }, + ], + }, + ], + output: ` + let str = 'asdf'; + str.length; + `, + }, + { + code: ` + let str = 'asdf'; + str.toString().length; + `, + errors: [ + { + column: 13, + endColumn: 23, + messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: ` + let str = 'asdf'; + (str satisfies string).length; + `, + }, + ], + }, + ], + output: ` + let str = 'asdf'; + str.length; + `, + }, ], }); From aac5d402e6695bd92a23d5b8490adba96892ca3c Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 26 Jan 2025 03:37:57 -0500 Subject: [PATCH 14/36] resolve bad autofix for !(!false) --- .../src/rules/no-unnecessary-type-conversion.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index c189616d84ff..fd999023c32c 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -99,7 +99,10 @@ export default createRule({ isDoubleOperator ? node.parent.range[0] : node.range[0], node.argument.range[0], ]), - fixer.removeRange([node.argument.range[1], node.range[1]]), + fixer.removeRange([ + node.argument.range[1], + isDoubleOperator ? node.parent.range[1] : node.range[1], + ]), ...surroundWithParentheses(keepParens, fixer, node.argument.range), ]; const typeString = checker.typeToString(type); From 988f383707b349589051d5576aa85d871957d8ff Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 26 Jan 2025 04:01:26 -0500 Subject: [PATCH 15/36] show satisfies suggestion for member expressions passed to type constructors as well --- .../rules/no-unnecessary-type-conversion.ts | 218 +++++++++--------- 1 file changed, 107 insertions(+), 111 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index fd999023c32c..c373e7115566 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -60,15 +60,19 @@ export default createRule({ return matchesType(type); } - function doesTypeRequireParentheses(type: AST_NODE_TYPES) { - return ![ + const doesTypeRequireParentheses = (type: AST_NODE_TYPES) => + ![ AST_NODE_TYPES.Literal, AST_NODE_TYPES.Identifier, AST_NODE_TYPES.UnaryExpression, AST_NODE_TYPES.CallExpression, AST_NODE_TYPES.MemberExpression, ].includes(type); - } + + const shouldHaveSatisfiesSuggestion = (type: AST_NODE_TYPES) => + [AST_NODE_TYPES.Identifier, AST_NODE_TYPES.MemberExpression].includes( + type, + ); const services = getParserServices(context); const checker = services.program.getTypeChecker(); @@ -84,7 +88,6 @@ export default createRule({ fixer.insertTextAfterRange(range, ')'), ] : []; - function handleUnaryOperator( node: TSESTree.UnaryExpression, typeFlag: ts.TypeFlags, @@ -118,24 +121,23 @@ export default createRule({ messageId: 'unnecessaryTypeConversion', data: reportDescriptorMessageData, fix: fixFunction, - suggest: - node.argument.type === AST_NODE_TYPES.Identifier - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: reportDescriptorMessageData.type }, - fix(fixer): RuleFix[] { - return [ - fixer.insertTextAfterRange( - node.argument.range, - ` satisfies ${typeString}`, - ), - ...fixFunction(fixer), - ]; - }, + suggest: shouldHaveSatisfiesSuggestion(node.argument.type) + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: reportDescriptorMessageData.type }, + fix(fixer): RuleFix[] { + return [ + fixer.insertTextAfterRange( + node.argument.range, + ` satisfies ${typeString}`, + ), + ...fixFunction(fixer), + ]; }, - ] - : null, + }, + ] + : null, }); } } @@ -164,22 +166,21 @@ export default createRule({ violation: "Concatenating a string with ''", }, fix: fixFunction, - suggest: - node.left.type === AST_NODE_TYPES.Identifier - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange( - node.range, - ' satisfies string', - ), - ...fixFunction(fixer), - ], - }, - ] - : null, + suggest: shouldHaveSatisfiesSuggestion(node.left.type) + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + fixer.insertTextAfterRange( + node.range, + ' satisfies string', + ), + ...fixFunction(fixer), + ], + }, + ] + : null, }); } }, @@ -210,22 +211,21 @@ export default createRule({ violation: "Concatenating a string with ''", }, fix: fixFunction, - suggest: - node.left.type === AST_NODE_TYPES.Identifier - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange( - node.range, - ' satisfies string', - ), - ...fixFunction(fixer), - ], - }, - ] - : null, + suggest: shouldHaveSatisfiesSuggestion(node.left.type) + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + fixer.insertTextAfterRange( + node.range, + ' satisfies string', + ), + ...fixFunction(fixer), + ], + }, + ] + : null, }); } else if ( node.left.type === AST_NODE_TYPES.Literal && @@ -251,22 +251,21 @@ export default createRule({ violation: "Concatenating '' with a string", }, fix: fixFunction, - suggest: - node.right.type === AST_NODE_TYPES.Identifier - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange( - node.range, - ' satisfies string', - ), - ...fixFunction(fixer), - ], - }, - ] - : null, + suggest: shouldHaveSatisfiesSuggestion(node.right.type) + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + fixer.insertTextAfterRange( + node.range, + ' satisfies string', + ), + ...fixFunction(fixer), + ], + }, + ] + : null, }); } }, @@ -329,27 +328,26 @@ export default createRule({ violation: `Passing a ${typeString} to ${node.callee.name}()`, }, fix: fixer => fixFunction(fixer, keepParens), - suggest: - node.arguments[0].type === AST_NODE_TYPES.Identifier - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: typeString }, - fix: fixer => [ - fixer.insertTextAfterRange( - node.range, - ` satisfies ${typeString}`, - ), - ...fixFunction( - fixer, - keepParens || - node.parent.type === - AST_NODE_TYPES.MemberExpression, - ), - ], - }, - ] - : null, + suggest: shouldHaveSatisfiesSuggestion(node.arguments[0].type) + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: typeString }, + fix: fixer => [ + fixer.insertTextAfterRange( + node.range, + ` satisfies ${typeString}`, + ), + ...fixFunction( + fixer, + keepParens || + node.parent.type === + AST_NODE_TYPES.MemberExpression, + ), + ], + }, + ] + : null, }); } } @@ -391,27 +389,25 @@ export default createRule({ violation: "Calling a string's .toString() method", }, fix: fixer => fixFunction(fixer, keepParens), - suggest: - memberExpr.object.type === AST_NODE_TYPES.Identifier - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange( - memberExpr.object.range, - ` satisfies string`, - ), - ...fixFunction( - fixer, - keepParens || - node.parent.type === - AST_NODE_TYPES.MemberExpression, - ), - ], - }, - ] - : null, + suggest: shouldHaveSatisfiesSuggestion(memberExpr.object.type) + ? [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + fixer.insertTextAfterRange( + memberExpr.object.range, + ` satisfies string`, + ), + ...fixFunction( + fixer, + keepParens || + node.parent.type === AST_NODE_TYPES.MemberExpression, + ), + ], + }, + ] + : null, }); } }, From 606556e809b98decfa190a8ed3890876a5293725 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 26 Jan 2025 04:49:26 -0500 Subject: [PATCH 16/36] different autofixes for the case where str += '' is used as a statement vs as an expression --- .../rules/no-unnecessary-type-conversion.ts | 10 ++++- .../no-unnecessary-type-conversion.test.ts | 44 +++++++++++++++---- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index c373e7115566..534bcf9a4526 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -165,7 +165,15 @@ export default createRule({ type: 'string', violation: "Concatenating a string with ''", }, - fix: fixFunction, + fix: + node.parent.type === AST_NODE_TYPES.ExpressionStatement + ? (fixer: RuleFixer): RuleFix[] => [ + fixer.removeRange([ + node.parent.range[0], + node.parent.range[1], + ]), + ] + : fixFunction, suggest: shouldHaveSatisfiesSuggestion(node.left.type) ? [ { diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index 4047b133879f..1b8e5ded0ff7 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -89,13 +89,13 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { }, { code: ` - let str = 'asdf'; - str += ''; +let str = 'asdf'; +str += ''; `, errors: [ { - column: 9, - endColumn: 18, + column: 1, + endColumn: 10, endLine: 3, line: 3, messageId: 'unnecessaryTypeConversion', @@ -103,16 +103,44 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { { messageId: 'unnecessaryTypeConversionSuggestion', output: ` - let str = 'asdf'; - str satisfies string; +let str = 'asdf'; +str satisfies string; `, }, ], }, ], output: ` - let str = 'asdf'; - str; +let str = 'asdf'; + + `, + }, + { + code: ` +let str = 'asdf'; +'asdf' + (str += ''); + `, + errors: [ + { + column: 11, + endColumn: 20, + endLine: 3, + line: 3, + messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: ` +let str = 'asdf'; +'asdf' + (str satisfies string); + `, + }, + ], + }, + ], + output: ` +let str = 'asdf'; +'asdf' + (str); `, }, { From 603370798bf0710b7abb9a4565a5c7d14d094492 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 2 Feb 2025 10:54:37 -0500 Subject: [PATCH 17/36] always provide a satisfies suggestion --- .../rules/no-unnecessary-type-conversion.ts | 185 ++++++++---------- 1 file changed, 79 insertions(+), 106 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 534bcf9a4526..2358f97a849e 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -69,11 +69,6 @@ export default createRule({ AST_NODE_TYPES.MemberExpression, ].includes(type); - const shouldHaveSatisfiesSuggestion = (type: AST_NODE_TYPES) => - [AST_NODE_TYPES.Identifier, AST_NODE_TYPES.MemberExpression].includes( - type, - ); - const services = getParserServices(context); const checker = services.program.getTypeChecker(); @@ -121,23 +116,21 @@ export default createRule({ messageId: 'unnecessaryTypeConversion', data: reportDescriptorMessageData, fix: fixFunction, - suggest: shouldHaveSatisfiesSuggestion(node.argument.type) - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: reportDescriptorMessageData.type }, - fix(fixer): RuleFix[] { - return [ - fixer.insertTextAfterRange( - node.argument.range, - ` satisfies ${typeString}`, - ), - ...fixFunction(fixer), - ]; - }, - }, - ] - : null, + suggest: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: reportDescriptorMessageData.type }, + fix(fixer): RuleFix[] { + return [ + fixer.insertTextAfterRange( + node.argument.range, + ` satisfies ${typeString}`, + ), + ...fixFunction(fixer), + ]; + }, + }, + ], }); } } @@ -174,21 +167,16 @@ export default createRule({ ]), ] : fixFunction, - suggest: shouldHaveSatisfiesSuggestion(node.left.type) - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange( - node.range, - ' satisfies string', - ), - ...fixFunction(fixer), - ], - }, - ] - : null, + suggest: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + fixer.insertTextAfterRange(node.range, ' satisfies string'), + ...fixFunction(fixer), + ], + }, + ], }); } }, @@ -219,21 +207,16 @@ export default createRule({ violation: "Concatenating a string with ''", }, fix: fixFunction, - suggest: shouldHaveSatisfiesSuggestion(node.left.type) - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange( - node.range, - ' satisfies string', - ), - ...fixFunction(fixer), - ], - }, - ] - : null, + suggest: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + fixer.insertTextAfterRange(node.range, ' satisfies string'), + ...fixFunction(fixer), + ], + }, + ], }); } else if ( node.left.type === AST_NODE_TYPES.Literal && @@ -259,21 +242,16 @@ export default createRule({ violation: "Concatenating '' with a string", }, fix: fixFunction, - suggest: shouldHaveSatisfiesSuggestion(node.right.type) - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange( - node.range, - ' satisfies string', - ), - ...fixFunction(fixer), - ], - }, - ] - : null, + suggest: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + fixer.insertTextAfterRange(node.range, ' satisfies string'), + ...fixFunction(fixer), + ], + }, + ], }); } }, @@ -336,26 +314,23 @@ export default createRule({ violation: `Passing a ${typeString} to ${node.callee.name}()`, }, fix: fixer => fixFunction(fixer, keepParens), - suggest: shouldHaveSatisfiesSuggestion(node.arguments[0].type) - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: typeString }, - fix: fixer => [ - fixer.insertTextAfterRange( - node.range, - ` satisfies ${typeString}`, - ), - ...fixFunction( - fixer, - keepParens || - node.parent.type === - AST_NODE_TYPES.MemberExpression, - ), - ], - }, - ] - : null, + suggest: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: typeString }, + fix: fixer => [ + fixer.insertTextAfterRange( + node.range, + ` satisfies ${typeString}`, + ), + ...fixFunction( + fixer, + keepParens || + node.parent.type === AST_NODE_TYPES.MemberExpression, + ), + ], + }, + ], }); } } @@ -397,25 +372,23 @@ export default createRule({ violation: "Calling a string's .toString() method", }, fix: fixer => fixFunction(fixer, keepParens), - suggest: shouldHaveSatisfiesSuggestion(memberExpr.object.type) - ? [ - { - messageId: 'unnecessaryTypeConversionSuggestion', - data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange( - memberExpr.object.range, - ` satisfies string`, - ), - ...fixFunction( - fixer, - keepParens || - node.parent.type === AST_NODE_TYPES.MemberExpression, - ), - ], - }, - ] - : null, + suggest: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + data: { type: 'string' }, + fix: fixer => [ + fixer.insertTextAfterRange( + memberExpr.object.range, + ` satisfies string`, + ), + ...fixFunction( + fixer, + keepParens || + node.parent.type === AST_NODE_TYPES.MemberExpression, + ), + ], + }, + ], }); } }, From d5b9f2f145338b38d1a12a0289bac8eb5e278eb6 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 2 Feb 2025 11:32:13 -0500 Subject: [PATCH 18/36] fix minor grammar mistakes in getWrappingFixer --- packages/eslint-plugin/src/util/getWrappingFixer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/util/getWrappingFixer.ts b/packages/eslint-plugin/src/util/getWrappingFixer.ts index 26afcaa6405b..5c38a3d626b1 100644 --- a/packages/eslint-plugin/src/util/getWrappingFixer.ts +++ b/packages/eslint-plugin/src/util/getWrappingFixer.ts @@ -27,7 +27,7 @@ interface WrappingFixerParams { } /** - * Wraps node with some code. Adds parenthesis as necessary. + * Wraps node with some code. Adds parentheses as necessary. * @returns Fixer which adds the specified code and parens if necessary. */ export function getWrappingFixer( @@ -105,7 +105,7 @@ export function getMovedNodeCode(params: { } /** - * Check if a node will always have the same precedence if it's parent changes. + * Check if a node will always have the same precedence if its parent changes. */ export function isStrongPrecedenceNode(innerNode: TSESTree.Node): boolean { return ( From 7568840de0010f7b4b6dc7dc4e11443c89c25372 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 2 Feb 2025 12:24:40 -0500 Subject: [PATCH 19/36] make satisfies suggestion fixes use getWrappingFixer --- .../rules/no-unnecessary-type-conversion.ts | 81 +++++++++---------- 1 file changed, 38 insertions(+), 43 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 2358f97a849e..6de34401d510 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -12,6 +12,7 @@ import { createRule, getConstrainedTypeAtLocation, getParserServices, + getWrappingFixer, isTypeFlagSet, } from '../util'; @@ -83,6 +84,7 @@ export default createRule({ fixer.insertTextAfterRange(range, ')'), ] : []; + function handleUnaryOperator( node: TSESTree.UnaryExpression, typeFlag: ts.TypeFlags, @@ -120,15 +122,12 @@ export default createRule({ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: reportDescriptorMessageData.type }, - fix(fixer): RuleFix[] { - return [ - fixer.insertTextAfterRange( - node.argument.range, - ` satisfies ${typeString}`, - ), - ...fixFunction(fixer), - ]; - }, + fix: getWrappingFixer({ + node: isDoubleOperator ? node.parent : node, + innerNode: [node.argument], + sourceCode: context.sourceCode, + wrap: expr => `${expr} satisfies ${typeString}`, + }), }, ], }); @@ -171,10 +170,12 @@ export default createRule({ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange(node.range, ' satisfies string'), - ...fixFunction(fixer), - ], + fix: getWrappingFixer({ + node, + innerNode: [node.left], + sourceCode: context.sourceCode, + wrap: expr => `${expr} satisfies string`, + }), }, ], }); @@ -211,10 +212,12 @@ export default createRule({ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange(node.range, ' satisfies string'), - ...fixFunction(fixer), - ], + fix: getWrappingFixer({ + node, + innerNode: [node.left], + sourceCode: context.sourceCode, + wrap: expr => `${expr} satisfies string`, + }), }, ], }); @@ -246,10 +249,12 @@ export default createRule({ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange(node.range, ' satisfies string'), - ...fixFunction(fixer), - ], + fix: getWrappingFixer({ + node, + innerNode: [node.right], + sourceCode: context.sourceCode, + wrap: expr => `${expr} satisfies string`, + }), }, ], }); @@ -318,17 +323,12 @@ export default createRule({ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: typeString }, - fix: fixer => [ - fixer.insertTextAfterRange( - node.range, - ` satisfies ${typeString}`, - ), - ...fixFunction( - fixer, - keepParens || - node.parent.type === AST_NODE_TYPES.MemberExpression, - ), - ], + fix: getWrappingFixer({ + node, + innerNode: [node.arguments[0]], + sourceCode: context.sourceCode, + wrap: expr => `${expr} satisfies ${typeString}`, + }), }, ], }); @@ -376,17 +376,12 @@ export default createRule({ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, - fix: fixer => [ - fixer.insertTextAfterRange( - memberExpr.object.range, - ` satisfies string`, - ), - ...fixFunction( - fixer, - keepParens || - node.parent.type === AST_NODE_TYPES.MemberExpression, - ), - ], + fix: getWrappingFixer({ + node: memberExpr.parent, + innerNode: [memberExpr.object], + sourceCode: context.sourceCode, + wrap: expr => `${expr} satisfies string`, + }), }, ], }); From 251bb05ee6c8d0e378687538f11a93852af2118c Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 2 Feb 2025 12:52:26 -0500 Subject: [PATCH 20/36] resolve issue where satisfies suggestion for unary operators gives literal types instead of boolean/number --- .../eslint-plugin/src/rules/no-unnecessary-type-conversion.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 6de34401d510..88e8f5bde653 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -71,7 +71,6 @@ export default createRule({ ].includes(type); const services = getParserServices(context); - const checker = services.program.getTypeChecker(); const surroundWithParentheses = ( keepParens: boolean, @@ -105,7 +104,8 @@ export default createRule({ ]), ...surroundWithParentheses(keepParens, fixer, node.argument.range), ]; - const typeString = checker.typeToString(type); + const typeString = + typeFlag === ts.TypeFlags.BooleanLike ? 'boolean' : 'number'; context.report({ loc: { From 0b12c0efa04a342b71ebcf9235f1c1e2357ff3c4 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 2 Feb 2025 12:54:19 -0500 Subject: [PATCH 21/36] make autofixes use getWrappingFixer --- .../rules/no-unnecessary-type-conversion.ts | 158 +++++++----------- 1 file changed, 60 insertions(+), 98 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 88e8f5bde653..78f22b401830 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -61,29 +61,8 @@ export default createRule({ return matchesType(type); } - const doesTypeRequireParentheses = (type: AST_NODE_TYPES) => - ![ - AST_NODE_TYPES.Literal, - AST_NODE_TYPES.Identifier, - AST_NODE_TYPES.UnaryExpression, - AST_NODE_TYPES.CallExpression, - AST_NODE_TYPES.MemberExpression, - ].includes(type); - const services = getParserServices(context); - const surroundWithParentheses = ( - keepParens: boolean, - fixer: RuleFixer, - range: TSESTree.Range, - ) => - keepParens - ? [ - fixer.insertTextBeforeRange(range, '('), - fixer.insertTextAfterRange(range, ')'), - ] - : []; - function handleUnaryOperator( node: TSESTree.UnaryExpression, typeFlag: ts.TypeFlags, @@ -92,18 +71,11 @@ export default createRule({ ) { const type = services.getTypeAtLocation(node.argument); if (doesUnderlyingTypeMatchFlag(type, typeFlag)) { - const keepParens = doesTypeRequireParentheses(node.argument.type); - const fixFunction = (fixer: RuleFixer): RuleFix[] => [ - fixer.removeRange([ - isDoubleOperator ? node.parent.range[0] : node.range[0], - node.argument.range[0], - ]), - fixer.removeRange([ - node.argument.range[1], - isDoubleOperator ? node.parent.range[1] : node.range[1], - ]), - ...surroundWithParentheses(keepParens, fixer, node.argument.range), - ]; + const wrappingFixerParams = { + node: isDoubleOperator ? node.parent : node, + innerNode: [node.argument], + sourceCode: context.sourceCode, + }; const typeString = typeFlag === ts.TypeFlags.BooleanLike ? 'boolean' : 'number'; @@ -117,15 +89,16 @@ export default createRule({ }, messageId: 'unnecessaryTypeConversion', data: reportDescriptorMessageData, - fix: fixFunction, + fix: getWrappingFixer({ + ...wrappingFixerParams, + wrap: expr => expr, + }), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: reportDescriptorMessageData.type }, fix: getWrappingFixer({ - node: isDoubleOperator ? node.parent : node, - innerNode: [node.argument], - sourceCode: context.sourceCode, + ...wrappingFixerParams, wrap: expr => `${expr} satisfies ${typeString}`, }), }, @@ -146,9 +119,11 @@ export default createRule({ ts.TypeFlags.StringLike, ) ) { - const fixFunction = (fixer: RuleFixer): RuleFix[] => [ - fixer.removeRange([node.left.range[1], node.range[1]]), - ]; + const wrappingFixerParams = { + node, + innerNode: [node.left], + sourceCode: context.sourceCode, + }; context.report({ node, @@ -165,15 +140,16 @@ export default createRule({ node.parent.range[1], ]), ] - : fixFunction, + : getWrappingFixer({ + ...wrappingFixerParams, + wrap: expr => expr, + }), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, fix: getWrappingFixer({ - node, - innerNode: [node.left], - sourceCode: context.sourceCode, + ...wrappingFixerParams, wrap: expr => `${expr} satisfies string`, }), }, @@ -192,10 +168,11 @@ export default createRule({ ts.TypeFlags.StringLike, ) ) { - const fixFunction = (fixer: RuleFixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.left.range[0]]), - fixer.removeRange([node.left.range[1], node.range[1]]), - ]; + const wrappingFixerParams = { + node, + innerNode: [node.left], + sourceCode: context.sourceCode, + }; context.report({ loc: { @@ -207,15 +184,16 @@ export default createRule({ type: 'string', violation: "Concatenating a string with ''", }, - fix: fixFunction, + fix: getWrappingFixer({ + ...wrappingFixerParams, + wrap: expr => expr, + }), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, fix: getWrappingFixer({ - node, - innerNode: [node.left], - sourceCode: context.sourceCode, + ...wrappingFixerParams, wrap: expr => `${expr} satisfies string`, }), }, @@ -229,10 +207,11 @@ export default createRule({ ts.TypeFlags.StringLike, ) ) { - const fixFunction = (fixer: RuleFixer): RuleFix[] => [ - fixer.removeRange([node.range[0], node.right.range[0]]), - fixer.removeRange([node.right.range[1], node.range[1]]), - ]; + const wrappingFixerParams = { + node, + innerNode: [node.right], + sourceCode: context.sourceCode, + }; context.report({ loc: { @@ -244,15 +223,16 @@ export default createRule({ type: 'string', violation: "Concatenating '' with a string", }, - fix: fixFunction, + fix: getWrappingFixer({ + ...wrappingFixerParams, + wrap: expr => expr, + }), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, fix: getWrappingFixer({ - node, - innerNode: [node.right], - sourceCode: context.sourceCode, + ...wrappingFixerParams, wrap: expr => `${expr} satisfies string`, }), }, @@ -298,17 +278,11 @@ export default createRule({ (isBuiltInCall('BigInt') && doesUnderlyingTypeMatchFlag(getType(), ts.TypeFlags.BigIntLike)) ) { - const keepParens = doesTypeRequireParentheses( - node.arguments[0].type, - ); - const fixFunction = ( - fixer: RuleFixer, - keepParens: boolean, - ): RuleFix[] => [ - fixer.removeRange([node.range[0], node.arguments[0].range[0]]), - fixer.removeRange([node.arguments[0].range[1], node.range[1]]), - ...surroundWithParentheses(keepParens, fixer, node.range), - ]; + const wrappingFixerParams = { + node, + innerNode: [node.arguments[0]], + sourceCode: context.sourceCode, + }; const typeString = node.callee.name.toLowerCase(); context.report({ @@ -318,15 +292,16 @@ export default createRule({ type: node.callee.name.toLowerCase(), violation: `Passing a ${typeString} to ${node.callee.name}()`, }, - fix: fixer => fixFunction(fixer, keepParens), + fix: getWrappingFixer({ + ...wrappingFixerParams, + wrap: expr => expr, + }), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: typeString }, fix: getWrappingFixer({ - node, - innerNode: [node.arguments[0]], - sourceCode: context.sourceCode, + ...wrappingFixerParams, wrap: expr => `${expr} satisfies ${typeString}`, }), }, @@ -341,25 +316,11 @@ export default createRule({ const memberExpr = node.parent as TSESTree.MemberExpression; const type = getConstrainedTypeAtLocation(services, memberExpr.object); if (doesUnderlyingTypeMatchFlag(type, ts.TypeFlags.StringLike)) { - const keepParens = doesTypeRequireParentheses(memberExpr.object.type); - const fixFunction = ( - fixer: RuleFixer, - keepParens: boolean, - ): RuleFix[] => [ - fixer.removeRange([ - memberExpr.parent.range[0], - memberExpr.object.range[0], - ]), - fixer.removeRange([ - memberExpr.object.range[1], - memberExpr.parent.range[1], - ]), - ...surroundWithParentheses( - keepParens, - fixer, - memberExpr.object.range, - ), - ]; + const wrappingFixerParams = { + node: memberExpr.parent, + innerNode: [memberExpr.object], + sourceCode: context.sourceCode, + }; context.report({ loc: { @@ -371,15 +332,16 @@ export default createRule({ type: 'string', violation: "Calling a string's .toString() method", }, - fix: fixer => fixFunction(fixer, keepParens), + fix: getWrappingFixer({ + ...wrappingFixerParams, + wrap: expr => expr, + }), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', data: { type: 'string' }, fix: getWrappingFixer({ - node: memberExpr.parent, - innerNode: [memberExpr.object], - sourceCode: context.sourceCode, + ...wrappingFixerParams, wrap: expr => `${expr} satisfies string`, }), }, From c85bf9a3b091e8a7d7fe47e7492689f7d12be0ca Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 2 Feb 2025 14:53:07 -0500 Subject: [PATCH 22/36] add suggestions to all tests --- .../no-unnecessary-type-conversion.test.ts | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index 1b8e5ded0ff7..bdebe62746ce 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -50,6 +50,12 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { column: 1, endColumn: 7, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: "'asdf' satisfies string;", + }, + ], }, ], output: "'asdf';", @@ -61,6 +67,12 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { column: 8, endColumn: 18, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: "'asdf' satisfies string;", + }, + ], }, ], output: "'asdf';", @@ -72,6 +84,12 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { column: 1, endColumn: 6, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: "'asdf' satisfies string;", + }, + ], }, ], output: "'asdf';", @@ -83,6 +101,12 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { column: 7, endColumn: 12, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: "'asdf' satisfies string;", + }, + ], }, ], output: "'asdf';", @@ -150,6 +174,12 @@ let str = 'asdf'; column: 1, endColumn: 7, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: '123 satisfies number;', + }, + ], }, ], output: '123;', @@ -161,6 +191,12 @@ let str = 'asdf'; column: 1, endColumn: 2, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: '123 satisfies number;', + }, + ], }, ], output: '123;', @@ -172,6 +208,12 @@ let str = 'asdf'; column: 1, endColumn: 3, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: '123 satisfies number;', + }, + ], }, ], output: '123;', @@ -183,6 +225,12 @@ let str = 'asdf'; column: 1, endColumn: 8, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: 'true satisfies boolean;', + }, + ], }, ], output: 'true;', @@ -194,6 +242,12 @@ let str = 'asdf'; column: 1, endColumn: 3, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: 'true satisfies boolean;', + }, + ], }, ], output: 'true;', @@ -205,6 +259,12 @@ let str = 'asdf'; column: 1, endColumn: 7, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: 'BigInt(1) satisfies bigint;', + }, + ], }, ], output: 'BigInt(1);', @@ -218,6 +278,12 @@ let str = 'asdf'; column: 1, endColumn: 7, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: "(('a' + 'b') satisfies string).length;", + }, + ], }, ], output: "('a' + 'b').length;", @@ -229,6 +295,12 @@ let str = 'asdf'; column: 13, endColumn: 23, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: "(('a' + 'b') satisfies string).length;", + }, + ], }, ], output: "('a' + 'b').length;", @@ -240,6 +312,12 @@ let str = 'asdf'; column: 5, endColumn: 6, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: '2 * ((2 + 2) satisfies number);', + }, + ], }, ], output: '2 * (2 + 2);', @@ -251,6 +329,12 @@ let str = 'asdf'; column: 5, endColumn: 11, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: '2 * ((2 + 2) satisfies number);', + }, + ], }, ], output: '2 * (2 + 2);', @@ -262,6 +346,12 @@ let str = 'asdf'; column: 5, endColumn: 7, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: '2 * ((2 + 2) satisfies number);', + }, + ], }, ], output: '2 * (2 + 2);', @@ -273,6 +363,12 @@ let str = 'asdf'; column: 10, endColumn: 12, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: 'false && ((false || true) satisfies boolean);', + }, + ], }, ], output: 'false && (false || true);', @@ -284,6 +380,12 @@ let str = 'asdf'; column: 10, endColumn: 17, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: 'false && ((false || true) satisfies boolean);', + }, + ], }, ], output: 'false && (false || true);', @@ -295,6 +397,12 @@ let str = 'asdf'; column: 6, endColumn: 12, messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: '2n * ((2n + 2n) satisfies bigint);', + }, + ], }, ], output: '2n * (2n + 2n);', From 2877d5fb30e734f5f36809f8ca41481d3ed8eb53 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 2 Feb 2025 14:55:46 -0500 Subject: [PATCH 23/36] basic implementation of optional wrap parameter to getWrappingFixer --- .../rules/no-unnecessary-type-conversion.ts | 30 ++++--------------- .../src/util/getWrappingFixer.ts | 6 +++- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 78f22b401830..b70e6e2028ae 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -89,10 +89,7 @@ export default createRule({ }, messageId: 'unnecessaryTypeConversion', data: reportDescriptorMessageData, - fix: getWrappingFixer({ - ...wrappingFixerParams, - wrap: expr => expr, - }), + fix: getWrappingFixer(wrappingFixerParams), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', @@ -140,10 +137,7 @@ export default createRule({ node.parent.range[1], ]), ] - : getWrappingFixer({ - ...wrappingFixerParams, - wrap: expr => expr, - }), + : getWrappingFixer(wrappingFixerParams), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', @@ -184,10 +178,7 @@ export default createRule({ type: 'string', violation: "Concatenating a string with ''", }, - fix: getWrappingFixer({ - ...wrappingFixerParams, - wrap: expr => expr, - }), + fix: getWrappingFixer(wrappingFixerParams), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', @@ -223,10 +214,7 @@ export default createRule({ type: 'string', violation: "Concatenating '' with a string", }, - fix: getWrappingFixer({ - ...wrappingFixerParams, - wrap: expr => expr, - }), + fix: getWrappingFixer(wrappingFixerParams), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', @@ -292,10 +280,7 @@ export default createRule({ type: node.callee.name.toLowerCase(), violation: `Passing a ${typeString} to ${node.callee.name}()`, }, - fix: getWrappingFixer({ - ...wrappingFixerParams, - wrap: expr => expr, - }), + fix: getWrappingFixer(wrappingFixerParams), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', @@ -332,10 +317,7 @@ export default createRule({ type: 'string', violation: "Calling a string's .toString() method", }, - fix: getWrappingFixer({ - ...wrappingFixerParams, - wrap: expr => expr, - }), + fix: getWrappingFixer(wrappingFixerParams), suggest: [ { messageId: 'unnecessaryTypeConversionSuggestion', diff --git a/packages/eslint-plugin/src/util/getWrappingFixer.ts b/packages/eslint-plugin/src/util/getWrappingFixer.ts index 5c38a3d626b1..3cb54a31b65d 100644 --- a/packages/eslint-plugin/src/util/getWrappingFixer.ts +++ b/packages/eslint-plugin/src/util/getWrappingFixer.ts @@ -23,7 +23,7 @@ interface WrappingFixerParams { * Receives multiple arguments if there are multiple innerNodes. * E.g. ``code => `${code} != null` `` */ - wrap: (...code: string[]) => string; + wrap?: (...code: string[]) => string; } /** @@ -55,6 +55,10 @@ export function getWrappingFixer( return code; }); + if (!wrap) { + return fixer.replaceText(node, innerCodes.join('')); + } + // do the wrapping let code = wrap(...innerCodes); From 178b961a030dd82a93bd5d5d44a7122e39798d08 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 2 Feb 2025 15:51:20 -0500 Subject: [PATCH 24/36] remove code path that doesn't seem to be possible to hit --- .../eslint-plugin/src/rules/no-unnecessary-type-conversion.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index b70e6e2028ae..652fbe4d64c4 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -54,10 +54,6 @@ export default createRule({ return type.types.every(matchesType); } - if (type.isIntersection()) { - return type.types.some(matchesType); - } - return matchesType(type); } From 74bf8a15187231cad9306bdc0ed63107c4a5d569 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sun, 23 Mar 2025 20:46:16 -0400 Subject: [PATCH 25/36] add tests based on feedback --- .../no-unnecessary-type-conversion.test.ts | 185 +++++++++++++++++- 1 file changed, 180 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index bdebe62746ce..b739c2b04685 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -16,6 +16,7 @@ const ruleTester = new RuleTester({ ruleTester.run('no-unnecessary-type-conversion', rule, { valid: [ + // standard type conversions are valid 'String(1);', '(1).toString();', '`${1}`;', @@ -31,15 +32,63 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { 'Boolean(0);', '!!0;', 'BigInt(3);', + + // things that are not type conversion idioms (but look similar) are valid "new String('asdf');", + 'new Number(2);', + 'new Boolean(true);', '!false;', - '~256;', + '~2;', ` function String(value) { return value; } String('asdf'); `, + ` + function Number(value) { + return value; + } + Number(2); + `, + ` + function Boolean(value) { + return value; + } + Boolean(true); + `, + ` + function BigInt(value) { + return value; + } + BigInt(3n); + `, + ` + function toString(value) { + return value; + } + toString('asdf'); + `, + ` + export {}; + declare const toString: string; + toString.toUpperCase(); + `, + + // using type conversion idioms to unbox boxed primitives is valid + 'String(new String);', + 'new String.toString();', + "'' + new String;", + "new String + '';", + ` + let str = new String; + str += ''; + `, + 'Number(new Number);', + '+new Number;', + '~~new Number;', + 'Boolean(new Boolean);', + '!!new Boolean;', ], invalid: [ @@ -253,7 +302,7 @@ let str = 'asdf'; output: 'true;', }, { - code: 'BigInt(BigInt(1));', + code: 'BigInt(3n);', errors: [ { column: 1, @@ -262,15 +311,141 @@ let str = 'asdf'; suggestions: [ { messageId: 'unnecessaryTypeConversionSuggestion', - output: 'BigInt(1) satisfies bigint;', + output: '3n satisfies bigint;', + }, + ], + }, + ], + output: '3n;', + }, + + // using type conversion idioms on generics that extend primitives is invalid + { + code: ` + function f(x: T) { + return String(x); + } + `, + errors: [ + { + column: 18, + endColumn: 24, + endLine: 3, + line: 3, + messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: ` + function f(x: T) { + return x satisfies string; + } + `, + }, + ], + }, + ], + output: ` + function f(x: T) { + return x; + } + `, + }, + { + code: ` + function f(x: T) { + return Number(x); + } + `, + errors: [ + { + column: 18, + endColumn: 24, + endLine: 3, + line: 3, + messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: ` + function f(x: T) { + return x satisfies number; + } + `, + }, + ], + }, + ], + output: ` + function f(x: T) { + return x; + } + `, + }, + { + code: ` + function f(x: T) { + return Boolean(x); + } + `, + errors: [ + { + column: 18, + endColumn: 25, + endLine: 3, + line: 3, + messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: ` + function f(x: T) { + return x satisfies boolean; + } + `, + }, + ], + }, + ], + output: ` + function f(x: T) { + return x; + } + `, + }, + { + code: ` + function f(x: T) { + return BigInt(x); + } + `, + errors: [ + { + column: 18, + endColumn: 24, + endLine: 3, + line: 3, + messageId: 'unnecessaryTypeConversion', + suggestions: [ + { + messageId: 'unnecessaryTypeConversionSuggestion', + output: ` + function f(x: T) { + return x satisfies bigint; + } + `, }, ], }, ], - output: 'BigInt(1);', + output: ` + function f(x: T) { + return x; + } + `, }, - // make sure autofixes preserve parentheses in cases where logic would otherwise break + // make sure fixes preserve parentheses in cases where logic would otherwise break { code: "String('a' + 'b').length;", errors: [ From 17dfbc96e619552d543c8ae679f22fc3a9860d44 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Fri, 4 Apr 2025 22:18:20 -0400 Subject: [PATCH 26/36] change all autofixes to suggestions --- .../rules/no-unnecessary-type-conversion.ts | 68 +++-- .../no-unnecessary-type-conversion.test.ts | 256 ++++++++++++------ 2 files changed, 211 insertions(+), 113 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 652fbe4d64c4..727a502f7ad3 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -18,8 +18,9 @@ import { type Options = []; type MessageIds = - | 'unnecessaryTypeConversion' - | 'unnecessaryTypeConversionSuggestion'; + | 'suggestRemove' + | 'suggestSatisfies' + | 'unnecessaryTypeConversion'; export default createRule({ name: 'no-unnecessary-type-conversion', @@ -33,10 +34,11 @@ export default createRule({ fixable: 'code', hasSuggestions: true, messages: { + suggestRemove: 'Remove the type conversion.', + suggestSatisfies: + 'Instead, assert that the value satisfies the {{type}} type.', unnecessaryTypeConversion: '{{violation}} does not change the type or value of the {{type}}.', - unnecessaryTypeConversionSuggestion: - 'Instead, assert that the value satisfies type {{type}}.', }, schema: [], }, @@ -85,10 +87,13 @@ export default createRule({ }, messageId: 'unnecessaryTypeConversion', data: reportDescriptorMessageData, - fix: getWrappingFixer(wrappingFixerParams), suggest: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + fix: getWrappingFixer(wrappingFixerParams), + }, + { + messageId: 'suggestSatisfies', data: { type: reportDescriptorMessageData.type }, fix: getWrappingFixer({ ...wrappingFixerParams, @@ -125,18 +130,21 @@ export default createRule({ type: 'string', violation: "Concatenating a string with ''", }, - fix: - node.parent.type === AST_NODE_TYPES.ExpressionStatement - ? (fixer: RuleFixer): RuleFix[] => [ - fixer.removeRange([ - node.parent.range[0], - node.parent.range[1], - ]), - ] - : getWrappingFixer(wrappingFixerParams), suggest: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + fix: + node.parent.type === AST_NODE_TYPES.ExpressionStatement + ? (fixer: RuleFixer): RuleFix[] => [ + fixer.removeRange([ + node.parent.range[0], + node.parent.range[1], + ]), + ] + : getWrappingFixer(wrappingFixerParams), + }, + { + messageId: 'suggestSatisfies', data: { type: 'string' }, fix: getWrappingFixer({ ...wrappingFixerParams, @@ -174,10 +182,13 @@ export default createRule({ type: 'string', violation: "Concatenating a string with ''", }, - fix: getWrappingFixer(wrappingFixerParams), suggest: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + fix: getWrappingFixer(wrappingFixerParams), + }, + { + messageId: 'suggestSatisfies', data: { type: 'string' }, fix: getWrappingFixer({ ...wrappingFixerParams, @@ -210,10 +221,13 @@ export default createRule({ type: 'string', violation: "Concatenating '' with a string", }, - fix: getWrappingFixer(wrappingFixerParams), suggest: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + fix: getWrappingFixer(wrappingFixerParams), + }, + { + messageId: 'suggestSatisfies', data: { type: 'string' }, fix: getWrappingFixer({ ...wrappingFixerParams, @@ -276,10 +290,13 @@ export default createRule({ type: node.callee.name.toLowerCase(), violation: `Passing a ${typeString} to ${node.callee.name}()`, }, - fix: getWrappingFixer(wrappingFixerParams), suggest: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + fix: getWrappingFixer(wrappingFixerParams), + }, + { + messageId: 'suggestSatisfies', data: { type: typeString }, fix: getWrappingFixer({ ...wrappingFixerParams, @@ -313,10 +330,13 @@ export default createRule({ type: 'string', violation: "Calling a string's .toString() method", }, - fix: getWrappingFixer(wrappingFixerParams), suggest: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + fix: getWrappingFixer(wrappingFixerParams), + }, + { + messageId: 'suggestSatisfies', data: { type: 'string' }, fix: getWrappingFixer({ ...wrappingFixerParams, diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index b739c2b04685..064a60f53de0 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -76,19 +76,19 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { `, // using type conversion idioms to unbox boxed primitives is valid - 'String(new String);', + 'String(new String());', 'new String.toString();', - "'' + new String;", - "new String + '';", + "'' + new String();", + "new String() + '';", ` - let str = new String; + let str = new String(); str += ''; `, - 'Number(new Number);', - '+new Number;', - '~~new Number;', - 'Boolean(new Boolean);', - '!!new Boolean;', + 'Number(new Number());', + '+new Number();', + '~~new Number();', + 'Boolean(new Boolean());', + '!!new Boolean();', ], invalid: [ @@ -101,13 +101,16 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: "'asdf';", + }, + { + messageId: 'suggestSatisfies', output: "'asdf' satisfies string;", }, ], }, ], - output: "'asdf';", }, { code: "'asdf'.toString();", @@ -118,13 +121,16 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: "'asdf';", + }, + { + messageId: 'suggestSatisfies', output: "'asdf' satisfies string;", }, ], }, ], - output: "'asdf';", }, { code: "'' + 'asdf';", @@ -135,13 +141,16 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: "'asdf';", + }, + { + messageId: 'suggestSatisfies', output: "'asdf' satisfies string;", }, ], }, ], - output: "'asdf';", }, { code: "'asdf' + '';", @@ -152,13 +161,16 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: "'asdf';", + }, + { + messageId: 'suggestSatisfies', output: "'asdf' satisfies string;", }, ], }, ], - output: "'asdf';", }, { code: ` @@ -174,7 +186,14 @@ str += ''; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: ` +let str = 'asdf'; + + `, + }, + { + messageId: 'suggestSatisfies', output: ` let str = 'asdf'; str satisfies string; @@ -183,10 +202,6 @@ str satisfies string; ], }, ], - output: ` -let str = 'asdf'; - - `, }, { code: ` @@ -202,7 +217,14 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: ` +let str = 'asdf'; +'asdf' + (str); + `, + }, + { + messageId: 'suggestSatisfies', output: ` let str = 'asdf'; 'asdf' + (str satisfies string); @@ -211,10 +233,6 @@ let str = 'asdf'; ], }, ], - output: ` -let str = 'asdf'; -'asdf' + (str); - `, }, { code: 'Number(123);', @@ -225,13 +243,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: '123;', + }, + { + messageId: 'suggestSatisfies', output: '123 satisfies number;', }, ], }, ], - output: '123;', }, { code: '+123;', @@ -242,13 +263,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: '123;', + }, + { + messageId: 'suggestSatisfies', output: '123 satisfies number;', }, ], }, ], - output: '123;', }, { code: '~~123;', @@ -259,13 +283,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: '123;', + }, + { + messageId: 'suggestSatisfies', output: '123 satisfies number;', }, ], }, ], - output: '123;', }, { code: 'Boolean(true);', @@ -276,13 +303,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: 'true;', + }, + { + messageId: 'suggestSatisfies', output: 'true satisfies boolean;', }, ], }, ], - output: 'true;', }, { code: '!!true;', @@ -293,13 +323,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: 'true;', + }, + { + messageId: 'suggestSatisfies', output: 'true satisfies boolean;', }, ], }, ], - output: 'true;', }, { code: 'BigInt(3n);', @@ -310,13 +343,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: '3n;', + }, + { + messageId: 'suggestSatisfies', output: '3n satisfies bigint;', }, ], }, ], - output: '3n;', }, // using type conversion idioms on generics that extend primitives is invalid @@ -335,7 +371,15 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: ` + function f(x: T) { + return x; + } + `, + }, + { + messageId: 'suggestSatisfies', output: ` function f(x: T) { return x satisfies string; @@ -345,11 +389,6 @@ let str = 'asdf'; ], }, ], - output: ` - function f(x: T) { - return x; - } - `, }, { code: ` @@ -366,7 +405,15 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: ` + function f(x: T) { + return x; + } + `, + }, + { + messageId: 'suggestSatisfies', output: ` function f(x: T) { return x satisfies number; @@ -376,11 +423,6 @@ let str = 'asdf'; ], }, ], - output: ` - function f(x: T) { - return x; - } - `, }, { code: ` @@ -397,7 +439,15 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: ` + function f(x: T) { + return x; + } + `, + }, + { + messageId: 'suggestSatisfies', output: ` function f(x: T) { return x satisfies boolean; @@ -407,11 +457,6 @@ let str = 'asdf'; ], }, ], - output: ` - function f(x: T) { - return x; - } - `, }, { code: ` @@ -428,7 +473,15 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: ` + function f(x: T) { + return x; + } + `, + }, + { + messageId: 'suggestSatisfies', output: ` function f(x: T) { return x satisfies bigint; @@ -438,11 +491,6 @@ let str = 'asdf'; ], }, ], - output: ` - function f(x: T) { - return x; - } - `, }, // make sure fixes preserve parentheses in cases where logic would otherwise break @@ -455,13 +503,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: "('a' + 'b').length;", + }, + { + messageId: 'suggestSatisfies', output: "(('a' + 'b') satisfies string).length;", }, ], }, ], - output: "('a' + 'b').length;", }, { code: "('a' + 'b').toString().length;", @@ -472,13 +523,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: "('a' + 'b').length;", + }, + { + messageId: 'suggestSatisfies', output: "(('a' + 'b') satisfies string).length;", }, ], }, ], - output: "('a' + 'b').length;", }, { code: '2 * +(2 + 2);', @@ -489,13 +543,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: '2 * (2 + 2);', + }, + { + messageId: 'suggestSatisfies', output: '2 * ((2 + 2) satisfies number);', }, ], }, ], - output: '2 * (2 + 2);', }, { code: '2 * Number(2 + 2);', @@ -506,13 +563,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: '2 * (2 + 2);', + }, + { + messageId: 'suggestSatisfies', output: '2 * ((2 + 2) satisfies number);', }, ], }, ], - output: '2 * (2 + 2);', }, { code: '2 * ~~(2 + 2);', @@ -523,13 +583,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: '2 * (2 + 2);', + }, + { + messageId: 'suggestSatisfies', output: '2 * ((2 + 2) satisfies number);', }, ], }, ], - output: '2 * (2 + 2);', }, { code: 'false && !!(false || true);', @@ -540,13 +603,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: 'false && (false || true);', + }, + { + messageId: 'suggestSatisfies', output: 'false && ((false || true) satisfies boolean);', }, ], }, ], - output: 'false && (false || true);', }, { code: 'false && Boolean(false || true);', @@ -557,13 +623,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: 'false && (false || true);', + }, + { + messageId: 'suggestSatisfies', output: 'false && ((false || true) satisfies boolean);', }, ], }, ], - output: 'false && (false || true);', }, { code: '2n * BigInt(2n + 2n);', @@ -574,13 +643,16 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: '2n * (2n + 2n);', + }, + { + messageId: 'suggestSatisfies', output: '2n * ((2n + 2n) satisfies bigint);', }, ], }, ], - output: '2n * (2n + 2n);', }, // make sure suggestions add parentheses in cases where syntax would otherwise break @@ -598,7 +670,14 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: ` + let str = 'asdf'; + str.length; + `, + }, + { + messageId: 'suggestSatisfies', output: ` let str = 'asdf'; (str satisfies string).length; @@ -607,10 +686,6 @@ let str = 'asdf'; ], }, ], - output: ` - let str = 'asdf'; - str.length; - `, }, { code: ` @@ -624,7 +699,14 @@ let str = 'asdf'; messageId: 'unnecessaryTypeConversion', suggestions: [ { - messageId: 'unnecessaryTypeConversionSuggestion', + messageId: 'suggestRemove', + output: ` + let str = 'asdf'; + str.length; + `, + }, + { + messageId: 'suggestSatisfies', output: ` let str = 'asdf'; (str satisfies string).length; @@ -633,10 +715,6 @@ let str = 'asdf'; ], }, ], - output: ` - let str = 'asdf'; - str.length; - `, }, ], }); From 6afafe9686faf065226cb0993df64047e8b58a90 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Fri, 4 Apr 2025 22:22:04 -0400 Subject: [PATCH 27/36] change getType to getTypeLazy --- .../src/rules/no-unnecessary-type-conversion.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 727a502f7ad3..b63856394980 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -243,7 +243,7 @@ export default createRule({ node.callee.type === AST_NODE_TYPES.Identifier && node.arguments.length === 1 ) { - const getType = () => + const getTypeLazy = () => getConstrainedTypeAtLocation(services, node.arguments[0]); const isBuiltInCall = (name: string) => { @@ -259,22 +259,25 @@ export default createRule({ // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum (isBuiltInCall('String') && doesUnderlyingTypeMatchFlag( - getType(), + getTypeLazy(), ts.TypeFlags.StringLike, )) || (isBuiltInCall('Number') && doesUnderlyingTypeMatchFlag( - getType(), + getTypeLazy(), ts.TypeFlags.NumberLike, )) || // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum (isBuiltInCall('Boolean') && doesUnderlyingTypeMatchFlag( - getType(), + getTypeLazy(), ts.TypeFlags.BooleanLike, )) || (isBuiltInCall('BigInt') && - doesUnderlyingTypeMatchFlag(getType(), ts.TypeFlags.BigIntLike)) + doesUnderlyingTypeMatchFlag( + getTypeLazy(), + ts.TypeFlags.BigIntLike, + )) ) { const wrappingFixerParams = { node, From c0976d1cd41b27037e7420502cfb265eace0af90 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Fri, 4 Apr 2025 22:33:13 -0400 Subject: [PATCH 28/36] alias node callee to avoid type assertion --- .../src/rules/no-unnecessary-type-conversion.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index b63856394980..0ea945fb636b 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -239,15 +239,16 @@ export default createRule({ } }, CallExpression(node: TSESTree.CallExpression): void { + const nodeCallee = node.callee; if ( - node.callee.type === AST_NODE_TYPES.Identifier && + nodeCallee.type === AST_NODE_TYPES.Identifier && node.arguments.length === 1 ) { const getTypeLazy = () => getConstrainedTypeAtLocation(services, node.arguments[0]); const isBuiltInCall = (name: string) => { - if ((node.callee as TSESTree.Identifier).name === name) { + if (nodeCallee.name === name) { const scope = context.sourceCode.getScope(node); const variable = scope.set.get(name); return !variable?.defs.length; @@ -284,14 +285,14 @@ export default createRule({ innerNode: [node.arguments[0]], sourceCode: context.sourceCode, }; - const typeString = node.callee.name.toLowerCase(); + const typeString = nodeCallee.name.toLowerCase(); context.report({ - loc: node.callee.loc, + node: nodeCallee, messageId: 'unnecessaryTypeConversion', data: { - type: node.callee.name.toLowerCase(), - violation: `Passing a ${typeString} to ${node.callee.name}()`, + type: nodeCallee.name.toLowerCase(), + violation: `Passing a ${typeString} to ${nodeCallee.name}()`, }, suggest: [ { From 973234fc3aef149755cd556b27b3a9e22780b034 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Fri, 4 Apr 2025 22:38:51 -0400 Subject: [PATCH 29/36] fix type in message --- .../eslint-plugin/src/rules/no-unnecessary-type-conversion.ts | 2 +- .../no-unnecessary-type-conversion.shot | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 0ea945fb636b..ddd23a2cd42b 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -382,7 +382,7 @@ export default createRule({ node, ts.TypeFlags.NumberLike, { - type: 'boolean', + type: 'number', violation: 'Using ~~ on a number', }, true, diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot index fa6f09e54441..dbbfcce1e97e 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot @@ -17,7 +17,7 @@ Number(123); +123; ~ Using the unary + operator on a number does not change the type or value of the number. ~~123; -~~ Using ~~ on a number does not change the type or value of the boolean. +~~ Using ~~ on a number does not change the type or value of the number. Boolean(true); ~~~~~~~ Passing a boolean to Boolean() does not change the type or value of the boolean. From e8956c0c8541a0b37f640caf1f81c0e194d6ed1e Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Fri, 4 Apr 2025 22:59:34 -0400 Subject: [PATCH 30/36] use unionTypeParts helper function --- .../src/rules/no-unnecessary-type-conversion.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index ddd23a2cd42b..a3a9585b43e3 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -6,6 +6,7 @@ import type { } from '@typescript-eslint/utils/ts-eslint'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { unionTypeParts } from 'ts-api-utils'; import * as ts from 'typescript'; import { @@ -48,15 +49,7 @@ export default createRule({ type: ts.Type, typeFlag: ts.TypeFlags, ): boolean { - const matchesType = (t: ts.Type): boolean => { - return isTypeFlagSet(t, typeFlag); - }; - - if (type.isUnion()) { - return type.types.every(matchesType); - } - - return matchesType(type); + return unionTypeParts(type).every(t => isTypeFlagSet(t, typeFlag)); } const services = getParserServices(context); From ad88ade0a480ccef4427bb4d73b3f2da116e6931 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Fri, 4 Apr 2025 23:12:47 -0400 Subject: [PATCH 31/36] make handleUnaryOperator cleaner --- .../rules/no-unnecessary-type-conversion.ts | 38 +++++++------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index a3a9585b43e3..6e5e5e5794e1 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -1,9 +1,5 @@ import type { TSESTree } from '@typescript-eslint/utils'; -import type { - ReportDescriptorMessageData, - RuleFix, - RuleFixer, -} from '@typescript-eslint/utils/ts-eslint'; +import type { RuleFix, RuleFixer } from '@typescript-eslint/utils/ts-eslint'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { unionTypeParts } from 'ts-api-utils'; @@ -57,29 +53,29 @@ export default createRule({ function handleUnaryOperator( node: TSESTree.UnaryExpression, typeFlag: ts.TypeFlags, - reportDescriptorMessageData: ReportDescriptorMessageData, + typeString: 'boolean' | 'number', + violation: string, isDoubleOperator: boolean, // !! or ~~ ) { + const outerNode = isDoubleOperator ? node.parent : node; const type = services.getTypeAtLocation(node.argument); if (doesUnderlyingTypeMatchFlag(type, typeFlag)) { const wrappingFixerParams = { - node: isDoubleOperator ? node.parent : node, + node: outerNode, innerNode: [node.argument], sourceCode: context.sourceCode, }; - const typeString = - typeFlag === ts.TypeFlags.BooleanLike ? 'boolean' : 'number'; context.report({ loc: { - start: isDoubleOperator ? node.parent.loc.start : node.loc.start, + start: outerNode.loc.start, end: { column: node.loc.start.column + 1, line: node.loc.start.line, }, }, messageId: 'unnecessaryTypeConversion', - data: reportDescriptorMessageData, + data: { type: typeString, violation }, suggest: [ { messageId: 'suggestRemove', @@ -87,7 +83,7 @@ export default createRule({ }, { messageId: 'suggestSatisfies', - data: { type: reportDescriptorMessageData.type }, + data: { type: typeString }, fix: getWrappingFixer({ ...wrappingFixerParams, wrap: expr => `${expr} satisfies ${typeString}`, @@ -350,10 +346,8 @@ export default createRule({ handleUnaryOperator( node, ts.TypeFlags.BooleanLike, - { - type: 'boolean', - violation: 'Using !! on a boolean', - }, + 'boolean', + 'Using !! on a boolean', true, ); }, @@ -361,10 +355,8 @@ export default createRule({ handleUnaryOperator( node, ts.TypeFlags.NumberLike, - { - type: 'number', - violation: 'Using the unary + operator on a number', - }, + 'number', + 'Using the unary + operator on a number', false, ); }, @@ -374,10 +366,8 @@ export default createRule({ handleUnaryOperator( node, ts.TypeFlags.NumberLike, - { - type: 'number', - violation: 'Using ~~ on a number', - }, + 'number', + 'Using ~~ on a number', true, ); }, From 853906929128ad5312df1b36de7bc414fae4602a Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sat, 5 Apr 2025 01:34:21 -0400 Subject: [PATCH 32/36] fix test after main merge --- .../tests/rules/no-array-constructor.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-array-constructor.test.ts b/packages/eslint-plugin/tests/rules/no-array-constructor.test.ts index b24fbc2c1335..ce97e7e6a8a2 100644 --- a/packages/eslint-plugin/tests/rules/no-array-constructor.test.ts +++ b/packages/eslint-plugin/tests/rules/no-array-constructor.test.ts @@ -58,7 +58,7 @@ ruleTester.run('no-array-constructor', rule, { code: 'Array?.();', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.CallExpression, }, ], @@ -68,7 +68,7 @@ ruleTester.run('no-array-constructor', rule, { code: '/* a */ /* b */ Array /* c */ /* d */ /* e */ /* f */?.(); /* g */ /* h */', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.CallExpression, }, ], @@ -98,7 +98,7 @@ ruleTester.run('no-array-constructor', rule, { code: 'Array?.(x, y);', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.CallExpression, }, ], @@ -108,7 +108,7 @@ ruleTester.run('no-array-constructor', rule, { code: '/* a */ /* b */ Array /* c */ /* d */ /* e */ /* f */?.(x, y); /* g */ /* h */', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.CallExpression, }, ], @@ -138,7 +138,7 @@ ruleTester.run('no-array-constructor', rule, { code: 'Array?.(0, 1, 2);', errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.CallExpression, }, ], @@ -154,7 +154,7 @@ ruleTester.run('no-array-constructor', rule, { `, errors: [ { - messageId, + messageId: 'useLiteral', type: AST_NODE_TYPES.CallExpression, }, ], From 94ebd1e06b0f3544dbe22cc2447cb8c5469fbe02 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Mon, 14 Apr 2025 20:28:48 -0400 Subject: [PATCH 33/36] update snapshots --- .../no-unnecessary-type-conversion.shot | 12 ++---------- .../no-unnecessary-type-conversion.shot | 6 +----- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot index dbbfcce1e97e..1231d273a43f 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-type-conversion.shot @@ -1,7 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Validating rule docs no-unnecessary-type-conversion.mdx code examples ESLint output 1`] = ` -"Incorrect +Incorrect String('123'); ~~~~~~ Passing a string to String() does not change the type or value of the string. @@ -30,11 +27,8 @@ BigInt(BigInt(1)); let str = '123'; str += ''; ~~~~~~~~~ Concatenating a string with '' does not change the type or value of the string. -" -`; -exports[`Validating rule docs no-unnecessary-type-conversion.mdx code examples ESLint output 2`] = ` -"Correct +Correct function foo(bar: string | number) { String(bar); @@ -53,5 +47,3 @@ function foo(bar: string | number) { bar += ''; } -" -`; diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-type-conversion.shot b/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-type-conversion.shot index d269eed9d351..42f81875ed94 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-type-conversion.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/no-unnecessary-type-conversion.shot @@ -1,7 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Rule schemas should be convertible to TS types for documentation purposes no-unnecessary-type-conversion 1`] = ` -" # SCHEMA: [] @@ -10,5 +7,4 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos # TYPES: /** No options declared */ -type Options = [];" -`; +type Options = []; \ No newline at end of file From ae8f4710418bd6eae5fd1795a384c914280e8e26 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Thu, 1 May 2025 09:24:55 -0600 Subject: [PATCH 34/36] review changes --- .../rules/no-unnecessary-type-conversion.mdx | 2 +- .../rules/no-unnecessary-type-conversion.test.ts | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx b/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx index af6c0297720f..01a69bd90031 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx @@ -11,7 +11,7 @@ import TabItem from '@theme/TabItem'; JavaScript has several idioms used to convert values to certain types. With TypeScript, it is possible to see at build time if the value is already of that type, making the conversion unnecessary. Performing unnecessary conversions increases visual clutter and harms code readability, so it's generally best practice to remove them if they don't change the type of an expression. -This rule reports when a type conversion idiom is identified which does not change the type of an expression. +This rule reports when a type conversion idiom is identified which does not change the runtime type of an expression. ## Examples diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts index 064a60f53de0..e0aaadacee2d 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-conversion.test.ts @@ -40,31 +40,35 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { '!false;', '~2;', ` - function String(value) { + function String(value: unknown) { return value; } String('asdf'); + export {}; `, ` - function Number(value) { + function Number(value: unknown) { return value; } Number(2); + export {}; `, ` - function Boolean(value) { + function Boolean(value: unknown) { return value; } Boolean(true); + export {}; `, ` - function BigInt(value) { + function BigInt(value: unknown) { return value; } BigInt(3n); + export {}; `, ` - function toString(value) { + function toString(value: unknown) { return value; } toString('asdf'); @@ -77,7 +81,7 @@ ruleTester.run('no-unnecessary-type-conversion', rule, { // using type conversion idioms to unbox boxed primitives is valid 'String(new String());', - 'new String.toString();', + 'new String().toString();', "'' + new String();", "new String() + '';", ` From 6f88f00d545c064c51b8f99a4653c00d8e17d3fc Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Thu, 1 May 2025 21:19:04 -0600 Subject: [PATCH 35/36] revert bad suggestion --- .../eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx b/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx index 01a69bd90031..af6c0297720f 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx @@ -11,7 +11,7 @@ import TabItem from '@theme/TabItem'; JavaScript has several idioms used to convert values to certain types. With TypeScript, it is possible to see at build time if the value is already of that type, making the conversion unnecessary. Performing unnecessary conversions increases visual clutter and harms code readability, so it's generally best practice to remove them if they don't change the type of an expression. -This rule reports when a type conversion idiom is identified which does not change the runtime type of an expression. +This rule reports when a type conversion idiom is identified which does not change the type of an expression. ## Examples From 6822a69fb7d81e5b28decf75df14ab6901911b25 Mon Sep 17 00:00:00 2001 From: Sasha Kondrashov Date: Sat, 3 May 2025 15:15:25 -0400 Subject: [PATCH 36/36] josh feedback --- .../rules/no-unnecessary-type-conversion.mdx | 11 +- .../rules/no-unnecessary-type-conversion.ts | 118 ++++++++---------- 2 files changed, 60 insertions(+), 69 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx b/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx index af6c0297720f..de0d7f1002bb 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-type-conversion.mdx @@ -9,9 +9,14 @@ import TabItem from '@theme/TabItem'; > > See **https://typescript-eslint.io/rules/no-unnecessary-type-conversion** for documentation. -JavaScript has several idioms used to convert values to certain types. With TypeScript, it is possible to see at build time if the value is already of that type, making the conversion unnecessary. -Performing unnecessary conversions increases visual clutter and harms code readability, so it's generally best practice to remove them if they don't change the type of an expression. -This rule reports when a type conversion idiom is identified which does not change the type of an expression. +JavaScript provides several commonly used idioms to convert values to a specific type: + +- Primitive coercion (e.g. `Boolean(value)`, `String(value)`): using a built-in primitive function +- String concatenation (e.g. `value + ''`): turning a value into a string +- Unary coercion (e.g. `+value`, `!!value`): using a built-in operator +- The `.toString()` method defined on many types + +These conversions are unnecessary if the value is already of that type. ## Examples diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts index 6e5e5e5794e1..dfa4a57de456 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-conversion.ts @@ -229,77 +229,63 @@ export default createRule({ }, CallExpression(node: TSESTree.CallExpression): void { const nodeCallee = node.callee; + const builtInTypeFlags = { + BigInt: ts.TypeFlags.BigIntLike, + Boolean: ts.TypeFlags.BooleanLike, + Number: ts.TypeFlags.NumberLike, + String: ts.TypeFlags.StringLike, + }; + if ( - nodeCallee.type === AST_NODE_TYPES.Identifier && - node.arguments.length === 1 + nodeCallee.type !== AST_NODE_TYPES.Identifier || + !(nodeCallee.name in builtInTypeFlags) ) { - const getTypeLazy = () => - getConstrainedTypeAtLocation(services, node.arguments[0]); + return; + } - const isBuiltInCall = (name: string) => { - if (nodeCallee.name === name) { - const scope = context.sourceCode.getScope(node); - const variable = scope.set.get(name); - return !variable?.defs.length; - } - return false; - }; + const typeFlag = + builtInTypeFlags[nodeCallee.name as keyof typeof builtInTypeFlags]; + const scope = context.sourceCode.getScope(node); + const variable = scope.set.get(nodeCallee.name); + if ( + !!variable?.defs.length || + !doesUnderlyingTypeMatchFlag( + getConstrainedTypeAtLocation(services, node.arguments[0]), + typeFlag, + ) + ) { + return; + } - if ( - // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum - (isBuiltInCall('String') && - doesUnderlyingTypeMatchFlag( - getTypeLazy(), - ts.TypeFlags.StringLike, - )) || - (isBuiltInCall('Number') && - doesUnderlyingTypeMatchFlag( - getTypeLazy(), - ts.TypeFlags.NumberLike, - )) || - // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum - (isBuiltInCall('Boolean') && - doesUnderlyingTypeMatchFlag( - getTypeLazy(), - ts.TypeFlags.BooleanLike, - )) || - (isBuiltInCall('BigInt') && - doesUnderlyingTypeMatchFlag( - getTypeLazy(), - ts.TypeFlags.BigIntLike, - )) - ) { - const wrappingFixerParams = { - node, - innerNode: [node.arguments[0]], - sourceCode: context.sourceCode, - }; - const typeString = nodeCallee.name.toLowerCase(); + const wrappingFixerParams = { + node, + innerNode: [node.arguments[0]], + sourceCode: context.sourceCode, + }; + const typeString = nodeCallee.name.toLowerCase(); - context.report({ - node: nodeCallee, - messageId: 'unnecessaryTypeConversion', - data: { - type: nodeCallee.name.toLowerCase(), - violation: `Passing a ${typeString} to ${nodeCallee.name}()`, - }, - suggest: [ - { - messageId: 'suggestRemove', - fix: getWrappingFixer(wrappingFixerParams), - }, - { - messageId: 'suggestSatisfies', - data: { type: typeString }, - fix: getWrappingFixer({ - ...wrappingFixerParams, - wrap: expr => `${expr} satisfies ${typeString}`, - }), - }, - ], - }); - } - } + context.report({ + node: nodeCallee, + messageId: 'unnecessaryTypeConversion', + data: { + type: nodeCallee.name.toLowerCase(), + violation: `Passing a ${typeString} to ${nodeCallee.name}()`, + }, + suggest: [ + { + messageId: 'suggestRemove', + fix: getWrappingFixer(wrappingFixerParams), + }, + { + messageId: 'suggestSatisfies', + data: { type: typeString }, + fix: getWrappingFixer({ + ...wrappingFixerParams, + wrap: expr => `${expr} satisfies ${typeString}`, + }), + }, + ], + }); }, 'CallExpression > MemberExpression.callee > Identifier[name = "toString"].property'( node: TSESTree.Expression,