Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Mar 29, 2025
Merged
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ package-lock.json
.cache-eslint-remote-tester
eslint-remote-tester-results
*.log
.vscode
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure add this to global ignore is a good idea, since many repos actually stores .vscode.
Adding this because my VSCode keeps creating this empty directory.

4 changes: 4 additions & 0 deletions docs/deleted-and-deprecated-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

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

### no-length-as-slice-end

Replaced by [`no-unnecessary-slice-end`](rules/no-unnecessary-slice-end.md) which covers more cases..

## Deleted rules

### ~import-index~
Expand Down
62 changes: 62 additions & 0 deletions docs/rules/no-unnecessary-array-splice-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Disallow using `.length` or `Infinity` as the `deleteCount` or `skipCount` argument of `Array#{splice,toSpliced}()`

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

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->

<!-- Remove this comment, add more detailed description. -->

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.

## Examples

```js
// ❌
const foo = array.toSpliced(1, string.length);

// ✅
const foo = array.toSpliced(1);
```

```js
// ❌
const foo = array.toSpliced(1, Infinity);

// ✅
const foo = array.toSpliced(1);
```

```js
// ❌
const foo = array.toSpliced(1, Number.POSITIVE_INFINITY);

// ✅
const foo = array.toSpliced(1);
```

```js
// ❌
array.splice(1, string.length);

// ✅
array.splice(1);
```

```js
// ❌
array.splice(1, Infinity);

// ✅
array.splice(1);
```

```js
// ❌
array.splice(1, Number.POSITIVE_INFINITY);

// ✅
array.splice(1);
```
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Disallow using `.length` as the `end` argument of `{Array,String,TypedArray}#slice()`
# Disallow using `.length` or `Infinity` as the `end` argument of `{Array,String,TypedArray}#slice()`

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

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

When calling `{String,Array,TypedArray}#slice(start, end)`, omitting the `end` argument defaults it to the object's `.length`. Passing it explicitly is unnecessary.
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.

## Fail
## Examples

```js
// ❌
const foo = string.slice(1, string.length);
```

```js
const foo = array.slice(1, array.length);
// ✅
const foo = string.slice(1);
```

## Pass

```js
// ❌
const foo = string.slice(1, Infinity);

// ✅
const foo = string.slice(1);
```

```js
const foo = bar.slice(1, baz.length);
// ❌
const foo = string.slice(1, Number.POSITIVE_INFINITY);

// ✅
const foo = string.slice(1);
```
4 changes: 4 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
message: 'Replaced by `unicorn/no-instanceof-builtins` which covers more cases.',
replacedBy: ['unicorn/no-instanceof-builtins'],
},
'no-length-as-slice-end': {
message: 'Replaced by `unicorn/no-unnecessary-slice-end` which covers more cases.',
replacedBy: ['unicorn/no-unnecessary-slice-end'],
},
});

