Skip to content

Commit

Permalink
prefer-number-is-integer: handle any reference in integer check
Browse files Browse the repository at this point in the history
  • Loading branch information
@tristanHessell
tristanHessell committed Sep 11, 2021
1 parent 917fd3a commit 2e39762
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 148 deletions.
1 change: 1 addition & 0 deletions configs/recommended.js
View file
Expand Up @@ -71,6 +71,7 @@ module.exports = {
'unicorn/prefer-negative-index': 'error',
'unicorn/prefer-node-protocol': 'error',
'unicorn/prefer-number-properties': 'error',
'unicorn/prefer-number-is-integer': 'error',
'unicorn/prefer-object-from-entries': 'error',
// TODO: Enable this by default when targeting a Node.js version that supports `Object.hasOwn`.
'unicorn/prefer-object-has-own': 'off',
Expand Down
14 changes: 11 additions & 3 deletions docs/rules/prefer-number-is-integer.md
View file
Expand Up @@ -2,14 +2,22 @@

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.

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

For example:

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

number % 1 === 0; // true
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.
Expand Down
120 changes: 2 additions & 118 deletions index.js
View file
Expand Up @@ -28,123 +28,7 @@ module.exports = {
...deprecatedRules,
},
configs: {
recommended: {
env: {
es6: true,
},
parserOptions: {
ecmaVersion: 2021,
sourceType: 'module',
},
plugins: [
'unicorn',
],
rules: {
'unicorn/better-regex': 'error',
'unicorn/catch-error-name': 'error',
'unicorn/consistent-destructuring': 'error',
'unicorn/consistent-function-scoping': 'error',
'unicorn/custom-error-definition': 'off',
'unicorn/empty-brace-spaces': 'error',
'unicorn/error-message': 'error',
'unicorn/escape-case': 'error',
'unicorn/expiring-todo-comments': 'error',
'unicorn/explicit-length-check': 'error',
'unicorn/filename-case': 'error',
'unicorn/import-index': 'off',
'unicorn/import-style': 'error',
'unicorn/new-for-builtins': 'error',
'unicorn/no-abusive-eslint-disable': 'error',
'unicorn/no-array-callback-reference': 'error',
'unicorn/no-array-for-each': 'error',
'unicorn/no-array-method-this-argument': 'error',
'unicorn/no-array-push-push': 'error',
'unicorn/no-array-reduce': 'error',
'unicorn/no-console-spaces': 'error',
'unicorn/no-document-cookie': 'error',
'unicorn/no-for-loop': 'error',
'unicorn/no-hex-escape': 'error',
'unicorn/no-instanceof-array': 'error',
'unicorn/no-invalid-remove-event-listener': 'error',
'unicorn/no-keyword-prefix': 'off',
'unicorn/no-lonely-if': 'error',
'no-nested-ternary': 'off',
'unicorn/no-nested-ternary': 'error',
'unicorn/no-new-array': 'error',
'unicorn/no-new-buffer': 'error',
'unicorn/no-null': 'error',
'unicorn/no-object-as-default-parameter': 'error',
'unicorn/no-process-exit': 'error',
'unicorn/no-static-only-class': 'error',
'unicorn/no-this-assignment': 'error',
'unicorn/no-unreadable-array-destructuring': 'error',
'unicorn/no-unsafe-regex': 'off',
'unicorn/no-unused-properties': 'off',
'unicorn/no-useless-fallback-in-spread': 'error',
'unicorn/no-useless-length-check': 'error',
'unicorn/no-useless-spread': 'error',
'unicorn/no-useless-undefined': 'error',
'unicorn/no-zero-fractions': 'error',
'unicorn/number-literal-case': 'error',
'unicorn/numeric-separators-style': 'error',
'unicorn/prefer-add-event-listener': 'error',
'unicorn/prefer-array-find': 'error',
'unicorn/prefer-array-flat': 'error',
'unicorn/prefer-array-flat-map': 'error',
'unicorn/prefer-array-index-of': 'error',
'unicorn/prefer-array-some': 'error',
// TODO: Enable this by default when targeting a Node.js version that supports `Array#at`.
'unicorn/prefer-at': 'off',
'unicorn/prefer-date-now': 'error',
'unicorn/prefer-default-parameters': 'error',
'unicorn/prefer-dom-node-append': 'error',
'unicorn/prefer-dom-node-dataset': 'error',
'unicorn/prefer-dom-node-remove': 'error',
'unicorn/prefer-dom-node-text-content': 'error',
'unicorn/prefer-includes': 'error',
'unicorn/prefer-keyboard-event-key': 'error',
'unicorn/prefer-math-trunc': 'error',
'unicorn/prefer-modern-dom-apis': 'error',
'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',
// TODO: Enable this by default when targeting a Node.js version that supports `Object.hasOwn`.
'unicorn/prefer-object-has-own': 'off',
'unicorn/prefer-optional-catch-binding': 'error',
'unicorn/prefer-prototype-methods': 'error',
'unicorn/prefer-query-selector': 'error',
'unicorn/prefer-reflect-apply': 'error',
'unicorn/prefer-regexp-test': 'error',
'unicorn/prefer-set-has': 'error',
'unicorn/prefer-spread': 'error',
// TODO: Enable this by default when targeting Node.js 16.
'unicorn/prefer-string-replace-all': 'off',
'unicorn/prefer-string-slice': 'error',
'unicorn/prefer-string-starts-ends-with': 'error',
'unicorn/prefer-string-trim-start-end': 'error',
'unicorn/prefer-switch': 'error',
'unicorn/prefer-ternary': 'error',
// TODO: Enable this by default when targeting Node.js 14.
'unicorn/prefer-top-level-await': 'off',
'unicorn/prefer-type-error': 'error',
'unicorn/prevent-abbreviations': 'error',
'unicorn/require-array-join-separator': 'error',
'unicorn/require-number-to-fixed-digits-argument': 'error',
'unicorn/require-post-message-target-origin': 'error',
'unicorn/string-content': 'off',
'unicorn/throw-new-error': 'error',
},
overrides: [
{
files: ['*.ts', '*.tsx'],
rules: {
'unicorn/require-post-message-target-origin': 'off',
},
},
],
},
recommended: recommendedConfig,
all,
},
};
61 changes: 34 additions & 27 deletions rules/prefer-number-is-integer.js
View file
@@ -1,5 +1,7 @@
'use strict';

