Skip to content

Commit 9b04e43

Browse files
fiskerfisker
authored
no-new-array & no-new-buffer: Improve argument type detection (#1648)
1 parent 2b92385 commit 9b04e43

File tree

9 files changed

+809
-37
lines changed

9 files changed

+809
-37
lines changed

‎rules/no-new-array.js‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
const {isParenthesized, getStaticValue} = require('eslint-utils');
33
const needsSemicolon = require('./utils/needs-semicolon.js');
44
const {newExpressionSelector} = require('./selectors/index.js');
5+
const isNumber = require('./utils/is-number.js');
56

67
const MESSAGE_ID_ERROR = 'error';
78
const MESSAGE_ID_LENGTH = 'array-length';
@@ -48,30 +49,30 @@ function getProblem(context, node) {
4849
return problem;
4950
}
5051

51-
const result = getStaticValue(argumentNode, context.getScope());
5252
const fromLengthText = `Array.from(${text === 'length' ? '{length}' : `{length: ${text}}`})`;
53-
const onlyElementText = `${maybeSemiColon}[${text}]`;
54-
55-
// We don't know the argument is number or not
56-
if (result === null) {
57-
problem.suggest = [
58-
{
59-
messageId: MESSAGE_ID_LENGTH,
60-
fix: fixer => fixer.replaceText(node, fromLengthText),
61-
},
62-
{
63-
messageId: MESSAGE_ID_ONLY_ELEMENT,
64-
fix: fixer => fixer.replaceText(node, onlyElementText),
65-
},
66-
];
53+
if (isNumber(argumentNode, context.getScope())) {
54+
problem.fix = fixer => fixer.replaceText(node, fromLengthText);
6755
return problem;
6856
}
6957

70-
problem.fix = fixer => fixer.replaceText(
71-
node,
72-
typeof result.value === 'number' ? fromLengthText : onlyElementText,
73-
);
58+
const onlyElementText = `${maybeSemiColon}[${text}]`;
59+
const result = getStaticValue(argumentNode, context.getScope());
60+
if (result !== null) {
61+
problem.fix = fixer => fixer.replaceText(node, onlyElementText);
62+
return problem;
63+
}
7464

65+
// We don't know the argument is number or not
66+
problem.suggest = [
67+
{
68+
messageId: MESSAGE_ID_LENGTH,
69+
fix: fixer => fixer.replaceText(node, fromLengthText),
70+
},
71+
{
72+
messageId: MESSAGE_ID_ONLY_ELEMENT,
73+
fix: fixer => fixer.replaceText(node, onlyElementText),
74+
},
75+
];
7576
return problem;
7677
}
7778

‎rules/no-new-buffer.js‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
const {getStaticValue} = require('eslint-utils');
33
const {newExpressionSelector} = require('./selectors/index.js');
44
const {switchNewExpressionToCallExpression} = require('./fix/index.js');
5+
const isNumber = require('./utils/is-number.js');
56

67
const ERROR = 'error';
78
const ERROR_UNKNOWN = 'error-unknown';
@@ -26,13 +27,13 @@ const inferMethod = (bufferArguments, scope) => {
2627
return 'from';
2728
}
2829

30+
if (isNumber(firstArgument, scope)) {
31+
return 'alloc';
32+
}
33+
2934
const staticResult = getStaticValue(firstArgument, scope);
3035
if (staticResult) {
3136
const {value} = staticResult;
32-
if (typeof value === 'number') {
33-
return 'alloc';
34-
}
35-
3637
if (
3738
typeof value === 'string'
3839
|| Array.isArray(value)

‎rules/utils/is-number.js‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
'use strict';
2+
const {getStaticValue} = require('eslint-utils');
3+
4+
const isStaticProperties = (node, object, properties) =>
5+
node.type === 'MemberExpression'
6+
&& !node.computed
7+
&& !node.optional
8+
&& node.object.type === 'Identifier'
9+
&& node.object.name === object
10+
&& node.property.type === 'Identifier'
11+
&& properties.has(node.property.name);
12+
const isFunctionCall = (node, functionName) => node.type === 'CallExpression'
13+
&& !node.optional
14+
&& node.callee.type === 'Identifier'
15+
&& node.callee.name === functionName;
16+
17+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math#static_properties
18+
const mathProperties = new Set([
19+
'E',
20+
'LN2',
21+
'LN10',
22+
'LOG2E',
23+
'LOG10E',
24+
'PI',
25+
'SQRT1_2',
26+
'SQRT2',
27+
]);
28+
29+
// `Math.{E,LN2,LN10,LOG2E,LOG10E,PI,SQRT1_2,SQRT2}`
30+
const isMathProperty = node => isStaticProperties(node, 'Math', mathProperties);
31+
32+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math#static_methods
33+
const mathMethods = new Set([
34+
'abs',
35+
'acos',
36+
'acosh',
37+
'asin',
38+
'asinh',
39+
'atan',
40+
'atanh',
41+
'atan2',
42+
'cbrt',
43+
'ceil',
44+
'clz32',
45+
'cos',
46+
'cosh',
47+
'exp',
48+
'expm1',
49+
'floor',
50+
'fround',
51+
'hypot',
52+
'imul',
53+
'log',
54+
'log1p',
55+
'log10',
56+
'log2',
57+
'max',
58+
'min',
59+
'pow',
60+
'random',
61+
'round',
62+
'sign',
63+
'sin',
64+
'sinh',
65+
'sqrt',
66+
'tan',
67+
'tanh',
68+
'trunc',
69+
]);
70+
// `Math.{abs, …, trunc}(…)`
71+
const isMathMethodCall = node =>
72+
node.type === 'CallExpression'
73+
&& !node.optional
74+
&& isStaticProperties(node.callee, 'Math', mathMethods);
75+
76+
// `Number(…)`
77+
const isNumberCall = node => isFunctionCall(node, 'Number');
78+
79+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number#static_properties
80+
const numberProperties = new Set([
81+
'EPSILON',
82+
'MAX_SAFE_INTEGER',
83+
'MAX_VALUE',
84+
'MIN_SAFE_INTEGER',
85+
'MIN_VALUE',
86+
'NaN',
87+
'NEGATIVE_INFINITY',
88+
'POSITIVE_INFINITY',
89+
]);
90+
const isNumberProperty = node => isStaticProperties(node, 'Number', numberProperties);
91+
92+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number#static_methods
93+
const numberMethods = new Set([
94+
'parseFloat',
95+
'parseInt',
96+
]);
97+
const isNumberMethodCall = node =>
98+
node.type === 'CallExpression'
99+
&& !node.optional
100+
&& isStaticProperties(node.callee, 'Number', numberMethods);
101+
const isGlobalParseToNumberFunctionCall = node => isFunctionCall(node, 'parseInt') || isFunctionCall(node, 'parseFloat');
102+
103+
// `+x`, `-x`
104+
const numberUnaryOperators = new Set(['-', '+', '~']);
105+
const isNumberUnaryExpression = node =>
106+
node.type === 'UnaryExpression'
107+
&& node.prefix
108+
&& numberUnaryOperators.has(node.operator);
109+
110+
const isStaticNumber = (node, scope) => {
111+
const staticResult = getStaticValue(node, scope);
112+
return staticResult !== null && typeof staticResult.value === 'number';
113+
};
114+
115+
const isNumberLiteral = node => node.type === 'Literal' && typeof node.value === 'number';
116+
const isLengthProperty = node =>
117+
node.type === 'MemberExpression'
118+
&& !node.computed
119+
&& !node.optional
120+
&& node.property.type === 'Identifier'
121+
&& node.property.name === 'length';
122+
123+
const mathOperators = new Set(['-', '*', '/', '%', '**', '<<', '>>', '>>>', '|', '^', '&']);
124+
function isNumber(node, scope) {
125+
if (
126+
isNumberLiteral(node)
127+
|| isMathProperty(node)
128+
|| isMathMethodCall(node)
129+
|| isNumberCall(node)
130+
|| isNumberProperty(node)
131+
|| isNumberMethodCall(node)
132+
|| isGlobalParseToNumberFunctionCall(node)
133+
|| isNumberUnaryExpression(node)
134+
|| isLengthProperty(node)
135+
) {
136+
return true;
137+
}
138+
139+
switch (node.type) {
140+
case 'BinaryExpression':
141+
case 'AssignmentExpression': {
142+
let {operator} = node;
143+
144+
if (node.type === 'AssignmentExpression') {
145+
operator = operator.slice(0, -1);
146+
}
147+
148+
if (operator === '+') {
149+
return isNumber(node.left, scope) && isNumber(node.right, scope);
150+
}
151+
152+
// `a + b` can be `BigInt`, we need make sure at least one side is number
153+
if (mathOperators.has(operator)) {
154+
return isNumber(node.left, scope) || isNumber(node.right, scope);
155+
}
156+
157+
break;
158+
}
159+
160+
case 'ConditionalExpression':
161+
return isNumber(node.consequent, scope) && isNumber(node.alternate, scope);
162+
case 'SequenceExpression':
163+
return isNumber(node.expressions[node.expressions.length - 1], scope);
164+
// No default
165+
}
166+
167+
return isStaticNumber(node, scope);
168+
}
169+
170+
module.exports = isNumber;

‎test/no-new-array.mjs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,5 +147,39 @@ test.snapshot({
147147
const foo = []
148148
new Array("bar").forEach(baz)
149149
`,
150+
// Number
151+
'new Array(0xff)',
152+
'new Array(Math.PI | foo)',
153+
'new Array(Math.min(foo, bar))',
154+
'new Array(Number(foo))',
155+
'new Array(Number.MAX_SAFE_INTEGER)',
156+
'new Array(parseInt(foo))',
157+
'new Array(Number.parseInt(foo))',
158+
'new Array(+foo)',
159+
'new Array(-foo)',
160+
'new Array(~foo)',
161+
'new Array(foo.length)',
162+
'const foo = 1; new Array(foo + 2)',
163+
'new Array(foo - 2)',
164+
'new Array(foo -= 2)',
165+
'new Array(foo ? 1 : 2)',
166+
'new Array((1n, 2))',
167+
'new Array(Number.NaN)',
168+
'new Array(NaN)',
169+
// Not number
170+
'new Array("0xff")',
171+
'new Array(Math.NON_EXISTS_PROPERTY)',
172+
'new Array(Math.NON_EXISTS_METHOD(foo))',
173+
'new Array(Math[min](foo, bar))',
174+
'new Array(Number[MAX_SAFE_INTEGER])',
175+
'new Array(new Number(foo))',
176+
'const foo = 1; new Array(foo + "2")',
177+
'new Array(foo - 2n)',
178+
'new Array(foo -= 2n)',
179+
'new Array(foo instanceof 1)',
180+
'new Array(foo || 1)',
181+
'new Array(foo ||= 1)',
182+
'new Array(foo ? 1n : 2)',
183+
'new Array((1, 2n))',
150184
],
151185
});

‎test/no-new-buffer.mjs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ test.snapshot({
4444
const size = 10;
4545
const buffer = new Buffer(size);
4646
`,
47+
'new Buffer(foo.length)',
48+
'new Buffer(Math.min(foo, bar))',
4749

4850
// `new Buffer(string[, encoding])`
4951
// https://nodejs.org/api/buffer.html#buffer_new_buffer_string_encoding

0 commit comments

Comments
 (0)