proper-object-iterable-methods

docs/rules/proper-object-iterable-methods.md

@@ -0,0 +1,17 @@
+# Enforce proper methods when iterate through objects.
+
+<!-- More detailed description. Remove this comment. -->
+
+This rule is partly fixable.
+
+## Fail
+
+```js
+const foo = 'unicorn';
+```
+
+## Pass
+
+```js
+const foo = '🦄';
+```

index.js

@@ -115,6 +115,7 @@ module.exports = {
 				'unicorn/prefer-ternary': 'error',
 				'unicorn/prefer-type-error': 'error',
 				'unicorn/prevent-abbreviations': 'error',
+				'unicorn/proper-object-iterable-methods': 'error',
 				'unicorn/string-content': 'off',
 				'unicorn/throw-new-error': 'error'
 			}

readme.md

@@ -105,6 +105,7 @@ Configure it in `package.json`.
 			"unicorn/prefer-ternary": "off",
 			"unicorn/prefer-type-error": "error",
 			"unicorn/prevent-abbreviations": "error",
+			"unicorn/proper-object-iterable-methods": "error",
 			"unicorn/string-content": "off",
 			"unicorn/throw-new-error": "error"
 		}
@@ -195,6 +196,7 @@ Each rule has emojis denoting:
 | [prefer-ternary](docs/rules/prefer-ternary.md) | Prefer ternary expressions over simple `if-else` statements. | ✅ | 🔧 |
 | [prefer-type-error](docs/rules/prefer-type-error.md) | Enforce throwing `TypeError` in type checking conditions. | ✅ | 🔧 |
 | [prevent-abbreviations](docs/rules/prevent-abbreviations.md) | Prevent abbreviations. | ✅ | 🔧 |
+| [proper-object-iterable-methods](docs/rules/proper-object-iterable-methods.md) | Enforce proper methods when iterate through objects. | ✅ | 🔧 |
 | [string-content](docs/rules/string-content.md) | Enforce better string content. |  | 🔧 |
 | [throw-new-error](docs/rules/throw-new-error.md) | Require `new` when throwing an error. | ✅ | 🔧 |
 

rules/proper-object-iterable-methods.js

@@ -0,0 +1,232 @@
+'use strict';
+const {findVariable} = require('eslint-utils');
+const getDocumentationUrl = require('./utils/get-documentation-url');
+const methodSelector = require('./utils/method-selector');
+const isSameReference = require('./utils/is-same-reference');
+
+const MESSAGE_ID = 'proper-object-iterable-methods';
+const messages = {
+	[MESSAGE_ID]: 'Prefer `Object.{{replacement}}()` over `Object.{{method}}()`.'
+};
+
+const objectStaticMethodSelector = property => methodSelector({
+	object: 'Object',
+	names: ['keys', 'entries'],
+	length: 1,
+	property,
+});
+
+const forOfSelector = [
+	'ForOfStatement',
+	'[await!=true]',
+	objectStaticMethodSelector('right'),
+	'> .body'
+].join('')
+
+const arrayMethodSelector = [
+	methodSelector({
+		names: [
+			'every',
+			'filter',
+			'find',
+			'findIndex',
+			'flatMap',
+			'forEach',
+			'map',
+			'reduce',
+			'reduceRight',
+			'some'
+		],
+		min: 1
+	}),
+	objectStaticMethodSelector('callee.object'),
+	'> :function.arguments'
+].join('')
+
+function getVariablesNode(node, method) {
+	const parameters = node.params;
+	const [firstParameter, secondParameter] = parameters;
+
+	if (method !== 'reduce' && method !== 'reduceRight') {
+		return firstParameter;
+	}
+
+	if (firstParameter && variablesNode.type !== 'RestElement') {
+		return secondParameter;
+	}
+}
+
+function getDeclaredVariables(node, method) {
+	if (!node) {
+		return;
+	}
+
+	let isConst = true;
+	if (
+		node.type === 'VariableDeclaration' &&
+		node.declarations.length === 1
+	) {
+		isConst = node.kind === 'const'
+		node = node.declarations[0].id;
+	}
+
+	if (method === 'entries' && node.type === 'ArrayPattern') {
+		const [key, value] = node.elements;
+		const result = {isConst};
+		if (key && key.type === 'Identifier') {
+			result.key = key;
+		}
+		if (value && value.type === 'Identifier') {
+			result.value = value;
+		}
+
+		return result;
+	}
+
+	if (node.type === 'Identifier') {
+		const type = method === 'keys' ? 'key' : 'pairs';
+
+		return {isConst, [type]: node}
+	}
+}
+
+function isSame(nodeA, nodeB, scope) {
+	if (!isSameReference(nodeA, nodeB)) {
+		return false;
+	}
+
+	if (nodeA.type === 'Identifier') {
+		// Check variable
+	}
+
+	return true;
+}
+
+function getProblem({node, block, scope, memberExpressions}) {
+	const {type} = node;
+	let variablesNode;
+	let objectMethodCallNode;
+	if (node.type === 'ForOfStatement') {
+		variablesNode = node.left;
+		objectMethodCallNode = node.right;
+	} else {
+		variablesNode = getVariablesNode(node.arguments[0], node.callee.property.name);
+		objectMethodCallNode = node.callee.object;
+	}
+
+	const methodNode = objectMethodCallNode.callee.property;
+	const method = methodNode.name;
+	const object = objectMethodCallNode.arguments[0];
+
+	const {key, pairs} = getDeclaredVariables(variablesNode, method) || {};
+	const accessingValue = memberExpressions.some(
+		node => {
+		const {object: memberObject, property, range, computed} = node;
+			if (
+				range[0] < block.range[0] ||
+				range[1] > block.range[1] ||
+				!computed
+			) {
+				return false;
+			}
+
+			if (!isSame(memberObject, object, scope)) {
+				return false;
+			}
+
+			if (
+				key &&
+				property.type === 'Identifier' &&
+				isSame(property, key, scope)
+			) {
+				return true;
+			}
+
+			if (
+				pairs &&
+				property.type === 'MemberExpression' &&
+				property.computed &&
+				isSame(property.object, pairs, scope) &&
+				property.property.type === 'Literal' &&
+				property.property.value === 0
+			) {
+				return true;
+			}
+
+			return false;
+		}
+	);
+
+	if (method === 'keys' && accessingValue) {
+		return {
+			node: methodNode,
+			messageId: MESSAGE_ID,
+			// TODO: if `key` is not used in other places, should suggest `.values()`
+			data: {method, replacement: 'entries'}
+		}
+	}
+
+	if (method === 'entries' && !accessingValue) {
+		return {
+			node: methodNode,
+			messageId: MESSAGE_ID,
+			data: {method, replacement: 'keys'}
+		}
+	}
+}
+
+const create = context => {
+	const nodes = new Set();
+	const memberExpressions = [];
+
+	return {
+		MemberExpression(node) {
+			memberExpressions.push(node);
+		},
+		[forOfSelector](node) {
+			nodes.add({
+				node: node.parent,
+				block: node,
+				scope: context.getScope()
+			});
+		},
+		[arrayMethodSelector](node) {
+			// Check function is the first argument (callback) of array method calls,
+			// Can't check this in selector
+			const arrayCall = node.parent;
+			if (arrayCall.arguments[0] !== node) {
+				return;
+			}
+
+			nodes.add({
+				node: node.parent,
+				block: node.body,
+				scope: context.getScope()
+			});
+		},
+		'Program:exit'() {
+			for (const nodeInfo of nodes) {
+				const problem = getProblem({...nodeInfo, memberExpressions});
+				if (problem) {
+					context.report(problem);
+				}
+			}
+		}
+	}
+};
+
+const schema = [];
+
+module.exports = {
+	create,
+	meta: {
+		type: 'suggestion',
+		docs: {
+			description: 'Enforce proper methods when iterate through objects.',
+			url: getDocumentationUrl(__filename)
+		},
+		fixable: 'code',
+		schema,
+		messages
+	}
+};

