Skip to content

feat(eslint-plugin): add eslint rule no-useless-constructor #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 1, 2019
3 changes: 2 additions & 1 deletion packages/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `/// <reference path="" />` 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: |
Expand Down
78 changes: 78 additions & 0 deletions packages/eslint-plugin/docs/rules/no-useless-constructor.md
Original file line number Diff line number Diff line change
@@ -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.

<sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/no-useless-constructor.md)</sup>
80 changes: 80 additions & 0 deletions packages/eslint-plugin/lib/rules/no-useless-constructor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* @fileoverview Disallow unnecessary constructors
* @author Armano <https://github.com/armano2>
*/
'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);
}
}
};
}
});
100 changes: 100 additions & 0 deletions packages/eslint-plugin/tests/lib/rules/no-useless-constructor.js
Original file line number Diff line number Diff line change
@@ -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]
}
]
});