Skip to content

Commit

Permalink
no-new-array & no-new-buffer: Improve argument type detection (#1648
Browse files Browse the repository at this point in the history
)
  • Loading branch information
fisker committed Dec 21, 2021
1 parent 2b92385 commit 9b04e43
Show file tree
Hide file tree
Showing 9 changed files with 809 additions and 37 deletions.
39 changes: 20 additions & 19 deletions rules/no-new-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const {isParenthesized, getStaticValue} = require('eslint-utils');
const needsSemicolon = require('./utils/needs-semicolon.js');
const {newExpressionSelector} = require('./selectors/index.js');
const isNumber = require('./utils/is-number.js');

const MESSAGE_ID_ERROR = 'error';
const MESSAGE_ID_LENGTH = 'array-length';
Expand Down Expand Up @@ -48,30 +49,30 @@ function getProblem(context, node) {
return problem;
}

const result = getStaticValue(argumentNode, context.getScope());
const fromLengthText = `Array.from(${text === 'length' ? '{length}' : `{length: ${text}}`})`;
const onlyElementText = `${maybeSemiColon}[${text}]`;

// We don't know the argument is number or not
if (result === null) {
problem.suggest = [
{
messageId: MESSAGE_ID_LENGTH,
fix: fixer => fixer.replaceText(node, fromLengthText),
},
{
messageId: MESSAGE_ID_ONLY_ELEMENT,
fix: fixer => fixer.replaceText(node, onlyElementText),
},
];
if (isNumber(argumentNode, context.getScope())) {
problem.fix = fixer => fixer.replaceText(node, fromLengthText);
return problem;
}

problem.fix = fixer => fixer.replaceText(
node,
typeof result.value === 'number' ? fromLengthText : onlyElementText,
);
const onlyElementText = `${maybeSemiColon}[${text}]`;
const result = getStaticValue(argumentNode, context.getScope());
if (result !== null) {
problem.fix = fixer => fixer.replaceText(node, onlyElementText);
return problem;
}

// We don't know the argument is number or not
problem.suggest = [
{
messageId: MESSAGE_ID_LENGTH,
fix: fixer => fixer.replaceText(node, fromLengthText),
},
{
messageId: MESSAGE_ID_ONLY_ELEMENT,
fix: fixer => fixer.replaceText(node, onlyElementText),
},
];
return problem;
}

Expand Down
9 changes: 5 additions & 4 deletions rules/no-new-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const {getStaticValue} = require('eslint-utils');
const {newExpressionSelector} = require('./selectors/index.js');
const {switchNewExpressionToCallExpression} = require('./fix/index.js');
const isNumber = require('./utils/is-number.js');

const ERROR = 'error';
const ERROR_UNKNOWN = 'error-unknown';
Expand All @@ -26,13 +27,13 @@ const inferMethod = (bufferArguments, scope) => {
return 'from';
}

if (isNumber(firstArgument, scope)) {
return 'alloc';
}

