Skip to content

Add no-array-sort rule #2713

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 6 commits into from
Aug 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/rules/no-array-reverse.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ Pass `allowExpressionStatement: false` to forbid `Array#reverse()` even if it's
#### Fail

```js
// eslint unicorn/no-array-reverse: ["error", {"allowExpressionStatement": true}]
// eslint unicorn/no-array-reverse: ["error", {"allowExpressionStatement": false}]
array.reverse();
```

## Related rules

- [unicorn/no-array-sort](./no-array-sort.md)
53 changes: 53 additions & 0 deletions docs/rules/no-array-sort.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Prefer `Array#toSorted()` over `Array#sort()`

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

💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions).

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

Prefer using [`Array#toSorted()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/toSorted) over [`Array#sort()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort).

`Array#sort()` modifies the original array, while `Array#toSorted()` returns a new reversed array.

## Examples

```js
// ❌
const sorted = [...array].sort();

// ✅
const sorted = [...array].toSorted();
```

```js
// ❌
const sorted = [...array].sort((a, b) => a - b);

// ✅
const sorted = [...array].toSorted((a, b) => a - b);
```

## Options

Type: `object`

### allowExpressionStatement

Type: `boolean`\
Default: `true`

This rule allows `array.sort()` to be used as an expression statement by default.\
Pass `allowExpressionStatement: false` to forbid `Array#sort()` even if it's an expression statement.

#### Fail

```js
// eslint unicorn/no-array-sort: ["error", {"allowExpressionStatement": false}]
array.sort();
```

## Related rules

- [unicorn/no-array-reverse](./no-array-reverse.md)
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export default [
| [no-array-method-this-argument](docs/rules/no-array-method-this-argument.md) | Disallow using the `this` argument in array methods. | ✅ | 🔧 | 💡 |
| [no-array-reduce](docs/rules/no-array-reduce.md) | Disallow `Array#reduce()` and `Array#reduceRight()`. | ✅ | | |
| [no-array-reverse](docs/rules/no-array-reverse.md) | Prefer `Array#toReversed()` over `Array#reverse()`. | ✅ | | 💡 |
| [no-array-sort](docs/rules/no-array-sort.md) | Prefer `Array#toSorted()` over `Array#sort()`. | ✅ | | 💡 |
| [no-await-expression-member](docs/rules/no-await-expression-member.md) | Disallow member access from await expression. | ✅ | 🔧 | |
| [no-await-in-promise-methods](docs/rules/no-await-in-promise-methods.md) | Disallow using `await` in `Promise` method parameters. | ✅ | | 💡 |
| [no-console-spaces](docs/rules/no-console-spaces.md) | Do not use leading/trailing space between `console.log` parameters. | ✅ | 🔧 | |
Expand Down
1 change: 1 addition & 0 deletions rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export {default as 'no-array-for-each'} from './no-array-for-each.js';
export {default as 'no-array-method-this-argument'} from './no-array-method-this-argument.js';
export {default as 'no-array-reduce'} from './no-array-reduce.js';
export {default as 'no-array-reverse'} from './no-array-reverse.js';
export {default as 'no-array-sort'} from './no-array-sort.js';
export {default as 'no-await-expression-member'} from './no-await-expression-member.js';
export {default as 'no-await-in-promise-methods'} from './no-await-in-promise-methods.js';
export {default as 'no-console-spaces'} from './no-console-spaces.js';
Expand Down
107 changes: 2 additions & 105 deletions rules/no-array-reverse.js
Original file line number Diff line number Diff line change
@@ -1,109 +1,6 @@
import {isMethodCall} from './ast/index.js';
import {getParenthesizedText} from './utils/index.js';

const MESSAGE_ID_ERROR = 'no-array-reverse/error';
const MESSAGE_ID_SUGGESTION_ONLY_FIX_METHOD = 'no-array-reverse/suggestion-only-fix-method';
const MESSAGE_ID_SUGGESTION_SPREADING_ARRAY = 'no-array-reverse/suggestion-spreading-array';
const MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY = 'no-array-reverse/suggestion-not-spreading-array';
const messages = {
[MESSAGE_ID_ERROR]: 'Use `Array#toReversed()` instead of `Array#reverse()`.',
[MESSAGE_ID_SUGGESTION_ONLY_FIX_METHOD]: 'Switch to `.toReversed()`.',
[MESSAGE_ID_SUGGESTION_SPREADING_ARRAY]: 'The spreading object is an array',
[MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY]: 'The spreading object is NOT an array',
};

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const {sourceCode} = context;
const {allowExpressionStatement} = context.options[0];

return {
CallExpression(callExpression) {
if (!isMethodCall(callExpression, {
method: 'reverse',
argumentsLength: 0,
optionalCall: false,
})) {
return;
}

const array = callExpression.callee.object;

// `[...array].reverse()`
const isSpreadAndReverse = array.type === 'ArrayExpression'
&& array.elements.length === 1
&& array.elements[0].type === 'SpreadElement';

if (allowExpressionStatement && !isSpreadAndReverse) {
const maybeExpressionStatement = callExpression.parent.type === 'ChainExpression'
? callExpression.parent.parent
: callExpression.parent;
if (maybeExpressionStatement.type === 'ExpressionStatement') {
return;
}
}

const reverseProperty = callExpression.callee.property;
const suggestions = [];
const fixMethodName = fixer => fixer.replaceText(reverseProperty, 'toReversed');

/*
For `[...array].reverse()`, provide two suggestion, let user choose if the object can be unwrapped,
otherwise only change `.reverse()` to `.toReversed()`
*/
if (isSpreadAndReverse) {
suggestions.push({
messageId: MESSAGE_ID_SUGGESTION_SPREADING_ARRAY,
* fix(fixer) {
const text = getParenthesizedText(array.elements[0].argument, sourceCode);
yield fixer.replaceText(array, text);
yield fixMethodName(fixer);
},
});
}

suggestions.push({
messageId: isSpreadAndReverse
? MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY
: MESSAGE_ID_SUGGESTION_ONLY_FIX_METHOD,
fix: fixMethodName,
});

return {
node: reverseProperty,
messageId: MESSAGE_ID_ERROR,
suggest: suggestions,
};
},
};
};

const schema = [
{
type: 'object',
additionalProperties: false,
properties: {
allowExpressionStatement: {
type: 'boolean',
},
},
},
];
import noArrayMutateRule from './shared/no-array-mutate-rule.js';

/** @type {import('eslint').Rule.RuleModule} */
const config = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Prefer `Array#toReversed()` over `Array#reverse()`.',
recommended: true,
},
hasSuggestions: true,
schema,
defaultOptions: [{allowExpressionStatement: true}],
messages,
},
};
const config = noArrayMutateRule('reverse');

