From f53f6cd9281ed9ba93521a9be7e05e951fe8bb42 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Mon, 1 Apr 2024 20:12:03 -0500 Subject: [PATCH 1/7] add failing tests --- .../tests/rules/no-unnecessary-type-assertion.test.ts | 6 ++++++ 1 file changed, 6 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 17efb3ff0baf..9c2e6a6dc308 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 @@ -292,6 +292,7 @@ const templateLiteral = \`\${myString}-somethingElse\` as const; const myString = 'foo'; const templateLiteral = \`\${myString}-somethingElse\`; `, + 'let a = `a` as const;', { code: ` declare const foo: { @@ -346,6 +347,11 @@ const bar = foo.a as string | undefined | bigint; output: 'const a = `a`;', errors: [{ messageId: 'unnecessaryAssertion', line: 1 }], }, + { + code: 'const a = `a${`b`}` as const;', + output: 'const a = `a${`b`}`;', + errors: [{ messageId: 'unnecessaryAssertion', line: 1 }], + }, { code: "const a = 'a' as const;", output: "const a = 'a';", From 99a1a0bf7632f4462c3beb1624e4afdea9211624 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Mon, 1 Apr 2024 20:19:12 -0500 Subject: [PATCH 2/7] remove unrelated test, and implement fix --- .../rules/no-unnecessary-type-assertion.ts | 29 +++++++++---------- .../no-unnecessary-type-assertion.test.ts | 5 ---- 2 files changed, 14 insertions(+), 20 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 a73962b9acbd..88b0bc4161da 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -108,23 +108,22 @@ export default createRule({ ); } - function isLiteralVariableDeclarationChangingTypeWithConst( - node: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion, - ): boolean { - /** - * If the type assertion is on a template literal WITH expressions we - * should keep the `const` casting - * @see https://github.com/typescript-eslint/typescript-eslint/issues/8737 - */ - if (node.expression.type === AST_NODE_TYPES.TemplateLiteral) { - return node.expression.expressions.length === 0; - } - + function isSafeConstVariableDeclaration({ + expression, + parent, + }: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion): boolean { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const maybeDeclarationNode = node.parent.parent!; + const maybeDeclarationNode = parent.parent!; return ( maybeDeclarationNode.type === AST_NODE_TYPES.VariableDeclaration && - maybeDeclarationNode.kind === 'const' + maybeDeclarationNode.kind === 'const' && + /** + * If the type assertion is on a template literal WITH expressions we + * should keep the `const` casting + * @see https://github.com/typescript-eslint/typescript-eslint/issues/8737 + */ + (expression.type !== AST_NODE_TYPES.TemplateLiteral || + !expression.expressions.length) ); } @@ -267,7 +266,7 @@ export default createRule({ const typeIsUnchanged = isTypeUnchanged(uncastType, castType); const wouldSameTypeBeInferred = castType.isLiteral() - ? isLiteralVariableDeclarationChangingTypeWithConst(node) + ? isSafeConstVariableDeclaration(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 9c2e6a6dc308..6e14eceedba0 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 @@ -347,11 +347,6 @@ const bar = foo.a as string | undefined | bigint; output: 'const a = `a`;', errors: [{ messageId: 'unnecessaryAssertion', line: 1 }], }, - { - code: 'const a = `a${`b`}` as const;', - output: 'const a = `a${`b`}`;', - errors: [{ messageId: 'unnecessaryAssertion', line: 1 }], - }, { code: "const a = 'a' as const;", output: "const a = 'a';", From a7255dd6835d466a0926d7e549036605d54abdc7 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Mon, 1 Apr 2024 20:21:59 -0500 Subject: [PATCH 3/7] clean up --- .../src/rules/no-unnecessary-type-assertion.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 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 88b0bc4161da..5e315da1ddc6 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -114,16 +114,17 @@ export default createRule({ }: 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' && /** - * If the type assertion is on a template literal WITH expressions we - * should keep the `const` casting + * Even on `const` variable declarations, type assertions on template literals with expressions are sometimes required. * @see https://github.com/typescript-eslint/typescript-eslint/issues/8737 */ - (expression.type !== AST_NODE_TYPES.TemplateLiteral || - !expression.expressions.length) + !isTemplateLiteralWithExpressions ); } From 5c856cd17d07125d1167caf1df886003d8d14c4d Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Mon, 1 Apr 2024 20:35:36 -0500 Subject: [PATCH 4/7] improve name --- .../eslint-plugin/src/rules/no-unnecessary-type-assertion.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 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 5e315da1ddc6..95c96743bc90 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -108,7 +108,7 @@ export default createRule({ ); } - function isSafeConstVariableDeclaration({ + function isImplicitlyNarrowedConstDeclaration({ expression, parent, }: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion): boolean { @@ -267,7 +267,7 @@ export default createRule({ const typeIsUnchanged = isTypeUnchanged(uncastType, castType); const wouldSameTypeBeInferred = castType.isLiteral() - ? isSafeConstVariableDeclaration(node) + ? isImplicitlyNarrowedConstDeclaration(node) : !isConstAssertion(node.typeAnnotation); if (typeIsUnchanged && wouldSameTypeBeInferred) { From e3f52f58f05044c5f08dde90e823d958b114a8c2 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Mon, 1 Apr 2024 20:47:19 -0500 Subject: [PATCH 5/7] clarify name and comment --- .../src/rules/no-unnecessary-type-assertion.ts | 6 +++--- 1 file changed, 3 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 95c96743bc90..7a38d3d47970 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -108,7 +108,7 @@ export default createRule({ ); } - function isImplicitlyNarrowedConstDeclaration({ + function isSafeConstVariableDeclaration({ expression, parent, }: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion): boolean { @@ -121,7 +121,7 @@ export default createRule({ maybeDeclarationNode.type === AST_NODE_TYPES.VariableDeclaration && maybeDeclarationNode.kind === 'const' && /** - * Even on `const` variable declarations, type assertions on template literals with expressions are sometimes required. + * 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 @@ -267,7 +267,7 @@ export default createRule({ const typeIsUnchanged = isTypeUnchanged(uncastType, castType); const wouldSameTypeBeInferred = castType.isLiteral() - ? isImplicitlyNarrowedConstDeclaration(node) + ? isSafeConstVariableDeclaration(node) : !isConstAssertion(node.typeAnnotation); if (typeIsUnchanged && wouldSameTypeBeInferred) { From fc1fb1da6f14a519c39badb2e572e697565a6c71 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Mon, 1 Apr 2024 20:52:29 -0500 Subject: [PATCH 6/7] invert check --- .../eslint-plugin/src/rules/no-unnecessary-type-assertion.ts | 2 +- .../tests/rules/no-unnecessary-type-assertion.test.ts | 4 ++-- 2 files changed, 3 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 7a38d3d47970..80aeaf5d8b45 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -116,7 +116,7 @@ export default createRule({ const maybeDeclarationNode = parent.parent!; const isTemplateLiteralWithExpressions = expression.type === AST_NODE_TYPES.TemplateLiteral && - expression.expressions.length === 0; + expression.expressions.length !== 0; return ( maybeDeclarationNode.type === AST_NODE_TYPES.VariableDeclaration && maybeDeclarationNode.kind === 'const' && 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 6e14eceedba0..1b60d0a4ba14 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 @@ -284,12 +284,12 @@ function bar(items: string[]) { }, // https://github.com/typescript-eslint/typescript-eslint/issues/8737 ` -const myString = 'foo'; +let myString = 'foo'; const templateLiteral = \`\${myString}-somethingElse\` as const; `, // https://github.com/typescript-eslint/typescript-eslint/issues/8737 ` -const myString = 'foo'; +let myString = 'foo'; const templateLiteral = \`\${myString}-somethingElse\`; `, 'let a = `a` as const;', From d8f2fe7c999632e871ed16ae1ef2de0952fa67a1 Mon Sep 17 00:00:00 2001 From: Abraham Guo Date: Mon, 1 Apr 2024 20:57:31 -0500 Subject: [PATCH 7/7] revert name --- .../eslint-plugin/src/rules/no-unnecessary-type-assertion.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 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 80aeaf5d8b45..c01b8f9edb73 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -108,7 +108,7 @@ export default createRule({ ); } - function isSafeConstVariableDeclaration({ + function isImplicitlyNarrowedConstDeclaration({ expression, parent, }: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion): boolean { @@ -267,7 +267,7 @@ export default createRule({ const typeIsUnchanged = isTypeUnchanged(uncastType, castType); const wouldSameTypeBeInferred = castType.isLiteral() - ? isSafeConstVariableDeclaration(node) + ? isImplicitlyNarrowedConstDeclaration(node) : !isConstAssertion(node.typeAnnotation); if (typeIsUnchanged && wouldSameTypeBeInferred) {