test/proper-object-iterable-methods.mjs

@@ -0,0 +1,24 @@
+import outdent from 'outdent';
+import {getTester} from './utils/test.mjs';
+
+const {test} = getTester(import.meta);
+
+// `Object.keys`
+test.snapshot({
+	valid: [
+	],
+	invalid: [
+		outdent`
+			for (const key of Object.keys(foo)) {
+				bar(foo[key]);
+			}
+		`,
+		outdent`
+			for (const key of Object.keys(foo)) {
+				bar(foo[key], key);
+			}
+		`,
+		'const foo = Object.keys(bar).map(key => baz(bar[key]));',
+		'const foo = Object.keys(bar).map(key => baz(bar[key], key));',
+	]
+});

test/snapshots/proper-object-iterable-methods.mjs.md

@@ -0,0 +1,53 @@
+# Snapshot report for `test/proper-object-iterable-methods.mjs`
+
+The actual snapshot is saved in `proper-object-iterable-methods.mjs.snap`.
+
+Generated by [AVA](https://avajs.dev).
+
+## Invalid #1
+      1 | for (const key of Object.keys(foo)) {
+      2 | 	bar(foo[key]);
+      3 | }
+
+> Error 1/1
+
+    `␊
+    > 1 | for (const key of Object.keys(foo)) {␊
+        |                          ^^^^ Prefer \`Object.entries()\` over \`Object.keys()\`.␊
+      2 | 	bar(foo[key]);␊
+      3 | }␊
+    `
+
+## Invalid #2
+      1 | for (const key of Object.keys(foo)) {
+      2 | 	bar(foo[key], key);
+      3 | }
+
+> Error 1/1
+
+    `␊
+    > 1 | for (const key of Object.keys(foo)) {␊
+        |                          ^^^^ Prefer \`Object.entries()\` over \`Object.keys()\`.␊
+      2 | 	bar(foo[key], key);␊
+      3 | }␊
+    `
+
+## Invalid #3
+      1 | const foo = Object.keys(bar).map(key => baz(bar[key]));
+
+> Error 1/1
+
+    `␊
+    > 1 | const foo = Object.keys(bar).map(key => baz(bar[key]));␊
+        |                    ^^^^ Prefer \`Object.entries()\` over \`Object.keys()\`.␊
+    `
+
+## Invalid #4
+      1 | const foo = Object.keys(bar).map(key => baz(bar[key], key));
+
+> Error 1/1
+
+    `␊
+    > 1 | const foo = Object.keys(bar).map(key => baz(bar[key], key));␊
+        |                    ^^^^ Prefer \`Object.entries()\` over \`Object.keys()\`.␊
+    `