Improve isNumber (#1653)

Author: fisker

rules/no-new-array.js

@@ -57,7 +57,7 @@ function getProblem(context, node) {
 
 	const onlyElementText = `${maybeSemiColon}[${text}]`;
 	const result = getStaticValue(argumentNode, context.getScope());
-	if (result !== null) {
+	if (result !== null && typeof result.value !== 'number') {
 		problem.fix = fixer => fixer.replaceText(node, onlyElementText);
 		return problem;
 	}

rules/utils/is-number.js

@@ -100,13 +100,6 @@ const isNumberMethodCall = node =>
 	&& 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';
@@ -120,7 +113,8 @@ const isLengthProperty = node =>
 	&& node.property.type === 'Identifier'
 	&& node.property.name === 'length';
 
-const mathOperators = new Set(['-', '*', '/', '%', '**', '<<', '>>', '>>>', '|', '^', '&']);
+// `+` and `>>>` operators are handled separately
+const mathOperators = new Set(['-', '*', '/', '%', '**', '<<', '>>', '|', '^', '&']);
 function isNumber(node, scope) {
 	if (
 		isNumberLiteral(node)
@@ -130,37 +124,97 @@ function isNumber(node, scope) {
 		|| isNumberProperty(node)
 		|| isNumberMethodCall(node)
 		|| isGlobalParseToNumberFunctionCall(node)
-		|| isNumberUnaryExpression(node)
 		|| isLengthProperty(node)
 	) {
 		return true;
 	}
 
 	switch (node.type) {
-		case 'BinaryExpression':
 		case 'AssignmentExpression': {
+			const {operator} = node;
+			if (operator === '=' && isNumber(node.right, scope)) {
+				return true;
+			}
+
+			// Fall through
+		}
+
+		case 'BinaryExpression': {
 			let {operator} = node;
 
 			if (node.type === 'AssignmentExpression') {
 				operator = operator.slice(0, -1);
 			}
 
+			if (operator === '+' && isNumber(node.left, scope) && isNumber(node.right, scope)) {
+				return true;
+			}
+
+			// `>>>` (zero-fill right shift) can't use on `BigInt`
+			// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#operators
+			if (operator === '>>>') {
+				return true;
+			}
+
+			// `a * b` can be `BigInt`, we need make sure at least one side is number
+			if (mathOperators.has(operator) && (isNumber(node.left, scope) || isNumber(node.right, scope))) {
+				return true;
+			}
+
+			break;
+		}
+
+		case 'UnaryExpression': {
+			const {operator} = node;
+
+			// `+` can't use on `BigInt`
+			// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#operators
 			if (operator === '+') {
-				return isNumber(node.left, scope) && isNumber(node.right, scope);
+				return true;
 			}
 
-			// `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);
+			if ((operator === '-' || operator === '~') && isNumber(node.argument, scope)) {
+				return true;
 			}
 
 			break;
 		}
 
-		case 'ConditionalExpression':
-			return isNumber(node.consequent, scope) && isNumber(node.alternate, scope);
-		case 'SequenceExpression':
-			return isNumber(node.expressions[node.expressions.length - 1], scope);
+		case 'UpdateExpression':
+			if (isNumber(node.argument, scope)) {
+				return true;
+			}
+
+			break;
+		case 'ConditionalExpression': {
+			const isConsequentNumber = isNumber(node.consequent, scope);
+			const isAlternateNumber = isNumber(node.alternate, scope);
+
+			if (isConsequentNumber && isAlternateNumber) {
+				return true;
+			}
+
+			const testStaticValueResult = getStaticValue(node.test, scope);
+			if (
+				testStaticValueResult !== null
+				&& (
+					(testStaticValueResult.value && isConsequentNumber)
+					|| (!testStaticValueResult.value && isAlternateNumber)
+				)
+			) {
+				return true;
+			}
+
+			break;
+		}
+
+		case 'SequenceExpression': {
+			if (isNumber(node.expressions[node.expressions.length - 1], scope)) {
+				return true;
+			}
+
+			break;
+		}
 		// No default
 	}
 

test/no-new-array.mjs

@@ -156,16 +156,23 @@ test.snapshot({
 		'new Array(parseInt(foo))',
 		'new Array(Number.parseInt(foo))',
 		'new Array(+foo)',
-		'new Array(-foo)',
-		'new Array(~foo)',
+		'new Array(-Math.PI)',
+		'new Array(-"-2")',
 		'new Array(foo.length)',
 		'const foo = 1; new Array(foo + 2)',
 		'new Array(foo - 2)',
 		'new Array(foo -= 2)',
 		'new Array(foo ? 1 : 2)',
+		'const truthy = "truthy"; new Array(truthy ? 1 : foo)',
+		'const falsy = !"truthy"; new Array(falsy ? foo : 1)',
 		'new Array((1n, 2))',
 		'new Array(Number.NaN)',
 		'new Array(NaN)',
+		'new Array(foo >>> bar)',
+		'new Array(foo >>>= bar)',
+		'new Array(++bar.length)',
+		'new Array(bar.length++)',
+		'new Array(foo = bar.length)',
 		// Not number
 		'new Array("0xff")',
 		'new Array(Math.NON_EXISTS_PROPERTY)',
@@ -181,5 +188,13 @@ test.snapshot({
 		'new Array(foo ||= 1)',
 		'new Array(foo ? 1n : 2)',
 		'new Array((1, 2n))',
+		'new Array(-foo)',
+		'new Array(~foo)',
+		'new Array(typeof 1)',
+		'const truthy = "truthy"; new Array(truthy ? foo : 1)',
+		'const falsy = !"truthy"; new Array(falsy ? 1 : foo)',
+		'new Array(unknown ? foo : 1)',
+		'new Array(unknown ? 1 : foo)',
+		'new Array(++foo)',
 	],
 });