Skip to content

Commit 21537d7

Browse files
fiskersindresorhus
fisker
and
sindresorhus
authored
Add no-array-push-push rule (#1015)
Co-authored-by: Sindre Sorhus <[email protected]>
1 parent 27bc3c3 commit 21537d7

File tree

10 files changed

+723
-5
lines changed

10 files changed

+723
-5
lines changed

‎docs/rules/no-array-push-push.md‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Enforce combining multiple `Array#push()` into one call
2+
3+
[`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.
4+
5+
This rule is partly fixable.
6+
7+
## Fail
8+
9+
```js
10+
foo.push(1);
11+
foo.push(2, 3);
12+
```
13+
14+
## Pass
15+
16+
```js
17+
foo.push(1, 2, 3);
18+
```
19+
20+
```js
21+
const length = foo.push(1);
22+
foo.push(2);
23+
```
24+
25+
```js
26+
foo.push(1);
27+
bar();
28+
foo.push(2);
29+
```

‎index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ module.exports = {
5555
'unicorn/new-for-builtins': 'error',
5656
'unicorn/no-abusive-eslint-disable': 'error',
5757
'unicorn/no-array-callback-reference': 'error',
58+
'unicorn/no-array-push-push': 'error',
5859
'unicorn/no-array-reduce': 'error',
5960
'unicorn/no-console-spaces': 'error',
6061
'unicorn/no-for-loop': 'error',

‎readme.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ Configure it in `package.json`.
4949
"unicorn/new-for-builtins": "error",
5050
"unicorn/no-abusive-eslint-disable": "error",
5151
"unicorn/no-array-callback-reference": "error",
52+
"unicorn/no-array-push-push": "error",
5253
"unicorn/no-array-reduce": "error",
5354
"unicorn/no-console-spaces": "error",
5455
"unicorn/no-for-loop": "error",
@@ -125,6 +126,7 @@ Configure it in `package.json`.
125126
- [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)*
126127
- [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) - Enforce specifying rules to disable in `eslint-disable` comments.
127128
- [no-array-callback-reference](docs/rules/no-array-callback-reference.md) - Prevent passing a function reference directly to iterator methods.
129+
- [no-array-push-push](docs/rules/no-array-push-push.md) - Enforce combining multiple `Array#push()` into one call. *(partly fixable)*
128130
- [no-array-reduce](docs/rules/no-array-reduce.md) - Disallow `Array#reduce()` and `Array#reduceRight()`.
129131
- [no-console-spaces](docs/rules/no-console-spaces.md) - Do not use leading/trailing space between `console.log` parameters. *(fixable)*
130132
- [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)*

‎rules/no-array-push-push.js‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
'use strict';
2+
const {hasSideEffect, isCommaToken, isOpeningParenToken, isSemicolonToken} = require('eslint-utils');
3+
const getDocumentationUrl = require('./utils/get-documentation-url');
4+
const methodSelector = require('./utils/method-selector');
5+
6+
const ERROR = 'error';
7+
const SUGGESTION = 'suggestion';
8+
const messages = {
9+
[ERROR]: 'Do not call `Array#push()` multiple times.',
10+
[SUGGESTION]: 'Merge with previous one.'
11+
};
12+
13+
const arrayPushExpressionStatement = [
14+
'ExpressionStatement',
15+
methodSelector({
16+
name: 'push',
17+
property: 'expression'
18+
})
19+
].join('');
20+
21+
const selector = `${arrayPushExpressionStatement} + ${arrayPushExpressionStatement}`;
22+
23+
const getCallExpressionArgumentsText = (node, sourceCode) => {
24+
const openingParenthesisToken = sourceCode.getTokenAfter(node.callee, isOpeningParenToken);
25+
const closingParenthesisToken = sourceCode.getLastToken(node);
26+
27+
return sourceCode.text.slice(
28+
openingParenthesisToken.range[1],
29+
closingParenthesisToken.range[0]
30+
);
31+
};
32+
33+
function getFirstExpression(node, sourceCode) {
34+
const {parent} = node;
35+
const visitorKeys = sourceCode.visitorKeys[parent.type] || Object.keys(parent);
36+
37+
for (const property of visitorKeys) {
38+
const value = parent[property];
39+
if (Array.isArray(value)) {
40+
const index = value.indexOf(node);
41+
42+
if (index !== -1) {
43+
return value[index - 1];
44+
}
45+
}
46+
}
47+
48+
/* istanbul ignore next */
49+
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');
50+
}
51+
52+
function create(context) {
53+
const sourceCode = context.getSourceCode();
54+
55+
return {
56+
[selector](secondExpression) {
57+
const firstExpression = getFirstExpression(secondExpression, sourceCode);
58+
const firstCall = firstExpression.expression;
59+
const secondCall = secondExpression.expression;
60+
61+
const firstCallArray = firstCall.callee.object;
62+
const secondCallArray = secondCall.callee.object;
63+
64+
// Not same array
65+
if (sourceCode.getText(firstCallArray) !== sourceCode.getText(secondCallArray)) {
66+
return;
67+
}
68+
69+
const secondCallArguments = secondCall.arguments;
70+
const problem = {
71+
node: secondCall.callee.property,
72+
messageId: ERROR
73+
};
74+
75+
const fix = function * (fixer) {
76+
if (secondCallArguments.length > 0) {
77+
const text = getCallExpressionArgumentsText(secondCall, sourceCode);
78+
79+
const [penultimateToken, lastToken] = sourceCode.getLastTokens(firstCall, 2);
80+
yield (isCommaToken(penultimateToken) ? fixer.insertTextAfter(penultimateToken, ` ${text}`) : fixer.insertTextBefore(
81+
lastToken,
82+
firstCall.arguments.length > 0 ? `, ${text}` : text
83+
));
84+
}
85+
86+
const shouldKeepSemicolon = !isSemicolonToken(sourceCode.getLastToken(firstExpression)) &&
87+
isSemicolonToken(sourceCode.getLastToken(secondExpression));
88+
89+
yield fixer.replaceTextRange(
90+
[firstExpression.range[1], secondExpression.range[1]],
91+
shouldKeepSemicolon ? ';' : ''
92+
);
93+
};
94+
95+
if (
96+
hasSideEffect(firstCallArray, sourceCode) ||
97+
secondCallArguments.some(element => hasSideEffect(element, sourceCode))
98+
) {
99+
problem.suggest = [
100+
{
101+
messageId: SUGGESTION,
102+
fix
103+
}
104+
];
105+
} else {
106+
problem.fix = fix;
107+
}
108+
109+
context.report(problem);
110+
}
111+
};
112+
}
113+
114+
module.exports = {
115+
create,
116+
meta: {
117+
type: 'suggestion',
118+
docs: {
119+
url: getDocumentationUrl(__filename)
120+
},
121+
fixable: 'code',
122+
messages
123+
}
124+
};

‎rules/prevent-abbreviations.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,8 +389,10 @@ const formatMessage = (discouragedName, replacements, nameTypeText) => {
389389
replacementsText += `, ... (${omittedReplacementsCount > 99 ? '99+' : omittedReplacementsCount} more omitted)`;
390390
}
391391

392-
message.push(`Please rename the ${nameTypeText} \`${discouragedName}\`.`);
393-
message.push(`Suggested names are: ${replacementsText}.`);
392+
message.push(
393+
`Please rename the ${nameTypeText} \`${discouragedName}\`.`,
394+
`Suggested names are: ${replacementsText}.`
395+
);
394396
}
395397

396398
message.push(anotherNameMessage);

‎rules/utils/method-selector.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@ module.exports = options => {
3737
}
3838

3939
if (object) {
40-
selector.push(`[${prefix}callee.object.type="Identifier"]`);
41-
selector.push(`[${prefix}callee.object.name="${object}"]`);
40+
selector.push(
41+
`[${prefix}callee.object.type="Identifier"]`,
42+
`[${prefix}callee.object.name="${object}"]`
43+
);
4244
}
4345

4446
if (typeof length === 'number') {

‎scripts/template/test.js.jst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {outdent} from 'outdent';
2-
import {test} from './utils/test';
2+
import {test} from './utils/test.js';
33

44
const MESSAGE_ID = '<%= id %>';
55
const errors = [

‎test/no-array-push-push.js‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
import {outdent} from 'outdent';
2+
import {test} from './utils/test.js';
3+
4+
test.visualize({
5+
valid: [
6+
outdent`
7+
foo.forEach(fn);
8+
foo.forEach(fn);
9+
`,
10+
'foo.push(1);',
11+
outdent`
12+
foo.push(1);
13+
foo.unshift(2);
14+
`,
15+
outdent`
16+
foo.push(1);; // <- there is a "EmptyStatement" between
17+
foo.push(2);
18+
`,
19+
// Not same array
20+
outdent`
21+
foo.push(1);
22+
bar.push(2);
23+
`,
24+
// `.push` selector
25+
'foo.push(1);push(2)',
26+
'push(1);foo.push(2)',
27+
'new foo.push(1);foo.push(2)',
28+
'foo.push(1);new foo.push(2)',
29+
'foo[push](1);foo.push(2)',
30+
'foo.push(1);foo[push](2)',
31+
'foo.push(foo.push(1));',
32+
// Not `ExpressionStatement`
33+
outdent`
34+
const length = foo.push(1);
35+
foo.push(2);
36+
`,
37+
outdent`
38+
foo.push(1);
39+
const length = foo.push(2);
40+
`,
41+
// We are comparing array with source code
42+
outdent`
43+
foo.bar.push(1);
44+
(foo).bar.push(2);
45+
`
46+
],
47+
invalid: [
48+
outdent`
49+
foo.push(1);
50+
foo.push(2);
51+
`,
52+
outdent`
53+
(foo.push)(1);
54+
(foo.push)(2);
55+
`,
56+
outdent`
57+
foo.bar.push(1);
58+
foo.bar.push(2);
59+
`,
60+
outdent`
61+
foo.push(1);
62+
(foo).push(2);
63+
`,
64+
outdent`
65+
foo.push();
66+
foo.push();
67+
`,
68+
outdent`
69+
foo.push(1);
70+
foo.push();
71+
`,
72+
outdent`
73+
foo.push();
74+
foo.push(2);
75+
`,
76+
outdent`
77+
foo.push(1, 2);
78+
foo.push((3), (4));
79+
`,
80+
outdent`
81+
foo.push(1, 2,);
82+
foo.push(3, 4);
83+
`,
84+
outdent`
85+
foo.push(1, 2);
86+
foo.push(3, 4,);
87+
`,
88+
outdent`
89+
foo.push(1, 2,);
90+
foo.push(3, 4,);
91+
`,
92+
outdent`
93+
foo.push(1, 2, ...a,);
94+
foo.push(...b,);
95+
`,
96+
outdent`
97+
foo.push(bar());
98+
foo.push(1);
99+
`,
100+
// `arguments` in second push has side effect
101+
outdent`
102+
foo.push(1);
103+
foo.push(bar());
104+
`,
105+
// Array has side effect
106+
outdent`
107+
foo().push(1);
108+
foo().push(2);
109+
`,
110+
outdent`
111+
foo().bar.push(1);
112+
foo().bar.push(2);
113+
`,
114+
// Multiple
115+
outdent`
116+
foo.push(1,);
117+
foo.push(2,);
118+
foo.push(3,);
119+
`,
120+
// Should be able to find the previous expression
121+
outdent`
122+
if (a) {
123+
foo.push(1);
124+
foo.push(2);
125+
}
126+
`,
127+
outdent`
128+
switch (a) {
129+
default:
130+
foo.push(1);
131+
foo.push(2);
132+
}
133+
`,
134+
outdent`
135+
function a() {
136+
foo.push(1);
137+
foo.push(2);
138+
}
139+
`,
140+
// ASI
141+
outdent`
142+
foo.push(1)
143+
foo.push(2)
144+
;[foo].forEach(bar)
145+
`
146+
]
147+
});

0 commit comments

Comments
 (0)