fix(linter): update no-unnecessary-template-expression docs and test case#17453
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Performance ReportMerging #17453 will not alter performanceComparing Summary
Footnotes
|
Merge activity
|
There was a problem hiding this comment.
Pull request overview
This PR updates the no-unnecessary-template-expression linter rule documentation and test fixtures to accurately reflect that the rule targets unnecessary template expressions (interpolations), not simple template literals without expressions. The key clarification is that `hello` (template literal without interpolation) is allowed, while `${text}` (unnecessary wrapping of a string variable) should be flagged.
- Updated rule documentation with clearer explanations and more accurate examples
- Added explicit note that template literals without substitution expressions are allowed
- Updated test fixture from a non-triggering case to a valid case that properly demonstrates the rule
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/oxc_linter/src/rules/typescript/no_unnecessary_template_expression.rs | Rewrote the rule documentation with accurate description, better examples, and clarification about what the rule does and doesn't flag |
| apps/oxlint/fixtures/tsgolint/no-unnecessary-template-expression.ts | Changed test case from `Hello world` (which doesn't trigger the rule) to `${text}` (which correctly triggers the rule) |
| apps/oxlint/src/snapshots/fixtures__tsgolint_--type-aware@oxlint.snap | Updated snapshot to reflect the new test case triggering the rule, increasing error count from 50 to 51 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// const ab1 = `ab`; | ||
| /// | ||
| /// const str2 = '42'; | ||
| /// // Template with non-trivial interpolation |
There was a problem hiding this comment.
The "correct" example on line 48 shows const greeting = Hello ${name}!; which is described as "Template with non-trivial interpolation". However, this example could be misleading because according to the rule's purpose, wrapping a simple string variable in a template literal is unnecessary. The reason this is considered "correct" is because it has additional static content ("Hello " and "!") around the variable, making the template literal necessary for string composition. Consider clarifying the comment to say "Template with interpolation and static content" to better distinguish it from the incorrect example of just wrapping a variable.
| /// // Template with non-trivial interpolation | |
| /// // Template with interpolation and static content |
| @@ -1,3 +1,4 @@ | |||
| const str1 = `Hello world`; | |||
| const text = 'hello'; | |||
| const wrapped = `${text}`; | |||
There was a problem hiding this comment.
Unused variable wrapped.
| const wrapped = `${text}`; |
…case (#17453) Update the rule documentation to match typescript-eslint behavior. The rule catches unnecessary template expressions (interpolations), not simple template literals without expressions. Update test fixture to use a valid test case that triggers the rule.
7679fea to
a38892a
Compare
# Oxlint ### 💥 BREAKING CHANGES - f7da875 oxlint: [**BREAKING**] Remove oxc_language_server binary (#17457) (Boshen) ### 🚀 Features - 659c23e linter: Init note field boilerplate (#17589) (Shrey Sudhir) - 6870b64 parser: Add TS1363 error code (#17609) (Sysix) - 6154c8c linter/eslint-plugin-vitest: Implemented vitest/warn-todo rule (#17228) (Said Atrahouch) - 0043cd6 linter/eslint-plugin-vitest: Implement consistent-vitest-vi rule (#17389) (Said Atrahouch) - a6d773d linter: Add full TS support to eslint/no-useless-constructor (#17592) (camc314) - f02c0e7 linter/eslint: Implement complexity (#17569) (Nguyen Tran) - bc7aae7 linter/no-unused-vars: Add fixer to remove unused catch bindings (#17567) (Don Isaac) - 9e8ec78 linter/only-throw-error rule: Add `allowRethrowing` option for (#17554) (camc314) - b67e819 linter: Add fixer for `unicorn/prefer-response-static-json` rule (#17559) (Mikhail Baev) - 44b0361 linter/vue: Implement no-this-in-before-route-enter (#17525) (yefan) - ee34716 linter/react: Implement no-will-update-set-state (#17530) (Kenzo Wada) - 3088e1d linter/react: Implement no-this-in-sfc (#17535) (Kenzo Wada) - 29a2868 linter/jsx-a11y: Implement no-static-element-interactions (#17538) (Kenzo Wada) - eadf057 linter: Enable tsconfig auto discovery by default (#17489) (Boshen) - 12a7d6e website_linter: Add a count of rules with fixes available to rules table. (#17476) (Connor Shea) ### 🐛 Bug Fixes - a702f13 oxlint/lsp: Correct position for "disable for this file" with shebang (#17613) (Sysix) - 19fdfb6 linter: Panic in `sort-keys` rule with Unicode numeric characters (#17629) (Adel Rodríguez) - 2e8f469 vscode: Search for `node_modules/.bin/oxlint.exe` too (bun setup) (#17597) (Sysix) - be39906 linter/aria-proptypes: Allow template literals with expressions for string-type ARIA props (#17460) (Jökull Sólberg Auðunsson) - 529901c linter: Include JS plugin rules when calculating total rule count (#17520) (connorshea) - 96ef2cc linter: Print total rule # when using a single nested config (#17517) (connorshea) - 9ad0f29 oxlint: Do not enable external plugin store when no external linter is passed (#17498) (Sysix) - 174375d oxfmt,oxlint: Disable mimalloc for 32-bit Arm targets (#17473) (Yaksh Bariya) - ff70fe9 linter/no-standalone-expect: Allows expect in wrapper functions passed to test blocks (#17427) (Copilot) - dab232f linter/catch-or-return: Handle arrow functions with implicit returns correctly (#17440) (Copilot) - a38892a linter: Update no-unnecessary-template-expression docs and test case (#17453) (camc314) ### ⚡ Performance - 605dbf1 vscode: Restrict searching for oxlint/oxfmt binaries only 3 levels deep + 10s timeout (#17345) (Sysix) ### 📚 Documentation - 884fb63 linter/react: Improve docs for jsx-curly-brace-presence (#17579) (connorshea) - 1d3ee07 linter: Improve rule explanation for `vue/no-this-in-before-route-enter`. (#17581) (connorshea) - 5f189f8 linter/arrow-body-style: Correctly document default mode option (#17566) (Rägnar O'ock) - bb2e8e4 linter: Add a note to the `typescript/no-var-requires` rule about the missing `allow` option (#17551) (connorshea) - 655afc1 linter: Improve docs for `import/extensions` and add a few more tests (#17539) (connorshea) - 7e5fc90 linter: Update list of plugins that are reserved. (#17516) (connorshea) # Oxfmt ### 💥 BREAKING CHANGES - f7da875 oxlint: [**BREAKING**] Remove oxc_language_server binary (#17457) (Boshen) ### 🚀 Features - 8fd4ea9 oxfmt: `options.embeddedLanguageFormatting` is now `"auto"` by default (#17649) (leaysgur) ### 🐛 Bug Fixes - c9b5d7d formatter/sort_imports: Handle alignable_comment correctly (#17646) (leaysgur) - 453222d formatter: Missing comment handling for end-of-line comments in member chains (#17659) (Dunqing) - 0805ff2 formatter: Incorrect inline comment placement in try-catch (#17657) (Dunqing) - 3a0c782 formatter: Don't move comments into optional call parentheses (#17582) (magic-akari) - 174375d oxfmt,oxlint: Disable mimalloc for 32-bit Arm targets (#17473) (Yaksh Bariya) ### ⚡ Performance - abb28dc oxfmt: Turn of pretty print from sort-package-json (#17452) (Boshen)

Update the rule documentation to match typescript-eslint behavior.
The rule catches unnecessary template expressions (interpolations),
not simple template literals without expressions.
Update test fixture to use a valid test case that triggers the rule.