diff --git a/docs/rules/proper-object-iterable-methods.md b/docs/rules/proper-object-iterable-methods.md new file mode 100644 index 0000000000..c679d6be84 --- /dev/null +++ b/docs/rules/proper-object-iterable-methods.md @@ -0,0 +1,17 @@ +# Enforce proper methods when iterate through objects. + + + +This rule is partly fixable. + +## Fail + +```js +const foo = 'unicorn'; +``` + +## Pass + +```js +const foo = '🦄'; +``` diff --git a/index.js b/index.js index c873a76bd5..3c8dbc0bd1 100644 --- a/index.js +++ b/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' } diff --git a/readme.md b/readme.md index c569eca7c1..8ba08601fe 100644 --- a/readme.md +++ b/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. | ✅ | 🔧 | diff --git a/rules/proper-object-iterable-methods.js b/rules/proper-object-iterable-methods.js new file mode 100644 index 0000000000..f1bcc87724 --- /dev/null +++ b/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 + } +}; diff --git a/test/proper-object-iterable-methods.mjs b/test/proper-object-iterable-methods.mjs new file mode 100644 index 0000000000..34f4268b4b --- /dev/null +++ b/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));', + ] +}); diff --git a/test/snapshots/proper-object-iterable-methods.mjs.md b/test/snapshots/proper-object-iterable-methods.mjs.md new file mode 100644 index 0000000000..3b461d8284 --- /dev/null +++ b/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()\`.␊ + ` diff --git a/test/snapshots/proper-object-iterable-methods.mjs.snap b/test/snapshots/proper-object-iterable-methods.mjs.snap new file mode 100644 index 0000000000..f61ac8e392 Binary files /dev/null and b/test/snapshots/proper-object-iterable-methods.mjs.snap differ