export default config;
6 changes: 6 additions & 0 deletions rules/no-array-sort.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import noArrayMutateRule from './shared/no-array-mutate-rule.js';

/** @type {import('eslint').Rule.RuleModule} */
const config = noArrayMutateRule('sort');

export default config;
2 changes: 1 addition & 1 deletion rules/prevent-abbreviations.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ const getWordReplacements = (word, {replacements, allowList}) => {
.map(name => transform(name));
}

return wordReplacement.length > 0 ? wordReplacement.sort() : [];
return wordReplacement.length > 0 ? wordReplacement.toSorted() : [];
};

const getNameReplacements = (name, options, limit = 3) => {
Expand Down
140 changes: 140 additions & 0 deletions rules/shared/no-array-mutate-rule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import {isMethodCall} from '../ast/index.js';
import {getParenthesizedText} from '../utils/index.js';

const MESSAGE_ID_ERROR = 'error';
const MESSAGE_ID_SUGGESTION_APPLY_REPLACEMENT = 'suggestion-apply-replacement';
const MESSAGE_ID_SUGGESTION_SPREADING_ARRAY = 'suggestion-spreading-array';
const MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY = 'suggestion-not-spreading-array';

const methods = new Map([
[
'reverse',
{
replacement: 'toReversed',
predicate: callExpression => isMethodCall(callExpression, {
method: 'reverse',
argumentsLength: 0,
optionalCall: false,
}),
},
],
[
'sort',
{
replacement: 'toSorted',
predicate: callExpression => isMethodCall(callExpression, {
method: 'sort',
maximumArguments: 1,
optionalCall: false,
}),
},
],
]);

const schema = [
{
type: 'object',
additionalProperties: false,
properties: {
allowExpressionStatement: {
type: 'boolean',
},
},
},
];

function noArrayMutateRule(methodName) {
const {
replacement,
predicate,
} = methods.get(methodName);

const messages = {
[MESSAGE_ID_ERROR]: `Use \`Array#${replacement}()\` instead of \`Array#${methodName}()\`.`,
[MESSAGE_ID_SUGGESTION_APPLY_REPLACEMENT]: `Switch to \`.${replacement}()\`.`,
[MESSAGE_ID_SUGGESTION_SPREADING_ARRAY]: 'The spreading object is an array.',
[MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY]: 'The spreading object is NOT an array.',
};

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const {sourceCode} = context;
const {allowExpressionStatement} = context.options[0];

return {
CallExpression(callExpression) {
if (!predicate(callExpression)) {
return;
}

const array = callExpression.callee.object;

// `[...array].reverse()`
const isSpreadAndMutate = array.type === 'ArrayExpression'
&& array.elements.length === 1
&& array.elements[0].type === 'SpreadElement';

if (allowExpressionStatement && !isSpreadAndMutate) {
const maybeExpressionStatement = callExpression.parent.type === 'ChainExpression'
? callExpression.parent.parent
: callExpression.parent;
if (maybeExpressionStatement.type === 'ExpressionStatement') {
return;
}
}

const methodProperty = callExpression.callee.property;
const suggestions = [];
const fixMethodName = fixer => fixer.replaceText(methodProperty, replacement);

/*
For `[...array].reverse()`, provide two suggestions, let user choose if the object can be unwrapped,
otherwise only change `.reverse()` to `.toReversed()`
*/
if (isSpreadAndMutate) {
suggestions.push({
messageId: MESSAGE_ID_SUGGESTION_SPREADING_ARRAY,
* fix(fixer) {
const text = getParenthesizedText(array.elements[0].argument, sourceCode);
yield fixer.replaceText(array, text);
yield fixMethodName(fixer);
},
});
}

suggestions.push({
messageId: isSpreadAndMutate
? MESSAGE_ID_SUGGESTION_NOT_SPREADING_ARRAY
: MESSAGE_ID_SUGGESTION_APPLY_REPLACEMENT,
fix: fixMethodName,
});

return {
node: methodProperty,
messageId: MESSAGE_ID_ERROR,
suggest: suggestions,
};
},
};
};

/** @type {import('eslint').Rule.RuleModule} */
const config = {
create,
meta: {
type: 'suggestion',
docs: {
description: `Prefer \`Array#${replacement}()\` over \`Array#${methodName}()\`.`,
recommended: true,
},
hasSuggestions: true,
schema,
defaultOptions: [{allowExpressionStatement: true}],
messages,
},
};

return config;
}

export default noArrayMutateRule;
2 changes: 1 addition & 1 deletion scripts/create-rules-index-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const DIRECTORY = new URL('../rules/', import.meta.url);
const files = fs.readdirSync(DIRECTORY, {withFileTypes: true})
.filter(file => file.isFile() && file.name.endsWith('.js') && file.name !== 'index.js')
.map(file => file.name)
.sort();
.toSorted();

const content = files
.map(file => `export {default as '${path.basename(file, '.js')}'} from './${file}';`)
Expand Down
Loading
Loading