diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index f0c11446353e..1ec6df216048 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -121,9 +121,10 @@ Install [`eslint-config-prettier`](https://github.com/prettier/eslint-config-pre | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` (`no-this-assignment` from TSLint) | | | | [`@typescript-eslint/no-triple-slash-reference`](./docs/rules/no-triple-slash-reference.md) | Disallow `/// ` comments (`no-reference` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases (`interface-over-type-literal` from TSLint) | | | -| [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression (`no-unnecessary-type-assertion` from TSLint) | | :wrench: | +| [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression (`no-unnecessary-type-assertion` from TSLint) | | :wrench: | | [`@typescript-eslint/no-unused-vars`](./docs/rules/no-unused-vars.md) | Disallow unused variables (`no-unused-variable` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-use-before-define`](./docs/rules/no-use-before-define.md) | Disallow the use of variables before they are defined | :heavy_check_mark: | | +| [`@typescript-eslint/no-useless-constructor`](./docs/rules/no-useless-constructor.md) | Disallow unnecessary constructors | | | | [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements (`no-var-requires` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/prefer-interface`](./docs/rules/prefer-interface.md) | Prefer an interface declaration over a type literal (type T = { ... }) (`interface-over-type-literal` from TSLint) | :heavy_check_mark: | :wrench: | | [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules. (`no-internal-module` from TSLint) | :heavy_check_mark: | :wrench: | diff --git a/packages/eslint-plugin/docs/rules/no-useless-constructor.md b/packages/eslint-plugin/docs/rules/no-useless-constructor.md new file mode 100644 index 000000000000..0e136d122ac1 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-useless-constructor.md @@ -0,0 +1,78 @@ +# Disallow unnecessary constructors (no-useless-constructor) + +ES2015 provides a default class constructor if one is not specified. As such, it is unnecessary to provide an empty constructor or one that simply delegates into its parent class, as in the following examples: + +```js +class A { + constructor() {} +} + +class A extends B { + constructor(value) { + super(value); + } +} +``` + +## Rule Details + +This rule flags class constructors that can be safely removed without changing how the class works. + +## Examples + +Examples of **incorrect** code for this rule: + +```js +/*eslint no-useless-constructor: "error"*/ + +class A { + constructor() {} +} + +class A extends B { + constructor(...args) { + super(...args); + } +} +``` + +Examples of **correct** code for this rule: + +```js +/*eslint no-useless-constructor: "error"*/ + +class A {} + +class A { + constructor() { + doSomething(); + } +} + +class A extends B { + constructor() { + super('foo'); + } +} + +class A extends B { + constructor() { + super(); + doSomething(); + } +} + +class A extends B { + constructor(protected name: string) {} +} + +class A extends B { + protected constructor() {} +} +``` + +## When Not To Use It + +If you don't want to be notified about unnecessary constructors, you can safely disable this rule. + +Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/no-useless-constructor.md) diff --git a/packages/eslint-plugin/lib/rules/no-useless-constructor.js b/packages/eslint-plugin/lib/rules/no-useless-constructor.js new file mode 100644 index 000000000000..c30cdcf872bf --- /dev/null +++ b/packages/eslint-plugin/lib/rules/no-useless-constructor.js @@ -0,0 +1,80 @@ +/** + * @fileoverview Disallow unnecessary constructors + * @author Armano + */ +'use strict'; + +const baseRule = require('eslint/lib/rules/no-useless-constructor'); +const util = require('../util'); + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +/** + * Check if method with accessibility is not useless + * @param {MethodDefinition} node + * @returns {boolean} + */ +function checkAccessibility(node) { + switch (node.accessibility) { + case 'protected': + case 'private': + return false; + case 'public': + if ( + node.parent.type === 'ClassBody' && + node.parent.parent && + node.parent.parent.superClass + ) { + return false; + } + break; + } + return true; +} + +/** + * Check if method is not unless due to typescript parameter properties + * @param {MethodDefinition} node + * @returns {boolean} + */ +function checkParams(node) { + return ( + !node.value.params || + !node.value.params.some(param => param.type === 'TSParameterProperty') + ); +} + +module.exports = Object.assign({}, baseRule, { + meta: { + type: 'problem', + docs: { + description: 'Disallow unnecessary constructors', + category: 'ECMAScript 6', + url: util.metaDocsUrl('no-useless-constructor') + }, + schema: baseRule.meta.schema, + messages: baseRule.meta.messages + }, + + create(context) { + const rules = baseRule.create(context); + + //---------------------------------------------------------------------- + // Public + //---------------------------------------------------------------------- + return { + MethodDefinition(node) { + if ( + node.value && + node.value.type === 'FunctionExpression' && + checkAccessibility(node) && + checkParams(node) + ) { + rules.MethodDefinition(node); + } + } + }; + } +}); diff --git a/packages/eslint-plugin/tests/lib/rules/no-useless-constructor.js b/packages/eslint-plugin/tests/lib/rules/no-useless-constructor.js new file mode 100644 index 000000000000..c73c5034f121 --- /dev/null +++ b/packages/eslint-plugin/tests/lib/rules/no-useless-constructor.js @@ -0,0 +1,100 @@ +'use strict'; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-useless-constructor'), + RuleTester = require('eslint').RuleTester; + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 6, + sourceType: 'module' + }, + parser: '@typescript-eslint/parser' +}); + +const error = { message: 'Useless constructor.', type: 'MethodDefinition' }; + +ruleTester.run('no-useless-constructor', rule, { + valid: [ + 'class A { }', + 'class A { constructor(){ doSomething(); } }', + 'class A extends B { constructor(){} }', + "class A extends B { constructor(){ super('foo'); } }", + 'class A extends B { constructor(foo, bar){ super(foo, bar, 1); } }', + 'class A extends B { constructor(){ super(); doSomething(); } }', + 'class A extends B { constructor(...args){ super(...args); doSomething(); } }', + 'class A { dummyMethod(){ doSomething(); } }', + 'class A extends B.C { constructor() { super(foo); } }', + 'class A extends B.C { constructor([a, b, c]) { super(...arguments); } }', + 'class A extends B.C { constructor(a = f()) { super(...arguments); } }', + 'class A extends B { constructor(a, b, c) { super(a, b); } }', + 'class A extends B { constructor(foo, bar){ super(foo); } }', + 'class A extends B { constructor(test) { super(); } }', + 'class A extends B { constructor() { foo; } }', + 'class A extends B { constructor(foo, bar) { super(bar); } }', + // https://github.com/typescript-eslint/typescript-eslint/issues/15 + 'declare class A { constructor(); }', + 'class A { constructor(); }', + 'abstract class A { constructor(); }', + 'abstract class A { abstract constructor(); }', + // https://github.com/typescript-eslint/typescript-eslint/issues/48 + 'class A { constructor(private name: string) {} }', + 'class A { constructor(public name: string) {} }', + 'class A { constructor(protected name: string) {} }', + // https://github.com/typescript-eslint/typescript-eslint/pull/167#discussion_r252638401 + 'class A { private constructor() {} }', + 'class A { protected constructor() {} }', + 'class A extends B { public constructor() {} }', + 'class A extends B { protected constructor(foo, bar) { super(bar); } }', + 'class A extends B { private constructor(foo, bar) { super(bar); } }', + 'class A extends B { public constructor(foo){ super(foo); } }', + 'class A extends B { public constructor(foo){} }' + ], + invalid: [ + { + code: 'class A { constructor(){} }', + errors: [error] + }, + { + code: "class A { 'constructor'(){} }", + errors: [error] + }, + { + code: 'class A extends B { constructor() { super(); } }', + errors: [error] + }, + { + code: 'class A extends B { constructor(foo){ super(foo); } }', + errors: [error] + }, + { + code: 'class A extends B { constructor(foo, bar){ super(foo, bar); } }', + errors: [error] + }, + { + code: 'class A extends B { constructor(...args){ super(...args); } }', + errors: [error] + }, + { + code: 'class A extends B.C { constructor() { super(...arguments); } }', + errors: [error] + }, + { + code: + 'class A extends B { constructor(a, b, ...c) { super(...arguments); } }', + errors: [error] + }, + { + code: + 'class A extends B { constructor(a, b, ...c) { super(a, b, ...c); } }', + errors: [error] + }, + { + code: 'class A { public constructor() {} }', + errors: [error] + } + ] +});