Add a fixer for "unnecessary-constructor" rule. (#4694)

bowenni · Josh Goldberg · commit be293b0b2005 · 2019-05-04T21:17:06.000-04:00
* Add a fixer for "unnecessary-constructor" rule.

* Handle decorated parameters.
diff --git 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
@@ -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
@@ -79,4 +79,8 @@ interface IConstructorSignature {
     new(): {};
 }
 
+class DecoratedParameters {
+    constructor(@Optional x: number) {}
+}
+
 [0]: Remove unnecessary empty constructor.

Original file line number	Diff line number	Diff line change
`@@ -79,4 +79,8 @@ interface IConstructorSignature {`
`79`	`79`	`new(): {};`
`80`	`80`	`}`
`81`	`81`
	`82`	`+class DecoratedParameters {`
	`83`	`+ constructor(@Optional x: number) {}`
	`84`	`+}`
	`85`	`+`
`82`	`86`	`[0]: Remove unnecessary empty constructor.`