diff --git a/docs/rules/prefer-destructuring-in-parameters.md b/docs/rules/prefer-destructuring-in-parameters.md new file mode 100644 index 0000000000..ddf3b1fadc --- /dev/null +++ b/docs/rules/prefer-destructuring-in-parameters.md @@ -0,0 +1,17 @@ +# Prefer destructuring in parameters over accessing properties. + + + +This rule is fixable. + +## Fail + +```js +const foo = 'unicorn'; +``` + +## Pass + +```js +const foo = '🦄'; +``` diff --git a/index.js b/index.js index d3247f14f6..ebec97db92 100644 --- a/index.js +++ b/index.js @@ -88,6 +88,7 @@ module.exports = { 'unicorn/prefer-array-some': 'error', 'unicorn/prefer-date-now': 'error', 'unicorn/prefer-default-parameters': 'error', + 'unicorn/prefer-destructuring-in-parameters': 'error', 'unicorn/prefer-dom-node-append': 'error', 'unicorn/prefer-dom-node-dataset': 'error', 'unicorn/prefer-dom-node-remove': 'error', diff --git a/readme.md b/readme.md index c68f804dcc..bab51ba8c1 100644 --- a/readme.md +++ b/readme.md @@ -80,6 +80,7 @@ Configure it in `package.json`. "unicorn/prefer-array-some": "error", "unicorn/prefer-date-now": "error", "unicorn/prefer-default-parameters": "error", + "unicorn/prefer-destructuring-in-parameters": "error", "unicorn/prefer-dom-node-append": "error", "unicorn/prefer-dom-node-dataset": "error", "unicorn/prefer-dom-node-remove": "error", @@ -158,6 +159,7 @@ Configure it in `package.json`. - [prefer-array-some](docs/rules/prefer-array-some.md) - Prefer `.some(…)` over `.find(…)`. - [prefer-date-now](docs/rules/prefer-date-now.md) - Prefer `Date.now()` to get the number of milliseconds since the Unix Epoch. *(fixable)* - [prefer-default-parameters](docs/rules/prefer-default-parameters.md) - Prefer default parameters over reassignment. *(fixable)* +- [prefer-destructuring-in-parameters](docs/rules/prefer-destructuring-in-parameters.md) - Prefer destructuring in parameters over accessing properties. *(fixable)* - [prefer-dom-node-append](docs/rules/prefer-dom-node-append.md) - Prefer `Node#append()` over `Node#appendChild()`. *(fixable)* - [prefer-dom-node-dataset](docs/rules/prefer-dom-node-dataset.md) - Prefer using `.dataset` on DOM elements over `.setAttribute(…)`. *(fixable)* - [prefer-dom-node-remove](docs/rules/prefer-dom-node-remove.md) - Prefer `childNode.remove()` over `parentNode.removeChild(childNode)`. *(fixable)* diff --git a/rules/prefer-destructuring-in-parameters.js b/rules/prefer-destructuring-in-parameters.js new file mode 100644 index 0000000000..a9e3657818 --- /dev/null +++ b/rules/prefer-destructuring-in-parameters.js @@ -0,0 +1,183 @@ +'use strict'; +const {upperFirst} = require('lodash'); +const {findVariable, isNotOpeningParenToken} = require('eslint-utils'); +const getDocumentationUrl = require('./utils/get-documentation-url'); +const avoidCapture = require('./utils/avoid-capture'); + +const MESSAGE_ID = 'prefer-destructuring-in-parameters'; +const messages = { + [MESSAGE_ID]: '`{{member}}` should be destructed in parameter `{{parameter}}`.' +}; + +const indexVariableNamePrefixes = ['first', 'second']; + +function getMemberExpressionProperty(node) { + const {parent} = node; + if (parent.type !== 'MemberExpression') { + return; + } + + const {computed, optional, object, property} = parent; + + if (optional || object !== node) { + return; + } + + if (computed) { + if (property.type !== 'Literal') { + return; + } + + const index = property.value; + if ( + typeof index !== 'number' || + !Number.isInteger(index) || + !(index >= 0 && index < indexVariableNamePrefixes.length) + ) { + return; + } + + return index; + } + + if (property.type === 'Identifier') { + return property.name; + } +} + +function fix({sourceCode, parameter, memberExpressions, isIndex}) { + function * fixArrowFunctionParentheses(fixer) { + const functionNode = parameter.parent; + if ( + functionNode.type === 'ArrowFunctionExpression' && + functionNode.params.length === 1 && + isNotOpeningParenToken(sourceCode.getFirstToken(parameter)) + ) { + yield fixer.insertTextBefore(parameter, '('); + yield fixer.insertTextAfter(parameter, ')'); + } + } + + function fixParameter(fixer) { + let text; + if (isIndex) { + const variables = []; + for (const [index, {variable}] of memberExpressions.entries()) { + variables[index] = variable; + } + + text = `[${variables.join(', ')}]`; + } else { + const variables = [...memberExpressions.keys()]; + + text = `{${variables.join(', ')}}` + } + + return fixer.replaceText(parameter, text); + } + + return function * (fixer) { + yield * fixArrowFunctionParentheses(fixer); + yield fixParameter(fixer); + + for (const {variable, expressions} of memberExpressions.values()) { + for (const expression of expressions) { + yield fixer.replaceText(expression, variable); + } + } + }; +} + +const create = context => { + const {ecmaVersion} = context.parserOptions; + const sourceCode = context.getSourceCode(); + return { + ':function > Identifier.params'(parameter) { + const scope = context.getScope(); + const {name} = parameter; + const variable = findVariable(scope, parameter); + const identifiers = variable.references.map(({identifier}) => identifier); + + const memberExpressions = new Map(); + let lastPropertyType; + let firstExpression; + for (const identifier of identifiers) { + const property = getMemberExpressionProperty(identifier); + const propertyType = typeof property; + if ( + propertyType === 'undefined' || + (lastPropertyType && propertyType !== lastPropertyType) + ) { + return; + } + + const memberExpression = identifier.parent; + + if (memberExpressions.has(property)) { + memberExpressions.get(property).expressions.push(memberExpression); + } else { + memberExpressions.set(property, {expressions: [memberExpression]}); + } + + lastPropertyType = propertyType; + firstExpression = ( + firstExpression && firstExpression.node.range[0] < memberExpression.range[0] + ) ? + firstExpression : + {node: memberExpression, property}; + } + + if (memberExpressions.size === 0) { + return; + } + + const isIndex = lastPropertyType === 'number'; + const scopes = [ + variable.scope, + ...variable.references.map(({from}) => from) + ]; + for (const [property, data] of memberExpressions.entries()) { + let variableName; + if (isIndex) { + const index = indexVariableNamePrefixes[property]; + variableName = avoidCapture(`${index}ElementOf${upperFirst(name)}`, scopes, ecmaVersion); + } else { + variableName = avoidCapture(property, scopes, ecmaVersion); + if (variableName !== property) { + return; + } + } + + data.variable = variableName; + } + + const {node, property} = firstExpression; + context.report({ + node, + messageId: MESSAGE_ID, + data: { + member: isIndex ? `${name}[${property}]` : `${name}.${property}`, + parameter: name + }, + fix: fix({ + sourceCode, + parameter, + memberExpressions, + isIndex + }) + }); + } + } +}; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocumentationUrl(__filename) + }, + fixable: 'code', + messages + } +}; diff --git a/test/prefer-destructuring-in-parameters.js b/test/prefer-destructuring-in-parameters.js new file mode 100644 index 0000000000..1ef668e01b --- /dev/null +++ b/test/prefer-destructuring-in-parameters.js @@ -0,0 +1,29 @@ +import {outdent} from 'outdent'; +import {test} from './utils/test.js'; + +test.snapshot({ + valid: [ + 'const foo = bar => bar', + 'const foo = function bar(baz) {return bar.name}', + 'const foo = ({bar}) => bar', + 'const foo = bar => bar[3]', + 'const foo = bar => bar[1.5]', + 'const foo = bar => bar[-1]', + 'const foo = bar => bar[0xFF]', + 'const foo = bar => bar[null]', + 'const foo = bar => bar[1n]', + 'const foo = bar => bar["baz"]', + 'const foo = bar => bar.length && bar[0]', + 'const foo = bar => bar.default', + 'const foo = bar => bar.function', + ], + invalid: [ + 'const foo = bar => bar[0]', + 'const foo = bar => bar[0] === firstElementOfBar', + 'const foo = (bar) => bar[0]', + 'const foo = (bar, {baz}) => bar[0] === baz', + 'const foo = bar => bar[0b01]', + 'const foo = bar => bar.length', + 'const foo = bar => bar.baz' + ] +}); diff --git a/test/snapshots/prefer-destructuring-in-parameters.js.md b/test/snapshots/prefer-destructuring-in-parameters.js.md new file mode 100644 index 0000000000..0f73a35c0b --- /dev/null +++ b/test/snapshots/prefer-destructuring-in-parameters.js.md @@ -0,0 +1,85 @@ +# Snapshot report for `test/prefer-destructuring-in-parameters.js` + +The actual snapshot is saved in `prefer-destructuring-in-parameters.js.snap`. + +Generated by [AVA](https://avajs.dev). + +## Invalid #1 + 1 | const foo = bar => bar[0] + +> Output + + `␊ + 1 | const foo = ([firstElementOfBar]) => firstElementOfBar␊ + ` + +> Error 1/1 + + `␊ + > 1 | const foo = bar => bar[0]␊ + | ^^^^^^ `bar[0]` should be destructed in parameter `bar`.␊ + ` + +## Invalid #2 + 1 | const foo = bar => bar[0] === firstElementOfBar + +> Output + + `␊ + 1 | const foo = ([firstElementOfBar_]) => firstElementOfBar_ === firstElementOfBar␊ + ` + +> Error 1/1 + + `␊ + > 1 | const foo = bar => bar[0] === firstElementOfBar␊ + | ^^^^^^ `bar[0]` should be destructed in parameter `bar`.␊ + ` + +## Invalid #3 + 1 | const foo = bar => bar[0b01] + +> Output + + `␊ + 1 | const foo = ([, secondElementOfBar]) => secondElementOfBar␊ + ` + +> Error 1/1 + + `␊ + > 1 | const foo = bar => bar[0b01]␊ + | ^^^^^^^^^ `bar[1]` should be destructed in parameter `bar`.␊ + ` + +## Invalid #4 + 1 | const foo = bar => bar.length + +> Output + + `␊ + 1 | const foo = ({length}) => length␊ + ` + +> Error 1/1 + + `␊ + > 1 | const foo = bar => bar.length␊ + | ^^^^^^^^^^ `bar.length` should be destructed in parameter `bar`.␊ + ` + +## Invalid #5 + 1 | const foo = bar => bar.baz + +> Output + + `␊ + 1 | const foo = ({baz}) => baz␊ + ` + +> Error 1/1 + + `␊ + > 1 | const foo = bar => bar.baz␊ + | ^^^^^^^ `bar.baz` should be destructed in parameter `bar`.␊ + ` diff --git a/test/snapshots/prefer-destructuring-in-parameters.js.snap b/test/snapshots/prefer-destructuring-in-parameters.js.snap new file mode 100644 index 0000000000..270cd3a898 Binary files /dev/null and b/test/snapshots/prefer-destructuring-in-parameters.js.snap differ