Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

4594 - fix unnecessary-constructor parameters for super call #4813

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
73 changes: 64 additions & 9 deletions src/rules/unnecessaryConstructorRule.ts
Expand Up @@ -15,18 +15,36 @@
* limitations under the License.
*/

import { isConstructorDeclaration, isParameterProperty } from "tsutils";
import {
isCallExpression,
isConstructorDeclaration,
isExpressionStatement,
isParameterProperty,
} from "tsutils";
import * as ts from "typescript";

import * as Lint from "../index";
import { Replacement } from "../language/rule/rule";

interface Options {
unnecessarySuperCall: boolean;
}

const OPTION_UNNECESSARY_SUPER_CALL = "unnecessary-super-call";
tanmoyopenroot marked this conversation as resolved.
Show resolved Hide resolved

export class Rule extends Lint.Rules.AbstractRule {
public static metadata: Lint.IRuleMetadata = {
description: "Prevents blank constructors, as they are redundant.",
optionExamples: [true],
options: null,
optionsDescription: "Not configurable.",
optionExamples: [true, [true, { [OPTION_UNNECESSARY_SUPER_CALL]: true }]],
options: {
properties: {
[OPTION_UNNECESSARY_SUPER_CALL]: { type: "boolean" },
},
type: "object",
},
optionsDescription: Lint.Utils.dedent`
An optional object with the property '${OPTION_UNNECESSARY_SUPER_CALL}'.
This is to check for unnecessary constructor parameters for super call`,
rationale: Lint.Utils.dedent`
JavaScript implicitly adds a blank constructor when there isn't one.
It's not necessary to manually add one in.
Expand All @@ -39,12 +57,49 @@ export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING = "Remove unnecessary empty constructor.";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk);
const options: Options = {
unnecessarySuperCall:
this.ruleArguments.length !== 0 &&
(this.ruleArguments[0] as { "unnecessary-super-call"?: boolean })[
"unnecessary-super-call"
] === true,
};

return this.applyWithFunction(sourceFile, walk, options);
}
}

const isEmptyConstructor = (node: ts.ConstructorDeclaration): boolean =>
node.body !== undefined && node.body.statements.length === 0;
const containsStatements = (statements: ts.NodeArray<ts.Statement>) => statements.length > 0;

const containsSuper = (statements: ts.NodeArray<ts.Statement>): boolean => {
for (const statement of statements) {
tanmoyopenroot marked this conversation as resolved.
Show resolved Hide resolved
if (
isExpressionStatement(statement) &&
isCallExpression(statement.expression) &&
ts.SyntaxKind.SuperKeyword === statement.expression.expression.kind
) {
return true;
}
}

return false;
};

const isEmptyOrContainsOnlySuper = (node: ts.ConstructorDeclaration, options: Options): boolean => {
if (node.body) {
const { unnecessarySuperCall } = options;

if (!containsStatements(node.body.statements)) {
tanmoyopenroot marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

if (unnecessarySuperCall) {
return node.body.statements.length === 1 && containsSuper(node.body.statements);
}
}

return false;
};

const containsConstructorParameter = (node: ts.ConstructorDeclaration): boolean =>
// If this has any parameter properties
Expand All @@ -59,11 +114,11 @@ const isAccessRestrictingConstructor = (node: ts.ConstructorDeclaration): boolea
const containsDecorator = (node: ts.ConstructorDeclaration): boolean =>
node.parameters.some(p => p.decorators !== undefined);

function walk(context: Lint.WalkContext) {
function walk(context: Lint.WalkContext<Options>) {
const callback = (node: ts.Node): void => {
if (
isConstructorDeclaration(node) &&
isEmptyConstructor(node) &&
isEmptyOrContainsOnlySuper(node, context.options) &&
!containsConstructorParameter(node) &&
!containsDecorator(node) &&
!isAccessRestrictingConstructor(node)
Expand Down
111 changes: 111 additions & 0 deletions test/rules/unnecessary-constructor/default/test.ts.fix
@@ -0,0 +1,111 @@
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) {}
}

class X {
constructor(private param1: number, param2: number) {}
}

export class Y extends X {
constructor(param1: number) {
super(param1, 2);
}
}

class Base {
constructor(public param: number) {}
}

class Super extends Base {
public param: number;
constructor(param1: number) {
super(param1);
this.param = param1;
}
}

class Super extends Base {
constructor(param: number) {
super(param);
}
}

class Super extends Base {
constructor(param1: number, public param2: number) {
super(param1);
}
}

class Super extends Base {
public param1: number;
constructor(public param2: number) {
super(param2);
this.param1 = param2;
}
}

Expand Up @@ -83,4 +83,46 @@ class DecoratedParameters {
constructor(@Optional x: number) {}
}

class X {
constructor(private param1: number, param2: number) {}
}

export class Y extends X {
constructor(param1: number) {
super(param1, 2);
}
}

class Base {
constructor(public param: number) {}
}

class Super extends Base {
public param: number;
constructor(param1: number) {
super(param1);
this.param = param1;
}
}

class Super extends Base {
constructor(param: number) {
super(param);
}
}

class Super extends Base {
constructor(param1: number, public param2: number) {
super(param1);
}
}

class Super extends Base {
public param1: number;
constructor(public param2: number) {
super(param2);
this.param1 = param2;
}
}

[0]: Remove unnecessary empty constructor.
Expand Up @@ -32,9 +32,6 @@ class ContainsContents {
}

class CallsSuper extends PublicConstructor {
constructor() {
super();
}
}

class ContainsParameter {
Expand Down Expand Up @@ -67,3 +64,39 @@ class DecoratedParameters {
constructor(@Optional x: number) {}
}

class X {
constructor(private param1: number, param2: number) {}
}

export class Y extends X {
}

class Base {
constructor(public param: number) {}
}

class Super extends Base {
public param: number;
constructor(param1: number) {
super(param1);
this.param = param1;
}
}

class Super extends Base {
}

class Super extends Base {
constructor(param1: number, public param2: number) {
super(param1);
}
}

class Super extends Base {
public param1: number;
constructor(public param2: number) {
super(param2);
this.param1 = param2;
}
}