Skip to content

Commit 4e50131

Browse files
sindresorhussindresorhus
authored
prefer-number-is-safe-integer: Check legacy integer patterns (#3214)
1 parent 0116e32 commit 4e50131

6 files changed

Lines changed: 668 additions & 18 deletions

‎docs/rules/prefer-number-is-safe-integer.md‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# prefer-number-is-safe-integer
22

3-
📝 Prefer `Number.isSafeInteger()` over `Number.isInteger()`.
3+
📝 Prefer `Number.isSafeInteger()` over integer checks.
44

55
💼🚫 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config). This rule is _disabled_ in the ☑️ `unopinionated` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config).
66

@@ -11,7 +11,11 @@
1111

1212
[`Number.isSafeInteger()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isSafeInteger) checks that a value is an integer _and_ that it can be exactly represented, that is, it is within the safe integer range `[-(2 ** 53 - 1), 2 ** 53 - 1]`. [`Number.isInteger()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isInteger) returns `true` for larger integers that can no longer be represented precisely, which is rarely what you want.
1313

14-
This rule only provides a suggestion, not an automatic fix, because the two methods are not equivalent: `Number.isInteger(2 ** 53)` is `true` while `Number.isSafeInteger(2 ** 53)` is `false`. Review each case before applying the suggestion, especially negated checks like `!Number.isInteger(x)`, where switching to `Number.isSafeInteger()` widens the set of values treated as “not an integer”.
14+
This rule also reports common integer checks like `value % 1 === 0`, `Math.trunc(value) === value`, `Math.floor(value) === value`, and Lodash/Underscore `isInteger()`/`isSafeInteger()` calls.
15+
16+
This rule only provides a suggestion, not an automatic fix, because these checks are not all equivalent: `Number.isInteger(2 ** 53)` is `true` while `Number.isSafeInteger(2 ** 53)` is `false`, and `[['1']] % 1 === 0` is `true` while `Number.isSafeInteger([['1']])` is `false`. Review each case before applying the suggestion, especially negated checks like `!Number.isInteger(x)`, where switching to `Number.isSafeInteger()` widens the set of values treated as “not an integer”.
17+
18+
This rule intentionally ignores less clear coercion and truncation patterns like bitwise integer checks, `parseInt(value, 10) === value`, and `Math.round(value) === value`.
1519

1620
## Examples
1721

