Add custom-error-definition rule - fixes #74 (#75)

* add custom-error rule - fixes #74

* add extra tests

* tweaks

* tweaks

Author: SamVerschueren

docs/rules/custom-error-definition.md

@@ -0,0 +1,108 @@
+# Enforce correct Error subclassing
+
+Enforces the only valid way of `Error` subclassing. It works with any super class that ends in `Error`.
+
+
+## Fail
+
+```js
+class CustomError extends Error {
+	constructor(message) {
+		super(message);
+		this.message = message;
+		this.name = 'CustomError';
+	}
+}
+```
+
+The `this.message` assignment is useless as it's already set via the `super()` call.
+
+
+```js
+class CustomError extends Error {
+	constructor(message) {
+		super();
+		this.message = message;
+		this.name = 'CustomError';
+	}
+}
+```
+
+Pass the error message to `super()` instead of setting `this.message`.
+
+
+```js
+class CustomError extends Error {
+	constructor(message) {
+		super(message);
+	}
+}
+```
+
+No `name` property set. The name property is needed so the error shows up as `[CustomError: foo]` and not `[Error: foo]`.
+
+
+```js
+class CustomError extends Error {
+	constructor(message) {
+		super(message);
+		this.name = this.constructor.name;
+	}
+}
+```
+
+Use a string literal to set the `name` property as it will not change after minifying.
+
+
+```js
+class CustomError extends Error {
+	constructor(message) {
+		super(message);
+		this.name = 'MyError';
+	}
+}
+```
+
+The `name` property should be set to the class name.
+
+
+```js
+class foo extends Error {
+	constructor(message) {
+		super(message);
+		this.name = 'foo';
+	}
+}
+```
+
+The class name is invalid. It should be capitalized and end with `Error`. In this case it should be `FooError`.
+
+
+## Pass
+
+```js
+class CustomError extends Error {
+	constructor(message) {
+		super(message);
+		this.name = 'CustomError';
+	}
+}
+```
+
+```js
+class CustomError extends Error {
+	constructor() {
+		super('My custom error');
+		this.name = 'CustomError';
+	}
+}
+```
+
+```js
+class CustomError extends TypeError {
+	constructor() {
+		super();
+		this.name = 'CustomError';
+	}
+}
+```

index.js

@@ -23,7 +23,8 @@ module.exports = {
 				'unicorn/escape-case': 'error',
 				'unicorn/no-array-instanceof': 'error',
 				'unicorn/no-new-buffer': 'error',
-				'unicorn/no-hex-escape': 'error'
+				'unicorn/no-hex-escape': 'error',
+				'unicorn/custom-error-definition': 'error'
 			}
 		}
 	}

readme.md

@@ -43,7 +43,8 @@ Configure it in `package.json`.
 			"unicorn/escape-case": "error",
 			"unicorn/no-array-instanceof": "error",
 			"unicorn/no-new-buffer": "error",
-			"unicorn/no-hex-escape": "error"
+			"unicorn/no-hex-escape": "error",
+			"unicorn/custom-error-definition": "error"
 		}
 	}
 }
@@ -63,6 +64,7 @@ Configure it in `package.json`.
 - [no-array-instanceof](docs/rules/no-array-instanceof.md) - Require `Array.isArray()` instead of `instanceof Array`. *(fixable)*
 - [no-new-buffer](docs/rules/no-new-buffer.md) - Enforce the use of `Buffer.from()` and `Buffer.alloc()` instead of the deprecated `new Buffer()`. *(fixable)*
 - [no-hex-escape](docs/rules/no-hex-escape.md) - Enforce the use of unicode escapes instead of hexadecimal escapes. *(fixable)*
+- [custom-error-definition](docs/rules/custom-error-definition.md) - Enforces the only valid way of `Error` subclassing. *(fixable)*
 
 
 ## Recommended config

rules/custom-error-definition.js

@@ -0,0 +1,132 @@
+'use strict';
+const upperfirst = require('lodash.upperfirst');
+
+const nameRegexp = /^(?:[A-Z][a-z0-9]*)*Error$/;
+
+const getClassName = name => upperfirst(name).replace(/(error|)$/i, 'Error');
+
+const getConstructorMethod = className => `
+	constructor() {
+		super();
+		this.name = '${className}';
+	}
+`;
+
+const hasValidSuperClass = node => {
+	if (!node.superClass) {
+		return false;
+	}
+
+	let name = node.superClass.name;
+
+	if (node.superClass.type === 'MemberExpression') {
+		name = node.superClass.property.name;
+	}
+
+	return nameRegexp.test(name);
+};
+
+const isSuperExpression = node => node.type === 'ExpressionStatement' && node.expression.type === 'CallExpression' && node.expression.callee.type === 'Super';
+
+const isAssignmentExpression = (node, name) => {
+	if (node.type !== 'ExpressionStatement' || node.expression.type !== 'AssignmentExpression') {
+		return false;
+	}
+
+	const lhs = node.expression.left;
+
+	if (!lhs.object || lhs.object.type !== 'ThisExpression') {
+		return false;
+	}
+
+	return lhs.property.name === name;
+};
+
+const create = context => {
+	return {
+		ClassDeclaration: node => {
+			if (!hasValidSuperClass(node)) {
+				return;
+			}
+
+			const name = node.id.name;
+			const className = getClassName(name);
+
+			if (name !== className) {
+				context.report({
+					node: node.id,
+					message: `Invalid class name, use \`${className}\`.`
+				});
+			}
+
+			const body = node.body.body;
+
+			const constructor = body.find(x => x.kind === 'constructor');
+
+			if (!constructor) {
+				context.report({
+					node,
+					message: 'Add a constructor to your error.',
+					fix: fixer => fixer.insertTextAfterRange([
+						node.body.start,
+						node.body.start + 1
+					], getConstructorMethod(name))
+				});
+				return;
+			}
+
+			const constructorBodyNode = constructor.value.body;
+			const constructorBody = constructorBodyNode.body;
+
+			const superExpression = constructorBody.find(isSuperExpression);
+			const messageExpressionIndex = constructorBody.findIndex(x => isAssignmentExpression(x, 'message'));
+
+			if (!superExpression) {
+				context.report({
+					node: constructorBodyNode,
+					message: 'Missing call to `super()` in constructor.'
+				});
+			} else if (messageExpressionIndex !== -1 && superExpression.expression.arguments.length === 0) {
+				const rhs = constructorBody[messageExpressionIndex].expression.right;
+
+				context.report({
+					node: superExpression,
+					message: 'Pass the error message to `super()`.',
+					fix: fixer => fixer.insertTextAfterRange([
+						superExpression.start,
+						superExpression.start + 6
+					], rhs.raw || rhs.name)
+				});
+			}
+
+			if (messageExpressionIndex !== -1) {
+				const expression = constructorBody[messageExpressionIndex];
+
+				context.report({
+					node: expression,
+					message: 'Pass the error message to `super()` instead of setting `this.message`.',
+					fix: fixer => fixer.removeRange([
+						messageExpressionIndex === 0 ? constructorBodyNode.start : constructorBody[messageExpressionIndex - 1].end,
+						expression.end
+					])
+				});
+			}
+
+			const nameExpression = constructorBody.find(x => isAssignmentExpression(x, 'name'));
+
+			if (!nameExpression || nameExpression.expression.right.value !== name) {
+				context.report({
+					node: nameExpression ? nameExpression.expression.right : constructorBodyNode,
+					message: `The \`name\` property should be set to \`${name}\`.`
+				});
+			}
+		}
+	};
+};
+
+module.exports = {
+	create,
+	meta: {
+		fixable: 'code'
+	}
+};