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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 64 additions & 9 deletions src/rules/unnecessaryConstructorRule.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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;
}
}

Original file line number Diff line number Diff line change
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.
Original file line number Diff line number Diff line change
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;
}
}

Loading