From 8d7954c4f1f7eb06e7bd19080b740a5501ca72f6 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 10 May 2024 13:49:55 +0800 Subject: [PATCH] Add `consistent-empty-array-spread` rule (#2349) Co-authored-by: Sindre Sorhus --- docs/rules/consistent-empty-array-spread.md | 42 ++++ readme.md | 1 + rules/consistent-empty-array-spread.js | 126 +++++++++++ scripts/template/rule.js.jst | 11 +- test/consistent-empty-array-spread.mjs | 57 +++++ .../consistent-empty-array-spread.mjs.md | 206 ++++++++++++++++++ .../consistent-empty-array-spread.mjs.snap | Bin 0 -> 782 bytes 7 files changed, 437 insertions(+), 6 deletions(-) create mode 100644 docs/rules/consistent-empty-array-spread.md create mode 100644 rules/consistent-empty-array-spread.js create mode 100644 test/consistent-empty-array-spread.mjs create mode 100644 test/snapshots/consistent-empty-array-spread.mjs.md create mode 100644 test/snapshots/consistent-empty-array-spread.mjs.snap diff --git a/docs/rules/consistent-empty-array-spread.md b/docs/rules/consistent-empty-array-spread.md new file mode 100644 index 0000000000..6317cf507f --- /dev/null +++ b/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). + + + + +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' : ''), +]; +``` diff --git a/readme.md b/readme.md index 86fa989b4a..f21ccaae94 100644 --- a/readme.md +++ b/readme.md @@ -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. | ✅ | 🔧 | | diff --git a/rules/consistent-empty-array-spread.js b/rules/consistent-empty-array-spread.js new file mode 100644 index 0000000000..7a8d0328d9 --- /dev/null +++ b/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, + }, +}; diff --git a/scripts/template/rule.js.jst b/scripts/template/rule.js.jst index eb3ae562e5..5ff569a392 100644 --- a/scripts/template/rule.js.jst +++ b/scripts/template/rule.js.jst @@ -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<% } %>, diff --git a/test/consistent-empty-array-spread.mjs b/test/consistent-empty-array-spread.mjs new file mode 100644 index 0000000000..d8d432aa8c --- /dev/null +++ b/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 ? "" : [])]', + ], +}); diff --git a/test/snapshots/consistent-empty-array-spread.mjs.md b/test/snapshots/consistent-empty-array-spread.mjs.md new file mode 100644 index 0000000000..c52617c284 --- /dev/null +++ b/test/snapshots/consistent-empty-array-spread.mjs.md @@ -0,0 +1,206 @@ +# Snapshot report for `test/consistent-empty-array-spread.mjs` + +The actual snapshot is saved in `consistent-empty-array-spread.mjs.snap`. + +Generated by [AVA](https://avajs.dev). + +## invalid(1): [ ...(test ? [] : "ab"), ...(test ? "ab" : []), ]; + +> Input + + `␊ + 1 | [␊ + 2 | ...(test ? [] : "ab"),␊ + 3 | ...(test ? "ab" : []),␊ + 4 | ];␊ + ` + +> Output + + `␊ + 1 | [␊ + 2 | ...(test ? '' : "ab"),␊ + 3 | ...(test ? "ab" : ''),␊ + 4 | ];␊ + ` + +> Error 1/2 + + `␊ + 1 | [␊ + > 2 | ...(test ? [] : "ab"),␊ + | ^^ Prefer using empty string since the alternate is a string.␊ + 3 | ...(test ? "ab" : []),␊ + 4 | ];␊ + ` + +> Error 2/2 + + `␊ + 1 | [␊ + 2 | ...(test ? [] : "ab"),␊ + > 3 | ...(test ? "ab" : []),␊ + | ^^ Prefer using empty string since the consequent is a string.␊ + 4 | ];␊ + ` + +## invalid(2): const STRING = "ab"; [ ...(test ? [] : STRING), ...(test ? STRING : []), ]; + +> Input + + `␊ + 1 | const STRING = "ab";␊ + 2 | [␊ + 3 | ...(test ? [] : STRING),␊ + 4 | ...(test ? STRING : []),␊ + 5 | ];␊ + ` + +> Output + + `␊ + 1 | const STRING = "ab";␊ + 2 | [␊ + 3 | ...(test ? '' : STRING),␊ + 4 | ...(test ? STRING : ''),␊ + 5 | ];␊ + ` + +> Error 1/2 + + `␊ + 1 | const STRING = "ab";␊ + 2 | [␊ + > 3 | ...(test ? [] : STRING),␊ + | ^^ Prefer using empty string since the alternate is a string.␊ + 4 | ...(test ? STRING : []),␊ + 5 | ];␊ + ` + +> Error 2/2 + + `␊ + 1 | const STRING = "ab";␊ + 2 | [␊ + 3 | ...(test ? [] : STRING),␊ + > 4 | ...(test ? STRING : []),␊ + | ^^ Prefer using empty string since the consequent is a string.␊ + 5 | ];␊ + ` + +## invalid(3): [ ...(test ? "" : [a, b]), ...(test ? [a, b] : ""), ]; + +> Input + + `␊ + 1 | [␊ + 2 | ...(test ? "" : [a, b]),␊ + 3 | ...(test ? [a, b] : ""),␊ + 4 | ];␊ + ` + +> Output + + `␊ + 1 | [␊ + 2 | ...(test ? [] : [a, b]),␊ + 3 | ...(test ? [a, b] : []),␊ + 4 | ];␊ + ` + +> Error 1/2 + + `␊ + 1 | [␊ + > 2 | ...(test ? "" : [a, b]),␊ + | ^^ Prefer using empty array since the alternate is an array.␊ + 3 | ...(test ? [a, b] : ""),␊ + 4 | ];␊ + ` + +> Error 2/2 + + `␊ + 1 | [␊ + 2 | ...(test ? "" : [a, b]),␊ + > 3 | ...(test ? [a, b] : ""),␊ + | ^^ Prefer using empty array since the consequent is an array.␊ + 4 | ];␊ + ` + +## invalid(4): const ARRAY = ["a", "b"]; [ /* hole */, ...(test ? "" : ARRAY), /* hole */, ...(test ? ARRAY : ""), /* hole */, ]; + +> Input + + `␊ + 1 | const ARRAY = ["a", "b"];␊ + 2 | [␊ + 3 | /* hole */,␊ + 4 | ...(test ? "" : ARRAY),␊ + 5 | /* hole */,␊ + 6 | ...(test ? ARRAY : ""),␊ + 7 | /* hole */,␊ + 8 | ];␊ + ` + +> Output + + `␊ + 1 | const ARRAY = ["a", "b"];␊ + 2 | [␊ + 3 | /* hole */,␊ + 4 | ...(test ? [] : ARRAY),␊ + 5 | /* hole */,␊ + 6 | ...(test ? ARRAY : []),␊ + 7 | /* hole */,␊ + 8 | ];␊ + ` + +> Error 1/2 + + `␊ + 1 | const ARRAY = ["a", "b"];␊ + 2 | [␊ + 3 | /* hole */,␊ + > 4 | ...(test ? "" : ARRAY),␊ + | ^^ Prefer using empty array since the alternate is an array.␊ + 5 | /* hole */,␊ + 6 | ...(test ? ARRAY : ""),␊ + 7 | /* hole */,␊ + 8 | ];␊ + ` + +> Error 2/2 + + `␊ + 1 | const ARRAY = ["a", "b"];␊ + 2 | [␊ + 3 | /* hole */,␊ + 4 | ...(test ? "" : ARRAY),␊ + 5 | /* hole */,␊ + > 6 | ...(test ? ARRAY : ""),␊ + | ^^ Prefer using empty array since the consequent is an array.␊ + 7 | /* hole */,␊ + 8 | ];␊ + ` + +## invalid(5): [...(foo ? "" : [])] + +> Input + + `␊ + 1 | [...(foo ? "" : [])]␊ + ` + +> Output + + `␊ + 1 | [...(foo ? [] : [])]␊ + ` + +> Error 1/1 + + `␊ + > 1 | [...(foo ? "" : [])]␊ + | ^^ Prefer using empty array since the alternate is an array.␊ + ` diff --git a/test/snapshots/consistent-empty-array-spread.mjs.snap b/test/snapshots/consistent-empty-array-spread.mjs.snap new file mode 100644 index 0000000000000000000000000000000000000000..c4ad96bdf12d6246fe5704b9c525eb7948ee0683 GIT binary patch literal 782 zcmV+p1M&PpRzV;ySQZ2NeM0OOjTMY!w53WW|S&ui5Yxt zUu`s#)Z|rWnBqqL8^X*&5E0x6BDi+r+TY?*k|u55%gaZdXm`!a&AI2kbIv^*^(|-c z+=5tG`EZB`di40RuqOCF@rrOwJdgkyFauozXu=HE`2Z5YCUY9YkpQ+}21ZB17V2XQ z@7t3pSsAZVDS%h0B=JUfr(_BEDGqq)ov=XEau|Yt3d`T&A>4EE2)i)#O#3O|qX{_y zkGTFH&$I^^$QT2%2zG5mFqj^o@VvI##LPRc>$uRUH$xI%C=&05g4d+wfU*=)nb z55JgVo3K=QWSaG6B;tpWh?`0~_zHph54!sYd$1ie*x@L9{sOJ-uqZ>_r=dC6=?SWx+Bnrh4SM=pUw M0X8sN2DlCY0NRgytpET3 literal 0 HcmV?d00001