Skip to content

Commit

Permalink
Add prefer-native-coercion-functions rule (#1767)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker committed Apr 1, 2022
1 parent 1d32db4 commit 51166f4
Show file tree
Hide file tree
Showing 7 changed files with 1,007 additions and 0 deletions.
1 change: 1 addition & 0 deletions configs/recommended.js
Expand Up @@ -87,6 +87,7 @@ module.exports = {
'unicorn/prefer-modern-dom-apis': 'error',
'unicorn/prefer-modern-math-apis': 'error',
'unicorn/prefer-module': 'error',
'unicorn/prefer-native-coercion-functions': 'error',
'unicorn/prefer-negative-index': 'error',
'unicorn/prefer-node-protocol': 'error',
'unicorn/prefer-number-properties': 'error',
Expand Down
68 changes: 68 additions & 0 deletions docs/rules/prefer-native-coercion-functions.md
@@ -0,0 +1,68 @@
# Prefer using `String`, `Number`, `BigInt`, `Boolean`, and `Symbol` directly

<!-- 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 is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems).*
<!-- /RULE_NOTICE -->

If a function is equivalent to [`String`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String), [`Number`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number), [`BigInt`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt), [`Boolean`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean), or [`Symbol`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol), you should use the built-in one directly. Wrapping the built-in in a function is moot.

## Fail

```js
const toBoolean = value => Boolean(value);
```

```js
function toNumber(value) {
return Number(value);
}

if (toNumber(foo) === 1) {}
```

```js
const hasTruthyValue = array.some(element => element);
```

## Pass

```js
const toBoolean = Boolean;
```

```js
if (Number(foo) === 1) {}
```

```js
const hasTruthyValue = array.some(Boolean);
```

```js
const toStringObject = value => new String(value);
```

```js
const toObject= value => Object(value);
```

## Note

We don't check implicit coercion like:

```js
const toString = value => '' + value;
```

```js
const toNumber = value => +value;
```

```js
const toBoolean = value => !!value;
```

It is recommended to enable the built-in ESLint rule [`no-implicit-coercion`](https://eslint.org/docs/rules/no-implicit-coercion) for a better experience.
1 change: 1 addition & 0 deletions readme.md
Expand Up @@ -127,6 +127,7 @@ Each rule has emojis denoting:
| [prefer-modern-dom-apis](docs/rules/prefer-modern-dom-apis.md) | Prefer `.before()` over `.insertBefore()`, `.replaceWith()` over `.replaceChild()`, prefer one of `.before()`, `.after()`, `.append()` or `.prepend()` over `insertAdjacentText()` and `insertAdjacentElement()`. || 🔧 | |
| [prefer-modern-math-apis](docs/rules/prefer-modern-math-apis.md) | Prefer modern `Math` APIs over legacy patterns. || 🔧 | |
| [prefer-module](docs/rules/prefer-module.md) | Prefer JavaScript modules (ESM) over CommonJS. || 🔧 | 💡 |
| [prefer-native-coercion-functions](docs/rules/prefer-native-coercion-functions.md) | Prefer using `String`, `Number`, `BigInt`, `Boolean`, and `Symbol` directly. || 🔧 | |
| [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-properties](docs/rules/prefer-number-properties.md) | Prefer `Number` static properties over global ones. || 🔧 | 💡 |
Expand Down
182 changes: 182 additions & 0 deletions rules/prefer-native-coercion-functions.js
@@ -0,0 +1,182 @@
'use strict';
const {getFunctionHeadLocation, getFunctionNameWithKind} = require('eslint-utils');
const {not} = require('./selectors/index.js');

const MESSAGE_ID = 'prefer-native-coercion-functions';
const messages = {
[MESSAGE_ID]: '{{functionNameWithKind}} is equivalent to `{{replacementFunction}}`. Use `{{replacementFunction}}` directly.',
};

const nativeCoercionFunctionNames = new Set(['String', 'Number', 'BigInt', 'Boolean', 'Symbol']);
const arrayMethodsWithBooleanCallback = new Set(['every', 'filter', 'find', 'findIndex', 'some']);

const isNativeCoercionFunctionCall = (node, firstArgumentName) =>
node
&& node.type === 'CallExpression'
&& !node.optional
&& node.callee.type === 'Identifier'
&& nativeCoercionFunctionNames.has(node.callee.name)
&& node.arguments[0]
&& node.arguments[0].type === 'Identifier'
&& node.arguments[0].name === firstArgumentName;

const isIdentityFunction = node =>
(
// `v => v`
node.type === 'ArrowFunctionExpression'
&& node.body.type === 'Identifier'
&& node.body.name === node.params[0].name
)
|| (
// `(v) => {return v;}`
// `function (v) {return v;}`
node.body.type === 'BlockStatement'
&& node.body.body.length === 1
&& node.body.body[0].type === 'ReturnStatement'
&& node.body.body[0].argument
&& node.body.body[0].argument.type === 'Identifier'
&& node.body.body[0].argument.name === node.params[0].name
);

const isArrayIdentityCallback = node =>
isIdentityFunction(node)
&& node.parent.type === 'CallExpression'
&& !node.parent.optional
&& node.parent.arguments[0] === node
&& node.parent.callee.type === 'MemberExpression'
&& !node.parent.callee.computed
&& !node.parent.callee.optional
&& node.parent.callee.property.type === 'Identifier'
&& arrayMethodsWithBooleanCallback.has(node.parent.callee.property.name);

function getCallExpression(node) {
const firstParameterName = node.params[0].name;

// `(v) => String(v)`
if (
node.type === 'ArrowFunctionExpression'
&& isNativeCoercionFunctionCall(node.body, firstParameterName)
) {
return node.body;
}

// `(v) => {return String(v);}`
// `function (v) {return String(v);}`
if (
node.body.type === 'BlockStatement'
&& node.body.body.length === 1
&& node.body.body[0].type === 'ReturnStatement'
&& isNativeCoercionFunctionCall(node.body.body[0].argument, firstParameterName)
) {
return node.body.body[0].argument;
}
}

const functionsSelector = [
':function',
'[async!=true]',
'[generator!=true]',
'[params.length>0]',
'[params.0.type="Identifier"]',
not([
'MethodDefinition[kind="constructor"] > .value',
'MethodDefinition[kind="set"] > .value',
'Property[kind="set"] > .value',
]),
].join('');

function getArrayCallbackProblem(node) {
if (!isArrayIdentityCallback(node)) {
return;
}

return {
replacementFunction: 'Boolean',
fix: fixer => fixer.replaceText(node, 'Boolean'),
};
}

function getCoercionFunctionProblem(node) {
const callExpression = getCallExpression(node);

if (!callExpression) {
return;
}

const {name} = callExpression.callee;

const problem = {replacementFunction: name};

if (node.type === 'FunctionDeclaration' || callExpression.arguments.length !== 1) {
return problem;
}

/** @param {import('eslint').Rule.RuleFixer} fixer */
problem.fix = fixer => {
let text = name;

if (
node.parent.type === 'Property'
&& node.parent.method
&& node.parent.value === node
) {
text = `: ${text}`;
} else if (node.parent.type === 'MethodDefinition') {
text = ` = ${text};`;
}

return fixer.replaceText(node, text);
};

return problem;
}

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
[functionsSelector](node) {
let problem = getArrayCallbackProblem(node) || getCoercionFunctionProblem(node);

if (!problem) {
return;
}

const sourceCode = context.getSourceCode();
const {replacementFunction, fix} = problem;

problem = {
node,
loc: getFunctionHeadLocation(node, sourceCode),
messageId: MESSAGE_ID,
data: {
functionNameWithKind: getFunctionNameWithKind(node, sourceCode),
replacementFunction,
},
};

/*
We do not fix if there are:
- Comments: No proper place to put them.
- Extra parameters: Removing them may break types.
*/
if (!fix || node.params.length !== 1 || sourceCode.getCommentsInside(node).length > 0) {
return problem;
}

problem.fix = fix;

return problem;
},
});

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Prefer using `String`, `Number`, `BigInt`, `Boolean`, and `Symbol` directly.',
},
fixable: 'code',
messages,
},
};

0 comments on commit 51166f4

Please sign in to comment.