Add no-useless-undefined rule (#718)

Author: fisker

docs/rules/no-useless-undefined.md

@@ -0,0 +1,100 @@
+# Disallow useless `undefined`
+
+This rule is fixable.
+
+## Fail
+
+```js
+let foo = undefined;
+```
+
+```js
+const {foo = undefined} = bar;
+```
+
+```js
+const noop = () => undefined;
+```
+
+```js
+function foo() {
+	return undefined;
+}
+```
+
+```js
+function* foo() {
+	yield undefined;
+}
+```
+
+```js
+function foo(bar = undefined) {
+}
+```
+
+```js
+function foo({bar = undefined}) {
+}
+```
+
+```js
+foo(undefined);
+```
+
+## Pass
+
+```js
+let foo;
+```
+
+```js
+const {foo} = bar;
+```
+
+```js
+const noop = () => {};
+```
+
+```js
+function foo() {
+	return;
+}
+```
+
+```js
+function* foo() {
+	yield;
+}
+```
+
+```js
+function foo(bar) {
+}
+```
+
+```js
+function foo({bar}) {
+}
+```
+
+```js
+foo();
+```
+
+## Conflict with ESLint `array-callback-return` rule
+
+We recommend setting the ESLint [`array-callback-return`](https://eslint.org/docs/rules/array-callback-return#top) rule option [`allowImplicit`](https://eslint.org/docs/rules/array-callback-return#options) to `true`:
+
+```json
+{
+	"rules": {
+		"array-callback-return": [
+			"error",
+			{
+				"allowImplicit": true
+			}
+		]
+	}
+}
+```

index.js

@@ -43,6 +43,7 @@ module.exports = {
 				'unicorn/no-unreadable-array-destructuring': 'error',
 				'unicorn/no-unsafe-regex': 'off',
 				'unicorn/no-unused-properties': 'off',
+				'unicorn/no-useless-undefined': 'error',
 				'unicorn/no-zero-fractions': 'error',
 				'unicorn/number-literal-case': 'error',
 				'unicorn/prefer-add-event-listener': 'error',

package.json

@@ -106,6 +106,12 @@
 		],
 		"rules": {
 			"strict": "error",
+			"array-callback-return": [
+				"error",
+				{
+					"allowImplicit": true
+				}
+			],
 			"unicorn/no-null": "error",
 			"unicorn/string-content": "off"
 		}

readme.md

@@ -59,6 +59,7 @@ Configure it in `package.json`.
 			"unicorn/no-unreadable-array-destructuring": "error",
 			"unicorn/no-unsafe-regex": "off",
 			"unicorn/no-unused-properties": "off",
+			"unicorn/no-useless-undefined": "error",
 			"unicorn/no-zero-fractions": "error",
 			"unicorn/number-literal-case": "error",
 			"unicorn/prefer-add-event-listener": "error",
@@ -117,6 +118,7 @@ Configure it in `package.json`.
 - [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) - Disallow unreadable array destructuring.
 - [no-unsafe-regex](docs/rules/no-unsafe-regex.md) - Disallow unsafe regular expressions.
 - [no-unused-properties](docs/rules/no-unused-properties.md) - Disallow unused object properties.
+- [no-useless-undefined](docs/rules/no-useless-undefined.md) - Disallow useless `undefined`. *(fixable)*
 - [no-zero-fractions](docs/rules/no-zero-fractions.md) - Disallow number literals with zero fractions or dangling dots. *(fixable)*
 - [number-literal-case](docs/rules/number-literal-case.md) - Enforce proper case for numeric literals. *(fixable)*
 - [prefer-add-event-listener](docs/rules/prefer-add-event-listener.md) - Prefer `.addEventListener()` and `.removeEventListener()` over `on`-functions. *(partly fixable)*

rules/no-for-loop.js

@@ -191,8 +191,6 @@ const resolveIdentifierName = (name, scope) => {
 
 		scope = scope.upper;
 	}
-
-	return undefined;
 };
 
 const scopeContains = (ancestor, descendant) => {
@@ -367,7 +365,7 @@ const create = context => {
 						], replacement),
 						...arrayReferences.map(reference => {
 							if (reference === elementReference) {
-								return undefined;
+								return;
 							}
 
 							return fixer.replaceText(reference.identifier.parent, element);

rules/no-unused-properties.js

@@ -139,7 +139,7 @@ const create = context => {
 							return {identifier: parent};
 						}
 
-						return undefined;
+						return;
 					}
 
 					if (parent.type === 'MemberExpression') {
@@ -152,7 +152,7 @@ const create = context => {
 							return {identifier: parent};
 						}
 
-						return undefined;
+						return;
 					}
 
 					if (
@@ -163,7 +163,7 @@ const create = context => {
 							return {identifier: parent};
 						}
 
-						return undefined;
+						return;
 					}
 
 					if (
@@ -174,7 +174,7 @@ const create = context => {
 							return {identifier: parent};
 						}
 
-						return undefined;
+						return;
 					}
 
 					return reference;

rules/no-useless-undefined.js

@@ -0,0 +1,126 @@
+'use strict';
+const {isCommaToken} = require('eslint-utils');
+const getDocumentationUrl = require('./utils/get-documentation-url');
+
+const messageId = 'no-useless-undefined';
+
+const getSelector = (parent, property) =>
+	`${parent} > Identifier.${property}[name="undefined"]`;
+
+// `return undefined`
+const returnSelector = getSelector('ReturnStatement', 'argument');
+
+// `yield undefined`
+const yieldSelector = getSelector('YieldExpression[delegate=false]', 'argument');
+
+// `() => undefined`
+const arrowFunctionSelector = getSelector('ArrowFunctionExpression', 'body');
+
+// `let foo = undefined` / `var foo = undefined`
+const variableInitSelector = getSelector(
+	[
+		'VariableDeclaration',
+		'[kind!="const"]',
+		'>',
+		'VariableDeclarator'
+	].join(''),
+	'init'
+);
+
+// `const {foo = undefined} = {}`
+const assignmentPatternSelector = getSelector('AssignmentPattern', 'right');
+
+const isUndefined = node => node && node.type === 'Identifier' && node.name === 'undefined';
+
+const create = context => {
+	const listener = fix => node => {
+		context.report({
+			node,
+			messageId,
+			fix: fixer => fix(node, fixer)
+		});
+	};
+
+	const code = context.getSourceCode().text;
+
+	const removeNodeAndLeadingSpace = (node, fixer) => {
+		const textBefore = code.slice(0, node.range[0]);
+		return fixer.removeRange([
+			node.range[0] - (textBefore.length - textBefore.trim().length),
+			node.range[1]
+		]);
+	};
+
+	return {
+		[returnSelector]: listener(removeNodeAndLeadingSpace),
+		[yieldSelector]: listener(removeNodeAndLeadingSpace),
+		[arrowFunctionSelector]: listener(
+			(node, fixer) => fixer.replaceText(node, '{}')
+		),
+		[variableInitSelector]: listener(
+			(node, fixer) => fixer.removeRange([node.parent.id.range[1], node.range[1]])
+		),
+		[assignmentPatternSelector]: listener(
+			(node, fixer) => fixer.removeRange([node.parent.left.range[1], node.range[1]])
+		),
+		CallExpression: node => {
+			const argumentNodes = node.arguments;
+			const undefinedArguments = [];
+			for (let index = argumentNodes.length - 1; index >= 0; index--) {
+				const node = argumentNodes[index];
+				if (isUndefined(node)) {
+					undefinedArguments.unshift(node);
+				} else {
+					break;
+				}
+			}
+
+			if (undefinedArguments.length === 0) {
+				return;
+			}
+
+			const firstUndefined = undefinedArguments[0];
+			const lastUndefined = undefinedArguments[undefinedArguments.length - 1];
+
+			context.report({
+				messageId,
+				loc: {
+					start: firstUndefined.loc.start,
+					end: lastUndefined.loc.end
+				},
+				fix: fixer => {
+					let start = firstUndefined.range[0];
+					let end = lastUndefined.range[1];
+
+					const previousArgument = argumentNodes[argumentNodes.length - undefinedArguments.length - 1];
+
+					if (previousArgument) {
+						start = previousArgument.range[1];
+					} else {
+						// If all arguments removed, and there is trailing comma, we need remove it.
+						const tokenAfter = context.getTokenAfter(lastUndefined);
+						if (isCommaToken(tokenAfter)) {
+							end = tokenAfter.range[1];
+						}
+					}
+
+					return fixer.removeRange([start, end]);
+				}
+			});
+		}
+	};
+};
+
+module.exports = {
+	create,
+	meta: {
+		type: 'suggestion',
+		docs: {
+			url: getDocumentationUrl(__filename)
+		},
+		messages: {
+			[messageId]: 'Do not use useless `undefined`.'
+		},
+		fixable: 'code'
+	}
+};

test/filename-case.js

@@ -87,7 +87,7 @@ ruleTester.run('filename-case', rule, {
 		testCase('src/foo/___foo-bar.js', 'kebabCase'),
 		testCase('src/foo/_FooBar.js', 'pascalCase'),
 		testCase('src/foo/___FooBar.js', 'pascalCase'),
-		testManyCases('src/foo/foo-bar.js', undefined),
+		testManyCases('src/foo/foo-bar.js'),
 		testManyCases('src/foo/foo-bar.js', {}),
 		testManyCases('src/foo/fooBar.js', {camelCase: true}),
 		testManyCases('src/foo/FooBar.js', {kebabCase: true, pascalCase: true}),