Skip to content

Commit def09f8

Browse files
JoshuaKGoldbergbradzacher
JoshuaKGoldberg
and
bradzacher
authoredJun 15, 2023
feat(eslint-plugin): [restrict-plus-operands] add allow* options (#6161)
* feat(eslint-plugin): [restrict-plus-operands] add allow* options from restrict-template-expressions * Warn explicitly * Update packages/eslint-plugin/docs/rules/restrict-plus-operands.md Co-authored-by: Brad Zacher <[email protected]> * Brad suggestion * Fix lint-markdown --------- Co-authored-by: Brad Zacher <[email protected]>
·
v8.27.0v5.60.0
1 parent a91bb9e commit def09f8

File tree

4 files changed

+975
-340
lines changed

4 files changed

+975
-340
lines changed
 

‎packages/eslint-plugin/docs/rules/restrict-plus-operands.md

+135-25
Original file line numberDiff line numberDiff line change
@@ -18,70 +18,180 @@ This rule reports when a `+` operation combines two values of different types, o
1818
### ❌ Incorrect
1919

2020
```ts
21-
var foo = '5.5' + 5;
22-
var foo = 1n + 1;
21+
let foo = '5.5' + 5;
22+
let foo = 1n + 1;
2323
```
2424

2525
### ✅ Correct
2626

2727
```ts
28-
var foo = parseInt('5.5', 10) + 10;
29-
var foo = 1n + 1n;
28+
let foo = parseInt('5.5', 10) + 10;
29+
let foo = 1n + 1n;
3030
```
3131

3232
## Options
3333

34-
### `checkCompoundAssignments`
34+
:::caution
35+
We generally recommend against using these options, as they limit which varieties of incorrect `+` usage can be checked.
36+
This in turn severely limits the validation that the rule can do to ensure that resulting strings and numbers are correct.
37+
38+
Safer alternatives to using the `allow*` options include:
39+
40+
- Using variadic forms of logging APIs to avoid needing to `+` values.
41+
```ts
42+
// Remove this line
43+
console.log('The result is ' + true);
44+
// Add this line
45+
console.log('The result is', true);
46+
```
47+
- Using `.toFixed()` to coerce numbers to well-formed string representations:
48+
```ts
49+
const number = 1.123456789;
50+
const result = 'The number is ' + number.toFixed(2);
51+
// result === 'The number is 1.12'
52+
```
53+
- Calling `.toString()` on other types to mark explicit and intentional string coercion:
54+
```ts
55+
const arg = '11';
56+
const regex = /[0-9]/;
57+
const result =
58+
'The result of ' +
59+
regex.toString() +
60+
'.test("' +
61+
arg +
62+
'") is ' +
63+
regex.test(arg).toString();
64+
// result === 'The result of /[0-9]/.test("11") is true'
65+
```
66+
67+
:::
3568

36-
Examples of code for this rule with `{ checkCompoundAssignments: true }`:
69+
### `allowAny`
70+
71+
Examples of code for this rule with `{ allowAny: true }`:
3772

3873
<!--tabs-->
3974

4075
#### ❌ Incorrect
4176

4277
```ts
43-
/*eslint @typescript-eslint/restrict-plus-operands: ["error", { "checkCompoundAssignments": true }]*/
78+
let fn = (a: number, b: []) => a + b;
79+
let fn = (a: string, b: []) => a + b;
80+
```
4481

45-
let foo: string | undefined;
46-
foo += 'some data';
82+
#### ✅ Correct
4783

48-
let bar: string = '';
49-
bar += 0;
84+
```ts
85+
let fn = (a: number, b: any) => a + b;
86+
let fn = (a: string, b: any) => a + b;
87+
```
88+
89+
### `allowBoolean`
90+
91+
Examples of code for this rule with `{ allowBoolean: true }`:
92+
93+
<!--tabs-->
94+
95+
#### ❌ Incorrect
96+
97+
```ts
98+
let fn = (a: number, b: unknown) => a + b;
99+
let fn = (a: string, b: unknown) => a + b;
50100
```
51101

52102
#### ✅ Correct
53103

54104
```ts
55-
/*eslint @typescript-eslint/restrict-plus-operands: ["error", { "checkCompoundAssignments": true }]*/
105+
let fn = (a: number, b: boolean) => a + b;
106+
let fn = (a: string, b: boolean) => a + b;
107+
```
56108

57-
let foo: number = 0;
58-
foo += 1;
109+
### `allowNullish`
59110

60-
let bar = '';
61-
bar += 'test';
111+
Examples of code for this rule with `{ allowNullish: true }`:
112+
113+
<!--tabs-->
114+
115+
#### ❌ Incorrect
116+
117+
```ts
118+
let fn = (a: number, b: unknown) => a + b;
119+
let fn = (a: number, b: never) => a + b;
120+
let fn = (a: string, b: unknown) => a + b;
121+
let fn = (a: string, b: never) => a + b;
62122
```
63123

64-
### `allowAny`
124+
#### ✅ Correct
65125

66-
Examples of code for this rule with `{ allowAny: true }`:
126+
```ts
127+
let fn = (a: number, b: undefined) => a + b;
128+
let fn = (a: number, b: null) => a + b;
129+
let fn = (a: string, b: undefined) => a + b;
130+
let fn = (a: string, b: null) => a + b;
131+
```
132+
133+
### `allowNumberAndString`
134+
135+
Examples of code for this rule with `{ allowNumberAndString: true }`:
67136

68137
<!--tabs-->
69138

70139
#### ❌ Incorrect
71140

72141
```ts
73-
var fn = (a: any, b: boolean) => a + b;
74-
var fn = (a: any, b: []) => a + b;
75-
var fn = (a: any, b: {}) => a + b;
142+
let fn = (a: number, b: unknown) => a + b;
143+
let fn = (a: number, b: never) => a + b;
76144
```
77145

78146
#### ✅ Correct
79147

80148
```ts
81-
var fn = (a: any, b: any) => a + b;
82-
var fn = (a: any, b: string) => a + b;
83-
var fn = (a: any, b: bigint) => a + b;
84-
var fn = (a: any, b: number) => a + b;
149+
let fn = (a: number, b: string) => a + b;
150+
let fn = (a: number, b: number | string) => a + b;
151+
```
152+
153+
### `allowRegExp`
154+
155+
Examples of code for this rule with `{ allowRegExp: true }`:
156+
157+
<!--tabs-->
158+
159+
#### ❌ Incorrect
160+
161+
```ts
162+
let fn = (a: number, b: RegExp) => a + b;
163+
```
164+
165+
#### ✅ Correct
166+
167+
```ts
168+
let fn = (a: string, b: RegExp) => a + b;
169+
```
170+
171+
### `checkCompoundAssignments`
172+
173+
Examples of code for this rule with `{ checkCompoundAssignments: true }`:
174+
175+
<!--tabs-->
176+
177+
#### ❌ Incorrect
178+
179+
```ts
180+
let foo: string | undefined;
181+
foo += 'some data';
182+
183+
let bar: string = '';
184+
bar += 0;
185+
```
186+
187+
#### ✅ Correct
188+
189+
```ts
190+
let foo: number = 0;
191+
foo += 1;
192+
193+
let bar = '';
194+
bar += 'test';
85195
```
86196

87197
## When Not To Use It

‎packages/eslint-plugin/src/rules/restrict-plus-operands.ts

+177-126
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
11
import type { TSESTree } from '@typescript-eslint/utils';
2+
import * as tsutils from 'tsutils';
23
import * as ts from 'typescript';
34

45
import * as util from '../util';
56

67
type Options = [
78
{
8-
checkCompoundAssignments?: boolean;
99
allowAny?: boolean;
10+
allowBoolean?: boolean;
11+
allowNullish?: boolean;
12+
allowNumberAndString?: boolean;
13+
allowRegExp?: boolean;
14+
checkCompoundAssignments?: boolean;
1015
},
1116
];
12-
type MessageIds =
13-
| 'notNumbers'
14-
| 'notStrings'
15-
| 'notBigInts'
16-
| 'notValidAnys'
17-
| 'notValidTypes';
17+
18+
type MessageIds = 'bigintAndNumber' | 'invalid' | 'mismatched';
1819

1920
export default util.createRule<Options, MessageIds>({
2021
name: 'restrict-plus-operands',
@@ -27,166 +28,204 @@ export default util.createRule<Options, MessageIds>({
2728
requiresTypeChecking: true,
2829
},
2930
messages: {
30-
notNumbers:
31-
"Operands of '+' operation must either be both strings or both numbers.",
32-
notStrings:
33-
"Operands of '+' operation must either be both strings or both numbers. Consider using a template literal.",
34-
notBigInts: "Operands of '+' operation must be both bigints.",
35-
notValidAnys:
36-
"Operands of '+' operation with any is possible only with string, number, bigint or any",
37-
notValidTypes:
38-
"Operands of '+' operation must either be one of string, number, bigint or any (if allowed by option)",
31+
bigintAndNumber:
32+
"Numeric '+' operations must either be both bigints or both numbers. Got `{{left}}` + `{{right}}`.",
33+
invalid:
34+
"Invalid operand for a '+' operation. Operands must each be a number or {{stringLike}}. Got `{{type}}`.",
35+
mismatched:
36+
"Operands of '+' operations must be a number or {{stringLike}}. Got `{{left}}` + `{{right}}`.",
3937
},
4038
schema: [
4139
{
4240
type: 'object',
4341
additionalProperties: false,
4442
properties: {
45-
checkCompoundAssignments: {
46-
description: 'Whether to check compound assignments such as `+=`.',
47-
type: 'boolean',
48-
},
4943
allowAny: {
5044
description: 'Whether to allow `any` typed values.',
5145
type: 'boolean',
5246
},
47+
allowBoolean: {
48+
description: 'Whether to allow `boolean` typed values.',
49+
type: 'boolean',
50+
},
51+
allowNullish: {
52+
description:
53+
'Whether to allow potentially `null` or `undefined` typed values.',
54+
type: 'boolean',
55+
},
56+
allowNumberAndString: {
57+
description:
58+
'Whether to allow `bigint`/`number` typed values and `string` typed values to be added together.',
59+
type: 'boolean',
60+
},
61+
allowRegExp: {
62+
description: 'Whether to allow `regexp` typed values.',
63+
type: 'boolean',
64+
},
65+
checkCompoundAssignments: {
66+
description: 'Whether to check compound assignments such as `+=`.',
67+
type: 'boolean',
68+
},
5369
},
5470
},
5571
],
5672
},
5773
defaultOptions: [
5874
{
5975
checkCompoundAssignments: false,
60-
allowAny: false,
6176
},
6277
],
63-
create(context, [{ checkCompoundAssignments, allowAny }]) {
78+
create(
79+
context,
80+
[
81+
{
82+
checkCompoundAssignments,
83+
allowAny,
84+
allowBoolean,
85+
allowNullish,
86+
allowNumberAndString,
87+
allowRegExp,
88+
},
89+
],
90+
) {
6491
const service = util.getParserServices(context);
6592
const typeChecker = service.program.getTypeChecker();
6693

67-
type BaseLiteral = 'string' | 'number' | 'bigint' | 'invalid' | 'any';
68-
69-
/**
70-
* Helper function to get base type of node
71-
*/
72-
function getBaseTypeOfLiteralType(type: ts.Type): BaseLiteral {
73-
if (type.isNumberLiteral()) {
74-
return 'number';
75-
}
76-
if (
77-
type.isStringLiteral() ||
78-
util.isTypeFlagSet(type, ts.TypeFlags.TemplateLiteral)
79-
) {
80-
return 'string';
81-
}
82-
// is BigIntLiteral
83-
if (type.flags & ts.TypeFlags.BigIntLiteral) {
84-
return 'bigint';
85-
}
86-
if (type.isUnion()) {
87-
const types = type.types.map(getBaseTypeOfLiteralType);
88-
89-
return types.every(value => value === types[0]) ? types[0] : 'invalid';
90-
}
91-
92-
if (type.isIntersection()) {
93-
const types = type.types.map(getBaseTypeOfLiteralType);
94-
95-
if (types.some(value => value === 'string')) {
96-
return 'string';
97-
}
98-
99-
if (types.some(value => value === 'number')) {
100-
return 'number';
101-
}
102-
103-
if (types.some(value => value === 'bigint')) {
104-
return 'bigint';
105-
}
106-
107-
return 'invalid';
108-
}
94+
const stringLikes = [
95+
allowAny && '`any`',
96+
allowBoolean && '`boolean`',
97+
allowNullish && '`null`',
98+
allowRegExp && '`RegExp`',
99+
allowNullish && '`undefined`',
100+
].filter((value): value is string => typeof value === 'string');
101+
const stringLike = stringLikes.length
102+
? stringLikes.length === 1
103+
? `string, allowing a string + ${stringLikes[0]}`
104+
: `string, allowing a string + any of: ${stringLikes.join(', ')}`
105+
: 'string';
106+
107+
function getTypeConstrained(node: TSESTree.Node): ts.Type {
108+
return typeChecker.getBaseTypeOfLiteralType(
109+
util.getConstrainedTypeAtLocation(
110+
typeChecker,
111+
service.esTreeNodeToTSNodeMap.get(node),
112+
),
113+
);
114+
}
109115

110-
const stringType = typeChecker.typeToString(type);
116+
function checkPlusOperands(
117+
node: TSESTree.AssignmentExpression | TSESTree.BinaryExpression,
118+
): void {
119+
const leftType = getTypeConstrained(node.left);
120+
const rightType = getTypeConstrained(node.right);
111121

112122
if (
113-
stringType === 'number' ||
114-
stringType === 'string' ||
115-
stringType === 'bigint' ||
116-
stringType === 'any'
123+
leftType === rightType &&
124+
tsutils.isTypeFlagSet(
125+
leftType,
126+
ts.TypeFlags.BigIntLike |
127+
ts.TypeFlags.NumberLike |
128+
ts.TypeFlags.StringLike,
129+
)
117130
) {
118-
return stringType;
131+
return;
119132
}
120-
return 'invalid';
121-
}
122-
123-
/**
124-
* Helper function to get base type of node
125-
* @param node the node to be evaluated.
126-
*/
127-
function getNodeType(
128-
node: TSESTree.Expression | TSESTree.PrivateIdentifier,
129-
): BaseLiteral {
130-
const tsNode = service.esTreeNodeToTSNodeMap.get(node);
131-
const type = util.getConstrainedTypeAtLocation(typeChecker, tsNode);
132-
133-
return getBaseTypeOfLiteralType(type);
134-
}
135133

136-
function checkPlusOperands(
137-
node: TSESTree.BinaryExpression | TSESTree.AssignmentExpression,
138-
): void {
139-
const leftType = getNodeType(node.left);
140-
const rightType = getNodeType(node.right);
141-
142-
if (leftType === rightType) {
143-
if (leftType === 'invalid') {
134+
let hadIndividualComplaint = false;
135+
136+
for (const [baseNode, baseType, otherType] of [
137+
[node.left, leftType, rightType],
138+
[node.right, rightType, leftType],
139+
] as const) {
140+
if (
141+
isTypeFlagSetInUnion(
142+
baseType,
143+
ts.TypeFlags.ESSymbolLike |
144+
ts.TypeFlags.Never |
145+
ts.TypeFlags.Unknown,
146+
) ||
147+
(!allowAny && isTypeFlagSetInUnion(baseType, ts.TypeFlags.Any)) ||
148+
(!allowBoolean &&
149+
isTypeFlagSetInUnion(baseType, ts.TypeFlags.BooleanLike)) ||
150+
(!allowNullish &&
151+
util.isTypeFlagSet(
152+
baseType,
153+
ts.TypeFlags.Null | ts.TypeFlags.Undefined,
154+
))
155+
) {
144156
context.report({
145-
node,
146-
messageId: 'notValidTypes',
157+
data: {
158+
stringLike,
159+
type: typeChecker.typeToString(baseType),
160+
},
161+
messageId: 'invalid',
162+
node: baseNode,
147163
});
164+
hadIndividualComplaint = true;
165+
continue;
148166
}
149167

150-
if (!allowAny && leftType === 'any') {
151-
context.report({
152-
node,
153-
messageId: 'notValidAnys',
154-
});
168+
// RegExps also contain ts.TypeFlags.Any & ts.TypeFlags.Object
169+
for (const subBaseType of tsutils.unionTypeParts(baseType)) {
170+
const typeName = util.getTypeName(typeChecker, subBaseType);
171+
if (
172+
typeName === 'RegExp'
173+
? !allowRegExp ||
174+
tsutils.isTypeFlagSet(otherType, ts.TypeFlags.NumberLike)
175+
: (!allowAny && util.isTypeAnyType(subBaseType)) ||
176+
isDeeplyObjectType(subBaseType)
177+
) {
178+
context.report({
179+
data: {
180+
stringLike,
181+
type: typeChecker.typeToString(subBaseType),
182+
},
183+
messageId: 'invalid',
184+
node: baseNode,
185+
});
186+
hadIndividualComplaint = true;
187+
continue;
188+
}
155189
}
190+
}
156191

192+
if (hadIndividualComplaint) {
157193
return;
158194
}
159195

160-
if (leftType === 'any' || rightType === 'any') {
161-
if (!allowAny || leftType === 'invalid' || rightType === 'invalid') {
162-
context.report({
196+
for (const [baseType, otherType] of [
197+
[leftType, rightType],
198+
[rightType, leftType],
199+
] as const) {
200+
if (
201+
!allowNumberAndString &&
202+
isTypeFlagSetInUnion(baseType, ts.TypeFlags.StringLike) &&
203+
isTypeFlagSetInUnion(otherType, ts.TypeFlags.NumberLike)
204+
) {
205+
return context.report({
206+
data: {
207+
stringLike,
208+
left: typeChecker.typeToString(leftType),
209+
right: typeChecker.typeToString(rightType),
210+
},
211+
messageId: 'mismatched',
163212
node,
164-
messageId: 'notValidAnys',
165213
});
166214
}
167215

168-
return;
169-
}
170-
171-
if (leftType === 'string' || rightType === 'string') {
172-
return context.report({
173-
node,
174-
messageId: 'notStrings',
175-
});
176-
}
177-
178-
if (leftType === 'bigint' || rightType === 'bigint') {
179-
return context.report({
180-
node,
181-
messageId: 'notBigInts',
182-
});
183-
}
184-
185-
if (leftType === 'number' || rightType === 'number') {
186-
return context.report({
187-
node,
188-
messageId: 'notNumbers',
189-
});
216+
if (
217+
isTypeFlagSetInUnion(baseType, ts.TypeFlags.NumberLike) &&
218+
isTypeFlagSetInUnion(otherType, ts.TypeFlags.BigIntLike)
219+
) {
220+
return context.report({
221+
data: {
222+
left: typeChecker.typeToString(leftType),
223+
right: typeChecker.typeToString(rightType),
224+
},
225+
messageId: 'bigintAndNumber',
226+
node,
227+
});
228+
}
190229
}
191230
}
192231

@@ -200,3 +239,15 @@ export default util.createRule<Options, MessageIds>({
200239
};
201240
},
202241
});
242+
243+
function isDeeplyObjectType(type: ts.Type): boolean {
244+
return type.isIntersection()
245+
? tsutils.intersectionTypeParts(type).every(tsutils.isObjectType)
246+
: tsutils.unionTypeParts(type).every(tsutils.isObjectType);
247+
}
248+
249+
function isTypeFlagSetInUnion(type: ts.Type, flag: ts.TypeFlags): boolean {
250+
return tsutils
251+
.unionTypeParts(type)
252+
.some(subType => tsutils.isTypeFlagSet(subType, flag));
253+
}

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import * as util from '../util';
66

77
type Options = [
88
{
9-
allowNumber?: boolean;
10-
allowBoolean?: boolean;
119
allowAny?: boolean;
10+
allowBoolean?: boolean;
1211
allowNullish?: boolean;
12+
allowNumber?: boolean;
1313
allowRegExp?: boolean;
1414
allowNever?: boolean;
1515
},
@@ -34,24 +34,24 @@ export default util.createRule<Options, MessageId>({
3434
{
3535
type: 'object',
3636
properties: {
37-
allowNumber: {
37+
allowAny: {
3838
description:
39-
'Whether to allow `number` typed values in template expressions.',
39+
'Whether to allow `any` typed values in template expressions.',
4040
type: 'boolean',
4141
},
4242
allowBoolean: {
4343
description:
4444
'Whether to allow `boolean` typed values in template expressions.',
4545
type: 'boolean',
4646
},
47-
allowAny: {
47+
allowNullish: {
4848
description:
49-
'Whether to allow `any` typed values in template expressions.',
49+
'Whether to allow `nullish` typed values in template expressions.',
5050
type: 'boolean',
5151
},
52-
allowNullish: {
52+
allowNumber: {
5353
description:
54-
'Whether to allow `nullish` typed values in template expressions.',
54+
'Whether to allow `number` typed values in template expressions.',
5555
type: 'boolean',
5656
},
5757
allowRegExp: {

‎packages/eslint-plugin/tests/rules/restrict-plus-operands.test.ts

+655-181
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)
Please sign in to comment.