prefer-math-min-max: Ignore BigInt

[chirokas]

Close #2465

[fisker]

Maybe we should only report when there is a number, since strings, Dates can also be compared.

[fisker]

@axetroy What do you think?

[chirokas]

You mean like this?

const isNumber = require("./utils/is-number.js");

// ...

const { sourceCode } = context;
const scope = sourceCode.getScope(conditionalExpression);
const isInvalidExpression = [left, right, alternate, consequent].some(
  (node) => !isNumber(node, scope),
);

if (isInvalidExpression) {
  return;
}

Ensure that everything, be it an Identifier, Literal, or any other expression, is a Number.

[axetroy]

Maybe we should only report when there is a number, since strings, Dates can also be compared.

Make sense.

Ensure that everything, be it an Identifier, Literal, or any other expression, is a Number.

But I think we should only need to determine whether one of the left or right is a number (Identifier/Literal/Expression). And ignore bigInt

const isNumber = require("./utils/is-number.js");

// ...

const { sourceCode } = context;
const scope = sourceCode.getScope(conditionalExpression);
const isValidExpression = [left, right].some((node) => isNumber(node, scope));

if (!isValidExpression) {
  return;
}

const isBigIntInResult = [alternate, consequent].some((node) => isBigIntLiteral(node)
				|| isCallExpression(node, {
					name: 'BigInt',
					argumentsLength: 1,
					optional: false,
				}))

if (isBigIntInResult) {
  return;
}

[fisker]

The logic is good, but it makes this rule almost useless.. since it's rare that both left and right matches isNumber.

[fisker]

Maybe we can skip this check with strictTypes: false (default: true) option? So this rule can be useful sometimes.

TypeTracker from eslint-plugin-regexp can be useful, but it's quit big.

@sindresorhus Not sure what we should do here.

[sindresorhus]

The logic is good, but it makes this rule almost useless.. since it's rare that both left and right matches isNumber.

Not sure I understand. We only need to check if one of the sides is a number, right?

[sindresorhus]

TypeTracker from eslint-plugin-regexp can be useful, but it's quit big.

If we were to do that, it would be better to use TypeScrip as its type inference is much more reliable.

[axetroy]

But I think we should only need to determine whether one of the left or right is a number
We only need to check if one of the sides is a number, right?

Just check one of them to make sure it is a comparison of numbers, not strings.

[sindresorhus]

Just check one of them to make sure it is a comparison of numbers, not strings.

Yes

[fisker]

We only need to check if one of the sides is a number, right?

I thought the same, #2467 (comment)

Turns out I was wrong

10n > 0 ? 10n : 0
10n
Math.max(0, 10n)
VM762:1 Uncaught TypeError: Cannot convert a BigInt value to a number

typeof (new Date > 0 ? new Date : 0)
'object'
typeof (Math.max(0, new Date))
'number'

typeof ('100' > 0 ? '100' : 0)
'string'
typeof (Math.max(0, '100'))
'number'

[sindresorhus]

Turns out I was wrong

That is awful code and not something we should care about. We should be pragmatic.