diff --git a/docs/rules/no-reduce.md b/docs/rules/no-reduce.md new file mode 100644 index 0000000000..381daf119a --- /dev/null +++ b/docs/rules/no-reduce.md @@ -0,0 +1,48 @@ +# Disallow `Array#reduce()` and `Array#reduceRight()` + +`Array#reduce()` and `Array#reduceRight()` usually [result in hard-to-read code](https://twitter.com/jaffathecake/status/1213077702300852224). In almost every case, it can be replaced by `.map`, `.filter`, or a `for-of` loop. It's only somewhat useful in the rare case of summing up numbers. + +Use `eslint-disable` comment if you really need to use it. + +This rule is not fixable. + +## Fail + +```js +array.reduce(reducer, initialValue); +``` + +```js +array.reduceRight(reducer, initialValue); +``` + +```js +array.reduce(reducer); +``` + +```js +[].reduce.call(array, reducer); +``` + +```js +[].reduce.apply(array, [reducer, initialValue]); +``` + +```js +Array.prototype.reduce.call(array, reducer); +``` + +## Pass + +```js +// eslint-disable-next-line +array.reduce(reducer, initialValue); +``` + +```js +let result = initialValue; + +for (const element of array) { + result += element; +} +``` diff --git a/index.js b/index.js index 60a11d7e8f..7a9355c12f 100644 --- a/index.js +++ b/index.js @@ -40,6 +40,7 @@ module.exports = { 'unicorn/no-new-buffer': 'error', 'unicorn/no-null': 'error', 'unicorn/no-process-exit': 'error', + 'unicorn/no-reduce': 'error', 'unicorn/no-unreadable-array-destructuring': 'error', 'unicorn/no-unsafe-regex': 'off', 'unicorn/no-unused-properties': 'off', diff --git a/readme.md b/readme.md index 106cbaded4..de9484b9d4 100644 --- a/readme.md +++ b/readme.md @@ -56,6 +56,7 @@ Configure it in `package.json`. "unicorn/no-new-buffer": "error", "unicorn/no-null": "error", "unicorn/no-process-exit": "error", + "unicorn/no-reduce": "error", "unicorn/no-unreadable-array-destructuring": "error", "unicorn/no-unsafe-regex": "off", "unicorn/no-unused-properties": "off", @@ -115,6 +116,7 @@ Configure it in `package.json`. - [no-new-buffer](docs/rules/no-new-buffer.md) - Enforce the use of `Buffer.from()` and `Buffer.alloc()` instead of the deprecated `new Buffer()`. *(fixable)* - [no-null](docs/rules/no-null.md) - Disallow the use of the `null` literal. - [no-process-exit](docs/rules/no-process-exit.md) - Disallow `process.exit()`. +- [no-reduce](docs/rules/no-reduce.md) - Disallow `Array#reduce()` and `Array#reduceRight()`. - [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) - Disallow unreadable array destructuring. - [no-unsafe-regex](docs/rules/no-unsafe-regex.md) - Disallow unsafe regular expressions. - [no-unused-properties](docs/rules/no-unused-properties.md) - Disallow unused object properties. diff --git a/rules/expiring-todo-comments.js b/rules/expiring-todo-comments.js index 2c3c1f6106..f8ec57d3df 100644 --- a/rules/expiring-todo-comments.js +++ b/rules/expiring-todo-comments.js @@ -4,6 +4,7 @@ const semver = require('semver'); const ci = require('ci-info'); const baseRule = require('eslint/lib/rules/no-warning-comments'); const getDocumentationUrl = require('./utils/get-documentation-url'); +const {flatten} = require('lodash'); // `unicorn/` prefix is added to avoid conflicts with core rule const MESSAGE_ID_AVOID_MULTIPLE_DATES = 'unicorn/avoidMultipleDates'; @@ -50,17 +51,21 @@ function parseTodoWithArguments(string, {terms}) { const {rawArguments} = result.groups; - return rawArguments + const parsedArguments = rawArguments .split(',') - .map(argument => parseArgument(argument.trim())) - .reduce((groups, argument) => { - if (!groups[argument.type]) { - groups[argument.type] = []; - } + .map(argument => parseArgument(argument.trim())); + + return createArgumentGroup(parsedArguments); +} + +function createArgumentGroup(arguments_) { + const groups = {}; + for (const {value, type} of arguments_) { + groups[type] = groups[type] || []; + groups[type].push(value); + } - groups[argument.type].push(argument.value); - return groups; - }, {}); + return groups; } function parseArgument(argumentString) { @@ -220,24 +225,23 @@ const create = context => { const sourceCode = context.getSourceCode(); const comments = sourceCode.getAllComments(); - const unusedComments = comments - .filter(token => token.type !== 'Shebang') - // Block comments come as one. - // Split for situations like this: - // /* - // * TODO [2999-01-01]: Validate this - // * TODO [2999-01-01]: And this - // * TODO [2999-01-01]: Also this - // */ - .map(comment => - comment.value.split('\n').map(line => ({ - ...comment, - value: line - })) - ) - // Flatten - .reduce((accumulator, array) => accumulator.concat(array), []) - .filter(comment => processComment(comment)); + const unusedComments = flatten( + comments + .filter(token => token.type !== 'Shebang') + // Block comments come as one. + // Split for situations like this: + // /* + // * TODO [2999-01-01]: Validate this + // * TODO [2999-01-01]: And this + // * TODO [2999-01-01]: Also this + // */ + .map(comment => + comment.value.split('\n').map(line => ({ + ...comment, + value: line + })) + ) + ).filter(comment => processComment(comment)); // This is highly dependable on ESLint's `no-warning-comments` implementation. // What we do is patch the parts we know the rule will use, `getAllComments`. diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js index 848e859617..283ace5367 100644 --- a/rules/no-for-loop.js +++ b/rules/no-for-loop.js @@ -1,6 +1,7 @@ 'use strict'; const getDocumentationUrl = require('./utils/get-documentation-url'); const isLiteralValue = require('./utils/is-literal-value'); +const {flatten} = require('lodash'); const defaultElementName = 'element'; const isLiteralZero = node => isLiteralValue(node, 0); @@ -256,9 +257,7 @@ const getReferencesInChildScopes = (scope, name) => { const references = scope.references.filter(reference => reference.identifier.name === name); return [ ...references, - ...scope.childScopes - .map(s => getReferencesInChildScopes(s, name)) - .reduce((accumulator, scopeReferences) => [...accumulator, ...scopeReferences], []) + ...flatten(scope.childScopes.map(s => getReferencesInChildScopes(s, name))) ]; }; diff --git a/rules/no-reduce.js b/rules/no-reduce.js new file mode 100644 index 0000000000..98f735f22f --- /dev/null +++ b/rules/no-reduce.js @@ -0,0 +1,78 @@ +'use strict'; +const methodSelector = require('./utils/method-selector'); +const getDocumentationUrl = require('./utils/get-documentation-url'); + +const MESSAGE_ID_REDUCE = 'reduce'; +const MESSAGE_ID_REDUCE_RIGHT = 'reduceRight'; + +const ignoredFirstArgumentSelector = `:not(${ + [ + '[arguments.0.type="Literal"]', + '[arguments.0.type="Identifier"][arguments.0.name="undefined"]' + ].join(',') +})`; + +const PROTOTYPE_SELECTOR = [ + methodSelector({names: ['call', 'apply']}), + ignoredFirstArgumentSelector, + '[callee.object.type="MemberExpression"]', + '[callee.object.computed=false]', + `:matches(${ + ['reduce', 'reduceRight'].map(method => `[callee.object.property.name="${method}"]`).join(', ') + })`, + '[callee.object.property.type="Identifier"]', + `:matches(${ + [ + // `[].reduce` + [ + 'type="ArrayExpression"', + 'elements.length=0' + ], + // `Array.prototype.reduce` + [ + 'type="MemberExpression"', + 'computed=false', + 'property.type="Identifier"', + 'property.name="prototype"', + 'object.type="Identifier"', + 'object.name="Array"' + ] + ].map( + selectors => selectors + .map(selector => `[callee.object.object.${selector}]`) + .join('') + ).join(', ') + })` +].join(''); + +const METHOD_SELECTOR = [ + methodSelector({names: ['reduce', 'reduceRight'], min: 1, max: 2}), + ignoredFirstArgumentSelector +].join(''); + +const create = context => { + return { + [METHOD_SELECTOR](node) { + // For arr.reduce() + context.report({node: node.callee.property, messageId: node.callee.property.name}); + }, + [PROTOTYPE_SELECTOR](node) { + // For cases [].reduce.call() and Array.prototype.reduce.call() + context.report({node: node.callee.object.property, messageId: node.callee.object.property.name}); + } + }; +}; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocumentationUrl(__filename) + }, + messages: { + [MESSAGE_ID_REDUCE]: '`Array#reduce()` is not allowed', + [MESSAGE_ID_REDUCE_RIGHT]: '`Array#reduceRight()` is not allowed' + } + } +}; diff --git a/rules/prefer-add-event-listener.js b/rules/prefer-add-event-listener.js index 88b03308d8..41c8aedb53 100644 --- a/rules/prefer-add-event-listener.js +++ b/rules/prefer-add-event-listener.js @@ -1,6 +1,7 @@ 'use strict'; const getDocumentationUrl = require('./utils/get-documentation-url'); const domEventsJson = require('./utils/dom-events.json'); +const {flatten} = require('lodash'); const message = 'Prefer `{{replacement}}` over `{{method}}`.{{extra}}'; const extraMessages = { @@ -9,7 +10,7 @@ const extraMessages = { }; const nestedEvents = Object.values(domEventsJson); -const eventTypes = new Set(nestedEvents.reduce((accumulatorEvents, events) => accumulatorEvents.concat(events), [])); +const eventTypes = new Set(flatten(nestedEvents)); const getEventMethodName = memberExpression => memberExpression.property.name; const getEventTypeName = eventMethodName => eventMethodName.slice('on'.length); diff --git a/rules/utils/cartesian-product-samples.js b/rules/utils/cartesian-product-samples.js index 276d25f611..c30773eff7 100644 --- a/rules/utils/cartesian-product-samples.js +++ b/rules/utils/cartesian-product-samples.js @@ -1,16 +1,29 @@ 'use strict'; +const getTotal = combinations => { + let total = 1; + for (const {length} of combinations) { + total *= length; + } + + return total; +}; + module.exports = (combinations, length = Infinity) => { - const total = combinations.reduce((total, {length}) => total * length, 1); + const total = getTotal(combinations); const samples = Array.from({length: Math.min(total, length)}, (_, sampleIndex) => { let indexRemaining = sampleIndex; - return combinations.reduceRight((combination, items) => { + const combination = []; + for (let i = combinations.length - 1; i >= 0; i--) { + const items = combinations[i]; const {length} = items; const index = indexRemaining % length; indexRemaining = (indexRemaining - index) / length; - return [items[index], ...combination]; - }, []); + combination.unshift(items[index]); + } + + return combination; }); return { diff --git a/test/lint/lint.js b/test/lint/lint.js index c7ba7a4d0f..9776b88101 100755 --- a/test/lint/lint.js +++ b/test/lint/lint.js @@ -22,6 +22,15 @@ const eslint = new ESLint({ } }); +const sum = (collection, fieldName) => { + let result = 0; + for (const item of collection) { + result += item[fieldName]; + } + + return result; +}; + (async function () { const results = await eslint.lintFiles(files); @@ -29,10 +38,10 @@ const eslint = new ESLint({ await ESLint.outputFixes(results); } - const errorCount = results.reduce((total, {errorCount}) => total + errorCount, 0); - const warningCount = results.reduce((total, {warningCount}) => total + warningCount, 0); - const fixableErrorCount = results.reduce((total, {fixableErrorCount}) => total + fixableErrorCount, 0); - const fixableWarningCount = results.reduce((total, {fixableWarningCount}) => total + fixableWarningCount, 0); + const errorCount = sum(results, 'errorCount'); + const warningCount = sum(results, 'warningCount'); + const fixableErrorCount = sum(results, 'fixableErrorCount'); + const fixableWarningCount = sum(results, 'fixableWarningCount'); const hasFixable = fixableErrorCount || fixableWarningCount; diff --git a/test/no-reduce.js b/test/no-reduce.js new file mode 100644 index 0000000000..4cbacc5201 --- /dev/null +++ b/test/no-reduce.js @@ -0,0 +1,138 @@ +import test from 'ava'; +import avaRuleTester from 'eslint-ava-rule-tester'; +import {flatten} from 'lodash'; +import rule from '../rules/no-reduce'; +import {outdent} from 'outdent'; + +const MESSAGE_ID_REDUCE = 'reduce'; +const MESSAGE_ID_REDUCE_RIGHT = 'reduceRight'; + +const ruleTester = avaRuleTester(test, { + parserOptions: { + ecmaVersion: 2020 + } +}); + +const errorsReduce = [{messageId: MESSAGE_ID_REDUCE}]; +const errorsReduceRight = [{messageId: MESSAGE_ID_REDUCE_RIGHT}]; + +const tests = { + valid: flatten([ + 'a[b.reduce]()', + 'a(b.reduce)', + 'a.reduce()', + 'a.reduce(1, 2, 3)', + 'a.reduce(b, c, d)', + '[][reduce].call()', + '[1, 2].reduce.call(() => {}, 34)', + + // First argument is not a function + 'a.reduce(123)', + 'a.reduce(\'abc\')', + 'a.reduce(null)', + 'a.reduce(undefined)', + 'a.reduce(123, initialValue)', + 'a.reduce(\'abc\', initialValue)', + 'a.reduce(null, initialValue)', + 'a.reduce(undefined, initialValue)', + + // Test `.reduce` + // Not `CallExpression` + 'new foo.reduce(fn);', + // Not `MemberExpression` + 'reduce(fn);', + // `callee.property` is not a `Identifier` + 'foo["reduce"](fn);', + // Computed + 'foo[reduce](fn);', + // Not listed method or property + 'foo.notListed(fn);', + // More or less argument(s) + 'foo.reduce();', + 'foo.reduce(fn, extraArgument1, extraArgument2);', + 'foo.reduce(...argumentsArray)', + + // Test `[].reduce.{call,apply}` + // Not `CallExpression` + 'new [].reduce.call(foo, fn);', + // Not `MemberExpression` + 'call(foo, fn);', + 'reduce.call(foo, fn);', + // `callee.property` is not a `Identifier` + '[].reduce["call"](foo, fn);', + '[]["reduce"].call(foo, fn);', + // Computed + '[].reduce[call](foo, fn);', + '[][reduce].call(foo, fn);', + // Not listed method or property + '[].reduce.notListed(foo, fn);', + '[].notListed.call(foo, fn);', + // Not empty + '[1].reduce.call(foo, fn)', + // Not ArrayExpression + '"".reduce.call(foo, fn)', + // More or less argument(s) + // We are not checking arguments length + + // Test `Array.prototype.{call,apply}` + // Not `CallExpression` + 'new Array.prototype.reduce.call(foo, fn);', + // Not `MemberExpression` + 'call(foo, fn);', + 'reduce.call(foo, fn);', + // `callee.property` is not a `Identifier` + 'Array.prototype.reduce["call"](foo, fn);', + 'Array.prototype["reduce"].call(foo, fn);', + 'Array["prototype"].reduce.call(foo, fn);', + '"Array".prototype.reduce.call(foo, fn);', + // Computed + 'Array.prototype.reduce[call](foo, fn);', + 'Array.prototype[reduce].call(foo, fn);', + 'Array[prototype].reduce.call(foo, fn);', + // Not listed method + 'Array.prototype.reduce.notListed(foo, fn);', + 'Array.prototype.notListed.call(foo, fn);', + 'Array.notListed.reduce.call(foo, fn);', + // Not `Array` + 'NotArray.prototype.reduce.call(foo, fn);', + // More or less argument(s) + // We are not checking arguments length + + // `reduce-like` + 'arr.reducex(foo)', + 'arr.xreduce(foo)', + '[].reducex.call(arr, foo)', + '[].xreduce.call(arr, foo)', + 'Array.prototype.reducex.call(arr, foo)', + 'Array.prototype.xreduce.call(arr, foo)' + ].map(code => [code, code.replace('reduce', 'reduceRight')])), + invalid: flatten([ + 'arr.reduce((total, item) => total + item)', + 'arr.reduce((total, item) => total + item, 0)', + 'arr.reduce(function (total, item) { return total + item }, 0)', + 'arr.reduce(function (total, item) { return total + item })', + 'arr.reduce((str, item) => str += item, "")', + outdent` + arr.reduce((obj, item) => { + obj[item] = null; + return obj; + }, {}) + `, + 'arr.reduce((obj, item) => ({ [item]: null }), {})', + outdent` + const hyphenate = (str, char) => \`\${str}-\${char}\`; + ["a", "b", "c"].reduce(hyphenate); + `, + '[].reduce.call(arr, (s, i) => s + i)', + '[].reduce.call(arr, sum);', + '[].reduce.call(sum);', + 'Array.prototype.reduce.call(arr, (s, i) => s + i)', + 'Array.prototype.reduce.call(arr, sum);', + '[].reduce.apply(arr, [(s, i) => s + i])', + '[].reduce.apply(arr, [sum]);', + 'Array.prototype.reduce.apply(arr, [(s, i) => s + i])', + 'Array.prototype.reduce.apply(arr, [sum]);' + ].map(code => [{code, errors: errorsReduce}, {code: code.replace('reduce', 'reduceRight'), errors: errorsReduceRight}])) +}; + +ruleTester.run('no-reduce', rule, tests);