From be293b0b2005aadfb0cc6e13ab4ca9247490d6c9 Mon Sep 17 00:00:00 2001 From: Bowen Ni Date: Sat, 4 May 2019 18:17:06 -0700 Subject: [PATCH] Add a fixer for "unnecessary-constructor" rule. (#4694) * Add a fixer for "unnecessary-constructor" rule. * Handle decorated parameters. --- src/rules/unnecessaryConstructorRule.ts | 16 ++++- .../rules/unnecessary-constructor/test.ts.fix | 69 +++++++++++++++++++ .../unnecessary-constructor/test.ts.lint | 4 ++ 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 test/rules/unnecessary-constructor/test.ts.fix diff --git a/src/rules/unnecessaryConstructorRule.ts b/src/rules/unnecessaryConstructorRule.ts index cb7d1ff0b74..616db5e3870 100644 --- a/src/rules/unnecessaryConstructorRule.ts +++ b/src/rules/unnecessaryConstructorRule.ts @@ -19,6 +19,7 @@ import { isConstructorDeclaration, isParameterProperty } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; +import { Replacement } from "../language/rule/rule"; export class Rule extends Lint.Rules.AbstractRule { public static metadata: Lint.IRuleMetadata = { @@ -55,15 +56,28 @@ const isAccessRestrictingConstructor = (node: ts.ConstructorDeclaration): boolea // If this has any modifier that isn't public, it's doing something node.modifiers.some(modifier => modifier.kind !== ts.SyntaxKind.PublicKeyword); +const containsDecorator = (node: ts.ConstructorDeclaration): boolean => + node.parameters.some(p => p.decorators !== undefined); + function walk(context: Lint.WalkContext) { const callback = (node: ts.Node): void => { if ( isConstructorDeclaration(node) && isEmptyConstructor(node) && !containsConstructorParameter(node) && + !containsDecorator(node) && !isAccessRestrictingConstructor(node) ) { - context.addFailureAtNode(node, Rule.FAILURE_STRING); + const replacements = []; + // Since only one constructor implementation is allowed and the one found above is empty, all other overloads can be safely removed. + for (const maybeConstructor of node.parent.members) { + if (isConstructorDeclaration(maybeConstructor)) { + replacements.push( + Replacement.deleteFromTo(maybeConstructor.pos, maybeConstructor.end), + ); + } + } + context.addFailureAtNode(node, Rule.FAILURE_STRING, replacements); } else { ts.forEachChild(node, callback); } diff --git a/test/rules/unnecessary-constructor/test.ts.fix b/test/rules/unnecessary-constructor/test.ts.fix new file mode 100644 index 00000000000..24e5163d8ba --- /dev/null +++ b/test/rules/unnecessary-constructor/test.ts.fix @@ -0,0 +1,69 @@ +export class ExportedClass { +} + +class PublicConstructor { +} + +class ProtectedConstructor { + protected constructor() { } +} + +class PrivateConstructor { + private constructor() { } +} + +class SameLine { } + +class WithPrecedingMember { + public member: string; +} + +class WithFollowingMethod { + public method() {} +} + +const classExpression = class { +} + +class ContainsContents { + constructor() { + console.log("Hello!"); + } +} + +class CallsSuper extends PublicConstructor { + constructor() { + super(); + } +} + +class ContainsParameter { +} + +class PrivateContainsParameter { + private constructor(x: number) { } +} + +class ProtectedContainsParameter { + protected constructor(x: number) { } +} + +class ContainsParameterDeclaration { + constructor(public x: number) { } +} + +class ContainsParameterAndParameterDeclaration { + constructor(x: number, public y: number) { } +} + +class ConstructorOverload { +} + +interface IConstructorSignature { + new(): {}; +} + +class DecoratedParameters { + constructor(@Optional x: number) {} +} + diff --git a/test/rules/unnecessary-constructor/test.ts.lint b/test/rules/unnecessary-constructor/test.ts.lint index ff619f7cc6f..86febfa49a7 100644 --- a/test/rules/unnecessary-constructor/test.ts.lint +++ b/test/rules/unnecessary-constructor/test.ts.lint @@ -79,4 +79,8 @@ interface IConstructorSignature { new(): {}; } +class DecoratedParameters { + constructor(@Optional x: number) {} +} + [0]: Remove unnecessary empty constructor.