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): [restrict-template-expressions] add allowArray option #8389

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c035774
WIP
abrahamguo Sep 29, 2023
4a41e8f
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Feb 6, 2024
f51c879
finish logic
abrahamguo Feb 6, 2024
0116b14
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Feb 12, 2024
37cfa52
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Feb 13, 2024
b7217e7
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Feb 15, 2024
592c036
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Feb 19, 2024
fbd2f30
add doc section
abrahamguo Feb 19, 2024
4ee7087
update snapshot
abrahamguo Feb 19, 2024
035b963
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Feb 23, 2024
bf925e5
refactors and renames
abrahamguo Feb 23, 2024
f56d8e9
simplify type flag testers
abrahamguo Feb 23, 2024
8245146
rename
abrahamguo Feb 23, 2024
bf1a087
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Feb 26, 2024
a6e0c7b
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Feb 26, 2024
f4bb1e8
write tests
abrahamguo Feb 26, 2024
0a60320
simplify test
abrahamguo Feb 26, 2024
d77d075
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Feb 26, 2024
2b03d56
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Mar 7, 2024
20994f5
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Mar 8, 2024
4c375ac
suppress lint error
abrahamguo Mar 8, 2024
9a3c36a
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
abrahamguo Mar 11, 2024
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
13 changes: 11 additions & 2 deletions packages/eslint-plugin/docs/rules/restrict-template-expressions.md
Expand Up @@ -12,10 +12,10 @@ This rule reports on values used in a template literal string that aren't string

:::note

This rule intentionally does not allow objects with a custom `toString()` method to be used in template literals, because the stringification result may not be user-friendly.
The default settings of this rule intentionally do not allow objects with a custom `toString()` method to be used in template literals, because the stringification result may not be user-friendly.

For example, arrays have a custom [`toString()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/toString) method, which only calls `join()` internally, which joins the array elements with commas. This means that (1) array elements are not necessarily stringified to useful results (2) the commas don't have spaces after them, making the result not user-friendly. The best way to format arrays is to use [`Intl.ListFormat`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/ListFormat), which even supports adding the "and" conjunction where necessary.
You must explicitly call `object.toString()` if you want to use this object in a template literal.
You must explicitly call `object.toString()` if you want to use this object in a template literal, or turn on the `allowArray` option to specifically allow arrays.
The [`no-base-to-string`](./no-base-to-string.md) rule can be used to guard this case against producing `"[object Object]"` by accident.

:::
Expand Down Expand Up @@ -111,6 +111,15 @@ const arg = 'something';
const msg1 = typeof arg === 'string' ? arg : `arg = ${arg}`;
```

### `allowArray`

Examples of additional **correct** code for this rule with `{ allowArray: true }`:

```ts option='{ "allowArray": true }' showPlaygroundButton
const arg = ['foo', 'bar'];
const msg1 = `arg = ${arg}`;
```

## When Not To Use It

If you're not worried about incorrectly stringifying non-string values in template literals, then you likely don't need this rule.
Expand Down
138 changes: 45 additions & 93 deletions packages/eslint-plugin/src/rules/restrict-template-expressions.ts
Expand Up @@ -12,16 +12,35 @@
isTypeNeverType,
} from '../util';

