diff --git a/docs/rules/prefer-reflect-apply.md b/docs/rules/prefer-reflect-apply.md new file mode 100644 index 0000000000..1da14684da --- /dev/null +++ b/docs/rules/prefer-reflect-apply.md @@ -0,0 +1,30 @@ +# Prefer `Reflect.apply()` over `Function#apply()` + +[`Reflect.apply)()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/apply) is arguably less verbose and easier to understand. In addition, when you accept arbitrary methods, it's not safe to assume `.apply()` exists or is not overridden. + +This rule is fixable. + + +## Fail + +```js +function foo() {} + +foo.apply(null, [42]); +Function.prototype.apply.call(foo, null, [42]); +foo.apply(this, [42]); +Function.prototype.apply.call(foo, this, [42]); +foo.apply(null, arguments); +Function.prototype.apply.call(foo, null, arguments); +foo.apply(this, arguments); +Function.prototype.apply.call(foo, this, arguments); +``` + + +## Pass + +```js +function foo() {} + +Reflect.apply(foo, null, [42]); +``` diff --git a/index.js b/index.js index 3aeb0d366a..8d096e1391 100644 --- a/index.js +++ b/index.js @@ -50,6 +50,7 @@ module.exports = { 'unicorn/prefer-node-append': 'error', 'unicorn/prefer-node-remove': 'error', 'unicorn/prefer-query-selector': 'error', + 'unicorn/prefer-reflect-apply': 'error', 'unicorn/prefer-spread': 'error', 'unicorn/prefer-starts-ends-with': 'error', 'unicorn/prefer-text-content': 'error', diff --git a/readme.md b/readme.md index 710f351b78..2cf67cedd1 100644 --- a/readme.md +++ b/readme.md @@ -68,6 +68,7 @@ Configure it in `package.json`. "unicorn/prefer-node-append": "error", "unicorn/prefer-node-remove": "error", "unicorn/prefer-query-selector": "error", + "unicorn/prefer-reflect-apply": "error", "unicorn/prefer-spread": "error", "unicorn/prefer-starts-ends-with": "error", "unicorn/prefer-text-content": "error", @@ -116,6 +117,7 @@ Configure it in `package.json`. - [prefer-node-append](docs/rules/prefer-node-append.md) - Prefer `Node#append()` over `Node#appendChild()`. *(fixable)* - [prefer-node-remove](docs/rules/prefer-node-remove.md) - Prefer `node.remove()` over `parentNode.removeChild(node)` and `parentElement.removeChild(node)`. *(fixable)* - [prefer-query-selector](docs/rules/prefer-query-selector.md) - Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`. *(partly fixable)* +- [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md) - Prefer `Reflect.apply()` over `Function#apply()`. *(fixable)* - [prefer-spread](docs/rules/prefer-spread.md) - Prefer the spread operator over `Array.from()`. *(fixable)* - [prefer-starts-ends-with](docs/rules/prefer-starts-ends-with.md) - Prefer `String#startsWith()` & `String#endsWith()` over more complex alternatives. - [prefer-text-content](docs/rules/prefer-text-content.md) - Prefer `.textContent` over `.innerText`. *(fixable)* diff --git a/rules/prefer-reflect-apply.js b/rules/prefer-reflect-apply.js new file mode 100644 index 0000000000..c8387b3961 --- /dev/null +++ b/rules/prefer-reflect-apply.js @@ -0,0 +1,86 @@ +'use strict'; +const astUtils = require('eslint-ast-utils'); +const getDocsUrl = require('./utils/get-docs-url'); + +const isApplySignature = (argument1, argument2) => ( + ((argument1.type === 'Literal' && argument1.raw === 'null') || + argument1.type === 'ThisExpression') && + (argument2.type === 'ArrayExpression' || + (argument2.type === 'Identifier' && + argument2.name === 'arguments')) +); + +const getReflectApplyCall = (sourceCode, func, receiver, args) => ( + `Reflect.apply(${sourceCode.getText(func)}, ${sourceCode.getText(receiver)}, ${sourceCode.getText(args)})` +); + +const fixDirectApplyCall = (node, sourceCode) => { + if ( + astUtils.getPropertyName(node.callee) === 'apply' && + node.arguments.length === 2 && + isApplySignature(node.arguments[0], node.arguments[1]) + ) { + return fixer => ( + fixer.replaceText( + node, + getReflectApplyCall(sourceCode, node.callee.object, node.arguments[0], node.arguments[1]) + ) + ); + } +}; + +const fixFunctionPrototypeCall = (node, sourceCode) => { + if ( + astUtils.getPropertyName(node.callee) === 'call' && + astUtils.getPropertyName(node.callee.object) === 'apply' && + astUtils.getPropertyName(node.callee.object.object) === 'prototype' && + node.callee.object.object.object && + node.callee.object.object.object.type === 'Identifier' && + node.callee.object.object.object.name === 'Function' && + node.arguments.length === 3 && + isApplySignature(node.arguments[1], node.arguments[2]) + ) { + return fixer => ( + fixer.replaceText( + node, + getReflectApplyCall(sourceCode, node.arguments[0], node.arguments[1], node.arguments[2]) + ) + ); + } +}; + +const create = context => { + return { + CallExpression: node => { + if ( + !( + node.callee.type === 'MemberExpression' && + !['Literal', 'ArrayExpression', 'ObjectExpression'].includes(node.callee.object.type) + ) + ) { + return; + } + + const sourceCode = context.getSourceCode(); + const fix = fixDirectApplyCall(node, sourceCode) || fixFunctionPrototypeCall(node, sourceCode); + if (fix) { + context.report({ + node, + message: 'Prefer `Reflect.apply()` over `Function#apply()`.', + fix + }); + } + } + }; +}; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocsUrl(__filename) + }, + fixable: 'code' + } +}; diff --git a/test/prefer-reflect-apply.js b/test/prefer-reflect-apply.js new file mode 100644 index 0000000000..c45aeae36d --- /dev/null +++ b/test/prefer-reflect-apply.js @@ -0,0 +1,85 @@ +import test from 'ava'; +import avaRuleTester from 'eslint-ava-rule-tester'; +import rule from '../rules/prefer-reflect-apply'; + +const ruleTester = avaRuleTester(test, { + env: { + es6: true + } +}); + +const errors = [ + { + ruleId: 'prefer-reflect-apply' + } +]; + +ruleTester.run('prefer-reflect-apply', rule, { + valid: [ + 'foo.apply();', + 'foo.apply(null);', + 'foo.apply(this);', + 'foo.apply(null, 42);', + 'foo.apply(this, 42);', + 'foo.apply(bar, arguments);', + '[].apply(null, [42]);', + 'foo.apply(bar);', + 'foo.apply(bar, []);', + 'foo.apply;', + 'apply;', + 'Reflect.apply(foo, null);', + 'Reflect.apply(foo, null, [bar]);' + ], + invalid: [ + { + code: 'foo.apply(null, [42]);', + output: 'Reflect.apply(foo, null, [42]);', + errors + }, + { + code: 'foo.bar.apply(null, [42]);', + output: 'Reflect.apply(foo.bar, null, [42]);', + errors + }, + { + code: 'Function.prototype.apply.call(foo, null, [42]);', + output: 'Reflect.apply(foo, null, [42]);', + errors + }, + { + code: 'Function.prototype.apply.call(foo.bar, null, [42]);', + output: 'Reflect.apply(foo.bar, null, [42]);', + errors + }, + { + code: 'foo.apply(null, arguments);', + output: 'Reflect.apply(foo, null, arguments);', + errors + }, + { + code: 'Function.prototype.apply.call(foo, null, arguments);', + output: 'Reflect.apply(foo, null, arguments);', + errors + }, + { + code: 'foo.apply(this, [42]);', + output: 'Reflect.apply(foo, this, [42]);', + errors + }, + { + code: 'Function.prototype.apply.call(foo, this, [42]);', + output: 'Reflect.apply(foo, this, [42]);', + errors + }, + { + code: 'foo.apply(this, arguments);', + output: 'Reflect.apply(foo, this, arguments);', + errors + }, + { + code: 'Function.prototype.apply.call(foo, this, arguments);', + output: 'Reflect.apply(foo, this, arguments);', + errors + } + ] +});