const isSameReference = require('./utils/is-same-reference.js');

const MESSAGE_ID = 'preferNumberIsInteger';
const MESSAGE_ID_SUGGEST = 'preferNumberIsIntegerSuggestion';
const messages = {
Expand All @@ -13,7 +15,6 @@ const equalsSelector = ':matches([operator="==="],[operator="=="])';
const modulo1Selector = [
'BinaryExpression',
'[left.type="BinaryExpression"]',
'[left.left.type="Identifier"]',
'[left.operator="%"]',
'[left.right.value=1]',
equalsSelector,
Expand All @@ -24,22 +25,18 @@ const modulo1Selector = [
const mathOperatorSelector = [
'BinaryExpression',
'[left.type="BinaryExpression"]',
'[left.left.type="Identifier"]',
`:matches(${['^', '|'].map(operator => `[left.operator="${operator}"]`).join(',')})`,
'[left.right.value=0]',
equalsSelector,
'[right.type="Identifier"]',
].join('');

// ParseInt(value,10) === value
const parseIntSelector = [
'BinaryExpression',
'[left.type="CallExpression"]',
'[left.callee.name="parseInt"]',
'[left.arguments.0.type="Identifier"]',
'[left.arguments.1.value=10]',
equalsSelector,
'[right.type="Identifier"]',
].join('');

// _.isInteger(value)
Expand All @@ -56,9 +53,7 @@ const mathRoundSelector = [
'[left.callee.type="MemberExpression"]',
'[left.callee.object.name="Math"]',
'[left.callee.property.name="round"]',
'[left.arguments.0.type="Identifier"]',
equalsSelector,
'[right.type="Identifier"]',
].join('');

// ~~value === value
Expand All @@ -68,9 +63,7 @@ const bitwiseNotSelector = [
'[left.operator="~"]',
'[left.argument.type="UnaryExpression"]',
'[left.argument.operator="~"]',
'[left.argument.argument.type="Identifier"]',
equalsSelector,
'[right.type="Identifier"]',
].join('');

function createNodeListener(sourceCode, variableGetter) {
Expand All @@ -97,41 +90,55 @@ function createNodeListener(sourceCode, variableGetter) {
};
}

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 '';
}
}
}

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

return {
[modulo1Selector]: createNodeListener(sourceCode, node => node.left.left.name),
[modulo1Selector]: createNodeListener(sourceCode, node => getNodeName(node.left.left)),
[mathOperatorSelector]: createNodeListener(sourceCode, node => {
const variableName = node.right.name;

if (variableName === node.left.left.name) {
return variableName;
if (isSameReference(node.left.left, node.right)) {
return getNodeName(node.right);
}
}),
[parseIntSelector]: createNodeListener(sourceCode, node => {
const variableName = node.right.name;

if (
node.left.arguments[0].name === variableName
isSameReference(node.left.arguments[0], node.right)
) {
return variableName;
return getNodeName(node.right);
}
}),
[lodashIsIntegerSelector]: createNodeListener(sourceCode, node => node.arguments[0].name),
[lodashIsIntegerSelector]: createNodeListener(sourceCode, node => getNodeName(node.arguments[0])),
[mathRoundSelector]: createNodeListener(sourceCode, node => {
const variableName = node.right.name;

if (node.left.arguments[0].name === variableName) {
return variableName;
if (
isSameReference(node.left.arguments[0], node.right)
) {
return getNodeName(node.right);
}
}),
[bitwiseNotSelector]: createNodeListener(sourceCode, node => {
const variableName = node.right.name;

if (node.left.argument.argument.name === variableName) {
return variableName;
if (isSameReference(node.left.argument.argument, node.right)) {
return getNodeName(node.right);
}
}),
};
Expand Down
20 changes: 20 additions & 0 deletions test/prefer-number-is-integer.mjs
View file
Expand Up @@ -84,5 +84,25 @@ test({
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)',
}]}),
],
});

0 comments on commit 2e39762

Please sign in to comment.