prefer-spread: Check String#split('') (#1489)

sindresorhus · Aug 16, 2021 · d51a197 · d51a197
1 parent 1675118
commit d51a197
Show file tree

Hide file tree

Showing 7 changed files with 259 additions and 6 deletions.
diff --git a/docs/rules/prefer-spread.md b/docs/rules/prefer-spread.md
@@ -1,4 +1,4 @@
-# Prefer the spread operator over `Array.from(…)`, `Array#concat(…)` and `Array#slice()`
+# Prefer the spread operator over `Array.from(…)`, `Array#concat(…)`, `Array#slice()` and `String#split('')`
 
 Enforces the use of [the spread operator (`...`)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) over
 
@@ -18,6 +18,12 @@ Enforces the use of [the spread operator (`...`)](https://developer.mozilla.org/
 
 	Variables named `arrayBuffer`, `blob`, `buffer`, `file`, and `this` are ignored.
 
+- `String#split('')`
+
+	Split a string into an array of characters.
+
+	Note: [The suggestion fix may get different result](https://stackoverflow.com/questions/4547609/how-to-get-character-array-from-a-string/34717402#34717402).
+
 This rule is partly fixable.
 
 ## Fail
@@ -34,6 +40,10 @@ const array = array1.concat(array2);
 const copy = array.slice();
 ```
 
+```js
+const characters = string.split('');
+```
+
 ## Pass
 
 ```js
@@ -52,6 +62,10 @@ const tail = array.slice(1);
 const copy = [...array];
 ```
 
+```js
+const characters = [...string];
+```
+
 ## With the `unicorn/no-useless-spread` rule
 
 Some cases are fixed using extra spread syntax. Therefore we recommend enabling the [`unicorn/no-useless-spread`](./no-useless-spread.md) rule to fix it.

diff --git a/readme.md b/readme.md
@@ -231,7 +231,7 @@ Each rule has emojis denoting:
 | [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md) | Prefer `Reflect.apply()` over `Function#apply()`. | ✅ | 🔧 |  |
 | [prefer-regexp-test](docs/rules/prefer-regexp-test.md) | Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`. | ✅ | 🔧 |  |
 | [prefer-set-has](docs/rules/prefer-set-has.md) | Prefer `Set#has()` over `Array#includes()` when checking for existence or non-existence. | ✅ | 🔧 | 💡 |
-| [prefer-spread](docs/rules/prefer-spread.md) | Prefer the spread operator over `Array.from(…)`, `Array#concat(…)` and `Array#slice()`. | ✅ | 🔧 | 💡 |
+| [prefer-spread](docs/rules/prefer-spread.md) | Prefer the spread operator over `Array.from(…)`, `Array#concat(…)`, `Array#slice()` and `String#split('')`. | ✅ | 🔧 | 💡 |
 | [prefer-string-replace-all](docs/rules/prefer-string-replace-all.md) | Prefer `String#replaceAll()` over regex searches with the global flag. |  | 🔧 |  |
 | [prefer-string-slice](docs/rules/prefer-string-slice.md) | Prefer `String#slice()` over `String#substr()` and `String#substring()`. | ✅ | 🔧 |  |
 | [prefer-string-starts-ends-with](docs/rules/prefer-string-starts-ends-with.md) | Prefer `String#startsWith()` & `String#endsWith()` over `RegExp#test()`. | ✅ | 🔧 | 💡 |

diff --git a/rules/prefer-spread.js b/rules/prefer-spread.js
@@ -15,18 +15,22 @@ const {
 const ERROR_ARRAY_FROM = 'array-from';
 const ERROR_ARRAY_CONCAT = 'array-concat';
 const ERROR_ARRAY_SLICE = 'array-slice';
+const ERROR_STRING_SPLIT = 'string-split';
 const SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE = 'argument-is-spreadable';
 const SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE = 'argument-is-not-spreadable';
 const SUGGESTION_CONCAT_TEST_ARGUMENT = 'test-argument';
 const SUGGESTION_CONCAT_SPREAD_ALL_ARGUMENTS = 'spread-all-arguments';
+const SUGGESTION_USE_SPREAD = 'use-spread';
 const messages = {
 	[ERROR_ARRAY_FROM]: 'Prefer the spread operator over `Array.from(…)`.',
 	[ERROR_ARRAY_CONCAT]: 'Prefer the spread operator over `Array#concat(…)`.',
 	[ERROR_ARRAY_SLICE]: 'Prefer the spread operator over `Array#slice()`.',
+	[ERROR_STRING_SPLIT]: 'Prefer the spread operator over `String#split(\'\')`.',
 	[SUGGESTION_CONCAT_ARGUMENT_IS_SPREADABLE]: 'First argument is an `array`.',
 	[SUGGESTION_CONCAT_ARGUMENT_IS_NOT_SPREADABLE]: 'First argument is not an `array`.',
 	[SUGGESTION_CONCAT_TEST_ARGUMENT]: 'Test first argument with `Array.isArray(…)`.',
 	[SUGGESTION_CONCAT_SPREAD_ALL_ARGUMENTS]: 'Spread all unknown arguments`.',
+	[SUGGESTION_USE_SPREAD]: 'Use `...` operator.',
 };
 
 const arrayFromCallSelector = [
@@ -67,6 +71,11 @@ const ignoredSliceCallee = [
 	'this',
 ];
 
+const stringSplitCallSelector = methodCallSelector({
+	method: 'split',
+	argumentsLength: 1,
+});
+
 const isArrayLiteral = node => node.type === 'ArrayExpression';
 const isArrayLiteralHasTrailingComma = (node, sourceCode) => {
 	if (node.elements.length === 0) {
@@ -281,7 +290,7 @@ function fixArrayFrom(node, sourceCode) {
 	};
 }
 
-function fixSlice(node, sourceCode) {
+function methodCallToSpread(node, sourceCode) {
 	return function * (fixer) {
 		// Fixed code always starts with `[`
 		if (needsSemicolon(sourceCode.getTokenBefore(node), sourceCode, '[')) {
@@ -291,7 +300,7 @@ function fixSlice(node, sourceCode) {
 		yield fixer.insertTextBefore(node, '[...');
 		yield fixer.insertTextAfter(node, ']');
 
-		// The array is already accessing `.slice`, there should not any case need add extra `()`
+		// The array is already accessing `.slice` or `.split`, there should not any case need add extra `()`
 
 		yield * removeMethodCall(fixer, node, sourceCode);
 	};
@@ -418,8 +427,49 @@ const create = context => {
 			return {
 				node: node.callee.property,
 				messageId: ERROR_ARRAY_SLICE,
-				fix: fixSlice(node, sourceCode),
+				fix: methodCallToSpread(node, sourceCode),
+			};
+		},
+		[stringSplitCallSelector](node) {
+			const [separator] = node.arguments;
+			if (!isLiteralValue(separator, '')) {
+				return;
+			}
+
+			const string = node.callee.object;
+			const staticValue = getStaticValue(string, context.getScope());
+			let hasSameResult = false;
+			if (staticValue) {
+				const {value} = staticValue;
+
+				if (typeof value !== 'string') {
+					return;
+				}
+
+				const resultBySplit = value.split('');
+				const resultBySpread = [...value];
+
+				hasSameResult = resultBySplit.length === resultBySpread.length
+					&& resultBySplit.every((character, index) => character === resultBySpread[index]);
+			}
+
+			const problem = {
+				node: node.callee.property,
+				messageId: ERROR_STRING_SPLIT,
 			};
+
+			if (hasSameResult) {
+				problem.fix = methodCallToSpread(node, sourceCode);
+			} else {
+				problem.suggest = [
+					{
+						messageId: SUGGESTION_USE_SPREAD,
+						fix: methodCallToSpread(node, sourceCode),
+					},
+				];
+			}
+
+			return problem;
 		},
 	};
 };
@@ -429,7 +479,7 @@ module.exports = {
 	meta: {
 		type: 'suggestion',
 		docs: {
-			description: 'Prefer the spread operator over `Array.from(…)`, `Array#concat(…)` and `Array#slice()`.',
+			description: 'Prefer the spread operator over `Array.from(…)`, `Array#concat(…)`, `Array#slice()` and `String#split(\'\')`.',
 		},
 		fixable: 'code',
 		messages,

diff --git a/test/prefer-spread.mjs b/test/prefer-spread.mjs
@@ -339,3 +339,46 @@ test.snapshot({
 		'array.slice(0.00, )',
 	],
 });
+
+// `String#slice('')`
+test.snapshot({
+	valid: [
+		'new foo.split("")',
+		'split("")',
+		'string[split]("")',
+		'string.split',
+		'string.split(1)',
+		'string.split(..."")',
+		'string.split(...[""])',
+		'string.split("" + "")',
+		'string.split(0)',
+		'string.split(false)',
+		'string.split(undefined)',
+		'string.split(0n)',
+		'string.split(null)',
+		'string.split(/""/)',
+		'string.split(``)',
+		'const EMPTY_STRING = ""; string.split(EMPTY_STRING)',
+		'string.split("", limit)',
+		'"".split(string)',
+		'string.split()',
+		'string.notSplit("")',
+		'const notString = 0; notString.split("")',
+	],
+	invalid: [
+		'"string".split("")',
+		'"string".split(\'\')',
+		'unknown.split("")',
+		'const characters = "string".split("")',
+		'(( (( (( "string" )).split ))( (("")) ) ))',
+		// Semicolon
+		outdent`
+			bar()
+			foo.split("")
+		`,
+		'unknown.split("")',
+		// Not result the same
+		'"🦄".split("")',
+		'const {length} = "🦄".split("")',
+	],
+});
diff --git a/test/run-rules-on-codebase/lint.mjs b/test/run-rules-on-codebase/lint.mjs
@@ -70,6 +70,15 @@ const eslint = new ESLint({
 					'unicorn/prefer-module': 'off',
 				},
 			},
+			{
+				files: [
+					'rules/prefer-spread.js',
+				],
+				rules: {
+					// TODO[xo@>=0.45.0]: Enable this rule when `xo` updated `eslint-plugin-unicorn`
+					'unicorn/prefer-spread': 'off',
+				},
+			},
 		],
 	},
 });

diff --git a/test/snapshots/prefer-spread.mjs.md b/test/snapshots/prefer-spread.mjs.md
@@ -2251,3 +2251,140 @@ Generated by [AVA](https://avajs.dev).
     > 1 | array.slice(0.00, )␊
         |       ^^^^^ Prefer the spread operator over \`Array#slice()\`.␊
     `
+
+## Invalid #1
+      1 | "string".split("")
+
+> Output
+
+    `␊
+      1 | [..."string"]␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | "string".split("")␊
+        |          ^^^^^ Prefer the spread operator over \`String#split('')\`.␊
+    `
+
+## Invalid #2
+      1 | "string".split('')
+
+> Output
+
+    `␊
+      1 | [..."string"]␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | "string".split('')␊
+        |          ^^^^^ Prefer the spread operator over \`String#split('')\`.␊
+    `
+
+## Invalid #3
+      1 | unknown.split("")
+
+> Error 1/1
+
+    `␊
+    > 1 | unknown.split("")␊
+        |         ^^^^^ Prefer the spread operator over \`String#split('')\`.␊
+    ␊
+    --------------------------------------------------------------------------------␊
+    Suggestion 1/1: Use \`...\` operator.␊
+      1 | [...unknown]␊
+    `
+
+## Invalid #4
+      1 | const characters = "string".split("")
+
+> Output
+
+    `␊
+      1 | const characters = [..."string"]␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | const characters = "string".split("")␊
+        |                             ^^^^^ Prefer the spread operator over \`String#split('')\`.␊
+    `
+
+## Invalid #5
+      1 | (( (( (( "string" )).split ))( (("")) ) ))
+
+> Output
+
+    `␊
+      1 | (( [...(( (( "string" )) ))] ))␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | (( (( (( "string" )).split ))( (("")) ) ))␊
+        |                      ^^^^^ Prefer the spread operator over \`String#split('')\`.␊
+    `
+
+## Invalid #6
+      1 | bar()
+      2 | foo.split("")
+
+> Error 1/1
+
+    `␊
+      1 | bar()␊
+    > 2 | foo.split("")␊
+        |     ^^^^^ Prefer the spread operator over \`String#split('')\`.␊
+    ␊
+    --------------------------------------------------------------------------------␊
+    Suggestion 1/1: Use \`...\` operator.␊
+      1 | bar()␊
+      2 | ;[...foo]␊
+    `
+
+## Invalid #7
+      1 | unknown.split("")
+
+> Error 1/1
+
+    `␊
+    > 1 | unknown.split("")␊
+        |         ^^^^^ Prefer the spread operator over \`String#split('')\`.␊
+    ␊
+    --------------------------------------------------------------------------------␊
+    Suggestion 1/1: Use \`...\` operator.␊
+      1 | [...unknown]␊
+    `
+
+## Invalid #8
+      1 | "🦄".split("")
+
+> Error 1/1
+
+    `␊
+    > 1 | "🦄".split("")␊
+        |      ^^^^^ Prefer the spread operator over \`String#split('')\`.␊
+    ␊
+    --------------------------------------------------------------------------------␊
+    Suggestion 1/1: Use \`...\` operator.␊
+      1 | [..."🦄"]␊
+    `
+
+## Invalid #9
+      1 | const {length} = "🦄".split("")
+
+> Error 1/1
+
+    `␊
+    > 1 | const {length} = "🦄".split("")␊
+        |                       ^^^^^ Prefer the spread operator over \`String#split('')\`.␊
+    ␊
+    --------------------------------------------------------------------------------␊
+    Suggestion 1/1: Use \`...\` operator.␊
+      1 | const {length} = [..."🦄"]␊
+    `
diff --git a/test/snapshots/prefer-spread.mjs.snap b/test/snapshots/prefer-spread.mjs.snap