Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): [no-useless-template-literals] add new rule #7957

Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 53 additions & 0 deletions packages/eslint-plugin/docs/rules/no-useless-template-literals.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
---
description: 'Disallow unnecessary template literals.'
---

> 🛑 This file is source code, not the primary documentation location! 🛑
>
> See **https://typescript-eslint.io/rules/no-useless-template-literals** for documentation.

This rule reports template literals that can be simplified to a normal string literal.

## Examples

<!--tabs-->
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

### ❌ Incorrect

```ts
const ab1 = `${'a'}${'b'}`;
const ab2 = `a${'b'}`;

const stringWithNumber = `1 + 1 = ${2}`;

const stringWithBoolean = `${'true is '}${true}`;

const string = 'a';
const wrappedString = `${string}`;

declare const intersectionWithString: string & { _brand: 'test-brand' };
const wrappedIntersection = `${intersectionWithString}`;
```

### ✅ Correct

```ts
const ab1 = 'ab';
const ab2 = 'ab';

const stringWithNumber = '1 + 1 = 2';

const stringWithBoolean = 'true is true';

const string = 'a';
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
const wrappedString = string;

declare const intersectionWithString: string & { _brand: 'test-brand' };
const wrappedIntersection = intersectionWithString;
```

<!--/tabs-->

## Related To