@@ -55,3 +59,27 @@ function processId(id) {
5559
// id is guaranteed to be safely representable
5660
}
5761
```
62+
63+
```js
64+
// ❌
65+
if (value % 1 === 0) {
66+
console.log('Integer');
67+
}
68+
69+
// ✅
70+
if (Number.isSafeInteger(value)) {
71+
console.log('Safe integer');
72+
}
73+
```
74+
75+
```js
76+
// ❌
77+
if (_.isInteger(value)) {
78+
console.log('Integer');
79+
}
80+
81+
// ✅
82+
if (Number.isSafeInteger(value)) {
83+
console.log('Safe integer');
84+
}
85+
```

‎readme.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ export default [
240240
| [prefer-negative-index](docs/rules/prefer-negative-index.md) | Prefer negative index over `.length - index` when possible. | ✅ ☑️ | 🔧 | |
241241
| [prefer-node-protocol](docs/rules/prefer-node-protocol.md) | Prefer using the `node:` protocol when importing Node.js builtin modules. | ✅ ☑️ | 🔧 | |
242242
| [prefer-number-coercion](docs/rules/prefer-number-coercion.md) | Prefer `Number()` over `parseFloat()` and base-10 `parseInt()`. | ✅ ☑️ | | 💡 |
243-
| [prefer-number-is-safe-integer](docs/rules/prefer-number-is-safe-integer.md) | Prefer `Number.isSafeInteger()` over `Number.isInteger()`. | ✅ | | 💡 |
243+
| [prefer-number-is-safe-integer](docs/rules/prefer-number-is-safe-integer.md) | Prefer `Number.isSafeInteger()` over integer checks. | ✅ | | 💡 |
244244
| [prefer-number-properties](docs/rules/prefer-number-properties.md) | Prefer `Number` static methods over global functions and optionally static properties over global constants. | ✅ ☑️ | 🔧 | 💡 |
245245
| [prefer-object-define-properties](docs/rules/prefer-object-define-properties.md) | Prefer `Object.defineProperties()` over multiple `Object.defineProperty()` calls. | ✅ ☑️ | 🔧 | |
246246
| [prefer-object-destructuring-defaults](docs/rules/prefer-object-destructuring-defaults.md) | Prefer object destructuring defaults over default object literals with spread. | ✅ | | 💡 |

‎rules/prefer-number-is-safe-integer.js‎

Lines changed: 150 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,175 @@
1-
import {isMethodCall} from './ast/index.js';
1+
import {findVariable} from '@eslint-community/eslint-utils';
2+
import {isLiteral, isMethodCall} from './ast/index.js';
3+
import isSameReference from './utils/is-same-reference.js';
24

35
const MESSAGE_ID_ERROR = 'prefer-number-is-safe-integer/error';
46
const MESSAGE_ID_SUGGESTION = 'prefer-number-is-safe-integer/suggestion';
7+
const MESSAGE_ID_INTEGER_CHECK_ERROR = 'prefer-number-is-safe-integer/integer-check-error';
8+
const MESSAGE_ID_INTEGER_CHECK_SUGGESTION = 'prefer-number-is-safe-integer/integer-check-suggestion';
59
const messages = {
610
[MESSAGE_ID_ERROR]: 'Prefer `Number.isSafeInteger()` over `Number.isInteger()`.',
711
[MESSAGE_ID_SUGGESTION]: 'Replace `Number.isInteger()` with `Number.isSafeInteger()`.',
12+
[MESSAGE_ID_INTEGER_CHECK_ERROR]: 'Prefer `Number.isSafeInteger()` over this integer check.',
13+
[MESSAGE_ID_INTEGER_CHECK_SUGGESTION]: 'Replace this integer check with `Number.isSafeInteger()`.',
14+
};
15+
16+
const lodashObjects = ['_', 'lodash', 'underscore'];
17+
const mathIntegerCheckMethods = ['floor', 'trunc'];
18+
19+
const getExpressionText = (node, sourceCode) => {
20+
const text = sourceCode.getText(node);
21+
return node.type === 'SequenceExpression' ? `(${text})` : text;
22+
};
23+
24+
const hasCommentsOutsideNode = (node, nodeToKeep, sourceCode) => {
25+
const keepRange = sourceCode.getRange(nodeToKeep);
26+
27+
return sourceCode.getCommentsInside(node).some(comment => {
28+
const commentRange = sourceCode.getRange(comment);
29+
return commentRange[0] < keepRange[0] || commentRange[1] > keepRange[1];
30+
});
31+
};
32+
33+
const isGlobalNumberAvailable = (node, sourceCode) => {
34+
const variable = findVariable(sourceCode.getScope(node), 'Number');
35+
return !variable || (variable.scope.type === 'global' && variable.defs.length === 0);
36+
};
37+
38+
const getModuloCheckArgument = node => {
39+
if (
40+
node.type !== 'BinaryExpression'
41+
|| node.operator !== '%'
42+
|| !isLiteral(node.right, 1)
43+
) {
44+
return;
45+
}
46+
47+
return node.left;
48+
};
49+
50+
const getModuloIntegerCheckArgument = node => {
51+
if (node.operator !== '===') {
52+
return;
53+
}
54+
55+
if (isLiteral(node.right, 0)) {
56+
return getModuloCheckArgument(node.left);
57+
}
58+
59+
if (isLiteral(node.left, 0)) {
60+
return getModuloCheckArgument(node.right);
61+
}
62+
};
63+
64+
const getMathIntegerCheckArgument = (node, sourceCode) => {
65+
if (
66+
!isMethodCall(node, {
67+
object: 'Math',
68+
methods: mathIntegerCheckMethods,
69+
argumentsLength: 1,
70+
optionalCall: false,
71+
optionalMember: false,
72+
computed: false,
73+
})
74+
|| !sourceCode.isGlobalReference(node.callee.object)
75+
) {
76+
return;
77+
}
78+
79+
return node.arguments[0];
80+
};
81+
82+
const getMathComparisonIntegerCheckArgument = (node, sourceCode) => {
83+
if (node.operator !== '===') {
84+
return;
85+
}
86+
87+
const leftArgument = getMathIntegerCheckArgument(node.left, sourceCode);
88+
if (leftArgument && isSameReference(leftArgument, node.right)) {
89+
return leftArgument;
90+
}
91+
92+
const rightArgument = getMathIntegerCheckArgument(node.right, sourceCode);
93+
if (rightArgument && isSameReference(rightArgument, node.left)) {
94+
return rightArgument;
95+
}
96+
};
97+
98+
const getLodashIntegerCheckArgument = node => {
99+
if (!isMethodCall(node, {
100+
objects: lodashObjects,
101+
methods: ['isInteger', 'isSafeInteger'],
102+
argumentsLength: 1,
103+
optionalCall: false,
104+
optionalMember: false,
105+
computed: false,
106+
})) {
107+
return;
108+
}
109+
110+
return node.arguments[0];
8111
};
9112

10113
/** @param {import('eslint').Rule.RuleContext} context */
11114
const create = context => {
115+
const {sourceCode} = context;
116+
117+
const createIntegerCheckProblem = (node, argument) => {
118+
const problem = {
119+
node,
120+
messageId: MESSAGE_ID_INTEGER_CHECK_ERROR,
121+
};
122+
123+
if (!hasCommentsOutsideNode(node, argument, sourceCode)) {
124+
problem.suggest = [
125+
{
126+
messageId: MESSAGE_ID_INTEGER_CHECK_SUGGESTION,
127+
fix: fixer => fixer.replaceText(node, `Number.isSafeInteger(${getExpressionText(argument, sourceCode)})`),
128+
},
129+
];
130+
}
131+
132+
return problem;
133+
};
134+
12135
context.on('CallExpression', callExpression => {
13136
if (
14-
!isMethodCall(callExpression, {
137+
isMethodCall(callExpression, {
15138
object: 'Number',
16139
method: 'isInteger',
17140
optionalCall: false,
18141
optionalMember: false,
19142
computed: false,
20143
})
21-
|| !context.sourceCode.isGlobalReference(callExpression.callee.object)
144+
&& sourceCode.isGlobalReference(callExpression.callee.object)
22145
) {
146+
return {
147+
node: callExpression.callee.property,
148+
messageId: MESSAGE_ID_ERROR,
149+
suggest: [
150+
{
151+
messageId: MESSAGE_ID_SUGGESTION,
152+
fix: fixer => fixer.replaceText(callExpression.callee.property, 'isSafeInteger'),
153+
},
154+
],
155+
};
156+
}
157+
158+
const argument = getLodashIntegerCheckArgument(callExpression);
159+
if (!argument || !isGlobalNumberAvailable(callExpression, sourceCode)) {
23160
return;
24161
}
25162

26-
const propertyNode = callExpression.callee.property;
163+
return createIntegerCheckProblem(callExpression, argument);
164+
});
27165

28-
return {
29-
node: propertyNode,
30-
messageId: MESSAGE_ID_ERROR,
31-
suggest: [
32-
{
33-
messageId: MESSAGE_ID_SUGGESTION,
34-
fix: fixer => fixer.replaceText(propertyNode, 'isSafeInteger'),
35-
},
36-
],
37-
};
166+
context.on('BinaryExpression', node => {
167+
const argument = getModuloIntegerCheckArgument(node) ?? getMathComparisonIntegerCheckArgument(node, sourceCode);
168+
if (!argument || !isGlobalNumberAvailable(node, sourceCode)) {
169+
return;
170+
}
171+
172+
return createIntegerCheckProblem(node, argument);
38173
});
39174
};
40175

@@ -44,7 +179,7 @@ const config = {
44179
meta: {
45180
type: 'suggestion',
46181
docs: {
47-
description: 'Prefer `Number.isSafeInteger()` over `Number.isInteger()`.',
182+
description: 'Prefer `Number.isSafeInteger()` over integer checks.',
48183
recommended: true,
49184
},
50185
hasSuggestions: true,

‎test/prefer-number-is-safe-integer.js‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,36 @@ test.snapshot({
2121
'const Number = {isInteger() {}}; Number.isInteger(x)',
2222
'import Number from "number"; Number.isInteger(x)',
2323
'function foo(Number) { return Number.isInteger(x); }',
24+
'const Number = {}; value % 1 === 0',
25+
'import Number from "number"; Math.trunc(value) === value',
26+
'function foo(Number) { return _.isInteger(value); }',
27+
'value % 1 == 0', // Loose equality
28+
'value % 1 !== 0', // Negative check
29+
'0 !== value % 1', // Negative check
30+
'(value | 0) === value', // Bitwise check
31+
'Number.parseInt(value, 10) === value',
32+
'Math.round(value) === value',
33+
'Math.floor(value) === otherValue',
34+
'Math.trunc(value) === otherValue',
35+
'Math.floor(value) == value', // Loose equality
36+
'Math.trunc(value) !== value', // Negative check
37+
'const Math = {trunc() {}}; Math.trunc(value) === value',
38+
'Math.trunc?.(value) === value',
39+
'Math?.trunc(value) === value',
40+
'Math[trunc](value) === value',
41+
'Math["trunc"](value) === value',
42+
'Math.floor(...value) === value',
43+
'Math.trunc(...value) === value',
44+
'Math.floor(value, extra) === value',
45+
'Math.trunc(value, extra) === value',
46+
'_.isInteger?.(value)',
47+
'_?.isInteger(value)',
48+
'_[isInteger](value)',
49+
'_["isInteger"](value)',
50+
'_.isInteger(...value)',
51+
'_.isInteger(value, extra)',
52+
'lodash.isSafeInteger(...value)',
53+
'underscore.isSafeInteger(value, extra)',
2454
],
2555
invalid: [
2656
'Number.isInteger(x)',
@@ -33,6 +63,26 @@ test.snapshot({
3363
'Number.isInteger(/* comment */ x)', // Comments are preserved
3464
'Number.isInteger(...x)', // Spread argument
3565
'Number.isInteger()', // Flagged regardless of argument count
66+
'value % 1 === 0',
67+
'0 === value % 1',
68+
'object.value % 1 === 0',
69+
'(foo, bar) % 1 === 0',
70+
'Math.floor(value) === value',
71+
'value === Math.floor(value)',
72+
'Math.trunc(value) === value',
73+
'value === Math.trunc(value)',
74+
'Math.trunc(object.value) === object.value',
75+
'Math.trunc(object["value"]) === object.value',
76+
'_.isInteger(value)',
77+
'lodash.isInteger(value)',
78+
'underscore.isInteger(value)',
79+
'_.isSafeInteger(value)',
80+
'lodash.isSafeInteger(value)',
81+
'underscore.isSafeInteger(value)',
82+
'value /* comment */ % 1 === 0', // Reported without suggestion to avoid dropping comments
83+
'0 === value /* comment */ % 1', // Reported without suggestion to avoid dropping comments
84+
'_.isInteger(/* comment */ value)', // Reported without suggestion to avoid dropping comments
85+
'Math.trunc(/* comment */ value) === value', // Reported without suggestion to avoid dropping comments
3686
],
3787
});
3888

@@ -48,5 +98,17 @@ test.snapshot({
4898
code: 'Number.isInteger(x!)', // Non-null assertion must survive the suggestion
4999
languageOptions: {parser: parsers.typescript},
50100
},
101+
{
102+
code: 'Math.trunc(value as number) === value',
103+
languageOptions: {parser: parsers.typescript},
104+
},
105+
{
106+
code: 'Math.trunc(value!) === value',
107+
languageOptions: {parser: parsers.typescript},
108+
},
109+
{
110+
code: 'Math.trunc(value) === value as number',
111+
languageOptions: {parser: parsers.typescript},
112+
},
51113
],
52114
});

0 commit comments

Comments
 (0)