Add no-static-only-class rule (#1120)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

docs/rules/no-static-only-class.md

@@ -0,0 +1,48 @@
+# Forbid classes that only have static members
+
+A class with only static members could just be an object instead.
+
+This rule is partly fixable.
+
+## Fail
+
+```js
+class X {
+	static foo = false;
+	static bar() {};
+}
+```
+
+## Pass
+
+```js
+const X = {
+	foo: false,
+	bar() {}
+};
+```
+
+```js
+class X {
+	static foo = false;
+	static bar() {};
+
+	constructor() {}
+}
+```
+
+```js
+class X {
+	static foo = false;
+	static bar() {};
+
+	unicorn() {}
+}
+```
+
+```js
+class X {
+	static #foo = false;
+	static bar() {}
+}
+```

index.js

@@ -71,6 +71,7 @@ module.exports = {
 				'unicorn/no-null': 'error',
 				'unicorn/no-object-as-default-parameter': 'error',
 				'unicorn/no-process-exit': 'error',
+				'unicorn/no-static-only-class': 'error',
 				'unicorn/no-this-assignment': 'error',
 				'unicorn/no-unreadable-array-destructuring': 'error',
 				'unicorn/no-unsafe-regex': 'off',

package.json

@@ -79,7 +79,8 @@
 	},
 	"ava": {
 		"files": [
-			"test/*.mjs"
+			"test/*.mjs",
+			"test/unit/*.mjs"
 		]
 	},
 	"nyc": {

readme.md

@@ -65,6 +65,7 @@ Configure it in `package.json`.
 			"unicorn/no-null": "error",
 			"unicorn/no-object-as-default-parameter": "error",
 			"unicorn/no-process-exit": "error",
+			"unicorn/no-static-only-class": "error",
 			"unicorn/no-this-assignment": "error",
 			"unicorn/no-unreadable-array-destructuring": "error",
 			"unicorn/no-unsafe-regex": "off",
@@ -143,6 +144,7 @@ Configure it in `package.json`.
 - [no-null](docs/rules/no-null.md) - Disallow the use of the `null` literal.
 - [no-object-as-default-parameter](docs/rules/no-object-as-default-parameter.md) - Disallow the use of objects as default parameters.
 - [no-process-exit](docs/rules/no-process-exit.md) - Disallow `process.exit()`.
+- [no-static-only-class](docs/rules/no-static-only-class.md) - Forbid classes that only have static members. *(partly fixable)*
 - [no-this-assignment](docs/rules/no-this-assignment.md) - Disallow assigning `this` to a variable.
 - [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) - Disallow unreadable array destructuring. *(partly fixable)*
 - [no-unsafe-regex](docs/rules/no-unsafe-regex.md) - Disallow unsafe regular expressions.

rules/no-array-for-each.js

@@ -15,6 +15,7 @@ const getParenthesizedTimes = require('./utils/get-parenthesized-times');
 const extendFixRange = require('./utils/extend-fix-range');
 const isFunctionSelfUsedInside = require('./utils/is-function-self-used-inside');
 const isNodeMatches = require('./utils/is-node-matches');
+const assertToken = require('./utils/assert-token');
 
 const MESSAGE_ID = 'no-array-for-each';
 const messages = {
@@ -90,11 +91,10 @@ function getFixFunction(callExpression, sourceCode, functionInfo) {
 
 	function * replaceReturnStatement(returnStatement, fixer) {
 		const returnToken = sourceCode.getFirstToken(returnStatement);
-
-		/* istanbul ignore next: `ReturnStatement` firstToken should be `return` */
-		if (returnToken.value !== 'return') {
-			throw new Error(`Unexpected token ${returnToken.value}.`);
-		}
+		assertToken(returnToken, {
+			expected: 'return',
+			ruleId: 'no-array-for-each'
+		});
 
 		if (!returnStatement.argument) {
 			yield fixer.replaceText(returnToken, 'continue');

rules/no-static-only-class.js

@@ -0,0 +1,233 @@
+'use strict';
+const {isSemicolonToken} = require('eslint-utils');
+const getDocumentationUrl = require('./utils/get-documentation-url');
+const getClassHeadLocation = require('./utils/get-class-head-location');
+const removeSpacesAfter = require('./utils/remove-spaces-after');
+const assertToken = require('./utils/assert-token');
+
+const MESSAGE_ID = 'no-static-only-class';
+const messages = {
+	[MESSAGE_ID]: 'Use an object instead of a class with only static members.'
+};
+
+const selector = [
+	':matches(ClassDeclaration, ClassExpression)',
+	':not([superClass], [decorators.length>0])',
+	'[body.type="ClassBody"]',
+	'[body.body.length>0]'
+].join('');
+
+const isEqualToken = ({type, value}) => type === 'Punctuator' && value === '=';
+const isDeclarationOfExportDefaultDeclaration = node =>
+	node.type === 'ClassDeclaration' &&
+	node.parent.type === 'ExportDefaultDeclaration' &&
+	node.parent.declaration === node;
+
+function isStaticMember(node) {
+	const {
+		type,
+		private: isPrivate,
+		static: isStatic,
+		declare: isDeclare,
+		readonly: isReadonly,
+		accessibility,
+		decorators,
+		key
+	} = node;
+
+	// Avoid matching unexpected node. For example: https://github.com/tc39/proposal-class-static-block
+	/* istanbul ignore next */
+	if (type !== 'ClassProperty' && type !== 'MethodDefinition') {
+		return false;
+	}
+
+	if (!isStatic || isPrivate) {
+		return false;
+	}
+
+	// TypeScript class
+	if (
+		isDeclare ||
+		isReadonly ||
+		typeof accessibility !== 'undefined' ||
+		(Array.isArray(decorators) && decorators.length > 0) ||
+		key.type === 'TSPrivateIdentifier'
+	) {
+		return false;
+	}
+
+	return true;
+}
+
+function * switchClassMemberToObjectProperty(node, sourceCode, fixer) {
+	const {type} = node;
+
+	const staticToken = sourceCode.getFirstToken(node);
+	assertToken(staticToken, {
+		expected: [
+			{type: 'Keyword', value: 'static'},
+			// `babel-eslint` and `@babel/eslint-parser` use `{type: 'Identifier', value: 'static'}`
+			{type: 'Identifier', value: 'static'}
+		],
+		ruleId: 'no-static-only-class'
+	});
+
+	yield fixer.remove(staticToken);
+	yield removeSpacesAfter(staticToken, sourceCode, fixer);
+
+	const maybeSemicolonToken = type === 'ClassProperty' ?
+		sourceCode.getLastToken(node) :
+		sourceCode.getTokenAfter(node);
+	const hasSemicolonToken = isSemicolonToken(maybeSemicolonToken);
+
+	if (type === 'ClassProperty') {
+		const {key, value} = node;
+
+		if (value) {
+			// Computed key may have `]` after `key`
+			const equalToken = sourceCode.getTokenAfter(key, isEqualToken);
+			yield fixer.replaceText(equalToken, ':');
+		} else if (hasSemicolonToken) {
+			yield fixer.insertTextBefore(maybeSemicolonToken, ': undefined');
+		} else {
+			yield fixer.insertTextAfter(node, ': undefined');
+		}
+	}
+
+	yield (
+		hasSemicolonToken ?
+			fixer.replaceText(maybeSemicolonToken, ',') :
+			fixer.insertTextAfter(node, ',')
+	);
+}
+
+function switchClassToObject(node, sourceCode) {
+	const {
+		type,
+		id,
+		body,
+		declare: isDeclare,
+		abstract: isAbstract,
+		implements: classImplements,
+		parent
+	} = node;
+
+	if (
+		isDeclare ||
+		isAbstract ||
+		(Array.isArray(classImplements) && classImplements.length > 0)
+	) {
+		return;
+	}
+
+	if (type === 'ClassExpression' && id) {
+		return;
+	}
+
+	const isExportDefault = isDeclarationOfExportDefaultDeclaration(node);
+
+	if (isExportDefault && id) {
+		return;
+	}
+
+	for (const node of body.body) {
+		if (
+			node.type === 'ClassProperty' &&
+			(
+				node.typeAnnotation ||
+				// This is a stupid way to check if `value` of `ClassProperty` uses `this`
+				(node.value && sourceCode.getText(node).includes('this'))
+			)
+		) {
+			return;
+		}
+	}
+
+	return function * (fixer) {
+		const classToken = sourceCode.getFirstToken(node);
+		/* istanbul ignore next */
+		assertToken(classToken, {
+			expected: {type: 'Keyword', value: 'class'},
+			ruleId: 'no-static-only-class'
+		});
+
+		if (isExportDefault || type === 'ClassExpression') {
+			/*
+				There are comments after return, and `{` is not on same line
+
+				```js
+				function a() {
+					return class // comment
+					{
+						static a() {}
+					}
+				}
+				```
+			*/
+			if (
+				type === 'ClassExpression' &&
+				parent.type === 'ReturnStatement' &&
+				body.loc.start.line !== parent.loc.start.line &&
+				sourceCode.text.slice(classToken.range[1], body.range[0]).trim()
+			) {
+				yield fixer.replaceText(classToken, '{');
+
+				const openingBraceToken = sourceCode.getFirstToken(body);
+				yield fixer.remove(openingBraceToken);
+			} else {
+				yield fixer.replaceText(classToken, '');
+
+				/*
+						Avoid breaking case like
+
+						```js
+						return class
+						{};
+						```
+				*/
+				yield removeSpacesAfter(classToken, sourceCode, fixer);
+			}
+
+			// There should not be ASI problem
+		} else {
+			yield fixer.replaceText(classToken, 'const');
+			yield fixer.insertTextBefore(body, '= ');
+			yield fixer.insertTextAfter(body, ';');
+		}
+
+		for (const node of body.body) {
+			yield * switchClassMemberToObjectProperty(node, sourceCode, fixer);
+		}
+	};
+}
+
+function create(context) {
+	const sourceCode = context.getSourceCode();
+
+	return {
+		[selector](node) {
+			if (node.body.body.some(node => !isStaticMember(node))) {
+				return;
+			}
+
+			context.report({
+				node,
+				loc: getClassHeadLocation(node, sourceCode),
+				messageId: MESSAGE_ID,
+				fix: switchClassToObject(node, sourceCode)
+			});
+		}
+	};
+}
+
+module.exports = {
+	create,
+	meta: {
+		type: 'suggestion',
+		docs: {
+			url: getDocumentationUrl(__filename)
+		},
+		fixable: 'code',
+		messages
+	}
+};

rules/prefer-optional-catch-binding.js

@@ -1,6 +1,7 @@
 'use strict';
 const getDocumentationUrl = require('./utils/get-documentation-url');
 const {findVariable, isOpeningParenToken, isClosingParenToken} = require('eslint-utils');
+const assertToken = require('./utils/assert-token');
 
 const ERROR_MESSAGE_ID = 'error';
 const messages = {
@@ -33,12 +34,18 @@ const create = context => {
 				data: {name},
 				* fix(fixer) {
 					const tokenBefore = context.getTokenBefore(node);
-					const tokenAfter = context.getTokenAfter(node);
+					assertToken(tokenBefore, {
+						test: isOpeningParenToken,
+						expected: '(',
+						ruleId: 'prefer-optional-catch-binding'
+					});
 
-					/* istanbul ignore next */
-					if (!isOpeningParenToken(tokenBefore) || !isClosingParenToken(tokenAfter)) {
-						throw new Error('Unexpected token.');
-					}
+					const tokenAfter = context.getTokenAfter(node);
+					assertToken(tokenAfter, {
+						test: isClosingParenToken,
+						expected: ')',
+						ruleId: 'prefer-optional-catch-binding'
+					});
 
 					yield fixer.remove(tokenBefore);
 					yield fixer.remove(node);