Skip to content

Commit

Permalink
Add consistent-empty-array-spread rule (#2349)
Browse files Browse the repository at this point in the history
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
  • Loading branch information
fisker and sindresorhus committed May 10, 2024
1 parent 6fde3fe commit 8d7954c
Show file tree
Hide file tree
Showing 7 changed files with 437 additions and 6 deletions.
42 changes: 42 additions & 0 deletions docs/rules/consistent-empty-array-spread.md
@@ -0,0 +1,42 @@
# Prefer consistent types when spreading a ternary in an array literal

💼 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 spreading a ternary in an array, we can use both `[]` and `''` as fallbacks, but it's better to have consistent types in both branches.

## Fail

```js
const array = [
a,
...(foo ? [b, c] : ''),
];
```

```js
const array = [
a,
...(foo ? 'bc' : []),
];
```

## Pass

```js
const array = [
a,
...(foo ? [b, c] : []),
];
```

```js
const array = [
a,
...(foo ? 'bc' : ''),
];
```
1 change: 1 addition & 0 deletions readme.md
Expand Up @@ -113,6 +113,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c
| [better-regex](docs/rules/better-regex.md) | Improve regexes by making them shorter, consistent, and safer. || 🔧 | |
| [catch-error-name](docs/rules/catch-error-name.md) | Enforce a specific parameter name in catch clauses. || 🔧 | |
| [consistent-destructuring](docs/rules/consistent-destructuring.md) | Use destructured variables over properties. | | 🔧 | 💡 |
| [consistent-empty-array-spread](docs/rules/consistent-empty-array-spread.md) | Prefer consistent types when spreading a ternary in an array literal. || 🔧 | |
| [consistent-function-scoping](docs/rules/consistent-function-scoping.md) | Move function definitions to the highest possible scope. || | |
| [custom-error-definition](docs/rules/custom-error-definition.md) | Enforce correct `Error` subclassing. | | 🔧 | |
| [empty-brace-spaces](docs/rules/empty-brace-spaces.md) | Enforce no spaces between braces. || 🔧 | |
Expand Down
126 changes: 126 additions & 0 deletions rules/consistent-empty-array-spread.js
@@ -0,0 +1,126 @@
'use strict';
const {getStaticValue} = require('@eslint-community/eslint-utils');

const MESSAGE_ID = 'consistent-empty-array-spread';
const messages = {
[MESSAGE_ID]: 'Prefer using empty {{replacementDescription}} since the {{anotherNodePosition}} is {{anotherNodeDescription}}.',
};

const isEmptyArrayExpression = node =>
node.type === 'ArrayExpression'
&& node.elements.length === 0;

const isEmptyStringLiteral = node =>
node.type === 'Literal'
&& node.value === '';

const isString = (node, context) => {
const staticValueResult = getStaticValue(node, context.sourceCode.getScope(node));
return typeof staticValueResult?.value === 'string';
};

const isArray = (node, context) => {
if (node.type === 'ArrayExpression') {
return true;
}

const staticValueResult = getStaticValue(node, context.sourceCode.getScope(node));
return Array.isArray(staticValueResult?.value);
};

const cases = [
{
oneSidePredicate: isEmptyStringLiteral,
anotherSidePredicate: isArray,
anotherNodeDescription: 'an array',
replacementDescription: 'array',
replacementCode: '[]',
},
{
oneSidePredicate: isEmptyArrayExpression,
anotherSidePredicate: isString,
anotherNodeDescription: 'a string',
replacementDescription: 'string',
replacementCode: '\'\'',
},
];

function createProblem({
problemNode,
anotherNodePosition,
anotherNodeDescription,
replacementDescription,
replacementCode,
}) {
return {
node: problemNode,
messageId: MESSAGE_ID,
data: {
replacementDescription,
anotherNodePosition,
anotherNodeDescription,
},
fix: fixer => fixer.replaceText(problemNode, replacementCode),
};
}

function getProblem(conditionalExpression, context) {
const {
consequent,
alternate,
} = conditionalExpression;

for (const problemCase of cases) {
const {
oneSidePredicate,
anotherSidePredicate,
} = problemCase;

if (oneSidePredicate(consequent, context) && anotherSidePredicate(alternate, context)) {
return createProblem({
...problemCase,
problemNode: consequent,
anotherNodePosition: 'alternate',
});
}

if (oneSidePredicate(alternate, context) && anotherSidePredicate(consequent, context)) {
return createProblem({
...problemCase,
problemNode: alternate,
anotherNodePosition: 'consequent',
});
}
}
}

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
* ArrayExpression(arrayExpression) {
for (const element of arrayExpression.elements) {
if (
element?.type !== 'SpreadElement'
|| element.argument.type !== 'ConditionalExpression'
) {
continue;
}

yield getProblem(element.argument, context);
}
},
});

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Prefer consistent types when spreading a ternary in an array literal.',
recommended: true,
},
fixable: 'code',

messages,
},
};
11 changes: 5 additions & 6 deletions scripts/template/rule.js.jst
Expand Up @@ -17,15 +17,14 @@ const messages = {
};
<% } %>

const selector = [
'Literal',
'[value="unicorn"]',
].join('');

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
return {
[selector](node) {
Literal(node) {
if (node.value !== 'unicorn') {
return;
}

return {
node,
messageId: <% if (hasSuggestions) { %>MESSAGE_ID_ERROR<% } else { %>MESSAGE_ID<% } %>,
Expand Down
57 changes: 57 additions & 0 deletions test/consistent-empty-array-spread.mjs
@@ -0,0 +1,57 @@
import outdent from 'outdent';
import {getTester} from './utils/test.mjs';

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

test.snapshot({
valid: [
'[,,,]',
'[...(test ? [] : [a, b])]',
'[...(test ? [a, b] : [])]',
'[...(test ? "" : "ab")]',
'[...(test ? "ab" : "")]',
'[...(test ? "" : unknown)]',
'[...(test ? unknown : "")]',
'[...(test ? [] : unknown)]',
'[...(test ? unknown : [])]',
'_ = {...(test ? "" : [a, b])}',
'_ = {...(test ? [] : "ab")}',
'call(...(test ? "" : [a, b]))',
'call(...(test ? [] : "ab"))',
'[...(test ? "ab" : [a, b])]',
// Not checking
'const EMPTY_STRING = ""; [...(test ? EMPTY_STRING : [a, b])]',
],
invalid: [
outdent`
[
...(test ? [] : "ab"),
...(test ? "ab" : []),
];
`,
outdent`
const STRING = "ab";
[
...(test ? [] : STRING),
...(test ? STRING : []),
];
`,
outdent`
[
...(test ? "" : [a, b]),
...(test ? [a, b] : ""),
];
`,
outdent`
const ARRAY = ["a", "b"];
[
/* hole */,
...(test ? "" : ARRAY),
/* hole */,
...(test ? ARRAY : ""),
/* hole */,
];
`,
'[...(foo ? "" : [])]',
],
});

0 comments on commit 8d7954c

Please sign in to comment.