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.

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

Add prefer-number-is-integer rule #1509

1 change: 1 addition & 0 deletions configs/recommended.js
Expand Up @@ -77,6 +77,7 @@ module.exports = {
'unicorn/prefer-module': 'error',
'unicorn/prefer-negative-index': 'error',
'unicorn/prefer-node-protocol': 'error',
'unicorn/prefer-number-is-integer': 'error',
'unicorn/prefer-number-properties': 'error',
'unicorn/prefer-object-from-entries': 'error',
'unicorn/prefer-optional-catch-binding': 'error',
Expand Down
51 changes: 51 additions & 0 deletions docs/rules/prefer-number-is-integer.md
@@ -0,0 +1,51 @@
# Prefer `Number.isInteger()` for integer checking

tristanHessell marked this conversation as resolved.
Show resolved Hide resolved
<!-- Do not manually modify RULE_NOTICE part. Run: `npm run generate-rule-notices` -->
<!-- RULE_NOTICE -->
✅ *This rule is part of the [recommended](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config) config.*

💡 *This rule provides [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).*
<!-- /RULE_NOTICE -->

Enforces the use of [Number.isInteger()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isInteger) for checking if a number is an integer.

There are multiple ways to check if a variable is an integer, but these approaches tend to have slightly different behaviours.

For example:

```js
// this is not an integer (or a number)
let notInteger = [['1']];

notInteger % 1 === 0; // true - ?! an array is defintely not an integer
Number.isInteger(notInteger); // false - makes sense

// this is an integer that is larger than Number.MAX_SAFE_INTEGER
let largeInteger = 1_000_000_000_000_000_000;

largeInteger^0 === largeInteger; // false - its an integer, should be true
Number.isInteger(largeInteger); // true - makes sense
```

Due to the difference in behaviours across the different implementations, this rule is fixable via the suggestions API.

## Fail

```js
(value^0) === value
(value | 0) === value
Math.round(value) === value
parseInt(value, 10) === value
~~value === value

// these will all trigger the lint warning
_.isInteger(value);
lodash.isInteger(value);
underscore.isInteger(value);
```

## Pass

```js
Number.isInteger(value);
```
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -110,6 +110,7 @@ Configure it in `package.json`.
"unicorn/prefer-module": "error",
"unicorn/prefer-negative-index": "error",
"unicorn/prefer-node-protocol": "error",
"unicorn/prefer-number-is-integer": "error",
"unicorn/prefer-number-properties": "error",
"unicorn/prefer-object-from-entries": "error",
"unicorn/prefer-optional-catch-binding": "error",
Expand Down Expand Up @@ -226,6 +227,7 @@ Each rule has emojis denoting:
| [prefer-module](docs/rules/prefer-module.md) | Prefer JavaScript modules (ESM) over CommonJS. | ✅ | 🔧 | 💡 |
| [prefer-negative-index](docs/rules/prefer-negative-index.md) | Prefer negative index over `.length - index` for `{String,Array,TypedArray}#slice()`, `Array#splice()` and `Array#at()`. | ✅ | 🔧 | |
| [prefer-node-protocol](docs/rules/prefer-node-protocol.md) | Prefer using the `node:` protocol when importing Node.js builtin modules. | ✅ | 🔧 | |
| [prefer-number-is-integer](docs/rules/prefer-number-is-integer.md) | Prefer `Number.isInteger()` for integer checking. | ✅ | | 💡 |
| [prefer-number-properties](docs/rules/prefer-number-properties.md) | Prefer `Number` static properties over global ones. | ✅ | 🔧 | 💡 |
| [prefer-object-from-entries](docs/rules/prefer-object-from-entries.md) | Prefer using `Object.fromEntries(…)` to transform a list of key-value pairs into an object. | ✅ | 🔧 | |
| [prefer-optional-catch-binding](docs/rules/prefer-optional-catch-binding.md) | Prefer omitting the `catch` binding parameter. | ✅ | 🔧 | |
Expand Down
157 changes: 157 additions & 0 deletions rules/prefer-number-is-integer.js
@@ -0,0 +1,157 @@
'use strict';

const {methodCallSelector} = require('./selectors/index.js');
const isSameReference = require('./utils/is-same-reference.js');

const MESSAGE_ID = 'preferNumberIsInteger';
const MESSAGE_ID_SUGGEST = 'preferNumberIsIntegerSuggestion';
const messages = {
[MESSAGE_ID]: 'Replace `{{original}}` with `Number.isInteger({{variable}})`.',
[MESSAGE_ID_SUGGEST]: 'Prefer `Number.isInteger()` for integer checks.',
};

const equalsSelector = ':matches([operator="==="],[operator="=="])';

// `value % 1 === 0`
const modulo1Selector = [
'BinaryExpression',
'[left.type="BinaryExpression"]',
'[left.operator="%"]',
'[left.right.value=1]',
equalsSelector,
'[right.value="0"]',
].join('');

// `(value ^ 0) === value`
// `(value | 0) === value`
const mathOperatorSelector = [
'BinaryExpression',
'[left.type="BinaryExpression"]',
`:matches(${['^', '|'].map(operator => `[left.operator="${operator}"]`).join(',')})`,
'[left.right.value=0]',
equalsSelector,
].join('');

// `Number.parseInt(value,10) === value`
const numberParseIntSelector = [
'BinaryExpression',
'[left.type="CallExpression"]',
'[left.callee.type="MemberExpression"]',
'[left.callee.object.name="Number"]',
'[left.callee.property.name="parseInt"]',
'[left.arguments.1.value=10]',
equalsSelector,
].join('');

// `_.isInteger(value)`
const lodashIsIntegerSelector = [
methodCallSelector({method: 'isInteger', objects: ['_', 'lodash', 'underscore']}),
].join('');

// `Math.round(value) === value`
const mathRoundSelector = [
'BinaryExpression',
methodCallSelector({method: 'round', object: 'Math', path: 'left'}),
equalsSelector,
].join('');

// `~~value === value`
const bitwiseNotSelector = [
'BinaryExpression',
'[left.type="UnaryExpression"]',
'[left.operator="~"]',
'[left.argument.type="UnaryExpression"]',
'[left.argument.operator="~"]',
equalsSelector,
].join('');

function createNodeListener(sourceCode, variableGetter) {
return node => {
const variable = variableGetter(node);

if (!variable) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not limit this to Identifier and MemberExpression, should allow anything

_.isInteger(a ? b : c)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return;
}

return {
node,
messageId: MESSAGE_ID,
data: {
variable,
original: sourceCode.getText(node),
},
/** @param {import('eslint').Rule.RuleFixer} fixer */
suggest: [{
messageId: MESSAGE_ID_SUGGEST,
fix: fixer => fixer.replaceText(node, `Number.isInteger(${variable})`),
}],
};
};
}

