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 all commits
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
95 changes: 86 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 {
checkSuperCall: boolean;
}

const OPTION_CHECK_SUPER_CALL = "check-super-calls";

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_CHECK_SUPER_CALL]: true }]],
options: {
properties: {
[OPTION_CHECK_SUPER_CALL]: { type: "boolean" },
},
type: "object",
},
optionsDescription: Lint.Utils.dedent`
An optional object with the property '${OPTION_CHECK_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,71 @@ 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 = {
checkSuperCall:
this.ruleArguments.length !== 0 &&
(this.ruleArguments[0] as { "check-super-calls"?: boolean })[
"check-super-calls"
] === true,
};

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

const isEmptyConstructor = (node: ts.ConstructorDeclaration): boolean =>
node.body !== undefined && node.body.statements.length === 0;
const containsSuper = (
statement: ts.Statement,
constructorParameters: ts.NodeArray<ts.ParameterDeclaration>,
): boolean => {
if (
isExpressionStatement(statement) &&
isCallExpression(statement.expression) &&
ts.SyntaxKind.SuperKeyword === statement.expression.expression.kind
) {
const superArguments = statement.expression.arguments;

if (superArguments.length < constructorParameters.length) {
return true;
}

if (superArguments.length === constructorParameters.length) {
if (constructorParameters.length === 0) {
return true;
}

for (const constructorParameter of constructorParameters) {
for (const superArgument of superArguments) {
if (constructorParameter.name.kind !== superArgument.kind) {
return false;
}
}
}

return true;
}
}

return false;
};

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

if (node.body.statements.length === 0) {
return true;
}

if (checkSuperCall) {
return (
node.body.statements.length === 1 &&
containsSuper(node.body.statements[0], node.parameters)
);
}
}

return false;
};

const containsConstructorParameter = (node: ts.ConstructorDeclaration): boolean =>
// If this has any parameter properties
Expand All @@ -59,11 +136,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
147 changes: 147 additions & 0 deletions test/rules/unnecessary-constructor/check-super-calls/test.ts.fix
@@ -0,0 +1,147 @@
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 {
}

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 {
}

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

class Super extends Base {
}

class Super extends Base {
constructor(param1: number) {
super(param1 + 1);
}
}

class Super extends Base {
constructor() {
super(1);
}
}

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

const test = (param: number) => number;

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

class Base2 {
constructor(public param1: number, param2: number) {}
}

class Super extends Base2 {
constructor(param1: number, param2: number) {
super(test(param2), param1);
}
}

class Super extends Base2 {
}

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