From 052ffa2daf4192c748d050dc6b7e9db6d2f7212f Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 6 Jan 2025 21:58:08 +0200 Subject: [PATCH 1/6] rough implementation --- .../rules/no-unnecessary-type-assertion.ts | 67 ++++++++++++++++++- 1 file changed, 64 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts index c73d8717c3e1..ab7c3037f944 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -2,7 +2,11 @@ import type { Scope } from '@typescript-eslint/scope-manager'; import type { TSESTree } from '@typescript-eslint/utils'; import type { ReportFixFunction } from '@typescript-eslint/utils/ts-eslint'; -import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; +import { + AST_NODE_TYPES, + AST_TOKEN_TYPES, + ASTUtils, +} from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; @@ -208,6 +212,53 @@ export default createRule({ return false; } + function isFreshLiteralFreeExpression(node: TSESTree.Expression): boolean { + if ( + node.type === AST_NODE_TYPES.TemplateLiteral || + node.type === AST_NODE_TYPES.Literal + ) { + return false; + } + + if (node.type === AST_NODE_TYPES.Identifier) { + // It seems fresh literals have a slightly different type at their + // definition rather than their reference. + const scope = context.sourceCode.getScope(node); + const superVar = ASTUtils.findVariable(scope, node.name); + + if (superVar) { + const definition = superVar.defs.at(0); + + if (definition) { + return !isFreshLiteralType( + services.getTypeAtLocation(definition.node), + ); + } + } + } + + if (node.type === AST_NODE_TYPES.ConditionalExpression) { + return ( + isFreshLiteralFreeExpression(node.alternate) && + isFreshLiteralFreeExpression(node.consequent) + ); + } + + return true; + } + + function wouldSameLiteralTypeBeInferred( + node: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion, + ): boolean { + // Literal widening only happens to literal types that originate from + // expressions, not types. + if (isFreshLiteralFreeExpression(node.expression)) { + return true; + } + + return isImplicitlyNarrowedConstDeclaration(node); + } + return { 'TSAsExpression, TSTypeAssertion'( node: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion, @@ -224,8 +275,13 @@ export default createRule({ const uncastType = services.getTypeAtLocation(node.expression); const typeIsUnchanged = isTypeUnchanged(uncastType, castType); - const wouldSameTypeBeInferred = castType.isLiteral() - ? isImplicitlyNarrowedConstDeclaration(node) + const isLiteral = + castType.isLiteral() || + (tsutils.isUnionType(castType) && + tsutils.unionTypeParts(castType).every(part => part.isLiteral())); + + const wouldSameTypeBeInferred = isLiteral + ? wouldSameLiteralTypeBeInferred(node) : !isConstAssertion(node.typeAnnotation); if (typeIsUnchanged && wouldSameTypeBeInferred) { @@ -387,3 +443,8 @@ export default createRule({ }; }, }); + +// https://github.com/microsoft/TypeScript/blob/21c1a61b49082915f93e3327dad0d73205cf4273/src/compiler/checker.ts/#L19679-L19681 +function isFreshLiteralType(type: ts.Type) { + return tsutils.isFreshableType(type) && type.freshType === type; +} From 87e3f235e3ac2d93930eb0a0373d8945727f9ce6 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 6 Jan 2025 22:18:51 +0200 Subject: [PATCH 2/6] roughly add tests --- .../no-unnecessary-type-assertion.test.ts | 224 ++++++++++++++++++ 1 file changed, 224 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts index db8309164efb..9b01b5633b34 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts @@ -358,6 +358,70 @@ if (Math.random()) { x!; `, }, + "let a = (Date.now() % 2 ? 'a' : 'b') as 'a' | 'b';", + ` + const state: 'expired' | 'pending' = 'pending'; + + function main() { + return { + type: \`\${state}Request\` as \`\${typeof state}Request\`, + }; + } + `, + ` +let conditionFreshCasted = (Math.random() > 0.5 ? 'foo' : 'bar') as + | 'foo' + | 'bar'; + `, + ` +let a = (Math.random() > 0.5 ? 'foo' : 'bar') as 'foo' | 'bar'; + `, + ` +let a = (Math.random() > 0.5 ? 1 : 2) as 1 | 2; + `, + ` +declare const foo: 'foo'; +const bar = 'bar'; + +let a = (Math.random() > 0.5 ? foo : bar) as bar | foo; + `, + ` +const foo = 'foo'; +let a = foo as 'foo'; + `, + ` +const foo = 'foo'; +function f() { + return foo as 'foo'; +} + `, + ` +const foo = 'foo'; +const a = { + foo: foo as 'foo', +}; + `, + ` +function f() { + return 'foo'; +} + +let callCasted = f() as 'foo'; + `, + ` +const obj = { + foo: 'foo', +}; + +let s = obj.foo as 'foo'; + `, + ` +const a = 1; + +foo(a as 1); + +function foo(a: T) {} + `, ], invalid: [ @@ -1151,5 +1215,165 @@ const a = ''; const b: string | undefined = (a ? undefined : a); `, }, + { + code: "const a = (Date.now() % 2 ? 'a' : 'b') as 'a' | 'b';", + errors: [ + { + column: 11, + line: 1, + messageId: 'unnecessaryAssertion', + }, + ], + output: `const a = (Date.now() % 2 ? 'a' : 'b');`, + }, + { + code: ` +declare const foo: 'foo'; +declare const bar: 'bar'; + +let a = (Math.random() > 0.5 ? foo : bar) as 'bar' | 'foo'; + `, + errors: [ + { + column: 9, + line: 5, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +declare const foo: 'foo'; +declare const bar: 'bar'; + +let a = (Math.random() > 0.5 ? foo : bar); + `, + }, + { + code: ` +declare const foo: 'foo'; +let a = foo as 'foo'; + `, + errors: [ + { + column: 9, + line: 3, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +declare const foo: 'foo'; +let a = foo; + `, + }, + { + code: ` +declare const foo: 'foo'; +function f() { + return foo as 'foo'; +} + `, + errors: [ + { + column: 10, + line: 4, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +declare const foo: 'foo'; +function f() { + return foo; +} + `, + }, + { + code: ` +declare const foo: 'foo'; +const a = { + foo: foo as 'foo', +}; + `, + errors: [ + { + column: 8, + line: 4, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +declare const foo: 'foo'; +const a = { + foo: foo, +}; + `, + }, + { + code: ` +function f(): 'foo' { + return 'foo'; +} + +let callCasted = f() as 'foo'; + `, + errors: [ + { + column: 18, + line: 6, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +function f(): 'foo' { + return 'foo'; +} + +let callCasted = f(); + `, + }, + { + code: ` +const obj = { + foo: 'foo' as const, +}; + +let s = obj.foo as 'foo'; + `, + errors: [ + { + column: 9, + line: 6, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +const obj = { + foo: 'foo' as const, +}; + +let s = obj.foo; + `, + }, + { + code: ` +declare const a: 1; + +foo(a as 1); + +function foo(a: T) {} + `, + errors: [ + { + column: 5, + line: 4, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +declare const a: 1; + +foo(a); + +function foo(a: T) {} + `, + }, ], }); From a967da85f31677ace30e14722841899671f382aa Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Tue, 7 Jan 2025 13:41:30 +0200 Subject: [PATCH 3/6] add tests for recent regression --- .../no-unnecessary-type-assertion.test.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts index 9b01b5633b34..29dca1e8e7c2 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts @@ -1375,5 +1375,55 @@ foo(a); function foo(a: T) {} `, }, + { + code: ` +const state: 'expired' | 'pending' = 'pending'; + +function main() { + return { + type: state as 'expired' | 'pending', + }; +} + `, + errors: [ + { + column: 11, + line: 6, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +const state: 'expired' | 'pending' = 'pending'; + +function main() { + return { + type: state, + }; +} + `, + }, + { + code: ` +const state: 'expired' | 'pending' = 'pending'; + +class Example { + type = state as 'expired' | 'pending'; +} + `, + errors: [ + { + column: 10, + line: 5, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +const state: 'expired' | 'pending' = 'pending'; + +class Example { + type = state; +} + `, + }, ], }); From 8805b8240a67f4564a4b9265fbad27e8f9f3e496 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Tue, 7 Jan 2025 13:58:40 +0200 Subject: [PATCH 4/6] refactor a bit --- .../rules/no-unnecessary-type-assertion.ts | 45 +++++++---- .../no-unnecessary-type-assertion.test.ts | 74 +++++++++++++++++++ 2 files changed, 105 insertions(+), 14 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts index ab7c3037f944..2a2a3e0d296d 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -212,39 +212,56 @@ export default createRule({ return false; } - function isFreshLiteralFreeExpression(node: TSESTree.Expression): boolean { + function doesExpressionHaveFreshLiterals( + node: TSESTree.Expression, + ): boolean { + // Actual literals don't seem to have the type of a "fresh literal" if ( node.type === AST_NODE_TYPES.TemplateLiteral || node.type === AST_NODE_TYPES.Literal ) { - return false; + return true; } if (node.type === AST_NODE_TYPES.Identifier) { - // It seems fresh literals have a slightly different type at their + // It seems "fresh literals" have a slightly different type at their // definition rather than their reference. const scope = context.sourceCode.getScope(node); const superVar = ASTUtils.findVariable(scope, node.name); - if (superVar) { - const definition = superVar.defs.at(0); + if (superVar == null) { + return true; + } - if (definition) { - return !isFreshLiteralType( - services.getTypeAtLocation(definition.node), - ); - } + const definition = superVar.defs[0]; + const typeAtDefinition = services.getTypeAtLocation(definition.node); + + if (tsutils.isUnionType(typeAtDefinition)) { + return tsutils + .unionTypeParts(typeAtDefinition) + .some(part => isFreshLiteralType(part)); } + + return isFreshLiteralType(typeAtDefinition); } if (node.type === AST_NODE_TYPES.ConditionalExpression) { return ( - isFreshLiteralFreeExpression(node.alternate) && - isFreshLiteralFreeExpression(node.consequent) + doesExpressionHaveFreshLiterals(node.alternate) || + doesExpressionHaveFreshLiterals(node.consequent) ); } - return true; + if ( + tsutils.isTypeFlagSet( + services.getTypeAtLocation(node), + ts.TypeFlags.EnumLiteral | ts.TypeFlags.UniqueESSymbol, + ) + ) { + return true; + } + + return false; } function wouldSameLiteralTypeBeInferred( @@ -252,7 +269,7 @@ export default createRule({ ): boolean { // Literal widening only happens to literal types that originate from // expressions, not types. - if (isFreshLiteralFreeExpression(node.expression)) { + if (!doesExpressionHaveFreshLiterals(node.expression)) { return true; } diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts index 29dca1e8e7c2..14ca079f7eca 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts @@ -422,6 +422,46 @@ foo(a as 1); function foo(a: T) {} `, + 'let a = b as 1;', + ` +const a = Math.random() > 0.5 ? 'foo' : 'bar'; + +let c = a as 'bar' | 'foo'; + `, + ` +enum Foo { + Bar, + Bazz, +} + +const data = { + x: Foo.Bar as Foo.Bar, +}; + `, + ` +enum Foo { + Bar, + Bazz, +} + +const a = Foo.Bar; + +const data = { + x: a as Foo.Bar, +}; + `, + ` +enum Foo { + Bar, + Bazz, +} + +const a = Foo; + +const data = { + x: a.Bar as Foo.Bar, +}; + `, ], invalid: [ @@ -1425,5 +1465,39 @@ class Example { } `, }, + + { + code: ` +enum Foo { + Bar, + Bazz, +} + +declare const a: Foo.Bar; + +const data = { + x: a as Foo.Bar, +}; + `, + errors: [ + { + column: 6, + line: 10, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +enum Foo { + Bar, + Bazz, +} + +declare const a: Foo.Bar; + +const data = { + x: a, +}; + `, + }, ], }); From 3d5d6d36dd2a842330c0377e23829e9dd5df7b67 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Thu, 9 Jan 2025 18:48:13 +0200 Subject: [PATCH 5/6] consider mixed expressions --- .../rules/no-unnecessary-type-assertion.ts | 23 +++----------- .../no-unnecessary-type-assertion.test.ts | 31 ++++++++++++++++++- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts index 2a2a3e0d296d..19804143bb40 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -264,18 +264,6 @@ export default createRule({ return false; } - function wouldSameLiteralTypeBeInferred( - node: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion, - ): boolean { - // Literal widening only happens to literal types that originate from - // expressions, not types. - if (!doesExpressionHaveFreshLiterals(node.expression)) { - return true; - } - - return isImplicitlyNarrowedConstDeclaration(node); - } - return { 'TSAsExpression, TSTypeAssertion'( node: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion, @@ -292,13 +280,12 @@ export default createRule({ const uncastType = services.getTypeAtLocation(node.expression); const typeIsUnchanged = isTypeUnchanged(uncastType, castType); - const isLiteral = - castType.isLiteral() || - (tsutils.isUnionType(castType) && - tsutils.unionTypeParts(castType).every(part => part.isLiteral())); + const hasFreshLiterals = doesExpressionHaveFreshLiterals( + node.expression, + ); - const wouldSameTypeBeInferred = isLiteral - ? wouldSameLiteralTypeBeInferred(node) + const wouldSameTypeBeInferred = hasFreshLiterals + ? isImplicitlyNarrowedConstDeclaration(node) : !isConstAssertion(node.typeAnnotation); if (typeIsUnchanged && wouldSameTypeBeInferred) { diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts index 14ca079f7eca..f7db71d87853 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts @@ -462,6 +462,13 @@ const data = { x: a.Bar as Foo.Bar, }; `, + ` +class Foo { + hey?: string; +} + +let b = (Math.random() > 0.5 ? new Foo() : '1') as '1' | Foo; + `, ], invalid: [ @@ -1465,7 +1472,6 @@ class Example { } `, }, - { code: ` enum Foo { @@ -1499,5 +1505,28 @@ const data = { }; `, }, + { + code: ` +class Foo { + hey?: string; +} + +const b = (Math.random() > 0.5 ? new Foo() : '1') as '1' | Foo; + `, + errors: [ + { + column: 11, + line: 6, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +class Foo { + hey?: string; +} + +const b = (Math.random() > 0.5 ? new Foo() : '1'); + `, + }, ], }); From 8d17d0973f78aacf8e9e32aef77c117fc3e51bd4 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Thu, 9 Jan 2025 19:17:19 +0200 Subject: [PATCH 6/6] fix runtime error --- .../src/rules/no-unnecessary-type-assertion.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts index 19804143bb40..5dfed1cff579 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -233,7 +233,12 @@ export default createRule({ return true; } - const definition = superVar.defs[0]; + const definition = superVar.defs.at(0); + + if (definition == null) { + return true; + } + const typeAtDefinition = services.getTypeAtLocation(definition.node); if (tsutils.isUnionType(typeAtDefinition)) {