type Options = [
{
allowAny?: boolean;
allowBoolean?: boolean;
allowNullish?: boolean;
allowNumber?: boolean;
allowRegExp?: boolean;
allowNever?: boolean;
const optionsObj = {
Any: isTypeAnyType,
Array: (type, checker, rec): boolean => {
if (!checker.isArrayType(type)) {
return false;

Check warning on line 19 in packages/eslint-plugin/src/rules/restrict-template-expressions.ts

View check run for this annotation

Codecov / codecov/patch

packages/eslint-plugin/src/rules/restrict-template-expressions.ts#L19

Added line #L19 was not covered by tests
}
const numberIndexType = type.getNumberIndexType();

Check warning on line 21 in packages/eslint-plugin/src/rules/restrict-template-expressions.ts

View check run for this annotation

Codecov / codecov/patch

packages/eslint-plugin/src/rules/restrict-template-expressions.ts#L21

Added line #L21 was not covered by tests
abrahamguo marked this conversation as resolved.
Show resolved Hide resolved
return !numberIndexType || rec(numberIndexType);
},
];
Boolean: (type): boolean => isTypeFlagSet(type, ts.TypeFlags.BooleanLike),
Nullish: (type): boolean =>
isTypeFlagSet(type, ts.TypeFlags.Null | ts.TypeFlags.Undefined),
Number: (type): boolean =>
isTypeFlagSet(type, ts.TypeFlags.NumberLike | ts.TypeFlags.BigIntLike),
RegExp: (type, checker): boolean => getTypeName(checker, type) === 'RegExp',
Never: isTypeNeverType,
} satisfies Record<
string,
(
type: ts.Type,
checker: ts.TypeChecker,
rec: (type: ts.Type) => boolean,
) => boolean
>;
abrahamguo marked this conversation as resolved.
Show resolved Hide resolved
const optionsArr = Object.entries(optionsObj).map(([_type, fn]) => {
const type = _type as keyof typeof optionsObj;
return { type, option: `allow${type}` as const, fn };
});
abrahamguo marked this conversation as resolved.
Show resolved Hide resolved
type Options = [{ [Type in (typeof optionsArr)[number]['option']]?: boolean }];

type MessageId = 'invalidType';

Expand All @@ -42,38 +61,15 @@
{
type: 'object',
additionalProperties: false,
properties: {
allowAny: {
description:
'Whether to allow `any` typed values in template expressions.',
type: 'boolean',
},
allowBoolean: {
description:
'Whether to allow `boolean` typed values in template expressions.',
type: 'boolean',
},
allowNullish: {
description:
'Whether to allow `nullish` typed values in template expressions.',
type: 'boolean',
},
allowNumber: {
description:
'Whether to allow `number` typed values in template expressions.',
type: 'boolean',
},
allowRegExp: {
description:
'Whether to allow `regexp` typed values in template expressions.',
type: 'boolean',
},
allowNever: {
description:
'Whether to allow `never` typed values in template expressions.',
type: 'boolean',
},
},
properties: Object.fromEntries(
optionsArr.map(({ option, type }) => [
option,
{
description: `Whether to allow \`${type.toLowerCase()}\` typed values in template expressions.`,
type: 'boolean',
},
]),
),
},
],
},
Expand All @@ -90,47 +86,6 @@
const services = getParserServices(context);
const checker = services.program.getTypeChecker();

function isUnderlyingTypePrimitive(type: ts.Type): boolean {
if (isTypeFlagSet(type, ts.TypeFlags.StringLike)) {
return true;
}

if (
options.allowNumber &&
isTypeFlagSet(type, ts.TypeFlags.NumberLike | ts.TypeFlags.BigIntLike)
) {
return true;
}

if (
options.allowBoolean &&
isTypeFlagSet(type, ts.TypeFlags.BooleanLike)
) {
return true;
}

if (options.allowAny && isTypeAnyType(type)) {
return true;
}

if (options.allowRegExp && getTypeName(checker, type) === 'RegExp') {
return true;
}

if (
options.allowNullish &&
isTypeFlagSet(type, ts.TypeFlags.Null | ts.TypeFlags.Undefined)
) {
return true;
}

if (options.allowNever && isTypeNeverType(type)) {
return true;
}

return false;
}

return {
TemplateLiteral(node: TSESTree.TemplateLiteral): void {
// don't check tagged template literals
Expand All @@ -144,12 +99,7 @@
expression,
);

if (
!isInnerUnionOrIntersectionConformingTo(
expressionType,
isUnderlyingTypePrimitive,
)
) {
if (!isInnerUnionOrIntersectionConformingTo(expressionType)) {
context.report({
node: expression,
messageId: 'invalidType',
Expand All @@ -160,10 +110,7 @@
},
};

function isInnerUnionOrIntersectionConformingTo(
type: ts.Type,
predicate: (underlyingType: ts.Type) => boolean,
): boolean {
function isInnerUnionOrIntersectionConformingTo(type: ts.Type): boolean {
return rec(type);

function rec(innerType: ts.Type): boolean {
Expand All @@ -175,7 +122,12 @@
return innerType.types.some(rec);
}

return predicate(innerType);
return (
isTypeFlagSet(innerType, ts.TypeFlags.StringLike) ||
optionsArr.some(
({ option, fn }) => options[option] && fn(innerType, checker, rec),
abrahamguo marked this conversation as resolved.
Show resolved Hide resolved
)
);
}
}
},
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.