Skip to content

Commit

Permalink
Add no-length-as-slice-end rule (#2400)
Browse files Browse the repository at this point in the history
Co-authored-by: Sindre Sorhus <[email protected]>
  • Loading branch information
fisker and sindresorhus authored Jul 12, 2024
1 parent 1deb9bb commit 3c33820
Show file tree
Hide file tree
Showing 6 changed files with 248 additions and 0 deletions.
30 changes: 30 additions & 0 deletions docs/rules/no-length-as-slice-end.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Disallow using `.length` as the `end` argument of `{Array,String,TypedArray}#slice()`

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

🔧 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` -->

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

## Fail

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

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

## Pass

```js
const foo = string.slice(1);
```

```js
const foo = bar.slice(1, baz.length);
```
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c
| [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-negated-condition](docs/rules/no-negated-condition.md) | Disallow negated conditions. || 🔧 | |
Expand Down
53 changes: 53 additions & 0 deletions rules/no-length-as-slice-end.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';
const {isMethodCall, isMemberExpression} = require('./ast/index.js');
const {removeArgument} = require('./fix/index.js');
const {isSameReference} = require('./utils/index.js');

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

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
context.on('CallExpression', callExpression => {
if (!isMethodCall(callExpression, {
method: 'slice',
argumentsLength: 2,
optionalCall: false,
})) {
return;
}

const secondArgument = callExpression.arguments[1];
const node = secondArgument.type === 'ChainExpression' ? secondArgument.expression : secondArgument;

if (
!isMemberExpression(node, {property: 'length', computed: false})
|| !isSameReference(callExpression.callee.object, node.object)
) {
return;
}

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

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Disallow using `.length` as the `end` argument of `{Array,String,TypedArray}#slice()`.',
recommended: true,
},
fixable: 'code',
messages,
},
};
33 changes: 33 additions & 0 deletions test/no-length-as-slice-end.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import {getTester} from './utils/test.mjs';

const {test} = getTester(import.meta);

test.snapshot({
valid: [
'foo.slice?.(1, foo.length)',
'foo.slice(foo.length, 1)',
'foo.slice()',
'foo.slice(1)',
'foo.slice(1, foo.length - 1)',
'foo.slice(1, foo.length, extraArgument)',
'foo.slice(...[1], foo.length)',
'foo.notSlice(1, foo.length)',
'new foo.slice(1, foo.length)',
'slice(1, foo.length)',
'foo.slice(1, foo.notLength)',
'foo.slice(1, length)',
'foo[slice](1, foo.length)',
'foo.slice(1, foo[length])',
'foo.slice(1, bar.length)',
// `isSameReference` consider they are not the same reference
'foo().slice(1, foo().length)',
],
invalid: [
'foo.slice(1, foo.length)',
'foo?.slice(1, foo.length)',
'foo.slice(1, foo.length,)',
'foo.slice(1, (( foo.length )))',
'foo.slice(1, foo?.length)',
'foo?.slice(1, foo?.length)',
],
});
131 changes: 131 additions & 0 deletions test/snapshots/no-length-as-slice-end.mjs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
# Snapshot report for `test/no-length-as-slice-end.mjs`

The actual snapshot is saved in `no-length-as-slice-end.mjs.snap`.

Generated by [AVA](https://avajs.dev).

## invalid(1): foo.slice(1, foo.length)

> Input
`␊
1 | foo.slice(1, foo.length)␊
`

> Output
`␊
1 | foo.slice(1)␊
`

> Error 1/1
`␊
> 1 | foo.slice(1, foo.length)␊
| ^^^^^^^^^^ Passing \`….length\` as the \`end\` argument is unnecessary.␊
`

## invalid(2): foo?.slice(1, foo.length)

> Input
`␊
1 | foo?.slice(1, foo.length)␊
`

> Output
`␊
1 | foo?.slice(1)␊
`

> Error 1/1
`␊
> 1 | foo?.slice(1, foo.length)␊
| ^^^^^^^^^^ Passing \`….length\` as the \`end\` argument is unnecessary.␊
`

## invalid(3): foo.slice(1, foo.length,)

> Input
`␊
1 | foo.slice(1, foo.length,)␊
`

> Output
`␊
1 | foo.slice(1,)␊
`

> Error 1/1
`␊
> 1 | foo.slice(1, foo.length,)␊
| ^^^^^^^^^^ Passing \`….length\` as the \`end\` argument is unnecessary.␊
`

## invalid(4): foo.slice(1, (( foo.length )))

> Input
`␊
1 | foo.slice(1, (( foo.length )))␊
`

> Output
`␊
1 | foo.slice(1)␊
`

> Error 1/1
`␊
> 1 | foo.slice(1, (( foo.length )))␊
| ^^^^^^^^^^ Passing \`….length\` as the \`end\` argument is unnecessary.␊
`

## invalid(5): foo.slice(1, foo?.length)

> Input
`␊
1 | foo.slice(1, foo?.length)␊
`

> Output
`␊
1 | foo.slice(1)␊
`

> Error 1/1
`␊
> 1 | foo.slice(1, foo?.length)␊
| ^^^^^^^^^^^ Passing \`….length\` as the \`end\` argument is unnecessary.␊
`

## invalid(6): foo?.slice(1, foo?.length)

> Input
`␊
1 | foo?.slice(1, foo?.length)␊
`

> Output
`␊
1 | foo?.slice(1)␊
`

> Error 1/1
`␊
> 1 | foo?.slice(1, foo?.length)␊
| ^^^^^^^^^^^ Passing \`….length\` as the \`end\` argument is unnecessary.␊
`
Binary file added test/snapshots/no-length-as-slice-end.mjs.snap
Binary file not shown.

0 comments on commit 3c33820

Please sign in to comment.