Skip to content

Commit 2d07c9a

Browse files
fiskerfisker
authored
Add no-useless-collection-argument rule (#2777)
1 parent ffe5943 commit 2d07c9a

13 files changed

+798
-12
lines changed

‎docs/rules/no-useless-collection-argument.md‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# Disallow useless values or fallbacks in `Set`, `Map`, `WeakSet`, or `WeakMap`
2+
3+
💼 This rule is enabled in the following [configs](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config): ✅ `recommended`, ☑️ `unopinionated`.
4+
5+
🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
6+
7+
<!-- end auto-generated rule header -->
8+
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
9+
10+
It's unnecessary to pass an empty array or string when constructing a `Set`, `Map`, `WeakSet`, or `WeakMap`, since they accept nullish values.
11+
12+
It's also unnecessary to provide a fallback for possible nullish values.
13+
14+
## Examples
15+
16+
```js
17+
// ❌
18+
const set = new Set([]);
19+
// ❌
20+
const set = new Set("");
21+
22+
// ✅
23+
const set = new Set();
24+
```
25+
26+
```js
27+
// ❌
28+
const set = new Set(foo ?? []);
29+
// ❌
30+
const set = new Set(foo ?? "");
31+
32+
// ✅
33+
const set = new Set(foo);
34+
```

‎readme.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ export default [
118118
| [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) | Disallow unreadable array destructuring. | ✅ ☑️ | 🔧 | |
119119
| [no-unreadable-iife](docs/rules/no-unreadable-iife.md) | Disallow unreadable IIFEs. | ✅ ☑️ | | |
120120
| [no-unused-properties](docs/rules/no-unused-properties.md) | Disallow unused object properties. | | | |
121+
| [no-useless-collection-argument](docs/rules/no-useless-collection-argument.md) | Disallow useless values or fallbacks in `Set`, `Map`, `WeakSet`, or `WeakMap`. | ✅ ☑️ | 🔧 | |
121122
| [no-useless-error-capture-stack-trace](docs/rules/no-useless-error-capture-stack-trace.md) | Disallow unnecessary `Error.captureStackTrace(…)`. | ✅ ☑️ | 🔧 | |
122123
| [no-useless-fallback-in-spread](docs/rules/no-useless-fallback-in-spread.md) | Disallow useless fallback when spreading in object literals. | ✅ ☑️ | 🔧 | |
123124
| [no-useless-length-check](docs/rules/no-useless-length-check.md) | Disallow useless array length check. | ✅ ☑️ | 🔧 | |

‎rules/ast/index.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export {
55
isBigIntLiteral,
66
isNullLiteral,
77
isRegexLiteral,
8+
isEmptyStringLiteral,
89
} from './literal.js';
910

1011
export {
@@ -16,6 +17,7 @@ export {
1617
export {default as isArrowFunctionBody} from './is-arrow-function-body.js';
1718
export {default as isDirective} from './is-directive.js';
1819
export {default as isEmptyNode} from './is-empty-node.js';
20+
export {default as isEmptyArrayExpression} from './is-empty-array-expression.js';
1921
export {default as isExpressionStatement} from './is-expression-statement.js';
2022
export {default as isFunction} from './is-function.js';
2123
export {default as isMemberExpression} from './is-member-expression.js';

‎rules/ast/is-empty-array-expression.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const isEmptyArrayExpression = node =>
2+
node.type === 'ArrayExpression'
3+
&& node.elements.length === 0;
4+
5+
export default isEmptyArrayExpression;

‎rules/ast/literal.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ export const isNullLiteral = node => isLiteral(node, null);
2121

2222
export const isBigIntLiteral = node => node.type === 'Literal' && Boolean(node.bigint);
2323

24+
export const isEmptyStringLiteral = node => isLiteral(node, '');

‎rules/fix/remove-argument.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import {isCommaToken} from '@eslint-community/eslint-utils';
22
import {getParentheses} from '../utils/parentheses.js';
33

44
export default function removeArgument(fixer, node, sourceCode) {
5-
const callExpression = node.parent;
6-
const index = callExpression.arguments.indexOf(node);
5+
const callOrNewExpression = node.parent;
6+
const index = callOrNewExpression.arguments.indexOf(node);
77
const parentheses = getParentheses(node, sourceCode);
88
const firstToken = parentheses[0] || node;
99
const lastToken = parentheses.at(-1) || node;
@@ -17,14 +17,12 @@ export default function removeArgument(fixer, node, sourceCode) {
1717
}
1818

1919
// If the removed argument is the only argument, the trailing comma must be removed too
20-
/* c8 ignore start */
21-
if (callExpression.arguments.length === 1) {
22-
const tokenAfter = sourceCode.getTokenBefore(lastToken);
20+
if (callOrNewExpression.arguments.length === 1) {
21+
const tokenAfter = sourceCode.getTokenAfter(lastToken);
2322
if (isCommaToken(tokenAfter)) {
2423
[, end] = sourceCode.getRange(tokenAfter);
2524
}
2625
}
27-
/* c8 ignore end */
2826

2927
return fixer.removeRange([start, end]);
3028
}

‎rules/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ export {default as 'no-unnecessary-slice-end'} from './no-unnecessary-slice-end.
6161
export {default as 'no-unreadable-array-destructuring'} from './no-unreadable-array-destructuring.js';
6262
export {default as 'no-unreadable-iife'} from './no-unreadable-iife.js';
6363
export {default as 'no-unused-properties'} from './no-unused-properties.js';
64+
export {default as 'no-useless-collection-argument'} from './no-useless-collection-argument.js';
6465
export {default as 'no-useless-error-capture-stack-trace'} from './no-useless-error-capture-stack-trace.js';
6566
export {default as 'no-useless-fallback-in-spread'} from './no-useless-fallback-in-spread.js';
6667
export {default as 'no-useless-length-check'} from './no-useless-length-check.js';

‎rules/no-useless-collection-argument.js‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import {isParenthesized, getParenthesizedRange} from './utils/index.js';
2+
import {
3+
isNewExpression,
4+
isEmptyArrayExpression,
5+
isEmptyStringLiteral,
6+
isNullLiteral,
7+
isUndefined,
8+
} from './ast/index.js';
9+
import {removeParentheses, removeArgument} from './fix/index.js';
10+
11+
const MESSAGE_ID = 'no-useless-collection-argument';
12+
const messages = {
13+
[MESSAGE_ID]: 'The {{description}} is useless.',
14+
};
15+
16+
const getDescription = node => {
17+
if (isEmptyArrayExpression(node)) {
18+
return 'empty array';
19+
}
20+
21+
if (isEmptyStringLiteral(node)) {
22+
return 'empty string';
23+
}
24+
25+
if (isNullLiteral(node)) {
26+
return '`null`';
27+
}
28+
29+
if (isUndefined(node)) {
30+
return '`undefined`';
31+
}
32+
};
33+
34+
const removeFallback = (node, context) =>
35+
// Same code from rules/no-useless-fallback-in-spread.js
36+
/** @param {import('eslint').Rule.RuleFixer} fixer */
37+
function * fix(fixer) {
38+
const {sourceCode} = context;
39+
const logicalExpression = node.parent;
40+
const {left} = logicalExpression;
41+
const isLeftObjectParenthesized = isParenthesized(left, sourceCode);
42+
const [, start] = isLeftObjectParenthesized
43+
? getParenthesizedRange(left, sourceCode)
44+
: sourceCode.getRange(left);
45+
const [, end] = sourceCode.getRange(logicalExpression);
46+
47+
yield fixer.removeRange([start, end]);
48+
49+
if (
50+
isLeftObjectParenthesized
51+
|| left.type !== 'SequenceExpression'
52+
) {
53+
yield * removeParentheses(logicalExpression, fixer, sourceCode);
54+
}
55+
};
56+
57+
/** @param {import('eslint').Rule.RuleContext} context */
58+
const create = context => ({
59+
NewExpression(newExpression) {
60+
if (!isNewExpression(newExpression, {
61+
names: ['Set', 'Map', 'WeakSet', 'WeakMap'],
62+
argumentsLength: 1,
63+
})) {
64+
return;
65+
}
66+
67+
const [iterable] = newExpression.arguments;
68+
const isCheckingFallback = iterable.type === 'LogicalExpression' && iterable.operator === '??';
69+
const node = isCheckingFallback ? iterable.right : iterable;
70+
const description = getDescription(node);
71+
72+
if (!description) {
73+
return;
74+
}
75+
76+
return {
77+
node,
78+
messageId: MESSAGE_ID,
79+
data: {description},
80+
fix: isCheckingFallback
81+
? removeFallback(node, context)
82+
: fixer => removeArgument(fixer, node, context.sourceCode),
83+
};
84+
},
85+
});
86+
87+
/** @type {import('eslint').Rule.RuleModule} */
88+
const config = {
89+
create,
90+
meta: {
91+
type: 'suggestion',
92+
docs: {
93+
description: 'Disallow useless values or fallbacks in `Set`, `Map`, `WeakSet`, or `WeakMap`.',
94+
recommended: 'unopinionated',
95+
},
96+
fixable: 'code',
97+
messages,
98+
},
99+
};
100+
101+
export default config;

‎rules/prefer-array-flat.js‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,13 @@ import {
99
shouldAddParenthesesToMemberExpressionObject,
1010
} from './utils/index.js';
1111
import {fixSpaceAroundKeyword} from './fix/index.js';
12-
import {isMethodCall, isCallExpression} from './ast/index.js';
12+
import {isMethodCall, isCallExpression, isEmptyArrayExpression} from './ast/index.js';
1313

1414
const MESSAGE_ID = 'prefer-array-flat';
1515
const messages = {
1616
[MESSAGE_ID]: 'Prefer `Array#flat()` over `{{description}}` to flatten an array.',
1717
};
1818

19-
const isEmptyArrayExpression = node =>
20-
node.type === 'ArrayExpression'
21-
&& node.elements.length === 0;
22-
2319
// `array.flatMap(x => x)`
2420
const arrayFlatMap = {
2521
testFunction(node) {

‎test/no-useless-collection-argument.js‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import {getTester} from './utils/test.js';
2+
3+
const {test} = getTester(import.meta);
4+
5+
// Useless value
6+
test.snapshot({
7+
valid: [
8+
'new Set()',
9+
'new Set',
10+
'new Set(foo)',
11+
'new Set(foo || [])',
12+
'new Set(foo && [])',
13+
'new Not_Set([])',
14+
'Set([])',
15+
'new Set([], extraArgument)',
16+
'new Set(...([]))',
17+
'new Set([""])',
18+
'new Set("not-empty")',
19+
'new Set(0)',
20+
'new ([])(Set)',
21+
// Not checking
22+
'new globalThis.Set([])',
23+
],
24+
invalid: [
25+
'new Set([])',
26+
'new Set("")',
27+
'new Set(undefined)',
28+
'new Set(null)',
29+
'new WeakSet([])',
30+
'new Map([])',
31+
'new WeakMap([])',
32+
'new Set( (([])) )',
33+
'new Set([],)',
34+
'new Set( (([])), )',
35+
],
36+
});
37+
38+
// Fallbacks
39+
test.snapshot({
40+
valid: [
41+
'new Set(foo || [])',
42+
'new Set(foo && [])',
43+
'new Not_Set(foo ?? [])',
44+
'Set(foo ?? [])',
45+
'new Set(foo ?? [], extraArgument)',
46+
'new Set(...(foo ?? []))',
47+
'new Set(foo ?? [""])',
48+
'new Set(foo ?? "not-empty")',
49+
'new Set(foo ?? 0)',
50+
'new (foo ?? [])(Set)',
51+
// Not checking
52+
'new globalThis.Set(foo ?? [])',
53+
],
54+
invalid: [
55+
'new Set(foo ?? [])',
56+
'new Set(foo ?? "")',
57+
'new Set(foo ?? undefined)',
58+
'new Set(foo ?? null)',
59+
'new WeakSet(foo ?? [])',
60+
'new Map(foo ?? [])',
61+
'new WeakMap(foo ?? [])',
62+
'new Set( ((foo ?? [])) )',
63+
'new Set( (( foo )) ?? [] )',
64+
'new Set( foo ?? (( [] )) )',
65+
'new Set( (await foo) ?? [] )',
66+
'new Set( (0, foo) ?? [] )',
67+
'new Set( (( (0, foo) ?? [] )) )',
68+
// Who cares?
69+
'new Set(document.all ?? [])',
70+
// Both sides are useless
71+
'new Set([] ?? "")',
72+
'new Set( (( (( "" )) ?? (( [] )) )) )',
73+
'new Set(foo ?? bar ?? [])',
74+
],
75+
});

0 commit comments

Comments
 (0)