const externalRules = {
Expand Down Expand Up @@ -59,7 +63,7 @@
recommended: createConfig(recommendedRules, 'unicorn/recommended'),
all: createConfig(allRules, 'unicorn/all'),

// TODO: Remove this at some point. Kept for now to avoid breaking users.

Check warning on line 66 in index.js

View workflow job for this annotation

GitHub Actions / lint-test (ubuntu-latest)

Unexpected 'todo' comment: 'TODO: Remove this at some point. Kept...'

Check warning on line 66 in index.js

View workflow job for this annotation

GitHub Actions / lint-test (windows-latest)

Unexpected 'todo' comment: 'TODO: Remove this at some point. Kept...'
'flat/recommended': createConfig(recommendedRules, 'unicorn/flat/recommended'),
'flat/all': createConfig(allRules, 'unicorn/flat/all'),
};
Expand Down
3 changes: 2 additions & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ export default [
| [no-invalid-fetch-options](docs/rules/no-invalid-fetch-options.md) | Disallow invalid options in `fetch()` and `new Request()`. | ✅ | | |
| [no-invalid-remove-event-listener](docs/rules/no-invalid-remove-event-listener.md) | Prevent calling `EventTarget#removeEventListener()` with the result of an expression. | ✅ | | |
| [no-keyword-prefix](docs/rules/no-keyword-prefix.md) | Disallow identifiers starting with `new` or `class`. | | | |
| [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()`. | ✅ | 🔧 | |
| [no-lonely-if](docs/rules/no-lonely-if.md) | Disallow `if` statements as the only statement in `if` blocks without `else`. | ✅ | 🔧 | |
| [no-magic-array-flat-depth](docs/rules/no-magic-array-flat-depth.md) | Disallow a magic number as the `depth` argument in `Array#flat(…).` | ✅ | | |
| [no-named-default](docs/rules/no-named-default.md) | Disallow named usage of default import and export. | ✅ | 🔧 | |
Expand All @@ -109,8 +108,10 @@ export default [
| [no-thenable](docs/rules/no-thenable.md) | Disallow `then` property. | ✅ | | |
| [no-this-assignment](docs/rules/no-this-assignment.md) | Disallow assigning `this` to a variable. | ✅ | | |
| [no-typeof-undefined](docs/rules/no-typeof-undefined.md) | Disallow comparing `undefined` using `typeof`. | ✅ | 🔧 | 💡 |
| [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}()`. | ✅ | 🔧 | |
| [no-unnecessary-await](docs/rules/no-unnecessary-await.md) | Disallow awaiting non-promise values. | ✅ | 🔧 | |
| [no-unnecessary-polyfills](docs/rules/no-unnecessary-polyfills.md) | Enforce the use of built-in methods instead of unnecessary polyfills. | ✅ | | |
| [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()`. | ✅ | 🔧 | |
| [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) | Disallow unreadable array destructuring. | ✅ | 🔧 | |
| [no-unreadable-iife](docs/rules/no-unreadable-iife.md) | Disallow unreadable IIFEs. | ✅ | | |
| [no-unused-properties](docs/rules/no-unused-properties.md) | Disallow unused object properties. | | | |
Expand Down
6 changes: 4 additions & 2 deletions rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import noInstanceofBuiltins from './no-instanceof-builtins.js';
import noInvalidFetchOptions from './no-invalid-fetch-options.js';
import noInvalidRemoveEventListener from './no-invalid-remove-event-listener.js';
import noKeywordPrefix from './no-keyword-prefix.js';
import noLengthAsSliceEnd from './no-length-as-slice-end.js';
import noLonelyIf from './no-lonely-if.js';
import noMagicArrayFlatDepth from './no-magic-array-flat-depth.js';
import noNamedDefault from './no-named-default.js';
Expand All @@ -54,8 +53,10 @@ import noStaticOnlyClass from './no-static-only-class.js';
import noThenable from './no-thenable.js';
import noThisAssignment from './no-this-assignment.js';
import noTypeofUndefined from './no-typeof-undefined.js';
import noUnnecessaryArraySpliceCount from './no-unnecessary-array-splice-count.js';
import noUnnecessaryAwait from './no-unnecessary-await.js';
import noUnnecessaryPolyfills from './no-unnecessary-polyfills.js';
import noUnnecessarySliceEnd from './no-unnecessary-slice-end.js';
import noUnreadableArrayDestructuring from './no-unreadable-array-destructuring.js';
import noUnreadableIife from './no-unreadable-iife.js';
import noUnusedProperties from './no-unused-properties.js';
Expand Down Expand Up @@ -166,7 +167,6 @@ const rules = {
'no-invalid-fetch-options': createRule(noInvalidFetchOptions, 'no-invalid-fetch-options'),
'no-invalid-remove-event-listener': createRule(noInvalidRemoveEventListener, 'no-invalid-remove-event-listener'),
'no-keyword-prefix': createRule(noKeywordPrefix, 'no-keyword-prefix'),
'no-length-as-slice-end': createRule(noLengthAsSliceEnd, 'no-length-as-slice-end'),
'no-lonely-if': createRule(noLonelyIf, 'no-lonely-if'),
'no-magic-array-flat-depth': createRule(noMagicArrayFlatDepth, 'no-magic-array-flat-depth'),
'no-named-default': createRule(noNamedDefault, 'no-named-default'),
Expand All @@ -183,8 +183,10 @@ const rules = {
'no-thenable': createRule(noThenable, 'no-thenable'),
'no-this-assignment': createRule(noThisAssignment, 'no-this-assignment'),
'no-typeof-undefined': createRule(noTypeofUndefined, 'no-typeof-undefined'),
'no-unnecessary-array-splice-count': createRule(noUnnecessaryArraySpliceCount, 'no-unnecessary-array-splice-count'),
'no-unnecessary-await': createRule(noUnnecessaryAwait, 'no-unnecessary-await'),
'no-unnecessary-polyfills': createRule(noUnnecessaryPolyfills, 'no-unnecessary-polyfills'),
'no-unnecessary-slice-end': createRule(noUnnecessarySliceEnd, 'no-unnecessary-slice-end'),
'no-unreadable-array-destructuring': createRule(noUnreadableArrayDestructuring, 'no-unreadable-array-destructuring'),
'no-unreadable-iife': createRule(noUnreadableIife, 'no-unreadable-iife'),
'no-unused-properties': createRule(noUnusedProperties, 'no-unused-properties'),
Expand Down
54 changes: 0 additions & 54 deletions rules/no-length-as-slice-end.js

This file was deleted.

23 changes: 23 additions & 0 deletions rules/no-unnecessary-array-splice-count.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import {listen} from './shared/no-unnecessary-length-or-infinity-rule.js';

const MESSAGE_ID = 'no-unnecessary-array-splice-count';
const messages = {
[MESSAGE_ID]: 'Passing `{{description}}` as the `{{argumentName}}` argument is unnecessary.',
};

/** @type {import('eslint').Rule.RuleModule} */
const config = {
create: context => listen(context, {methods: ['splice', 'toSpliced'], messageId: MESSAGE_ID}),
meta: {
type: 'suggestion',
docs: {
description: 'Disallow using `.length` or `Infinity` as the `deleteCount` or `skipCount` argument of `Array#{splice,toSpliced}()`.',
recommended: true,
},
fixable: 'code',

messages,
},
};

export default config;
22 changes: 22 additions & 0 deletions rules/no-unnecessary-slice-end.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {listen} from './shared/no-unnecessary-length-or-infinity-rule.js';

const MESSAGE_ID = 'no-unnecessary-slice-end';
const messages = {
[MESSAGE_ID]: 'Passing `{{description}}` as the `end` argument is unnecessary.',
};

/** @type {import('eslint').Rule.RuleModule} */
const config = {
create: context => listen(context, {methods: ['slice'], messageId: MESSAGE_ID}),
meta: {
type: 'suggestion',
docs: {
description: 'Disallow using `.length` or `Infinity` as the `end` argument of `{Array,String,TypedArray}#slice()`.',
recommended: true,
},
fixable: 'code',
messages,
},
};

export default config;
80 changes: 80 additions & 0 deletions rules/shared/no-unnecessary-length-or-infinity-rule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import {isMethodCall, isMemberExpression} from '../ast/index.js';
import {isSameReference} from '../utils/index.js';
import {removeArgument} from '../fix/index.js';

function getObjectLengthOrInfinityDescription(node, object) {
// `Infinity`
if (node.type === 'Identifier' && node.name === 'Infinity') {
return 'Infinity';
}

// `Number.POSITIVE_INFINITY`
if (isMemberExpression(node, {
object: 'Number',
property: 'POSITIVE_INFINITY',
computed: false,
optional: false,
})) {
return 'Number.POSITIVE_INFINITY';
}

// `object?.length`
const isOptional = node.type === 'ChainExpression';
if (isOptional) {
node = node.expression;
}

// `object.length`
if (!(
isMemberExpression(node, {property: 'length', computed: false})
&& isSameReference(object, node.object)
)) {
return;
}

return `${object.type === 'Identifier' ? object.name : '…'}${isOptional ? '?.' : '.'}length`;
}

/** @param {import('eslint').Rule.RuleContext} context */
function listen(context, {methods, messageId}) {
context.on('CallExpression', callExpression => {
if (!isMethodCall(callExpression, {
methods,
argumentsLength: 2,
optionalCall: false,
})) {
return;
}

const secondArgument = callExpression.arguments[1];
const description = getObjectLengthOrInfinityDescription(
secondArgument,
callExpression.callee.object,
);

if (!description) {
return;
}

const methodName = callExpression.callee.property.name;
const messageData = {
description,
};

if (methodName === 'splice') {
messageData.argumentName = 'deleteCount';
} else if (methodName === 'toSpliced') {
messageData.argumentName = 'skipCount';
}

return {
node: secondArgument,
messageId,
data: messageData,
/** @param {import('eslint').Rule.RuleFixer} fixer */
fix: fixer => removeArgument(fixer, secondArgument, context.sourceCode),
};
});
}

export {listen};
3 changes: 1 addition & 2 deletions scripts/rename-rule.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ async function renameRule(from, to) {
`rules/${to}.js`,
`test/${to}.js`,
`test/snapshots/${to}.js.md`,
].map(file => resolveFile(file))
) {
].map(file => resolveFile(file))) {
if (!fs.existsSync(file)) {
continue;
}
Expand Down
Loading
Loading