- [`restrict-template-expressions`](./restrict-template-expressions.md)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export = {
'no-useless-constructor': 'off',
'@typescript-eslint/no-useless-constructor': 'error',
'@typescript-eslint/no-useless-empty-export': 'error',
'@typescript-eslint/no-useless-template-literals': 'error',
'@typescript-eslint/no-var-requires': 'error',
'@typescript-eslint/non-nullable-type-assertion-style': 'error',
'object-curly-spacing': 'off',
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/disable-type-checked.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export = {
'@typescript-eslint/no-unsafe-enum-comparison': 'off',
'@typescript-eslint/no-unsafe-member-access': 'off',
'@typescript-eslint/no-unsafe-return': 'off',
'@typescript-eslint/no-useless-template-literals': 'off',
'@typescript-eslint/non-nullable-type-assertion-style': 'off',
'@typescript-eslint/prefer-includes': 'off',
'@typescript-eslint/prefer-nullish-coalescing': 'off',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export = {
'@typescript-eslint/no-unsafe-return': 'error',
'no-unused-vars': 'off',
'@typescript-eslint/no-unused-vars': 'error',
'@typescript-eslint/no-useless-template-literals': 'error',
'@typescript-eslint/no-var-requires': 'error',
'@typescript-eslint/prefer-as-const': 'error',
'require-await': 'off',
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/strict-type-checked.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export = {
'@typescript-eslint/no-unused-vars': 'error',
'no-useless-constructor': 'off',
'@typescript-eslint/no-useless-constructor': 'error',
'@typescript-eslint/no-useless-template-literals': 'error',
'@typescript-eslint/no-var-requires': 'error',
'@typescript-eslint/prefer-as-const': 'error',
'@typescript-eslint/prefer-includes': 'error',
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ import noUnusedVars from './no-unused-vars';
import noUseBeforeDefine from './no-use-before-define';
import noUselessConstructor from './no-useless-constructor';
import noUselessEmptyExport from './no-useless-empty-export';
import noUselessTemplateLiterals from './no-useless-template-literals';
import noVarRequires from './no-var-requires';
import nonNullableTypeAssertionStyle from './non-nullable-type-assertion-style';
import objectCurlySpacing from './object-curly-spacing';
Expand Down Expand Up @@ -231,6 +232,7 @@ export default {
'no-use-before-define': noUseBeforeDefine,
'no-useless-constructor': noUselessConstructor,
'no-useless-empty-export': noUselessEmptyExport,
'no-useless-template-literals': noUselessTemplateLiterals,
'no-var-requires': noVarRequires,
'non-nullable-type-assertion-style': nonNullableTypeAssertionStyle,
'object-curly-spacing': objectCurlySpacing,
Expand Down
13 changes: 7 additions & 6 deletions packages/eslint-plugin/src/rules/member-ordering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,8 @@ function getNodeType(node: Member): MemberKind | null {
return node.value && functionExpressions.includes(node.value.type)
? 'method'
: node.readonly
? 'readonly-field'
: 'field';
? 'readonly-field'
: 'field';
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
case AST_NODE_TYPES.TSPropertySignature:
return node.readonly ? 'readonly-field' : 'field';
case AST_NODE_TYPES.TSIndexSignature:
Expand Down Expand Up @@ -555,8 +555,8 @@ function getRank(
'static' in node && node.static
? 'static'
: abstract
? 'abstract'
: 'instance';
? 'abstract'
: 'instance';
const accessibility = getAccessibility(node);

// Collect all existing member groups that apply to this node...
Expand All @@ -578,7 +578,7 @@ function getRank(

if (type === 'readonly-field') {
memberGroups.push(`${accessibility}-decorated-field`);
memberGroups.push(`decorated-field`);
memberGroups.push('decorated-field');
}
}

Expand Down Expand Up @@ -666,7 +666,8 @@ export default createRule<Options, MessageIds>({
'Member {{member}} should be declared before member {{beforeMember}}.',
incorrectGroupOrder:
'Member {{name}} should be declared before all {{rank}} definitions.',
incorrectRequiredMembersOrder: `Member {{member}} should be declared after all {{optionalOrRequired}} members.`,
incorrectRequiredMembersOrder:
'Member {{member}} should be declared after all {{optionalOrRequired}} members.',
},
schema: [
{
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/rules/no-mixed-enums.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default createRule({
requiresTypeChecking: true,
},
messages: {
mixed: `Mixing number and string enums can be confusing.`,
mixed: 'Mixing number and string enums can be confusing.',
},
schema: [],
type: 'problem',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,10 @@ export default createRule({
requiresTypeChecking: true,
},
messages: {
literalOverridden: `{{literal}} is overridden by {{primitive}} in this union type.`,
primitiveOverridden: `{{primitive}} is overridden by the {{literal}} in this intersection type.`,
literalOverridden:
'{{literal}} is overridden by {{primitive}} in this union type.',
primitiveOverridden:
'{{primitive}} is overridden by the {{literal}} in this intersection type.',
overridden: `'{{typeName}}' is overridden by other types in this {{container}} type.`,
overrides: `'{{typeName}}' overrides all other types in this {{container}} type.`,
},
Expand Down
97 changes: 97 additions & 0 deletions packages/eslint-plugin/src/rules/no-useless-template-literals.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import type { TSESTree } from '@typescript-eslint/utils';
import { AST_NODE_TYPES } from '@typescript-eslint/utils';
import * as ts from 'typescript';

import {
createRule,
getConstrainedTypeAtLocation,
getParserServices,
isTypeFlagSet,
} from '../util';

type MessageId = 'noUselessTemplateLiteral';

export default createRule<[], MessageId>({
name: 'no-useless-template-literals',
meta: {
type: 'problem',
docs: {
description: 'Disallow unnecessary template literals',
recommended: 'recommended',
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
requiresTypeChecking: true,
},
messages: {
noUselessTemplateLiteral:
'Template literal expression is unnecessary and can be simplified.',
},
schema: [],
},
defaultOptions: [],
create(context) {
const services = getParserServices(context);

function isUnderlyingTypeString(expression: TSESTree.Expression): boolean {
const type = getConstrainedTypeAtLocation(services, expression);

const isString = (t: ts.Type): boolean => {
return isTypeFlagSet(t, ts.TypeFlags.StringLike);
};

if (type.isUnion()) {
return type.types.every(isString);
}

if (type.isIntersection()) {
return type.types.some(isString);
}

return isString(type);
}

return {
TemplateLiteral(node: TSESTree.TemplateLiteral): void {
if (node.parent.type === AST_NODE_TYPES.TaggedTemplateExpression) {
return;
}

const hasSingleStringVariable =
node.quasis.length === 2 &&
node.quasis[0].value.raw === '' &&
node.quasis[1].value.raw === '' &&
node.expressions.length === 1 &&
node.expressions[0].type === AST_NODE_TYPES.Identifier &&
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
isUnderlyingTypeString(node.expressions[0]);

if (hasSingleStringVariable) {
context.report({
node,
messageId: 'noUselessTemplateLiteral',
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably add a fixer in another PR
will raise a new issue after this one is merged


return;
}

const allowedChars = ['\r', '\n', "'", '"'];

const hasStringWithAllowedChars = node.quasis.some(quasi => {
return new RegExp(`[${allowedChars.join('')}]`).test(quasi.value.raw);
});
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

if (hasStringWithAllowedChars) {
return;
}

const allAreLiterals = node.expressions.every(expression => {
return expression.type === AST_NODE_TYPES.Literal;
});

if (allAreLiterals) {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
context.report({
node,
messageId: 'noUselessTemplateLiteral',
});
}
},
};
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ export default createRule({
requiresTypeChecking: false,
},
messages: {
notLiteral: `Explicit enum value must only be a literal value (string, number, boolean, etc).`,
notLiteral:
'Explicit enum value must only be a literal value (string, number, boolean, etc).',
},
schema: [
{
Expand Down
10 changes: 5 additions & 5 deletions packages/eslint-plugin/tests/docs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,16 @@ describe('Validating rule docs', () => {
test(`${ruleName}.md must next have a blockquote directing to website`, () => {
expect(tokens[2]).toMatchObject({
text: [
`🛑 This file is source code, not the primary documentation location! 🛑`,
``,
'🛑 This file is source code, not the primary documentation location! 🛑',
'',
`See **https://typescript-eslint.io/rules/${ruleName}** for documentation.`,
``,
'',
].join('\n'),
type: 'blockquote',
});
});

test(`headers must be title-cased`, () => {
test('headers must be title-cased', () => {
// Get all H2 headers objects as the other levels are variable by design.
const headers = tokens.filter(tokenIsH2);

Expand Down Expand Up @@ -130,7 +130,7 @@ describe('Validating rule metadata', () => {
}

for (const [ruleName, rule] of rulesData) {
describe(`${ruleName}`, () => {
describe(ruleName, () => {
it('`name` field in rule must match the filename', () => {
// validate if rule name is same as url
// there is no way to access this field but its used only in generation of docs url
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/tests/rules/array-type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2162,14 +2162,14 @@ describe('schema validation', () => {
// https://github.com/typescript-eslint/typescript-eslint/issues/6852
test("array-type does not accept 'simple-array' option", () => {
if (areOptionsValid(rule, [{ default: 'simple-array' }])) {
throw new Error(`Options succeeded validation for bad options`);
throw new Error('Options succeeded validation for bad options');
}
});

// https://github.com/typescript-eslint/typescript-eslint/issues/6892
test('array-type does not accept non object option', () => {
if (areOptionsValid(rule, ['array'])) {
throw new Error(`Options succeeded validation for bad options`);
throw new Error('Options succeeded validation for bad options');
}
});
});
2 changes: 1 addition & 1 deletion packages/eslint-plugin/tests/rules/ban-types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ let baz: object = {};
},
{
code: noFormat`let a: Foo< F >;`,
output: `let a: Foo< T >;`,
output: 'let a: Foo< T >;',
errors: [
{
messageId: 'bannedTypeMessage',
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/tests/rules/comma-spacing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ ruleTester.run('comma-spacing', rule, {
data: { loc: 'before' },
},
{
messageId: `missing`,
messageId: 'missing',
column: 16,
line: 1,
data: { loc: 'after' },
Expand Down