From 62b77e706f3f72c54d3de2deaa7e77e70c14d59a Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Mon, 6 Jan 2025 18:37:54 +0900 Subject: [PATCH 1/8] fix: handle readonly class properties --- .../src/rules/no-unnecessary-type-assertion.ts | 11 ++++++++++- 1 file changed, 10 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 d29bd1292d02..29a433458ebb 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -177,6 +177,14 @@ export default createRule({ ); } + function isReadonlyClassProperty({ + parent, + }: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion) { + return ( + parent.type === AST_NODE_TYPES.PropertyDefinition && parent.readonly + ); + } + function isTypeUnchanged(uncast: ts.Type, cast: ts.Type): boolean { if (uncast === cast) { return true; @@ -230,7 +238,8 @@ export default createRule({ const wouldSameTypeBeInferred = castType.isLiteral() || isAssertionTypeUnionStringlike - ? isImplicitlyNarrowedConstDeclaration(node) + ? isImplicitlyNarrowedConstDeclaration(node) || + isReadonlyClassProperty(node) : !isConstAssertion(node.typeAnnotation); if (typeIsUnchanged && wouldSameTypeBeInferred) { From 80b23fd7e3b2c1a1769e6b5988404673e579eba3 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Mon, 6 Jan 2025 18:38:09 +0900 Subject: [PATCH 2/8] test: add test --- .../no-unnecessary-type-assertion.test.ts | 34 +++++++++++++++++++ 1 file changed, 34 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 a4d34d1881bb..098aa93344a2 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 @@ -1148,6 +1148,40 @@ var x = 1; }, { code: ` +type S = 10; + +class T { + readonly a = 'a' as const; + readonly b = 3 as 3; + readonly c = 10 as S; +} + `, + errors: [ + { + line: 5, + messageId: 'unnecessaryAssertion', + }, + { + line: 6, + messageId: 'unnecessaryAssertion', + }, + { + line: 7, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +type S = 10; + +class T { + readonly a = 'a'; + readonly b = 3; + readonly c = 10; +} + `, + }, + { + code: ` const a = ''; const b: string | undefined = (a ? undefined : a)!; `, From d20d45873c54de411e398e40f2d11bf9bf1a983b Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Fri, 17 Jan 2025 23:22:40 +0900 Subject: [PATCH 3/8] test: add valid case --- .../rules/no-unnecessary-type-assertion.test.ts | 15 +++++++++++++++ 1 file changed, 15 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 4d22b0119bc5..ccd90f5f0243 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 @@ -151,6 +151,21 @@ const y: number = x!; const x: number | null = null; class Foo { prop: number = x!; +} + `, + ` +type S = 10; + +class T { + a = 'a' as const; + b = 3 as 3; + c = 10 as S; +} + `, + ` +class T { + readonly a = 'a'; + readonly b = 3; } `, ` From b0924e7f3d5951742cea388f9ecd27d2a0d17730 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Fri, 17 Jan 2025 23:45:34 +0900 Subject: [PATCH 4/8] fix: check that template literals nodes have no expressions in them --- .../src/rules/no-unnecessary-type-assertion.ts | 15 ++++++++++----- .../rules/no-unnecessary-type-assertion.test.ts | 9 +++++++-- 2 files changed, 17 insertions(+), 7 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 2e4e86123f18..9920c2596225 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -156,15 +156,19 @@ export default createRule({ ); } + function isTemplateLiteralWithExpressions(expression: TSESTree.Expression) { + return ( + expression.type === AST_NODE_TYPES.TemplateLiteral && + expression.expressions.length !== 0 + ); + } + function isImplicitlyNarrowedConstDeclaration({ expression, parent, }: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion): boolean { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const maybeDeclarationNode = parent.parent!; - const isTemplateLiteralWithExpressions = - expression.type === AST_NODE_TYPES.TemplateLiteral && - expression.expressions.length !== 0; return ( maybeDeclarationNode.type === AST_NODE_TYPES.VariableDeclaration && maybeDeclarationNode.kind === 'const' && @@ -172,7 +176,7 @@ export default createRule({ * Even on `const` variable declarations, template literals with expressions can sometimes be widened without a type assertion. * @see https://github.com/typescript-eslint/typescript-eslint/issues/8737 */ - !isTemplateLiteralWithExpressions + !isTemplateLiteralWithExpressions(expression) ); } @@ -234,7 +238,8 @@ export default createRule({ const wouldSameTypeBeInferred = castType.isLiteral() ? isImplicitlyNarrowedConstDeclaration(node) || - isReadonlyClassProperty(node) + (isReadonlyClassProperty(node) && + !isTemplateLiteralWithExpressions(node.expression)) : !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 ccd90f5f0243..92b2aff0fba4 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 @@ -162,12 +162,17 @@ class T { c = 10 as S; } `, - ` + { + code: ` +const foo = 'foo'; + class T { readonly a = 'a'; readonly b = 3; + readonly test = \`\${foo}\` as const; } - `, + `, + }, ` declare const y: number | null; console.log(y!); From d000227ab083d6587add8b8f2d381f74bb71c6c6 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Wed, 22 Jan 2025 07:59:37 +0900 Subject: [PATCH 5/8] test: split invalid test --- .../no-unnecessary-type-assertion.test.ts | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) 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 92b2aff0fba4..35682cb0aa33 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 @@ -1158,25 +1158,51 @@ var x = 1; }, { code: ` -type S = 10; - class T { readonly a = 'a' as const; - readonly b = 3 as 3; - readonly c = 10 as S; } `, errors: [ { - line: 5, + line: 3, messageId: 'unnecessaryAssertion', }, + ], + output: ` +class T { + readonly a = 'a'; +} + `, + }, + { + code: ` +class T { + readonly a = 3 as 3; +} + `, + errors: [ { - line: 6, + line: 3, messageId: 'unnecessaryAssertion', }, + ], + output: ` +class T { + readonly a = 3; +} + `, + }, + { + code: ` +type S = 10; + +class T { + readonly a = 10 as S; +} + `, + errors: [ { - line: 7, + line: 5, messageId: 'unnecessaryAssertion', }, ], @@ -1184,9 +1210,7 @@ class T { type S = 10; class T { - readonly a = 'a'; - readonly b = 3; - readonly c = 10; + readonly a = 10; } `, }, From cc7a71877c90123349434663e1e4617e51855453 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Wed, 22 Jan 2025 08:06:46 +0900 Subject: [PATCH 6/8] refactor: create isImplicitlyNarrowedLiteralDeclaration function --- .../rules/no-unnecessary-type-assertion.ts | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 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 9920c2596225..83c8afe9b78c 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -163,28 +163,25 @@ export default createRule({ ); } - function isImplicitlyNarrowedConstDeclaration({ + function isImplicitlyNarrowedLiteralDeclaration({ expression, parent, }: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion): boolean { + /** + * Even on `const` variable declarations, template literals with expressions can sometimes be widened without a type assertion. + * @see https://github.com/typescript-eslint/typescript-eslint/issues/8737 + */ + if (isTemplateLiteralWithExpressions(expression)) { + return false; + } + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const maybeDeclarationNode = parent.parent!; - return ( - maybeDeclarationNode.type === AST_NODE_TYPES.VariableDeclaration && - maybeDeclarationNode.kind === 'const' && - /** - * Even on `const` variable declarations, template literals with expressions can sometimes be widened without a type assertion. - * @see https://github.com/typescript-eslint/typescript-eslint/issues/8737 - */ - !isTemplateLiteralWithExpressions(expression) - ); - } - function isReadonlyClassProperty({ - parent, - }: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion) { return ( - parent.type === AST_NODE_TYPES.PropertyDefinition && parent.readonly + (maybeDeclarationNode.type === AST_NODE_TYPES.VariableDeclaration && + maybeDeclarationNode.kind === 'const') || + (parent.type === AST_NODE_TYPES.PropertyDefinition && parent.readonly) ); } @@ -237,9 +234,7 @@ export default createRule({ const typeIsUnchanged = isTypeUnchanged(uncastType, castType); const wouldSameTypeBeInferred = castType.isLiteral() - ? isImplicitlyNarrowedConstDeclaration(node) || - (isReadonlyClassProperty(node) && - !isTemplateLiteralWithExpressions(node.expression)) + ? isImplicitlyNarrowedLiteralDeclaration(node) : !isConstAssertion(node.typeAnnotation); if (typeIsUnchanged && wouldSameTypeBeInferred) { From 6f2326bea63527e94dff6b2d2ae38f51387d894e Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Fri, 24 Jan 2025 21:18:43 +0900 Subject: [PATCH 7/8] test: add test --- .../no-unnecessary-type-assertion.test.ts | 50 +++++++++++++++---- 1 file changed, 40 insertions(+), 10 deletions(-) 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 35682cb0aa33..fc4005346cb8 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 @@ -154,25 +154,37 @@ class Foo { } `, ` -type S = 10; - class T { a = 'a' as const; - b = 3 as 3; - c = 10 as S; } `, - { - code: ` + ` +class T { + a = 3 as 3; +} + `, + ` +class T { + a = 10; +} + `, + ` const foo = 'foo'; class T { - readonly a = 'a'; - readonly b = 3; readonly test = \`\${foo}\` as const; } - `, - }, + `, + ` +class T { + readonly a = 3 + 5; +} + `, + ` +class T { + readonly a = { foo: 'foo' } as const; +} + `, ` declare const y: number | null; console.log(y!); @@ -1216,6 +1228,24 @@ class T { }, { code: ` +class T { + readonly a = (3 + 5) as number; +} + `, + errors: [ + { + line: 3, + messageId: 'unnecessaryAssertion', + }, + ], + output: ` +class T { + readonly a = (3 + 5); +} + `, + }, + { + code: ` const a = ''; const b: string | undefined = (a ? undefined : a)!; `, From a320eaf6cd280f6d6941eede2c368a0e57b9cd84 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Tue, 28 Jan 2025 08:26:13 +0900 Subject: [PATCH 8/8] test: remove test --- .../tests/rules/no-unnecessary-type-assertion.test.ts | 10 ---------- 1 file changed, 10 deletions(-) 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 fc4005346cb8..a8a1e132e8e2 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 @@ -164,11 +164,6 @@ class T { } `, ` -class T { - a = 10; -} - `, - ` const foo = 'foo'; class T { @@ -176,11 +171,6 @@ class T { } `, ` -class T { - readonly a = 3 + 5; -} - `, - ` class T { readonly a = { foo: 'foo' } as const; }