From 9bde9b8a2dea53f9855e7532e1edb5bfa9176287 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Sun, 5 Jan 2025 19:29:48 +0900 Subject: [PATCH 01/10] fix(eslint-plugin): handle expressions in template literal type --- .../no-unnecessary-template-expression.ts | 541 +++++++++++------- ...no-unnecessary-template-expression.test.ts | 135 +++++ 2 files changed, 465 insertions(+), 211 deletions(-) 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 8556c51191dc..25ff4ab61fad 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -16,6 +16,15 @@ import { import { rangeToLoc } from '../util/rangeToLoc'; type MessageId = 'noUnnecessaryTemplateExpression'; +type TemplateLiteralTypeOrValue = + | TSESTree.TemplateLiteral + | TSESTree.TSTemplateLiteralType; + +interface InterpolationInfo { + interpolation: TSESTree.Expression | TSESTree.TypeNode; + prevQuasi: TSESTree.TemplateElement; + nextQuasi: TSESTree.TemplateElement; +} const evenNumOfBackslashesRegExp = /(?({ create(context) { const services = getParserServices(context); - function isUnderlyingTypeString( - expression: TSESTree.Expression, - ): expression is TSESTree.Identifier | TSESTree.StringLiteral { + function isUnderlyingTypeString(node: TSESTree.Node): boolean { const checker = services.program.getTypeChecker(); const { constraintType } = getConstraintInfo( checker, - services.getTypeAtLocation(expression), + services.getTypeAtLocation(node), ); if (constraintType == null) { @@ -76,29 +83,41 @@ export default createRule<[], MessageId>({ return isString(constraintType); } - function isLiteral( - expression: TSESTree.Expression, - ): expression is TSESTree.Literal { - return expression.type === AST_NODE_TYPES.Literal; + function isEnumType(node: TSESTree.TypeNode): boolean { + const type = services.getTypeAtLocation(node); + const symbol = type.getSymbol(); + + return !!( + symbol?.valueDeclaration && + ts.isEnumDeclaration(symbol.valueDeclaration) + ); + } + + function isLiteral(node: TSESTree.Node): node is TSESTree.Literal { + return node.type === AST_NODE_TYPES.Literal; } function isTemplateLiteral( - expression: TSESTree.Expression, - ): expression is TSESTree.TemplateLiteral { - return expression.type === AST_NODE_TYPES.TemplateLiteral; + node: TSESTree.Node, + ): node is TSESTree.TemplateLiteral { + return node.type === AST_NODE_TYPES.TemplateLiteral; } - function isInfinityIdentifier(expression: TSESTree.Expression): boolean { + function isInfinityIdentifier(node: TSESTree.Node): boolean { return ( - expression.type === AST_NODE_TYPES.Identifier && - expression.name === 'Infinity' + node.type === AST_NODE_TYPES.Identifier && node.name === 'Infinity' ); } - function isNaNIdentifier(expression: TSESTree.Expression): boolean { + function isNaNIdentifier(node: TSESTree.Node): boolean { + return node.type === AST_NODE_TYPES.Identifier && node.name === 'NaN'; + } + + function isFixableIdentifier(node: TSESTree.Node): boolean { return ( - expression.type === AST_NODE_TYPES.Identifier && - expression.name === 'NaN' + isUndefinedIdentifier(node) || + isInfinityIdentifier(node) || + isNaNIdentifier(node) ); } @@ -118,221 +137,321 @@ export default createRule<[], MessageId>({ return context.sourceCode.commentsExistBetween(startToken, endToken); } - return { - TemplateLiteral(node: TSESTree.TemplateLiteral): void { - if (node.parent.type === AST_NODE_TYPES.TaggedTemplateExpression) { - return; - } + function hasSingleInterpolation( + node: TSESTree.TemplateLiteral | TSESTree.TSTemplateLiteralType, + ) { + return ( + node.quasis.length === 2 && + node.quasis[0].value.raw === '' && + node.quasis[1].value.raw === '' + ); + } - const hasSingleStringVariable = - node.quasis.length === 2 && - node.quasis[0].value.raw === '' && - node.quasis[1].value.raw === '' && - node.expressions.length === 1 && - isUnderlyingTypeString(node.expressions[0]); + function getInterpolations( + node: TemplateLiteralTypeOrValue, + ): TSESTree.Expression[] | TSESTree.TypeNode[] { + if (node.type === AST_NODE_TYPES.TemplateLiteral) { + return node.expressions; + } + return node.types; + } - if (hasSingleStringVariable) { - if (hasCommentsBetweenQuasi(node.quasis[0], node.quasis[1])) { - return; - } + function getInterpolationInfos( + node: TemplateLiteralTypeOrValue, + ): InterpolationInfo[] { + return getInterpolations(node) + .map((interpolation, index) => ({ + interpolation, + nextQuasi: node.quasis[index + 1], + prevQuasi: node.quasis[index], + })) + .reverse(); + } - context.report({ - loc: rangeToLoc(context.sourceCode, [ - node.expressions[0].range[0] - 2, - node.expressions[0].range[1] + 1, - ]), - messageId: 'noUnnecessaryTemplateExpression', - fix(fixer): TSESLint.RuleFix | null { - const wrappingCode = getMovedNodeCode({ - destinationNode: node, - nodeToMove: node.expressions[0], - sourceCode: context.sourceCode, - }); - - return fixer.replaceText(node, wrappingCode); - }, + function getLiteral( + node: TSESTree.Expression | TSESTree.TypeNode, + ): TSESTree.Literal | null { + const maybeLiteral = + node.type === AST_NODE_TYPES.TSLiteralType ? node.literal : node; + return isLiteral(maybeLiteral) ? maybeLiteral : null; + } + + function getTemplateLiteral( + node: TSESTree.Expression | TSESTree.TypeNode, + ): TSESTree.TemplateLiteral | null { + const maybeTemplateLiteral = + node.type === AST_NODE_TYPES.TSLiteralType ? node.literal : node; + return isTemplateLiteral(maybeTemplateLiteral) + ? maybeTemplateLiteral + : null; + } + + function reportSingleInterpolation(node: TemplateLiteralTypeOrValue): void { + const interpolations = getInterpolations(node); + context.report({ + loc: rangeToLoc(context.sourceCode, [ + interpolations[0].range[0] - 2, + interpolations[0].range[1] + 1, + ]), + messageId: 'noUnnecessaryTemplateExpression', + fix(fixer): TSESLint.RuleFix | null { + const wrappingCode = getMovedNodeCode({ + destinationNode: node, + nodeToMove: interpolations[0], + sourceCode: context.sourceCode, }); - return; + return fixer.replaceText(node, wrappingCode); + }, + }); + } + + function isUnncessaryInterpolation({ + interpolation, + nextQuasi, + prevQuasi, + }: InterpolationInfo): boolean { + if (isFixableIdentifier(interpolation)) { + return true; + } + if (hasCommentsBetweenQuasi(prevQuasi, nextQuasi)) { + return false; + } + + if (isLiteral(interpolation)) { + // allow trailing whitespace literal + if (startsWithNewLine(nextQuasi.value.raw)) { + return !( + typeof interpolation.value === 'string' && + isWhitespace(interpolation.value) + ); } + return true; + } - const fixableExpressionsReversed = node.expressions - .map((expression, index) => ({ - expression, - nextQuasi: node.quasis[index + 1], - prevQuasi: node.quasis[index], - })) - .filter(({ expression, nextQuasi, prevQuasi }) => { - if ( - isUndefinedIdentifier(expression) || - isInfinityIdentifier(expression) || - isNaNIdentifier(expression) - ) { - return true; - } - - // allow expressions that include comments - if (hasCommentsBetweenQuasi(prevQuasi, nextQuasi)) { - return false; - } - - if (isLiteral(expression)) { - // allow trailing whitespace literal - if (startsWithNewLine(nextQuasi.value.raw)) { - return !( - typeof expression.value === 'string' && - isWhitespace(expression.value) - ); - } - return true; - } - - if (isTemplateLiteral(expression)) { - // allow trailing whitespace literal - if (startsWithNewLine(nextQuasi.value.raw)) { - return !( - expression.quasis.length === 1 && - isWhitespace(expression.quasis[0].value.raw) - ); - } - return true; - } - - return false; - }) - .reverse(); - - let nextCharacterIsOpeningCurlyBrace = false; - - for (const { - expression, - nextQuasi, - prevQuasi, - } of fixableExpressionsReversed) { - const fixers: ((fixer: TSESLint.RuleFixer) => TSESLint.RuleFix[])[] = - []; - - if (nextQuasi.value.raw !== '') { - nextCharacterIsOpeningCurlyBrace = - nextQuasi.value.raw.startsWith('{'); - } + if (isTemplateLiteral(interpolation)) { + // allow trailing whitespace literal + if (startsWithNewLine(nextQuasi.value.raw)) { + return !( + interpolation.quasis.length === 1 && + isWhitespace(interpolation.quasis[0].value.raw) + ); + } + return true; + } - if (isLiteral(expression)) { - let escapedValue = ( - typeof expression.value === 'string' - ? // The value is already a string, so we're removing quotes: - // "'va`lue'" -> "va`lue" - expression.raw.slice(1, -1) - : // The value may be one of number | bigint | boolean | RegExp | null. - // In regular expressions, we escape every backslash - String(expression.value).replaceAll('\\', '\\\\') - ) - // The string or RegExp may contain ` or ${. - // We want both of these to be escaped in the final template expression. - // - // A pair of backslashes means "escaped backslash", so backslashes - // from this pair won't escape ` or ${. Therefore, to escape these - // sequences in the resulting template expression, we need to escape - // all sequences that are preceded by an even number of backslashes. - // - // This RegExp does the following transformations: - // \` -> \` - // \\` -> \\\` - // \${ -> \${ - // \\${ -> \\\${ - .replaceAll( - new RegExp( - `${String(evenNumOfBackslashesRegExp.source)}(\`|\\\${)`, - 'g', - ), - '\\$1', - ); - - // `...${'...$'}{...` - // ^^^^ - if ( - nextCharacterIsOpeningCurlyBrace && - endsWithUnescapedDollarSign(escapedValue) - ) { - escapedValue = escapedValue.replaceAll(/\$$/g, '\\$'); - } - - if (escapedValue.length !== 0) { - nextCharacterIsOpeningCurlyBrace = escapedValue.startsWith('{'); - } - - fixers.push(fixer => [fixer.replaceText(expression, escapedValue)]); - } else if (isTemplateLiteral(expression)) { - // Since we iterate from the last expression to the first, - // a subsequent expression can tell the current expression - // that it starts with {. + return false; + } + + function isUnncessaryTypeInterpolatione({ + interpolation, + nextQuasi, + prevQuasi, + }: InterpolationInfo): boolean { + if (hasCommentsBetweenQuasi(prevQuasi, nextQuasi)) { + return false; + } + + const literal = getLiteral(interpolation); + if (literal) { + // allow trailing whitespace literal + if (startsWithNewLine(nextQuasi.value.raw)) { + return !( + typeof literal.value === 'string' && isWhitespace(literal.value) + ); + } + return true; + } + + const templateLiteral = getTemplateLiteral(interpolation); + if (templateLiteral) { + // allow trailing whitespace literal + if (startsWithNewLine(nextQuasi.value.raw)) { + return !( + templateLiteral.quasis.length === 1 && + isWhitespace(templateLiteral.quasis[0].value.raw) + ); + } + return true; + } + + return false; + } + + function report(infos: InterpolationInfo[]): void { + let nextCharacterIsOpeningCurlyBrace = false; + + for (const { interpolation, nextQuasi, prevQuasi } of infos) { + const fixers: ((fixer: TSESLint.RuleFixer) => TSESLint.RuleFix[])[] = + []; + + if (nextQuasi.value.raw !== '') { + nextCharacterIsOpeningCurlyBrace = + nextQuasi.value.raw.startsWith('{'); + } + + const literal = getLiteral(interpolation); + const templateLiteral = getTemplateLiteral(interpolation); + if (literal) { + let escapedValue = ( + typeof literal.value === 'string' + ? // The value is already a string, so we're removing quotes: + // "'va`lue'" -> "va`lue" + literal.raw.slice(1, -1) + : // The value may be one of number | bigint | boolean | RegExp | null. + // In regular expressions, we escape every backslash + String(literal.value).replaceAll('\\', '\\\\') + ) + // The string or RegExp may contain ` or ${. + // We want both of these to be escaped in the final template expression. // - // `... ${`... $`}${'{...'} ...` - // ^ ^ subsequent expression starts with { - // current expression ends with a dollar sign, - // so '$' + '{' === '${' (bad news for us). - // Let's escape the dollar sign at the end. - if ( - nextCharacterIsOpeningCurlyBrace && - endsWithUnescapedDollarSign( - expression.quasis[expression.quasis.length - 1].value.raw, - ) - ) { - fixers.push(fixer => [ - fixer.replaceTextRange( - [expression.range[1] - 2, expression.range[1] - 2], - '\\', - ), - ]); - } - if ( - expression.quasis.length === 1 && - expression.quasis[0].value.raw.length !== 0 - ) { - nextCharacterIsOpeningCurlyBrace = - expression.quasis[0].value.raw.startsWith('{'); - } - - // Remove the beginning and trailing backtick characters. - fixers.push(fixer => [ - fixer.removeRange([expression.range[0], expression.range[0] + 1]), - fixer.removeRange([expression.range[1] - 1, expression.range[1]]), - ]); - } else { - nextCharacterIsOpeningCurlyBrace = false; + // A pair of backslashes means "escaped backslash", so backslashes + // from this pair won't escape ` or ${. Therefore, to escape these + // sequences in the resulting template expression, we need to escape + // all sequences that are preceded by an even number of backslashes. + // + // This RegExp does the following transformations: + // \` -> \` + // \\` -> \\\` + // \${ -> \${ + // \\${ -> \\\${ + .replaceAll( + new RegExp( + `${String(evenNumOfBackslashesRegExp.source)}(\`|\\\${)`, + 'g', + ), + '\\$1', + ); + + // `...${'...$'}{...` + // ^^^^ + if ( + nextCharacterIsOpeningCurlyBrace && + endsWithUnescapedDollarSign(escapedValue) + ) { + escapedValue = escapedValue.replaceAll(/\$$/g, '\\$'); + } + + if (escapedValue.length !== 0) { + nextCharacterIsOpeningCurlyBrace = escapedValue.startsWith('{'); } - // `... $${'{...'} ...` - // ^^^^^ + fixers.push(fixer => [fixer.replaceText(literal, escapedValue)]); + } else if (templateLiteral) { + // Since we iterate from the last expression to the first, + // a subsequent expression can tell the current expression + // that it starts with {. + // + // `... ${`... $`}${'{...'} ...` + // ^ ^ subsequent expression starts with { + // current expression ends with a dollar sign, + // so '$' + '{' === '${' (bad news for us). + // Let's escape the dollar sign at the end. if ( nextCharacterIsOpeningCurlyBrace && - endsWithUnescapedDollarSign(prevQuasi.value.raw) + endsWithUnescapedDollarSign( + templateLiteral.quasis[templateLiteral.quasis.length - 1].value + .raw, + ) ) { fixers.push(fixer => [ fixer.replaceTextRange( - [prevQuasi.range[1] - 3, prevQuasi.range[1] - 2], - '\\$', + [templateLiteral.range[1] - 2, templateLiteral.range[1] - 2], + '\\', ), ]); } + if ( + templateLiteral.quasis.length === 1 && + templateLiteral.quasis[0].value.raw.length !== 0 + ) { + nextCharacterIsOpeningCurlyBrace = + templateLiteral.quasis[0].value.raw.startsWith('{'); + } - const warnLocStart = prevQuasi.range[1] - 2; - const warnLocEnd = nextQuasi.range[0] + 1; - - context.report({ - loc: rangeToLoc(context.sourceCode, [warnLocStart, warnLocEnd]), - messageId: 'noUnnecessaryTemplateExpression', - fix(fixer): TSESLint.RuleFix[] { - return [ - // Remove the quasis' parts that are related to the current expression. - fixer.removeRange([warnLocStart, expression.range[0]]), - fixer.removeRange([expression.range[1], warnLocEnd]), - - ...fixers.flatMap(cb => cb(fixer)), - ]; - }, - }); + // Remove the beginning and trailing backtick characters. + fixers.push(fixer => [ + fixer.removeRange([ + templateLiteral.range[0], + templateLiteral.range[0] + 1, + ]), + fixer.removeRange([ + templateLiteral.range[1] - 1, + templateLiteral.range[1], + ]), + ]); + } else { + nextCharacterIsOpeningCurlyBrace = false; + } + + // `... $${'{...'} ...` + // ^^^^^ + if ( + nextCharacterIsOpeningCurlyBrace && + endsWithUnescapedDollarSign(prevQuasi.value.raw) + ) { + fixers.push(fixer => [ + fixer.replaceTextRange( + [prevQuasi.range[1] - 3, prevQuasi.range[1] - 2], + '\\$', + ), + ]); + } + + const warnLocStart = prevQuasi.range[1] - 2; + const warnLocEnd = nextQuasi.range[0] + 1; + + context.report({ + loc: rangeToLoc(context.sourceCode, [warnLocStart, warnLocEnd]), + messageId: 'noUnnecessaryTemplateExpression', + fix(fixer): TSESLint.RuleFix[] { + return [ + // Remove the quasis' parts that are related to the current expression. + fixer.removeRange([warnLocStart, interpolation.range[0]]), + fixer.removeRange([interpolation.range[1], warnLocEnd]), + + ...fixers.flatMap(cb => cb(fixer)), + ]; + }, + }); + } + } + + return { + TemplateLiteral(node: TSESTree.TemplateLiteral): void { + if (node.parent.type === AST_NODE_TYPES.TaggedTemplateExpression) { + return; } + if ( + hasSingleInterpolation(node) && + !hasCommentsBetweenQuasi(node.quasis[0], node.quasis[1]) && + isUnderlyingTypeString(node.expressions[0]) + ) { + reportSingleInterpolation(node); + return; + } + + const infos = getInterpolationInfos(node).filter( + isUnncessaryInterpolation, + ); + + report(infos); + }, + TSTemplateLiteralType(node: TSESTree.TSTemplateLiteralType): void { + if ( + hasSingleInterpolation(node) && + !hasCommentsBetweenQuasi(node.quasis[0], node.quasis[1]) && + isUnderlyingTypeString(node.types[0]) && + !isEnumType(node.types[0]) + ) { + reportSingleInterpolation(node); + return; + } + const infos = getInterpolationInfos(node).filter( + isUnncessaryTypeInterpolatione, + ); + + report(infos); }, }; }, diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts index eb87d7f3755d..3bfeba77c9ca 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts @@ -1,6 +1,10 @@ +import type { ParserServicesWithTypeInformation } from '@typescript-eslint/parser'; import type { InvalidTestCase } from '@typescript-eslint/rule-tester'; +import type { TSESTree } from '@typescript-eslint/utils'; +import { parseForESLint } from '@typescript-eslint/parser'; import { noFormat, RuleTester } from '@typescript-eslint/rule-tester'; +import path from 'node:path'; import rule from '../../src/rules/no-unnecessary-template-expression'; import { getFixturesRootDir } from '../RuleTester'; @@ -972,6 +976,104 @@ describe('fixer should not change runtime value', () => { } }); +const invalidTypeCases: readonly InvalidTestCase< + 'noUnnecessaryTemplateExpression', + [] +>[] = [ + { + code: 'type Foo = `${1}`;', + errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], + output: 'type Foo = `1`;', + }, + { + code: 'type Foo = `${"foo"}`;', + errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], + output: 'type Foo = "foo";', + }, + { + code: ` +type Foo = 'A' | 'B'; +type Bar = \`\${Foo}\`; +`, + errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], + output: ` +type Foo = 'A' | 'B'; +type Bar = Foo; +`, + }, + { + code: ` +type Foo = 'A' | 'B'; +type Bar = \`\${\`\${Foo}\`}\`; +`, + errors: [ + { messageId: 'noUnnecessaryTemplateExpression' }, + { messageId: 'noUnnecessaryTemplateExpression' }, + ], + output: [ + ` +type Foo = 'A' | 'B'; +type Bar = \`\${Foo}\`; +`, + + ` +type Foo = 'A' | 'B'; +type Bar = Foo; +`, + ], + }, +]; + +describe('fixer should not change type', () => { + const options = { + filePath: path.join(rootPath, 'file.ts'), + project: './tsconfig.json', + tsconfigRootDir: rootPath, + }; + + for (const { code, output } of invalidTypeCases) { + if (!output) { + continue; + } + + test(code, () => { + const { ast: inputAst, services: inputServices } = parseForESLint( + code, + options, + ); + + const lastOutput = Array.isArray(output) ? output.at(-1)! : output; + const { ast: outputAst, services: outputServices } = parseForESLint( + lastOutput, + options, + ); + + const inputDeclaration = inputAst.body.at( + -1, + ) as TSESTree.TSTypeAliasDeclaration; + + const outputDeclaration = outputAst.body.at( + -1, + ) as TSESTree.TSTypeAliasDeclaration; + + const inputType = ( + inputServices as ParserServicesWithTypeInformation + ).getTypeAtLocation(inputDeclaration.id); + + const outputType = ( + outputServices as ParserServicesWithTypeInformation + ).getTypeAtLocation(outputDeclaration.id); + + const inputChecker = inputServices.program!.getTypeChecker(); + const outputChecker = outputServices.program!.getTypeChecker(); + + expect(inputChecker.typeToString(inputType)).toBe( + outputChecker.typeToString(outputType), + ); + }); + } +}); + ruleTester.run('no-unnecessary-template-expression', rule, { valid: [ "const string = 'a';", @@ -1147,10 +1249,25 @@ this code has trailing whitespace: \${' '} return \`\${input}\`; } `, + ` +type FooBarBaz = \`foo\${/* comment */ 'bar'}"baz"\`; + `, + ` +enum Foo { + A = 'A', + B = 'B', +} +type Foos = \`\${Foo}\`; + `, + ` +type Foo = 'A' | 'B'; +type Bar = \`foo\${Foo}foo\`; + `, ], invalid: [ ...invalidCases, + ...invalidTypeCases, { code: ` function func(arg: T) { @@ -1264,5 +1381,23 @@ declare const nested: string, interpolation: string; ], output: "true ? ('test' || '').trim() : undefined;", }, + { + code: "type Foo = `${'foo'}`;", + errors: [ + { + messageId: 'noUnnecessaryTemplateExpression', + }, + ], + output: "type Foo = 'foo';", + }, + { + code: "type FooBarBaz = `foo${'bar'}baz`;", + errors: [ + { + messageId: 'noUnnecessaryTemplateExpression', + }, + ], + output: 'type FooBarBaz = `foobarbaz`;', + }, ], }); From d7bfc3c946aaf15d01581991a210e07167e8e91b Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Jan 2025 22:27:37 +0900 Subject: [PATCH 02/10] Update no-unnecessary-template-expression.test.ts --- ...no-unnecessary-template-expression.test.ts | 185 ++++++++---------- 1 file changed, 83 insertions(+), 102 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts index 3bfeba77c9ca..d9711a2e6cf6 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts @@ -8,6 +8,7 @@ import path from 'node:path'; import rule from '../../src/rules/no-unnecessary-template-expression'; import { getFixturesRootDir } from '../RuleTester'; +import { TypeFormatFlags } from 'typescript'; const rootPath = getFixturesRootDir(); @@ -976,104 +977,6 @@ describe('fixer should not change runtime value', () => { } }); -const invalidTypeCases: readonly InvalidTestCase< - 'noUnnecessaryTemplateExpression', - [] ->[] = [ - { - code: 'type Foo = `${1}`;', - errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], - output: 'type Foo = `1`;', - }, - { - code: 'type Foo = `${"foo"}`;', - errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], - output: 'type Foo = "foo";', - }, - { - code: ` -type Foo = 'A' | 'B'; -type Bar = \`\${Foo}\`; -`, - errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], - output: ` -type Foo = 'A' | 'B'; -type Bar = Foo; -`, - }, - { - code: ` -type Foo = 'A' | 'B'; -type Bar = \`\${\`\${Foo}\`}\`; -`, - errors: [ - { messageId: 'noUnnecessaryTemplateExpression' }, - { messageId: 'noUnnecessaryTemplateExpression' }, - ], - output: [ - ` -type Foo = 'A' | 'B'; -type Bar = \`\${Foo}\`; -`, - - ` -type Foo = 'A' | 'B'; -type Bar = Foo; -`, - ], - }, -]; - -describe('fixer should not change type', () => { - const options = { - filePath: path.join(rootPath, 'file.ts'), - project: './tsconfig.json', - tsconfigRootDir: rootPath, - }; - - for (const { code, output } of invalidTypeCases) { - if (!output) { - continue; - } - - test(code, () => { - const { ast: inputAst, services: inputServices } = parseForESLint( - code, - options, - ); - - const lastOutput = Array.isArray(output) ? output.at(-1)! : output; - const { ast: outputAst, services: outputServices } = parseForESLint( - lastOutput, - options, - ); - - const inputDeclaration = inputAst.body.at( - -1, - ) as TSESTree.TSTypeAliasDeclaration; - - const outputDeclaration = outputAst.body.at( - -1, - ) as TSESTree.TSTypeAliasDeclaration; - - const inputType = ( - inputServices as ParserServicesWithTypeInformation - ).getTypeAtLocation(inputDeclaration.id); - - const outputType = ( - outputServices as ParserServicesWithTypeInformation - ).getTypeAtLocation(outputDeclaration.id); - - const inputChecker = inputServices.program!.getTypeChecker(); - const outputChecker = outputServices.program!.getTypeChecker(); - - expect(inputChecker.typeToString(inputType)).toBe( - outputChecker.typeToString(outputType), - ); - }); - } -}); - ruleTester.run('no-unnecessary-template-expression', rule, { valid: [ "const string = 'a';", @@ -1263,11 +1166,21 @@ type Foos = \`\${Foo}\`; type Foo = 'A' | 'B'; type Bar = \`foo\${Foo}foo\`; `, + ` +type Foo = + \`trailing position interpolated empty string also makes whitespace clear \${''} +\`; + `, + "type Foo = `${'foo' | 'bar' | null}`;", + + ` +type StringOrNumber = string | number; +type Foo = \`\${StringOrNumber}\`; + `, ], invalid: [ ...invalidCases, - ...invalidTypeCases, { code: ` function func(arg: T) { @@ -1381,23 +1294,91 @@ declare const nested: string, interpolation: string; ], output: "true ? ('test' || '').trim() : undefined;", }, + { + code: 'type Foo = `${1}`;', + errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], + output: 'type Foo = `1`;', + }, { code: "type Foo = `${'foo'}`;", + errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], + output: "type Foo = 'foo';", + }, + { + code: ` +type Foo = 'A' | 'B'; +type Bar = \`\${Foo}\`; + `, + errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], + output: ` +type Foo = 'A' | 'B'; +type Bar = Foo; + `, + }, + { + code: ` +type Foo = 'A' | 'B'; +type Bar = \`\${\`\${Foo}\`}\`; + `, + errors: [ + { messageId: 'noUnnecessaryTemplateExpression' }, + { messageId: 'noUnnecessaryTemplateExpression' }, + ], + output: [ + ` +type Foo = 'A' | 'B'; +type Bar = \`\${Foo}\`; + `, + + ` +type Foo = 'A' | 'B'; +type Bar = Foo; + `, + ], + }, + { + code: "type FooBarBaz = `foo${'bar'}baz`;", errors: [ { messageId: 'noUnnecessaryTemplateExpression', }, ], - output: "type Foo = 'foo';", + output: 'type FooBarBaz = `foobarbaz`;', }, { - code: "type FooBarBaz = `foo${'bar'}baz`;", + code: 'type FooBar = `foo${`bar`}`;', errors: [ { messageId: 'noUnnecessaryTemplateExpression', }, ], - output: 'type FooBarBaz = `foobarbaz`;', + output: 'type FooBar = `foobar`;', + }, + { + code: "type FooBar = `${'foo' | 'bar'}`;", + errors: [ + { + messageId: 'noUnnecessaryTemplateExpression', + }, + ], + output: "type FooBar = 'foo' | 'bar';", + }, + { + code: ` +function foo() { + const a: \`\${T}\` = 'a'; +} + `, + errors: [ + { + messageId: 'noUnnecessaryTemplateExpression', + }, + ], + output: ` +function foo() { + const a: T = 'a'; +} + `, }, ], }); From 556b4838cae6df3c0d9e0c91d4d3394f41605415 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Jan 2025 22:38:08 +0900 Subject: [PATCH 03/10] add docs --- .../no-unnecessary-template-expression.mdx | 19 +++++++++++++++++++ ...no-unnecessary-template-expression.test.ts | 5 ----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-template-expression.mdx b/packages/eslint-plugin/docs/rules/no-unnecessary-template-expression.mdx index 1fe39d412dd1..c720652ebf7a 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-template-expression.mdx +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-template-expression.mdx @@ -28,6 +28,8 @@ The new name is a drop-in replacement with identical functionality. const ab1 = `${'a'}${'b'}`; const ab2 = `a${'b'}`; +type AB1 = `${'A'}${'B'}`; +type AB2 = `A${'B'}`; const stringWithNumber = `${'1 + 1 = '}${2}`; @@ -38,9 +40,13 @@ const stringWithBoolean = `${'true is '}${true}`; const text = 'a'; const wrappedText = `${text}`; +type Text = 'A'; +type WrappedText = `${Text}`; declare const intersectionWithString: string & { _brand: 'test-brand' }; const wrappedIntersection = `${intersectionWithString}`; +type IntersectionWithString = string & { _brand: 'test-brand' }; +type WrappedIntersection = `${IntersectionWithString}`; ``` @@ -51,6 +57,15 @@ const wrappedIntersection = `${intersectionWithString}`; const ab1 = `ab`; const ab2 = `ab`; +type AB = `AB`; + +// Transforming enum members into string unions using template literals is allowed. +enum ABC { + A = 'A', + B = 'B', + C = 'C', +} +type ABCUnion = `${ABC}`; const stringWithNumber = `1 + 1 = 2`; @@ -61,9 +76,13 @@ const stringWithBoolean = `true is true`; const text = 'a'; const wrappedText = text; +type Text = 'A'; +type WrappedText = Text; declare const intersectionWithString: string & { _brand: 'test-brand' }; const wrappedIntersection = intersectionWithString; +type IntersectionWithString = string & { _brand: 'test-brand' }; +type WrappedIntersection = IntersectionWithString; ``` diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts index d9711a2e6cf6..6f5d8bddc1f5 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts @@ -1,14 +1,9 @@ -import type { ParserServicesWithTypeInformation } from '@typescript-eslint/parser'; import type { InvalidTestCase } from '@typescript-eslint/rule-tester'; -import type { TSESTree } from '@typescript-eslint/utils'; -import { parseForESLint } from '@typescript-eslint/parser'; import { noFormat, RuleTester } from '@typescript-eslint/rule-tester'; -import path from 'node:path'; import rule from '../../src/rules/no-unnecessary-template-expression'; import { getFixturesRootDir } from '../RuleTester'; -import { TypeFormatFlags } from 'typescript'; const rootPath = getFixturesRootDir(); From d16be87f2cdf0da2864f114f793b70a2fe06ae5a Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Jan 2025 22:39:50 +0900 Subject: [PATCH 04/10] update snapshots --- .../no-unnecessary-template-expression.shot | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-template-expression.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-template-expression.shot index 47aa863d7203..0065786d6ef9 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-template-expression.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unnecessary-template-expression.shot @@ -10,6 +10,11 @@ const ab1 = \`\${'a'}\${'b'}\`; ~~~~~~ Template literal expression is unnecessary and can be simplified. const ab2 = \`a\${'b'}\`; ~~~~~~ Template literal expression is unnecessary and can be simplified. +type AB1 = \`\${'A'}\${'B'}\`; + ~~~~~~ Template literal expression is unnecessary and can be simplified. + ~~~~~~ Template literal expression is unnecessary and can be simplified. +type AB2 = \`A\${'B'}\`; + ~~~~~~ Template literal expression is unnecessary and can be simplified. const stringWithNumber = \`\${'1 + 1 = '}\${2}\`; ~~~~~~~~~~~~~ Template literal expression is unnecessary and can be simplified. @@ -25,10 +30,16 @@ const stringWithBoolean = \`\${'true is '}\${true}\`; const text = 'a'; const wrappedText = \`\${text}\`; ~~~~~~~ Template literal expression is unnecessary and can be simplified. +type Text = 'A'; +type WrappedText = \`\${Text}\`; + ~~~~~~~ Template literal expression is unnecessary and can be simplified. declare const intersectionWithString: string & { _brand: 'test-brand' }; const wrappedIntersection = \`\${intersectionWithString}\`; ~~~~~~~~~~~~~~~~~~~~~~~~~ Template literal expression is unnecessary and can be simplified. +type IntersectionWithString = string & { _brand: 'test-brand' }; +type WrappedIntersection = \`\${IntersectionWithString}\`; + ~~~~~~~~~~~~~~~~~~~~~~~~~ Template literal expression is unnecessary and can be simplified. " `; @@ -39,6 +50,15 @@ exports[`Validating rule docs no-unnecessary-template-expression.mdx code exampl const ab1 = \`ab\`; const ab2 = \`ab\`; +type AB = \`AB\`; + +// Transforming enum members into string unions using template literals is allowed. +enum ABC { + A = 'A', + B = 'B', + C = 'C', +} +type ABCUnion = \`\${ABC}\`; const stringWithNumber = \`1 + 1 = 2\`; @@ -49,8 +69,12 @@ const stringWithBoolean = \`true is true\`; const text = 'a'; const wrappedText = text; +type Text = 'A'; +type WrappedText = Text; declare const intersectionWithString: string & { _brand: 'test-brand' }; const wrappedIntersection = intersectionWithString; +type IntersectionWithString = string & { _brand: 'test-brand' }; +type WrappedIntersection = IntersectionWithString; " `; From 2ae997b72faa0d1a83237119185dfeabf7eadf6f Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Jan 2025 23:31:19 +0900 Subject: [PATCH 05/10] handle undefined & null --- .../src/rules/no-unnecessary-template-expression.ts | 7 +++++++ .../rules/no-unnecessary-template-expression.test.ts | 10 ++++++++++ 2 files changed, 17 insertions(+) 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 25ff4ab61fad..64a9bb19ad3c 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -263,6 +263,13 @@ export default createRule<[], MessageId>({ return true; } + if ( + interpolation.type === AST_NODE_TYPES.TSNullKeyword || + interpolation.type === AST_NODE_TYPES.TSUndefinedKeyword + ) { + return true; + } + const templateLiteral = getTemplateLiteral(interpolation); if (templateLiteral) { // allow trailing whitespace literal diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts index 6f5d8bddc1f5..ede19dd5dc07 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts @@ -1294,6 +1294,16 @@ declare const nested: string, interpolation: string; errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], output: 'type Foo = `1`;', }, + { + code: 'type Foo = `${null}`;', + errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], + output: 'type Foo = `null`;', + }, + { + code: 'type Foo = `${undefined}`;', + errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], + output: 'type Foo = `undefined`;', + }, { code: "type Foo = `${'foo'}`;", errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], From c9e1fe8120cb972d512b3ab641db3d240e2c3249 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 6 Jan 2025 23:37:55 +0900 Subject: [PATCH 06/10] add tests --- ...no-unnecessary-template-expression.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts index ede19dd5dc07..6c7e69f14563 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts @@ -1172,6 +1172,13 @@ type Foo = type StringOrNumber = string | number; type Foo = \`\${StringOrNumber}\`; `, + ` +enum Foo { + A = 1, + B = 2, +} +type Bar = \`\${Foo.A}\`; + `, ], invalid: [ @@ -1370,6 +1377,27 @@ type Bar = Foo; }, { code: ` +enum Foo { + A = 'A', + B = 'B', +} +type Bar = \`\${Foo.A}\`; + `, + errors: [ + { + messageId: 'noUnnecessaryTemplateExpression', + }, + ], + output: ` +enum Foo { + A = 'A', + B = 'B', +} +type Bar = Foo.A; + `, + }, + { + code: ` function foo() { const a: \`\${T}\` = 'a'; } From 435bb86c652b5a27b670985a6ec6746e82a517b2 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Tue, 7 Jan 2025 00:02:24 +0900 Subject: [PATCH 07/10] add tests --- .../rules/no-unnecessary-template-expression.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts index 6c7e69f14563..5370beb4447b 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts @@ -1097,6 +1097,11 @@ ruleTester.run('no-unnecessary-template-expression', rule, { ` \` this code has trailing whitespace: \${' '} + \`; + `, + ` +\` +this code has trailing whitespace: \${\` \`} \`; `, noFormat` @@ -1165,6 +1170,9 @@ type Bar = \`foo\${Foo}foo\`; type Foo = \`trailing position interpolated empty string also makes whitespace clear \${''} \`; + `, + noFormat` +type Foo = \`this code has trailing whitespace with a windows \\\r new line: \${\` \`}\r\n\`; `, "type Foo = `${'foo' | 'bar' | null}`;", From de4d42834f879c950321c98bc6924b8d0502be7c Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 20 Jan 2025 20:38:16 +0900 Subject: [PATCH 08/10] apply review --- .../no-unnecessary-template-expression.ts | 119 ++++++++++-------- ...no-unnecessary-template-expression.test.ts | 3 + 2 files changed, 68 insertions(+), 54 deletions(-) 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 02e7782d876b..d720e3b0a85e 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -57,35 +57,25 @@ export default createRule<[], MessageId>({ defaultOptions: [], create(context) { const services = getParserServices(context); + const checker = services.program.getTypeChecker(); - function isUnderlyingTypeString(node: TSESTree.Node): boolean { - const checker = services.program.getTypeChecker(); - const { constraintType } = getConstraintInfo( - checker, - services.getTypeAtLocation(node), - ); - - if (constraintType == null) { - return false; - } - - const isString = (t: ts.Type): boolean => { - return isTypeFlagSet(t, ts.TypeFlags.StringLike); - }; + function isStringLike(type: ts.Type): boolean { + return isTypeFlagSet(type, ts.TypeFlags.StringLike); + } - if (constraintType.isUnion()) { - return constraintType.types.every(isString); + function isUnderlyingTypeString(type: ts.Type): boolean { + if (type.isUnion()) { + return type.types.every(isStringLike); } - if (constraintType.isIntersection()) { - return constraintType.types.some(isString); + if (type.isIntersection()) { + return type.types.some(isStringLike); } - return isString(constraintType); + return isStringLike(type); } - function isEnumType(node: TSESTree.TypeNode): boolean { - const type = services.getTypeAtLocation(node); + function isEnumType(type: ts.Type): boolean { const symbol = type.getSymbol(); return !!( @@ -138,7 +128,7 @@ export default createRule<[], MessageId>({ return context.sourceCode.commentsExistBetween(startToken, endToken); } - function hasSingleInterpolation( + function isTrivialInterpolation( node: TSESTree.TemplateLiteral | TSESTree.TSTemplateLiteralType, ) { return ( @@ -160,13 +150,11 @@ export default createRule<[], MessageId>({ function getInterpolationInfos( node: TemplateLiteralTypeOrValue, ): InterpolationInfo[] { - return getInterpolations(node) - .map((interpolation, index) => ({ - interpolation, - nextQuasi: node.quasis[index + 1], - prevQuasi: node.quasis[index], - })) - .reverse(); + return getInterpolations(node).map((interpolation, index) => ({ + interpolation, + nextQuasi: node.quasis[index + 1], + prevQuasi: node.quasis[index], + })); } function getLiteral( @@ -207,18 +195,19 @@ export default createRule<[], MessageId>({ }); } - function isUnncessaryInterpolation({ + function isUnncessaryValueInterpolation({ interpolation, nextQuasi, prevQuasi, }: InterpolationInfo): boolean { - if (isFixableIdentifier(interpolation)) { - return true; - } if (hasCommentsBetweenQuasi(prevQuasi, nextQuasi)) { return false; } + if (isFixableIdentifier(interpolation)) { + return true; + } + if (isLiteral(interpolation)) { // allow trailing whitespace literal if (startsWithNewLine(nextQuasi.value.raw)) { @@ -244,7 +233,7 @@ export default createRule<[], MessageId>({ return false; } - function isUnncessaryTypeInterpolatione({ + function isUnncessaryTypeInterpolation({ interpolation, nextQuasi, prevQuasi, @@ -286,10 +275,13 @@ export default createRule<[], MessageId>({ return false; } - function report(infos: InterpolationInfo[]): void { + function getReportDescriptors( + infos: InterpolationInfo[], + ): TSESLint.ReportDescriptor[] { let nextCharacterIsOpeningCurlyBrace = false; - - for (const { interpolation, nextQuasi, prevQuasi } of infos) { + const reportDescriptors: TSESLint.ReportDescriptor[] = []; + const reversedInfos = [...infos].reverse(); + for (const { interpolation, nextQuasi, prevQuasi } of reversedInfos) { const fixers: ((fixer: TSESLint.RuleFixer) => TSESLint.RuleFix[])[] = []; @@ -408,8 +400,7 @@ export default createRule<[], MessageId>({ const warnLocStart = prevQuasi.range[1] - 2; const warnLocEnd = nextQuasi.range[0] + 1; - - context.report({ + reportDescriptors.push({ loc: rangeToLoc(context.sourceCode, [warnLocStart, warnLocEnd]), messageId: 'noUnnecessaryTemplateExpression', fix(fixer): TSESLint.RuleFix[] { @@ -423,6 +414,7 @@ export default createRule<[], MessageId>({ }, }); } + return reportDescriptors; } return { @@ -431,35 +423,54 @@ export default createRule<[], MessageId>({ return; } if ( - hasSingleInterpolation(node) && - !hasCommentsBetweenQuasi(node.quasis[0], node.quasis[1]) && - isUnderlyingTypeString(node.expressions[0]) + isTrivialInterpolation(node) && + !hasCommentsBetweenQuasi(node.quasis[0], node.quasis[1]) ) { - reportSingleInterpolation(node); - return; + const { constraintType } = getConstraintInfo( + checker, + services.getTypeAtLocation(node.expressions[0]), + ); + if (constraintType && isUnderlyingTypeString(constraintType)) { + reportSingleInterpolation(node); + return; + } } const infos = getInterpolationInfos(node).filter( - isUnncessaryInterpolation, + isUnncessaryValueInterpolation, ); - report(infos); + for (const reportDescriptor of getReportDescriptors(infos)) { + context.report(reportDescriptor); + } }, TSTemplateLiteralType(node: TSESTree.TSTemplateLiteralType): void { if ( - hasSingleInterpolation(node) && - !hasCommentsBetweenQuasi(node.quasis[0], node.quasis[1]) && - isUnderlyingTypeString(node.types[0]) && - !isEnumType(node.types[0]) + isTrivialInterpolation(node) && + !hasCommentsBetweenQuasi(node.quasis[0], node.quasis[1]) ) { - reportSingleInterpolation(node); - return; + const { constraintType } = getConstraintInfo( + checker, + services.getTypeAtLocation(node.types[0]), + ); + + if ( + constraintType && + isUnderlyingTypeString(constraintType) && + !isEnumType(constraintType) + ) { + reportSingleInterpolation(node); + return; + } } + const infos = getInterpolationInfos(node).filter( - isUnncessaryTypeInterpolatione, + isUnncessaryTypeInterpolation, ); - report(infos); + for (const reportDescriptor of getReportDescriptors(infos)) { + context.report(reportDescriptor); + } }, }; }, diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts index 5370beb4447b..14f093bddf42 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts @@ -976,6 +976,9 @@ ruleTester.run('no-unnecessary-template-expression', rule, { valid: [ "const string = 'a';", 'const string = `a`;', + 'const string = `NaN: ${/* comment */ NaN}`;', + 'const string = `undefined: ${/* comment */ undefined}`;', + 'const string = `Infinity: ${Infinity /* comment */}`;', ` declare const string: 'a'; \`\${string}b\`; From 256040fd5a5169e7652b840a3083134ceba32ebc Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Thu, 23 Jan 2025 20:28:30 +0900 Subject: [PATCH 09/10] apply reviews --- .../no-unnecessary-template-expression.ts | 9 +++-- ...no-unnecessary-template-expression.test.ts | 34 +++++++++++++++++-- 2 files changed, 35 insertions(+), 8 deletions(-) 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 d720e3b0a85e..f287fcb721aa 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -1,6 +1,6 @@ -import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import type { TSESLint } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as ts from 'typescript'; import { @@ -8,6 +8,7 @@ import { getConstraintInfo, getMovedNodeCode, getParserServices, + isNodeOfType, isTypeFlagSet, isUndefinedIdentifier, nullThrows, @@ -84,9 +85,7 @@ export default createRule<[], MessageId>({ ); } - function isLiteral(node: TSESTree.Node): node is TSESTree.Literal { - return node.type === AST_NODE_TYPES.Literal; - } + const isLiteral = isNodeOfType(TSESTree.AST_NODE_TYPES.Literal); function isTemplateLiteral( node: TSESTree.Node, diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts index 14f093bddf42..160c9114f2aa 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-template-expression.test.ts @@ -1332,7 +1332,15 @@ declare const nested: string, interpolation: string; type Foo = 'A' | 'B'; type Bar = \`\${Foo}\`; `, - errors: [{ messageId: 'noUnnecessaryTemplateExpression' }], + errors: [ + { + column: 13, + endColumn: 19, + endLine: 3, + line: 3, + messageId: 'noUnnecessaryTemplateExpression', + }, + ], output: ` type Foo = 'A' | 'B'; type Bar = Foo; @@ -1344,8 +1352,20 @@ type Foo = 'A' | 'B'; type Bar = \`\${\`\${Foo}\`}\`; `, errors: [ - { messageId: 'noUnnecessaryTemplateExpression' }, - { messageId: 'noUnnecessaryTemplateExpression' }, + { + column: 13, + endColumn: 24, + endLine: 3, + line: 3, + messageId: 'noUnnecessaryTemplateExpression', + }, + { + column: 16, + endColumn: 22, + endLine: 3, + line: 3, + messageId: 'noUnnecessaryTemplateExpression', + }, ], output: [ ` @@ -1396,6 +1416,10 @@ type Bar = \`\${Foo.A}\`; `, errors: [ { + column: 13, + endColumn: 21, + endLine: 6, + line: 6, messageId: 'noUnnecessaryTemplateExpression', }, ], @@ -1415,6 +1439,10 @@ function foo() { `, errors: [ { + column: 13, + endColumn: 17, + endLine: 3, + line: 3, messageId: 'noUnnecessaryTemplateExpression', }, ], From 14beb1ae27ec0f55601540f5235eac8e9200cb1e Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Fri, 24 Jan 2025 19:31:09 +0900 Subject: [PATCH 10/10] add comments --- .../src/rules/no-unnecessary-template-expression.ts | 3 +++ 1 file changed, 3 insertions(+) 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 f287fcb721aa..308855686d79 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -76,6 +76,9 @@ export default createRule<[], MessageId>({ return isStringLike(type); } + /** + * Checks for whole enum types, i.e. `MyEnum`, and not their values, i.e. `MyEnum.A` + */ function isEnumType(type: ts.Type): boolean { const symbol = type.getSymbol();