function getNodeName(node) {
switch (node.type) {
case 'Identifier': {
return node.name;
}

case 'ChainExpression': {
return getNodeName(node.expression);
}

case 'MemberExpression': {
return `${getNodeName(node.object)}.${getNodeName(node.property)}`;
}

default: {
return '';
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sindresorhus what should we do here?

I've added the default case as to appease linting, but the calling code protects us from this branch being taken.

How should I approach appeasing Codecov?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use an ignore comment:

/* c8 ignore next 3 */

}
}

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const sourceCode = context.getSourceCode();

return {
[modulo1Selector]: createNodeListener(sourceCode, node => getNodeName(node.left.left)),
[mathOperatorSelector]: createNodeListener(sourceCode, node => {
if (isSameReference(node.left.left, node.right)) {
return getNodeName(node.right);
}
}),
[numberParseIntSelector]: createNodeListener(sourceCode, node => {
if (
isSameReference(node.left.arguments[0], node.right)
) {
return getNodeName(node.right);
}
}),
[lodashIsIntegerSelector]: createNodeListener(sourceCode, node => getNodeName(node.arguments[0])),
[mathRoundSelector]: createNodeListener(sourceCode, node => {
if (
isSameReference(node.left.arguments[0], node.right)
) {
return getNodeName(node.right);
}
}),
[bitwiseNotSelector]: createNodeListener(sourceCode, node => {
if (isSameReference(node.left.argument.argument, node.right)) {
return getNodeName(node.right);
}
}),
};
};

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
tristanHessell marked this conversation as resolved.
Show resolved Hide resolved
create,
meta: {
type: 'suggestion',
docs: {
description: 'Prefer `Number.isInteger()` for integer checking.',
},
hasSuggestions: true,
messages,
},
};
108 changes: 108 additions & 0 deletions test/prefer-number-is-integer.mjs
@@ -0,0 +1,108 @@
import {getTester} from './utils/test.mjs';

const {test} = getTester(import.meta);

const suggestionCase = ({code, suggestions}) => ({
code,
errors: [
{
messageId: 'preferNumberIsInteger',
suggestions,
},
],
});

test({
valid: [
'Number.isInteger(13)',
'Number.isInteger(13.0)',
'Number.isInteger(value)',
'(value^0) === notValue',
'(value | 0) === notValue',
'Math.round(value) === notValue',
'Number.parseInt(value, 10) === notValue',
'~~value === notValue',
],
invalid: [
suggestionCase({code: 'value % 1 === 0', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: 'value % 1 == 0', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: '(value^0) === value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: '(value^0) == value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: '(value | 0) === value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: '(value | 0) == value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: 'Number.parseInt(value, 10) === value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: 'Number.parseInt(value, 10) == value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: '_.isInteger(value)', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: 'lodash.isInteger(value)', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: 'underscore.isInteger(value)', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: 'Math.round(value) === value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: 'Math.round(value) == value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: '~~value === value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: '~~value == value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(value)',
}]}),
suggestionCase({code: '~~object.value === object.value', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(object.value)',
}]}),
suggestionCase({code: '~~object.a.b.c === object.a.b.c', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(object.a.b.c)',
}]}),
suggestionCase({code: '~~object["a"] === object.a', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(object.a)',
}]}),
suggestionCase({code: '~~object?.a === object.a', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(object.a)',
}]}),
suggestionCase({code: '~~object?.a === object?.a', suggestions: [{
messageId: 'preferNumberIsIntegerSuggestion',
output: 'Number.isInteger(object.a)',
}]}),
],
});