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.

Sign up for GitHub

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

no-new-array & no-new-buffer: Improve argument type detection #1648

Merged
merged 6 commits into from
Dec 21, 2021
Merged

no-new-array & no-new-buffer: Improve argument type detection #1648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
354908f
Refactor
fisker Dec 21, 2021
bf49f82
Add tests
fisker Dec 21, 2021
2de196c
Fix
fisker Dec 21, 2021
f39ba24
`NaN`
fisker Dec 21, 2021
872f3e7
Coverage
fisker Dec 21, 2021
933220a
Use in `no-new-buffer` rule
fisker Dec 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 20 additions & 19 deletions rules/no-new-array.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading