Skip to content

Commit 1922df1

Browse files
fiskerfisker
authored
Add no-unnecessary-array-splice-count rule, Rename no-length-as-slice-end to no-unnecessary-slice-end, Support checking Infinity in no-unnecessary-slice-end (#2614)
1 parent 052c281 commit 1922df1

21 files changed

+847
-232
lines changed

‎docs/deleted-and-deprecated-rules.md

+4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66

77
Replaced by [`no-instanceof-builtins`](rules/no-instanceof-builtins.md) which covers more cases.
88

9+
### no-length-as-slice-end
10+
11+
Replaced by [`no-unnecessary-slice-end`](rules/no-unnecessary-slice-end.md) which covers more cases.
12+
913
## Deleted rules
1014

1115
### ~import-index~

‎docs/rules/no-unnecessary-array-splice-count.md

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Disallow using `.length` or `Infinity` as the `deleteCount` or `skipCount` argument of `Array#{splice,toSpliced}()`
2+
3+
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config).
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+
<!-- Remove this comment, add more detailed description. -->
11+
12+
When calling `Array#splice(start, deleteCount)` and `Array#toSpliced(start, skipCount)`, omitting the `deleteCount` and `skipCount` argument will delete or skip all elements after `start`. Using `.length` or `Infinity` is unnecessary.
13+
14+
## Examples
15+
16+
```js
17+
// ❌
18+
const foo = array.toSpliced(1, string.length);
19+
20+
// ✅
21+
const foo = array.toSpliced(1);
22+
```
23+
24+
```js
25+
// ❌
26+
const foo = array.toSpliced(1, Infinity);
27+
28+
// ✅
29+
const foo = array.toSpliced(1);
30+
```
31+
32+
```js
33+
// ❌
34+
const foo = array.toSpliced(1, Number.POSITIVE_INFINITY);
35+
36+
// ✅
37+
const foo = array.toSpliced(1);
38+
```
39+
40+
```js
41+
// ❌
42+
array.splice(1, string.length);
43+
44+
// ✅
45+
array.splice(1);
46+
```
47+
48+
```js
49+
// ❌
50+
array.splice(1, Infinity);
51+
52+
// ✅
53+
array.splice(1);
54+
```
55+
56+
```js
57+
// ❌
58+
array.splice(1, Number.POSITIVE_INFINITY);
59+
60+
// ✅
61+
array.splice(1);
62+
```

‎docs/rules/no-length-as-slice-end.md renamed to ‎docs/rules/no-unnecessary-slice-end.md

+15-9
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Disallow using `.length` as the `end` argument of `{Array,String,TypedArray}#slice()`
1+
# Disallow using `.length` or `Infinity` as the `end` argument of `{Array,String,TypedArray}#slice()`
22

33
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config).
44

@@ -7,24 +7,30 @@
77
<!-- end auto-generated rule header -->
88
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
99

10-
When calling `{String,Array,TypedArray}#slice(start, end)`, omitting the `end` argument defaults it to the object's `.length`. Passing it explicitly is unnecessary.
10+
When calling `{String,Array,TypedArray}#slice(start, end)`, omitting the `end` argument defaults it to the object's `.length`. Passing it explicitly or using `Infinity` is unnecessary.
1111

12-
## Fail
12+
## Examples
1313

1414
```js
15+
// ❌
1516
const foo = string.slice(1, string.length);
16-
```
1717

18-
```js
19-
const foo = array.slice(1, array.length);
18+
// ✅
19+
const foo = string.slice(1);
2020
```
2121

22-
## Pass
23-
2422
```js
23+
// ❌
24+
const foo = string.slice(1, Infinity);
25+
26+
// ✅
2527
const foo = string.slice(1);
2628
```
2729

2830
```js
29-
const foo = bar.slice(1, baz.length);
31+
// ❌
32+
const foo = string.slice(1, Number.POSITIVE_INFINITY);
33+
34+
// ✅
35+
const foo = string.slice(1);
3036
```

‎index.js

+4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ const deprecatedRules = createDeprecatedRules({
99
message: 'Replaced by `unicorn/no-instanceof-builtins` which covers more cases.',
1010
replacedBy: ['unicorn/no-instanceof-builtins'],
1111
},
12+
'no-length-as-slice-end': {
13+
message: 'Replaced by `unicorn/no-unnecessary-slice-end` which covers more cases.',
14+
replacedBy: ['unicorn/no-unnecessary-slice-end'],
15+
},
1216
});
1317

1418
const externalRules = {

‎readme.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ export default [
9292
| [no-invalid-fetch-options](docs/rules/no-invalid-fetch-options.md) | Disallow invalid options in `fetch()` and `new Request()`. | ✅ | | |
9393
| [no-invalid-remove-event-listener](docs/rules/no-invalid-remove-event-listener.md) | Prevent calling `EventTarget#removeEventListener()` with the result of an expression. | ✅ | | |
9494
| [no-keyword-prefix](docs/rules/no-keyword-prefix.md) | Disallow identifiers starting with `new` or `class`. | | | |
95-
| [no-length-as-slice-end](docs/rules/no-length-as-slice-end.md) | Disallow using `.length` as the `end` argument of `{Array,String,TypedArray}#slice()`. | ✅ | 🔧 | |
9695
| [no-lonely-if](docs/rules/no-lonely-if.md) | Disallow `if` statements as the only statement in `if` blocks without `else`. | ✅ | 🔧 | |
9796
| [no-magic-array-flat-depth](docs/rules/no-magic-array-flat-depth.md) | Disallow a magic number as the `depth` argument in `Array#flat(…).` | ✅ | | |
9897
| [no-named-default](docs/rules/no-named-default.md) | Disallow named usage of default import and export. | ✅ | 🔧 | |
@@ -109,8 +108,10 @@ export default [
109108
| [no-thenable](docs/rules/no-thenable.md) | Disallow `then` property. | ✅ | | |
110109
| [no-this-assignment](docs/rules/no-this-assignment.md) | Disallow assigning `this` to a variable. | ✅ | | |
111110
| [no-typeof-undefined](docs/rules/no-typeof-undefined.md) | Disallow comparing `undefined` using `typeof`. | ✅ | 🔧 | 💡 |
111+
| [no-unnecessary-array-splice-count](docs/rules/no-unnecessary-array-splice-count.md) | Disallow using `.length` or `Infinity` as the `deleteCount` or `skipCount` argument of `Array#{splice,toSpliced}()`. | ✅ | 🔧 | |
112112
| [no-unnecessary-await](docs/rules/no-unnecessary-await.md) | Disallow awaiting non-promise values. | ✅ | 🔧 | |
113113
| [no-unnecessary-polyfills](docs/rules/no-unnecessary-polyfills.md) | Enforce the use of built-in methods instead of unnecessary polyfills. | ✅ | | |
114+
| [no-unnecessary-slice-end](docs/rules/no-unnecessary-slice-end.md) | Disallow using `.length` or `Infinity` as the `end` argument of `{Array,String,TypedArray}#slice()`. | ✅ | 🔧 | |
114115
| [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) | Disallow unreadable array destructuring. | ✅ | 🔧 | |
115116
| [no-unreadable-iife](docs/rules/no-unreadable-iife.md) | Disallow unreadable IIFEs. | ✅ | | |
116117
| [no-unused-properties](docs/rules/no-unused-properties.md) | Disallow unused object properties. | | | |

‎rules/index.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import noInstanceofBuiltins from './no-instanceof-builtins.js';
3737
import noInvalidFetchOptions from './no-invalid-fetch-options.js';
3838
import noInvalidRemoveEventListener from './no-invalid-remove-event-listener.js';
3939
import noKeywordPrefix from './no-keyword-prefix.js';
40-
import noLengthAsSliceEnd from './no-length-as-slice-end.js';
4140
import noLonelyIf from './no-lonely-if.js';
4241
import noMagicArrayFlatDepth from './no-magic-array-flat-depth.js';
4342
import noNamedDefault from './no-named-default.js';
@@ -54,8 +53,10 @@ import noStaticOnlyClass from './no-static-only-class.js';
5453
import noThenable from './no-thenable.js';
5554
import noThisAssignment from './no-this-assignment.js';
5655
import noTypeofUndefined from './no-typeof-undefined.js';
56+
import noUnnecessaryArraySpliceCount from './no-unnecessary-array-splice-count.js';
5757
import noUnnecessaryAwait from './no-unnecessary-await.js';
5858
import noUnnecessaryPolyfills from './no-unnecessary-polyfills.js';
59+
import noUnnecessarySliceEnd from './no-unnecessary-slice-end.js';
5960
import noUnreadableArrayDestructuring from './no-unreadable-array-destructuring.js';
6061
import noUnreadableIife from './no-unreadable-iife.js';
6162
import noUnusedProperties from './no-unused-properties.js';
@@ -166,7 +167,6 @@ const rules = {
166167
'no-invalid-fetch-options': createRule(noInvalidFetchOptions, 'no-invalid-fetch-options'),
167168
'no-invalid-remove-event-listener': createRule(noInvalidRemoveEventListener, 'no-invalid-remove-event-listener'),
168169
'no-keyword-prefix': createRule(noKeywordPrefix, 'no-keyword-prefix'),
169-
'no-length-as-slice-end': createRule(noLengthAsSliceEnd, 'no-length-as-slice-end'),
170170
'no-lonely-if': createRule(noLonelyIf, 'no-lonely-if'),
171171
'no-magic-array-flat-depth': createRule(noMagicArrayFlatDepth, 'no-magic-array-flat-depth'),
172172
'no-named-default': createRule(noNamedDefault, 'no-named-default'),
@@ -183,8 +183,10 @@ const rules = {
183183
'no-thenable': createRule(noThenable, 'no-thenable'),
184184
'no-this-assignment': createRule(noThisAssignment, 'no-this-assignment'),
185185
'no-typeof-undefined': createRule(noTypeofUndefined, 'no-typeof-undefined'),
186+
'no-unnecessary-array-splice-count': createRule(noUnnecessaryArraySpliceCount, 'no-unnecessary-array-splice-count'),
186187
'no-unnecessary-await': createRule(noUnnecessaryAwait, 'no-unnecessary-await'),
187188
'no-unnecessary-polyfills': createRule(noUnnecessaryPolyfills, 'no-unnecessary-polyfills'),
189+
'no-unnecessary-slice-end': createRule(noUnnecessarySliceEnd, 'no-unnecessary-slice-end'),
188190
'no-unreadable-array-destructuring': createRule(noUnreadableArrayDestructuring, 'no-unreadable-array-destructuring'),
189191
'no-unreadable-iife': createRule(noUnreadableIife, 'no-unreadable-iife'),
190192
'no-unused-properties': createRule(noUnusedProperties, 'no-unused-properties'),

‎rules/no-length-as-slice-end.js

-54
This file was deleted.

‎rules/no-unnecessary-array-splice-count.js

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import {listen} from './shared/no-unnecessary-length-or-infinity-rule.js';
2+
3+
const MESSAGE_ID = 'no-unnecessary-array-splice-count';
4+
const messages = {
5+
[MESSAGE_ID]: 'Passing `{{description}}` as the `{{argumentName}}` argument is unnecessary.',
6+
};
7+
8+
/** @type {import('eslint').Rule.RuleModule} */
9+
const config = {
10+
create: context => listen(context, {methods: ['splice', 'toSpliced'], messageId: MESSAGE_ID}),
11+
meta: {
12+
type: 'suggestion',
13+
docs: {
14+
description: 'Disallow using `.length` or `Infinity` as the `deleteCount` or `skipCount` argument of `Array#{splice,toSpliced}()`.',
15+
recommended: true,
16+
},
17+
fixable: 'code',
18+
19+
messages,
20+
},
21+
};
22+
23+
export default config;

‎rules/no-unnecessary-slice-end.js

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import {listen} from './shared/no-unnecessary-length-or-infinity-rule.js';
2+
3+
const MESSAGE_ID = 'no-unnecessary-slice-end';
4+
const messages = {
5+
[MESSAGE_ID]: 'Passing `{{description}}` as the `end` argument is unnecessary.',
6+
};
7+
8+
/** @type {import('eslint').Rule.RuleModule} */
9+
const config = {
10+
create: context => listen(context, {methods: ['slice'], messageId: MESSAGE_ID}),
11+
meta: {
12+
type: 'suggestion',
13+
docs: {
14+
description: 'Disallow using `.length` or `Infinity` as the `end` argument of `{Array,String,TypedArray}#slice()`.',
15+
recommended: true,
16+
},
17+
fixable: 'code',
18+
messages,
19+
},
20+
};
21+
22+
export default config;

‎rules/shared/no-unnecessary-length-or-infinity-rule.js

+80
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import {isMethodCall, isMemberExpression} from '../ast/index.js';
2+
import {isSameReference} from '../utils/index.js';
3+
import {removeArgument} from '../fix/index.js';
4+
5+
function getObjectLengthOrInfinityDescription(node, object) {
6+
// `Infinity`
7+
if (node.type === 'Identifier' && node.name === 'Infinity') {
8+
return 'Infinity';
9+
}
10+
11+
// `Number.POSITIVE_INFINITY`
12+
if (isMemberExpression(node, {
13+
object: 'Number',
14+
property: 'POSITIVE_INFINITY',
15+
computed: false,
16+
optional: false,
17+
})) {
18+
return 'Number.POSITIVE_INFINITY';
19+
}
20+
21+
// `object?.length`
22+
const isOptional = node.type === 'ChainExpression';
23+
if (isOptional) {
24+
node = node.expression;
25+
}
26+
27+
// `object.length`
28+
if (!(
29+
isMemberExpression(node, {property: 'length', computed: false})
30+
&& isSameReference(object, node.object)
31+
)) {
32+
return;
33+
}
34+
35+
return `${object.type === 'Identifier' ? object.name : '…'}${isOptional ? '?.' : '.'}length`;
36+
}
37+
38+
/** @param {import('eslint').Rule.RuleContext} context */
39+
function listen(context, {methods, messageId}) {
40+
context.on('CallExpression', callExpression => {
41+
if (!isMethodCall(callExpression, {
42+
methods,
43+
argumentsLength: 2,
44+
optionalCall: false,
45+
})) {
46+
return;
47+
}
48+
49+
const secondArgument = callExpression.arguments[1];
50+
const description = getObjectLengthOrInfinityDescription(
51+
secondArgument,
52+
callExpression.callee.object,
53+
);
54+
55+
if (!description) {
56+
return;
57+
}
58+
59+
const methodName = callExpression.callee.property.name;
60+
const messageData = {
61+
description,
62+
};
63+
64+
if (methodName === 'splice') {
65+
messageData.argumentName = 'deleteCount';
66+
} else if (methodName === 'toSpliced') {
67+
messageData.argumentName = 'skipCount';
68+
}
69+
70+
return {
71+
node: secondArgument,
72+
messageId,
73+
data: messageData,
74+
/** @param {import('eslint').Rule.RuleFixer} fixer */
75+
fix: fixer => removeArgument(fixer, secondArgument, context.sourceCode),
76+
};
77+
});
78+
}
79+
80+
export {listen};

‎scripts/rename-rule.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ async function renameRule(from, to) {
4545
`rules/${to}.js`,
4646
`test/${to}.js`,
4747
`test/snapshots/${to}.js.md`,
48-
].map(file => resolveFile(file))
49-
) {
48+
].map(file => resolveFile(file))) {
5049
if (!fs.existsSync(file)) {
5150
continue;
5251
}

0 commit comments

Comments
 (0)