From e0b6a37157c04e761a597d62a5c0f642e41f7356 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 2 Dec 2024 22:02:50 +0200 Subject: [PATCH 1/7] initial implementation --- .../no-unnecessary-template-expression.ts | 30 ++++++++++++++++- ...no-unnecessary-template-expression.test.ts | 33 ++++++++++++++----- 2 files changed, 54 insertions(+), 9 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 1eb00c2872ba..0efb4dc33aaa 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -92,6 +92,26 @@ export default createRule<[], MessageId>({ ); } + function hasCommentsAfterOffset(offset: number): boolean { + const token = context.sourceCode.getTokenByRangeStart(offset); + + if (token) { + return context.sourceCode.getCommentsAfter(token).length > 0; + } + + return false; + } + + function hasCommentsBeforeOffset(offset: number): boolean { + const token = context.sourceCode.getTokenByRangeStart(offset); + + if (token) { + return context.sourceCode.getCommentsBefore(token).length > 0; + } + + return false; + } + return { TemplateLiteral(node: TSESTree.TemplateLiteral): void { if (node.parent.type === AST_NODE_TYPES.TaggedTemplateExpression) { @@ -132,7 +152,7 @@ export default createRule<[], MessageId>({ nextQuasi: node.quasis[index + 1], prevQuasi: node.quasis[index], })) - .filter(({ expression, nextQuasi }) => { + .filter(({ expression, nextQuasi, prevQuasi }) => { if ( isUndefinedIdentifier(expression) || isInfinityIdentifier(expression) || @@ -141,6 +161,14 @@ export default createRule<[], MessageId>({ return true; } + // allow expressions that include comments + if ( + hasCommentsAfterOffset(prevQuasi.range[0]) || + hasCommentsBeforeOffset(nextQuasi.range[0]) + ) { + return false; + } + if (isLiteral(expression)) { // allow trailing whitespace literal if (startsWithNewLine(nextQuasi.value.raw)) { 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 d178ddd95f6a..c9b8550f7f39 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 @@ -310,10 +310,6 @@ const invalidCases: readonly InvalidTestCase< }\`; `, errors: [ - { - line: 2, - messageId: 'noUnnecessaryTemplateExpression', - }, { column: 2, endColumn: 2, @@ -331,12 +327,18 @@ const invalidCases: readonly InvalidTestCase< ], output: [ ` -\`use\${ - \`less\` -}\`; +\`u\${ + // hopefully this comment is not needed. + 'se' + +}le\${ \`ss\` }\`; `, ` -\`useless\`; +\`u\${ + // hopefully this comment is not needed. + 'se' + +}less\`; `, ], }, @@ -1104,6 +1106,21 @@ this code has trailing whitespace: \${' '} \`trailing position interpolated empty string also makes whitespace clear \${''} \`; `, + ` +\` +\${/* intentional comment before */ 'bar'} +...\`; + `, + ` +\` +\${'bar' /* intentional comment after */} +...\`; + `, + ` +\` +\${/* intentional comment before */ 'bar' /* intentional comment after */} +...\`; + `, ], invalid: [ From fe8c81f627c56b83ea4c0f5c8a46f52f6f088f31 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 2 Dec 2024 22:10:47 +0200 Subject: [PATCH 2/7] refactor and handle the case of a single string variable --- .../no-unnecessary-template-expression.ts | 35 +++++++++---------- ...no-unnecessary-template-expression.test.ts | 3 ++ 2 files changed, 19 insertions(+), 19 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 0efb4dc33aaa..5423d6ca737f 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -92,21 +92,15 @@ export default createRule<[], MessageId>({ ); } - function hasCommentsAfterOffset(offset: number): boolean { - const token = context.sourceCode.getTokenByRangeStart(offset); - - if (token) { - return context.sourceCode.getCommentsAfter(token).length > 0; - } - - return false; - } - - function hasCommentsBeforeOffset(offset: number): boolean { - const token = context.sourceCode.getTokenByRangeStart(offset); - - if (token) { - return context.sourceCode.getCommentsBefore(token).length > 0; + function hasCommentsBetween( + startOffset: number, + endOffset: number, + ): boolean { + const startToken = context.sourceCode.getTokenByRangeStart(startOffset); + const endToken = context.sourceCode.getTokenByRangeStart(endOffset); + + if (startToken && endToken) { + return context.sourceCode.commentsExistBetween(startToken, endToken); } return false; @@ -126,6 +120,12 @@ export default createRule<[], MessageId>({ isUnderlyingTypeString(node.expressions[0]); if (hasSingleStringVariable) { + if ( + hasCommentsBetween(node.quasis[0].range[0], node.quasis[1].range[0]) + ) { + return; + } + context.report({ loc: rangeToLoc(context.sourceCode, [ node.expressions[0].range[0] - 2, @@ -162,10 +162,7 @@ export default createRule<[], MessageId>({ } // allow expressions that include comments - if ( - hasCommentsAfterOffset(prevQuasi.range[0]) || - hasCommentsBeforeOffset(nextQuasi.range[0]) - ) { + if (hasCommentsBetween(prevQuasi.range[0], nextQuasi.range[0])) { return false; } 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 c9b8550f7f39..8454ca045e5f 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 @@ -1121,6 +1121,9 @@ this code has trailing whitespace: \${' '} \${/* intentional comment before */ 'bar' /* intentional comment after */} ...\`; `, + ` +\`\${'bar' /* intentional comment */}\`; + `, ], invalid: [ From eaa7eae55ca501ae50df6dd37ea4e9ecb8cc60dd Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 2 Dec 2024 22:12:36 +0200 Subject: [PATCH 3/7] refactor hasCommentsBetween => hasCommentsBetweenQuasi --- .../no-unnecessary-template-expression.ts | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 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 5423d6ca737f..0448e12d0317 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -92,12 +92,16 @@ export default createRule<[], MessageId>({ ); } - function hasCommentsBetween( - startOffset: number, - endOffset: number, + function hasCommentsBetweenQuasi( + startQuasi: TSESTree.TemplateElement, + endQuasi: TSESTree.TemplateElement, ): boolean { - const startToken = context.sourceCode.getTokenByRangeStart(startOffset); - const endToken = context.sourceCode.getTokenByRangeStart(endOffset); + const startToken = context.sourceCode.getTokenByRangeStart( + startQuasi.range[0], + ); + const endToken = context.sourceCode.getTokenByRangeStart( + endQuasi.range[0], + ); if (startToken && endToken) { return context.sourceCode.commentsExistBetween(startToken, endToken); @@ -120,9 +124,7 @@ export default createRule<[], MessageId>({ isUnderlyingTypeString(node.expressions[0]); if (hasSingleStringVariable) { - if ( - hasCommentsBetween(node.quasis[0].range[0], node.quasis[1].range[0]) - ) { + if (hasCommentsBetweenQuasi(node.quasis[0], node.quasis[1])) { return; } @@ -162,7 +164,7 @@ export default createRule<[], MessageId>({ } // allow expressions that include comments - if (hasCommentsBetween(prevQuasi.range[0], nextQuasi.range[0])) { + if (hasCommentsBetweenQuasi(prevQuasi, nextQuasi)) { return false; } From 5cc8485fe710a19889f3a5bef6ee6bb5fa2f8c65 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 2 Dec 2024 22:13:39 +0200 Subject: [PATCH 4/7] add an istanbul-ignore-next-comment to what seems like an unreachable line --- .../src/rules/no-unnecessary-template-expression.ts | 1 + 1 file changed, 1 insertion(+) 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 0448e12d0317..3ddf51cba0d9 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -107,6 +107,7 @@ export default createRule<[], MessageId>({ return context.sourceCode.commentsExistBetween(startToken, endToken); } + /* istanbul ignore next */ return false; } From cfcb4a08f6f4d29436a48bebbc1ab63e207e793d Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 2 Dec 2024 22:38:53 +0200 Subject: [PATCH 5/7] remove istanbul-ignore-next comment and use nullThrows instead --- .../no-unnecessary-template-expression.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 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 3ddf51cba0d9..90ad42fb0169 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-template-expression.ts @@ -10,6 +10,8 @@ import { getParserServices, isTypeFlagSet, isUndefinedIdentifier, + nullThrows, + NullThrowsReasons, } from '../util'; import { rangeToLoc } from '../util/rangeToLoc'; @@ -96,19 +98,16 @@ export default createRule<[], MessageId>({ startQuasi: TSESTree.TemplateElement, endQuasi: TSESTree.TemplateElement, ): boolean { - const startToken = context.sourceCode.getTokenByRangeStart( - startQuasi.range[0], + const startToken = nullThrows( + context.sourceCode.getTokenByRangeStart(startQuasi.range[0]), + NullThrowsReasons.MissingToken('`${', 'opening template literal'), ); - const endToken = context.sourceCode.getTokenByRangeStart( - endQuasi.range[0], + const endToken = nullThrows( + context.sourceCode.getTokenByRangeStart(endQuasi.range[0]), + NullThrowsReasons.MissingToken('}', 'closing template literal'), ); - if (startToken && endToken) { - return context.sourceCode.commentsExistBetween(startToken, endToken); - } - - /* istanbul ignore next */ - return false; + return context.sourceCode.commentsExistBetween(startToken, endToken); } return { From fead0e09c7f5508ebc717d8f8b359e7054bcba38 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 2 Dec 2024 22:57:24 +0200 Subject: [PATCH 6/7] missing tests --- .../rules/no-unnecessary-template-expression.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 8454ca045e5f..67cdc8a924d8 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 @@ -1122,7 +1122,13 @@ this code has trailing whitespace: \${' '} ...\`; `, ` -\`\${'bar' /* intentional comment */}\`; +\`\${/* intentional before */ 'bar'}\`; + `, + ` +\`\${'bar' /* intentional comment after */}\`; + `, + ` +\`\${/* intentional comment before */ 'bar' /* intentional comment after */}\`; `, ], From 869b2ae26be66c15db00b29ca66e7935c93e05e5 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Tue, 3 Dec 2024 19:01:05 +0200 Subject: [PATCH 7/7] test line comments, not just block comments --- .../rules/no-unnecessary-template-expression.test.ts | 12 ++++++++++++ 1 file changed, 12 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 67cdc8a924d8..6043b2b8b50d 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 @@ -1130,6 +1130,18 @@ this code has trailing whitespace: \${' '} ` \`\${/* intentional comment before */ 'bar' /* intentional comment after */}\`; `, + ` +\`\${ + // intentional comment before + 'bar' +}\`; + `, + ` +\`\${ + 'bar' + // intentional comment after +}\`; + `, ], invalid: [