diff --git a/docs/rules/no-array-push-push.md b/docs/rules/no-array-push-push.md new file mode 100644 index 0000000000..2e3bd5b92c --- /dev/null +++ b/docs/rules/no-array-push-push.md @@ -0,0 +1,29 @@ +# Enforce combining multiple `Array#push()` into one call + +[`Array#push()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/push) accepts multiple arguments. Multiple calls should be combined into one. + +This rule is partly fixable. + +## Fail + +```js +foo.push(1); +foo.push(2, 3); +``` + +## Pass + +```js +foo.push(1, 2, 3); +``` + +```js +const length = foo.push(1); +foo.push(2); +``` + +```js +foo.push(1); +bar(); +foo.push(2); +``` diff --git a/index.js b/index.js index 41017ea22c..568a0eecb2 100644 --- a/index.js +++ b/index.js @@ -55,6 +55,7 @@ module.exports = { 'unicorn/new-for-builtins': 'error', 'unicorn/no-abusive-eslint-disable': 'error', 'unicorn/no-array-callback-reference': 'error', + 'unicorn/no-array-push-push': 'error', 'unicorn/no-array-reduce': 'error', 'unicorn/no-console-spaces': 'error', 'unicorn/no-for-loop': 'error', diff --git a/readme.md b/readme.md index 4dc8fdb51b..f8fe151ce5 100644 --- a/readme.md +++ b/readme.md @@ -49,6 +49,7 @@ Configure it in `package.json`. "unicorn/new-for-builtins": "error", "unicorn/no-abusive-eslint-disable": "error", "unicorn/no-array-callback-reference": "error", + "unicorn/no-array-push-push": "error", "unicorn/no-array-reduce": "error", "unicorn/no-console-spaces": "error", "unicorn/no-for-loop": "error", @@ -125,6 +126,7 @@ Configure it in `package.json`. - [new-for-builtins](docs/rules/new-for-builtins.md) - Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. *(partly fixable)* - [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) - Enforce specifying rules to disable in `eslint-disable` comments. - [no-array-callback-reference](docs/rules/no-array-callback-reference.md) - Prevent passing a function reference directly to iterator methods. +- [no-array-push-push](docs/rules/no-array-push-push.md) - Enforce combining multiple `Array#push()` into one call. *(partly fixable)* - [no-array-reduce](docs/rules/no-array-reduce.md) - Disallow `Array#reduce()` and `Array#reduceRight()`. - [no-console-spaces](docs/rules/no-console-spaces.md) - Do not use leading/trailing space between `console.log` parameters. *(fixable)* - [no-for-loop](docs/rules/no-for-loop.md) - Do not use a `for` loop that can be replaced with a `for-of` loop. *(partly fixable)* diff --git a/rules/no-array-push-push.js b/rules/no-array-push-push.js new file mode 100644 index 0000000000..fae21f69d4 --- /dev/null +++ b/rules/no-array-push-push.js @@ -0,0 +1,124 @@ +'use strict'; +const {hasSideEffect, isCommaToken, isOpeningParenToken, isSemicolonToken} = require('eslint-utils'); +const getDocumentationUrl = require('./utils/get-documentation-url'); +const methodSelector = require('./utils/method-selector'); + +const ERROR = 'error'; +const SUGGESTION = 'suggestion'; +const messages = { + [ERROR]: 'Do not call `Array#push()` multiple times.', + [SUGGESTION]: 'Merge with previous one.' +}; + +const arrayPushExpressionStatement = [ + 'ExpressionStatement', + methodSelector({ + name: 'push', + property: 'expression' + }) +].join(''); + +const selector = `${arrayPushExpressionStatement} + ${arrayPushExpressionStatement}`; + +const getCallExpressionArgumentsText = (node, sourceCode) => { + const openingParenthesisToken = sourceCode.getTokenAfter(node.callee, isOpeningParenToken); + const closingParenthesisToken = sourceCode.getLastToken(node); + + return sourceCode.text.slice( + openingParenthesisToken.range[1], + closingParenthesisToken.range[0] + ); +}; + +function getFirstExpression(node, sourceCode) { + const {parent} = node; + const visitorKeys = sourceCode.visitorKeys[parent.type] || Object.keys(parent); + + for (const property of visitorKeys) { + const value = parent[property]; + if (Array.isArray(value)) { + const index = value.indexOf(node); + + if (index !== -1) { + return value[index - 1]; + } + } + } + + /* istanbul ignore next */ + throw new Error('Cannot find the first `Array#push()` call.\nPlease open an issue at https://github.com/sindresorhus/eslint-plugin-unicorn/issues/new?title=%60no-array-push-push%60%3A%20Cannot%20find%20first%20%60push()%60'); +} + +function create(context) { + const sourceCode = context.getSourceCode(); + + return { + [selector](secondExpression) { + const firstExpression = getFirstExpression(secondExpression, sourceCode); + const firstCall = firstExpression.expression; + const secondCall = secondExpression.expression; + + const firstCallArray = firstCall.callee.object; + const secondCallArray = secondCall.callee.object; + + // Not same array + if (sourceCode.getText(firstCallArray) !== sourceCode.getText(secondCallArray)) { + return; + } + + const secondCallArguments = secondCall.arguments; + const problem = { + node: secondCall.callee.property, + messageId: ERROR + }; + + const fix = function * (fixer) { + if (secondCallArguments.length > 0) { + const text = getCallExpressionArgumentsText(secondCall, sourceCode); + + const [penultimateToken, lastToken] = sourceCode.getLastTokens(firstCall, 2); + yield (isCommaToken(penultimateToken) ? fixer.insertTextAfter(penultimateToken, ` ${text}`) : fixer.insertTextBefore( + lastToken, + firstCall.arguments.length > 0 ? `, ${text}` : text + )); + } + + const shouldKeepSemicolon = !isSemicolonToken(sourceCode.getLastToken(firstExpression)) && + isSemicolonToken(sourceCode.getLastToken(secondExpression)); + + yield fixer.replaceTextRange( + [firstExpression.range[1], secondExpression.range[1]], + shouldKeepSemicolon ? ';' : '' + ); + }; + + if ( + hasSideEffect(firstCallArray, sourceCode) || + secondCallArguments.some(element => hasSideEffect(element, sourceCode)) + ) { + problem.suggest = [ + { + messageId: SUGGESTION, + fix + } + ]; + } else { + problem.fix = fix; + } + + context.report(problem); + } + }; +} + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocumentationUrl(__filename) + }, + fixable: 'code', + messages + } +}; diff --git a/rules/prevent-abbreviations.js b/rules/prevent-abbreviations.js index 0f3135ca80..1af88d2f3e 100644 --- a/rules/prevent-abbreviations.js +++ b/rules/prevent-abbreviations.js @@ -389,8 +389,10 @@ const formatMessage = (discouragedName, replacements, nameTypeText) => { replacementsText += `, ... (${omittedReplacementsCount > 99 ? '99+' : omittedReplacementsCount} more omitted)`; } - message.push(`Please rename the ${nameTypeText} \`${discouragedName}\`.`); - message.push(`Suggested names are: ${replacementsText}.`); + message.push( + `Please rename the ${nameTypeText} \`${discouragedName}\`.`, + `Suggested names are: ${replacementsText}.` + ); } message.push(anotherNameMessage); diff --git a/rules/utils/method-selector.js b/rules/utils/method-selector.js index de52a01483..aa78bad0b3 100644 --- a/rules/utils/method-selector.js +++ b/rules/utils/method-selector.js @@ -37,8 +37,10 @@ module.exports = options => { } if (object) { - selector.push(`[${prefix}callee.object.type="Identifier"]`); - selector.push(`[${prefix}callee.object.name="${object}"]`); + selector.push( + `[${prefix}callee.object.type="Identifier"]`, + `[${prefix}callee.object.name="${object}"]` + ); } if (typeof length === 'number') { diff --git a/scripts/template/test.js.jst b/scripts/template/test.js.jst index b512106e7e..95c8963615 100644 --- a/scripts/template/test.js.jst +++ b/scripts/template/test.js.jst @@ -1,5 +1,5 @@ import {outdent} from 'outdent'; -import {test} from './utils/test'; +import {test} from './utils/test.js'; const MESSAGE_ID = '<%= id %>'; const errors = [ diff --git a/test/no-array-push-push.js b/test/no-array-push-push.js new file mode 100644 index 0000000000..c63ac3a849 --- /dev/null +++ b/test/no-array-push-push.js @@ -0,0 +1,147 @@ +import {outdent} from 'outdent'; +import {test} from './utils/test.js'; + +test.visualize({ + valid: [ + outdent` + foo.forEach(fn); + foo.forEach(fn); + `, + 'foo.push(1);', + outdent` + foo.push(1); + foo.unshift(2); + `, + outdent` + foo.push(1);; // <- there is a "EmptyStatement" between + foo.push(2); + `, + // Not same array + outdent` + foo.push(1); + bar.push(2); + `, + // `.push` selector + 'foo.push(1);push(2)', + 'push(1);foo.push(2)', + 'new foo.push(1);foo.push(2)', + 'foo.push(1);new foo.push(2)', + 'foo[push](1);foo.push(2)', + 'foo.push(1);foo[push](2)', + 'foo.push(foo.push(1));', + // Not `ExpressionStatement` + outdent` + const length = foo.push(1); + foo.push(2); + `, + outdent` + foo.push(1); + const length = foo.push(2); + `, + // We are comparing array with source code + outdent` + foo.bar.push(1); + (foo).bar.push(2); + ` + ], + invalid: [ + outdent` + foo.push(1); + foo.push(2); + `, + outdent` + (foo.push)(1); + (foo.push)(2); + `, + outdent` + foo.bar.push(1); + foo.bar.push(2); + `, + outdent` + foo.push(1); + (foo).push(2); + `, + outdent` + foo.push(); + foo.push(); + `, + outdent` + foo.push(1); + foo.push(); + `, + outdent` + foo.push(); + foo.push(2); + `, + outdent` + foo.push(1, 2); + foo.push((3), (4)); + `, + outdent` + foo.push(1, 2,); + foo.push(3, 4); + `, + outdent` + foo.push(1, 2); + foo.push(3, 4,); + `, + outdent` + foo.push(1, 2,); + foo.push(3, 4,); + `, + outdent` + foo.push(1, 2, ...a,); + foo.push(...b,); + `, + outdent` + foo.push(bar()); + foo.push(1); + `, + // `arguments` in second push has side effect + outdent` + foo.push(1); + foo.push(bar()); + `, + // Array has side effect + outdent` + foo().push(1); + foo().push(2); + `, + outdent` + foo().bar.push(1); + foo().bar.push(2); + `, + // Multiple + outdent` + foo.push(1,); + foo.push(2,); + foo.push(3,); + `, + // Should be able to find the previous expression + outdent` + if (a) { + foo.push(1); + foo.push(2); + } + `, + outdent` + switch (a) { + default: + foo.push(1); + foo.push(2); + } + `, + outdent` + function a() { + foo.push(1); + foo.push(2); + } + `, + // ASI + outdent` + foo.push(1) + foo.push(2) + ;[foo].forEach(bar) + ` + ] +}); diff --git a/test/snapshots/no-array-push-push.js.md b/test/snapshots/no-array-push-push.js.md new file mode 100644 index 0000000000..37d8cbb040 --- /dev/null +++ b/test/snapshots/no-array-push-push.js.md @@ -0,0 +1,411 @@ +# Snapshot report for `test/no-array-push-push.js` + +The actual snapshot is saved in `no-array-push-push.js.snap`. + +Generated by [AVA](https://avajs.dev). + +## Invalid #1 + 1 | foo.push(1); + 2 | foo.push(2); + +> Output + + `␊ + 1 | foo.push(1, 2);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(1);␊ + > 2 | foo.push(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #2 + 1 | (foo.push)(1); + 2 | (foo.push)(2); + +> Output + + `␊ + 1 | (foo.push)(1, 2);␊ + ` + +> Error 1/1 + + `␊ + 1 | (foo.push)(1);␊ + > 2 | (foo.push)(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #3 + 1 | foo.bar.push(1); + 2 | foo.bar.push(2); + +> Output + + `␊ + 1 | foo.bar.push(1, 2);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.bar.push(1);␊ + > 2 | foo.bar.push(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #4 + 1 | foo.push(1); + 2 | (foo).push(2); + +> Output + + `␊ + 1 | foo.push(1, 2);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(1);␊ + > 2 | (foo).push(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #5 + 1 | foo.push(); + 2 | foo.push(); + +> Output + + `␊ + 1 | foo.push();␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push();␊ + > 2 | foo.push();␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #6 + 1 | foo.push(1); + 2 | foo.push(); + +> Output + + `␊ + 1 | foo.push(1);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(1);␊ + > 2 | foo.push();␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #7 + 1 | foo.push(); + 2 | foo.push(2); + +> Output + + `␊ + 1 | foo.push(2);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push();␊ + > 2 | foo.push(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #8 + 1 | foo.push(1, 2); + 2 | foo.push((3), (4)); + +> Output + + `␊ + 1 | foo.push(1, 2, (3), (4));␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(1, 2);␊ + > 2 | foo.push((3), (4));␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #9 + 1 | foo.push(1, 2,); + 2 | foo.push(3, 4); + +> Output + + `␊ + 1 | foo.push(1, 2, 3, 4);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(1, 2,);␊ + > 2 | foo.push(3, 4);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #10 + 1 | foo.push(1, 2); + 2 | foo.push(3, 4,); + +> Output + + `␊ + 1 | foo.push(1, 2, 3, 4,);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(1, 2);␊ + > 2 | foo.push(3, 4,);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #11 + 1 | foo.push(1, 2,); + 2 | foo.push(3, 4,); + +> Output + + `␊ + 1 | foo.push(1, 2, 3, 4,);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(1, 2,);␊ + > 2 | foo.push(3, 4,);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #12 + 1 | foo.push(1, 2, ...a,); + 2 | foo.push(...b,); + +> Output + + `␊ + 1 | foo.push(1, 2, ...a, ...b,);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(1, 2, ...a,);␊ + > 2 | foo.push(...b,);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #13 + 1 | foo.push(bar()); + 2 | foo.push(1); + +> Output + + `␊ + 1 | foo.push(bar(), 1);␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(bar());␊ + > 2 | foo.push(1);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #14 + 1 | foo.push(1); + 2 | foo.push(bar()); + +> Error 1/1 + + `␊ + 1 | foo.push(1);␊ + > 2 | foo.push(bar());␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Merge with previous one.␊ + 1 | foo.push(1, bar());␊ + ` + +## Invalid #15 + 1 | foo().push(1); + 2 | foo().push(2); + +> Error 1/1 + + `␊ + 1 | foo().push(1);␊ + > 2 | foo().push(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Merge with previous one.␊ + 1 | foo().push(1, 2);␊ + ` + +## Invalid #16 + 1 | foo().bar.push(1); + 2 | foo().bar.push(2); + +> Error 1/1 + + `␊ + 1 | foo().bar.push(1);␊ + > 2 | foo().bar.push(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Merge with previous one.␊ + 1 | foo().bar.push(1, 2);␊ + ` + +## Invalid #17 + 1 | foo.push(1,); + 2 | foo.push(2,); + 3 | foo.push(3,); + +> Output + + `␊ + 1 | foo.push(1, 2, 3,);␊ + ` + +> Error 1/2 + + `␊ + 1 | foo.push(1,);␊ + > 2 | foo.push(2,);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + 3 | foo.push(3,);␊ + ` + +> Error 2/2 + + `␊ + 1 | foo.push(1,);␊ + 2 | foo.push(2,);␊ + > 3 | foo.push(3,);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + ` + +## Invalid #18 + 1 | if (a) { + 2 | foo.push(1); + 3 | foo.push(2); + 4 | } + +> Output + + `␊ + 1 | if (a) {␊ + 2 | foo.push(1, 2);␊ + 3 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | if (a) {␊ + 2 | foo.push(1);␊ + > 3 | foo.push(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + 4 | }␊ + ` + +## Invalid #19 + 1 | switch (a) { + 2 | default: + 3 | foo.push(1); + 4 | foo.push(2); + 5 | } + +> Output + + `␊ + 1 | switch (a) {␊ + 2 | default:␊ + 3 | foo.push(1, 2);␊ + 4 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | switch (a) {␊ + 2 | default:␊ + 3 | foo.push(1);␊ + > 4 | foo.push(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + 5 | }␊ + ` + +## Invalid #20 + 1 | function a() { + 2 | foo.push(1); + 3 | foo.push(2); + 4 | } + +> Output + + `␊ + 1 | function a() {␊ + 2 | foo.push(1, 2);␊ + 3 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function a() {␊ + 2 | foo.push(1);␊ + > 3 | foo.push(2);␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + 4 | }␊ + ` + +## Invalid #21 + 1 | foo.push(1) + 2 | foo.push(2) + 3 | ;[foo].forEach(bar) + +> Output + + `␊ + 1 | foo.push(1, 2);[foo].forEach(bar)␊ + ` + +> Error 1/1 + + `␊ + 1 | foo.push(1)␊ + > 2 | foo.push(2)␊ + | ^^^^ Do not call `Array#push()` multiple times.␊ + 3 | ;[foo].forEach(bar)␊ + ` diff --git a/test/snapshots/no-array-push-push.js.snap b/test/snapshots/no-array-push-push.js.snap new file mode 100644 index 0000000000..351a9e0c19 Binary files /dev/null and b/test/snapshots/no-array-push-push.js.snap differ