const staticResult = getStaticValue(firstArgument, scope);
if (staticResult) {
const {value} = staticResult;
if (typeof value === 'number') {
return 'alloc';
}

if (
typeof value === 'string'
|| Array.isArray(value)
Expand Down
170 changes: 170 additions & 0 deletions rules/utils/is-number.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
'use strict';
const {getStaticValue} = require('eslint-utils');

const isStaticProperties = (node, object, properties) =>
node.type === 'MemberExpression'
&& !node.computed
&& !node.optional
&& node.object.type === 'Identifier'
&& node.object.name === object
&& node.property.type === 'Identifier'
&& properties.has(node.property.name);
const isFunctionCall = (node, functionName) => node.type === 'CallExpression'
&& !node.optional
&& node.callee.type === 'Identifier'
&& node.callee.name === functionName;

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math#static_properties
const mathProperties = new Set([
'E',
'LN2',
'LN10',
'LOG2E',
'LOG10E',
'PI',
'SQRT1_2',
'SQRT2',
]);

// `Math.{E,LN2,LN10,LOG2E,LOG10E,PI,SQRT1_2,SQRT2}`
const isMathProperty = node => isStaticProperties(node, 'Math', mathProperties);

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math#static_methods
const mathMethods = new Set([
'abs',
'acos',
'acosh',
'asin',
'asinh',
'atan',
'atanh',
'atan2',
'cbrt',
'ceil',
'clz32',
'cos',
'cosh',
'exp',
'expm1',
'floor',
'fround',
'hypot',
'imul',
'log',
'log1p',
'log10',
'log2',
'max',
'min',
'pow',
'random',
'round',
'sign',
'sin',
'sinh',
'sqrt',
'tan',
'tanh',
'trunc',
]);
// `Math.{abs, …, trunc}(…)`
const isMathMethodCall = node =>
node.type === 'CallExpression'
&& !node.optional
&& isStaticProperties(node.callee, 'Math', mathMethods);

// `Number(…)`
const isNumberCall = node => isFunctionCall(node, 'Number');

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number#static_properties
const numberProperties = new Set([
'EPSILON',
'MAX_SAFE_INTEGER',
'MAX_VALUE',
'MIN_SAFE_INTEGER',
'MIN_VALUE',
'NaN',
'NEGATIVE_INFINITY',
'POSITIVE_INFINITY',
]);
const isNumberProperty = node => isStaticProperties(node, 'Number', numberProperties);

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number#static_methods
const numberMethods = new Set([
'parseFloat',
'parseInt',
]);
const isNumberMethodCall = node =>
node.type === 'CallExpression'
&& !node.optional
&& isStaticProperties(node.callee, 'Number', numberMethods);
const isGlobalParseToNumberFunctionCall = node => isFunctionCall(node, 'parseInt') || isFunctionCall(node, 'parseFloat');

// `+x`, `-x`
const numberUnaryOperators = new Set(['-', '+', '~']);
const isNumberUnaryExpression = node =>
node.type === 'UnaryExpression'
&& node.prefix
&& numberUnaryOperators.has(node.operator);

const isStaticNumber = (node, scope) => {
const staticResult = getStaticValue(node, scope);
return staticResult !== null && typeof staticResult.value === 'number';
};

const isNumberLiteral = node => node.type === 'Literal' && typeof node.value === 'number';
const isLengthProperty = node =>
node.type === 'MemberExpression'
&& !node.computed
&& !node.optional
&& node.property.type === 'Identifier'
&& node.property.name === 'length';

const mathOperators = new Set(['-', '*', '/', '%', '**', '<<', '>>', '>>>', '|', '^', '&']);
function isNumber(node, scope) {
if (
isNumberLiteral(node)
|| isMathProperty(node)
|| isMathMethodCall(node)
|| isNumberCall(node)
|| isNumberProperty(node)
|| isNumberMethodCall(node)
|| isGlobalParseToNumberFunctionCall(node)
|| isNumberUnaryExpression(node)
|| isLengthProperty(node)
) {
return true;
}

switch (node.type) {
case 'BinaryExpression':
case 'AssignmentExpression': {
let {operator} = node;

if (node.type === 'AssignmentExpression') {
operator = operator.slice(0, -1);
}

if (operator === '+') {
return isNumber(node.left, scope) && isNumber(node.right, scope);
}

// `a + b` can be `BigInt`, we need make sure at least one side is number
if (mathOperators.has(operator)) {
return isNumber(node.left, scope) || isNumber(node.right, scope);
}

break;
}

case 'ConditionalExpression':
return isNumber(node.consequent, scope) && isNumber(node.alternate, scope);
case 'SequenceExpression':
return isNumber(node.expressions[node.expressions.length - 1], scope);
// No default
}

return isStaticNumber(node, scope);
}

module.exports = isNumber;
34 changes: 34 additions & 0 deletions test/no-new-array.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -147,5 +147,39 @@ test.snapshot({
const foo = []
new Array("bar").forEach(baz)
`,
// Number
'new Array(0xff)',
'new Array(Math.PI | foo)',
'new Array(Math.min(foo, bar))',
'new Array(Number(foo))',
'new Array(Number.MAX_SAFE_INTEGER)',
'new Array(parseInt(foo))',
'new Array(Number.parseInt(foo))',
'new Array(+foo)',
'new Array(-foo)',
'new Array(~foo)',
'new Array(foo.length)',
'const foo = 1; new Array(foo + 2)',
'new Array(foo - 2)',
'new Array(foo -= 2)',
'new Array(foo ? 1 : 2)',
'new Array((1n, 2))',
'new Array(Number.NaN)',
'new Array(NaN)',
// Not number
'new Array("0xff")',
'new Array(Math.NON_EXISTS_PROPERTY)',
'new Array(Math.NON_EXISTS_METHOD(foo))',
'new Array(Math[min](foo, bar))',
'new Array(Number[MAX_SAFE_INTEGER])',
'new Array(new Number(foo))',
'const foo = 1; new Array(foo + "2")',
'new Array(foo - 2n)',
'new Array(foo -= 2n)',
'new Array(foo instanceof 1)',
'new Array(foo || 1)',
'new Array(foo ||= 1)',
'new Array(foo ? 1n : 2)',
'new Array((1, 2n))',
],
});
2 changes: 2 additions & 0 deletions test/no-new-buffer.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ test.snapshot({
const size = 10;
const buffer = new Buffer(size);
`,
'new Buffer(foo.length)',
'new Buffer(Math.min(foo, bar))',

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

0 comments on commit 9b04e43

Please sign in to comment.