Add no-reduce rule (#704)

Co-authored-by: fisker Cheung <[email protected]>
Co-authored-by: Sindre Sorhus <[email protected]>

Author: 3 people

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;
+}
+```

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',

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.

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`.

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)))
 	];
 };
 

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

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);
 

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 {

test/lint/lint.js

@@ -22,17 +22,26 @@ 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);
 
 	if (fix) {
 		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;