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

Commit b6c8b0c

Browse files
tanmoyopenrootadidahiya
authored andcommitted
[new-rule-option] check-super-calls option for unnecessary-constructor rule (#4813)
1 parent 7659cd9 commit b6c8b0c

File tree

8 files changed

+678
-78
lines changed

8 files changed

+678
-78
lines changed

src/rules/unnecessaryConstructorRule.ts

Lines changed: 86 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,36 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { isConstructorDeclaration, isParameterProperty } from "tsutils";
18+
import {
19+
isCallExpression,
20+
isConstructorDeclaration,
21+
isExpressionStatement,
22+
isParameterProperty,
23+
} from "tsutils";
1924
import * as ts from "typescript";
2025

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

29+
interface Options {
30+
checkSuperCall: boolean;
31+
}
32+
33+
const OPTION_CHECK_SUPER_CALL = "check-super-calls";
34+
2435
export class Rule extends Lint.Rules.AbstractRule {
2536
public static metadata: Lint.IRuleMetadata = {
2637
description: "Prevents blank constructors, as they are redundant.",
27-
optionExamples: [true],
28-
options: null,
29-
optionsDescription: "Not configurable.",
38+
optionExamples: [true, [true, { [OPTION_CHECK_SUPER_CALL]: true }]],
39+
options: {
40+
properties: {
41+
[OPTION_CHECK_SUPER_CALL]: { type: "boolean" },
42+
},
43+
type: "object",
44+
},
45+
optionsDescription: Lint.Utils.dedent`
46+
An optional object with the property '${OPTION_CHECK_SUPER_CALL}'.
47+
This is to check for unnecessary constructor parameters for super call`,
3048
rationale: Lint.Utils.dedent`
3149
JavaScript implicitly adds a blank constructor when there isn't one.
3250
It's not necessary to manually add one in.
@@ -39,12 +57,71 @@ export class Rule extends Lint.Rules.AbstractRule {
3957
public static FAILURE_STRING = "Remove unnecessary empty constructor.";
4058

4159
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
42-
return this.applyWithFunction(sourceFile, walk);
60+
const options: Options = {
61+
checkSuperCall:
62+
this.ruleArguments.length !== 0 &&
63+
(this.ruleArguments[0] as { "check-super-calls"?: boolean })[
64+
"check-super-calls"
65+
] === true,
66+
};
67+
68+
return this.applyWithFunction(sourceFile, walk, options);
4369
}
4470
}
4571

46-
const isEmptyConstructor = (node: ts.ConstructorDeclaration): boolean =>
47-
node.body !== undefined && node.body.statements.length === 0;
72+
const containsSuper = (
73+
statement: ts.Statement,
74+
constructorParameters: ts.NodeArray<ts.ParameterDeclaration>,
75+
): boolean => {
76+
if (
77+
isExpressionStatement(statement) &&
78+
isCallExpression(statement.expression) &&
79+
ts.SyntaxKind.SuperKeyword === statement.expression.expression.kind
80+
) {
81+
const superArguments = statement.expression.arguments;
82+
83+
if (superArguments.length < constructorParameters.length) {
84+
return true;
85+
}
86+
87+
if (superArguments.length === constructorParameters.length) {
88+
if (constructorParameters.length === 0) {
89+
return true;
90+
}
91+
92+
for (const constructorParameter of constructorParameters) {
93+
for (const superArgument of superArguments) {
94+
if (constructorParameter.name.kind !== superArgument.kind) {
95+
return false;
96+
}
97+
}
98+
}
99+
100+
return true;
101+
}
102+
}
103+
104+
return false;
105+
};
106+
107+
const isEmptyOrContainsOnlySuper = (node: ts.ConstructorDeclaration, options: Options): boolean => {
108+
if (node.body !== undefined) {
109+
const { checkSuperCall } = options;
110+
111+
if (node.body.statements.length === 0) {
112+
return true;
113+
}
114+
115+
if (checkSuperCall) {
116+
return (
117+
node.body.statements.length === 1 &&
118+
containsSuper(node.body.statements[0], node.parameters)
119+
);
120+
}
121+
}
122+
123+
return false;
124+
};
48125

49126
const containsConstructorParameter = (node: ts.ConstructorDeclaration): boolean =>
50127
// If this has any parameter properties
@@ -59,11 +136,11 @@ const isAccessRestrictingConstructor = (node: ts.ConstructorDeclaration): boolea
59136
const containsDecorator = (node: ts.ConstructorDeclaration): boolean =>
60137
node.parameters.some(p => p.decorators !== undefined);
61138

62-
function walk(context: Lint.WalkContext) {
139+
function walk(context: Lint.WalkContext<Options>) {
63140
const callback = (node: ts.Node): void => {
64141
if (
65142
isConstructorDeclaration(node) &&
66-
isEmptyConstructor(node) &&
143+
isEmptyOrContainsOnlySuper(node, context.options) &&
67144
!containsConstructorParameter(node) &&
68145
!containsDecorator(node) &&
69146
!isAccessRestrictingConstructor(node)
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
export class ExportedClass {
2+
}
3+
4+
class PublicConstructor {
5+
}
6+
7+
class ProtectedConstructor {
8+
protected constructor() { }
9+
}
10+
11+
class PrivateConstructor {
12+
private constructor() { }
13+
}
14+
15+
class SameLine { }
16+
17+
class WithPrecedingMember {
18+
public member: string;
19+
}
20+
21+
class WithFollowingMethod {
22+
public method() {}
23+
}
24+
25+
const classExpression = class {
26+
}
27+
28+
class ContainsContents {
29+
constructor() {
30+
console.log("Hello!");
31+
}
32+
}
33+
34+
class CallsSuper extends PublicConstructor {
35+
}
36+
37+
class ContainsParameter {
38+
}
39+
40+
class PrivateContainsParameter {
41+
private constructor(x: number) { }
42+
}
43+
44+
class ProtectedContainsParameter {
45+
protected constructor(x: number) { }
46+
}
47+
48+
class ContainsParameterDeclaration {
49+
constructor(public x: number) { }
50+
}
51+
52+
class ContainsParameterAndParameterDeclaration {
53+
constructor(x: number, public y: number) { }
54+
}
55+
56+
class ConstructorOverload {
57+
}
58+
59+
interface IConstructorSignature {
60+
new(): {};
61+
}
62+
63+
class DecoratedParameters {
64+
constructor(@Optional x: number) {}
65+
}
66+
67+
class X {
68+
constructor(private param1: number, param2: number) {}
69+
}
70+
71+
export class Y extends X {
72+
constructor(param1: number) {
73+
super(param1, 2);
74+
}
75+
}
76+
77+
class Base {
78+
constructor(public param: number) {}
79+
}
80+
81+
class Super extends Base {
82+
public param: number;
83+
constructor(param1: number) {
84+
super(param1);
85+
this.param = param1;
86+
}
87+
}
88+
89+
class Super extends Base {
90+
}
91+
92+
class Super extends Base {
93+
constructor(param1: number, public param2: number) {
94+
super(param1);
95+
}
96+
}
97+
98+
class Super extends Base {
99+
}
100+
101+
class Super extends Base {
102+
constructor(param1: number) {
103+
super(param1 + 1);
104+
}
105+
}
106+
107+
class Super extends Base {
108+
constructor() {
109+
super(1);
110+
}
111+
}
112+
113+
class Super extends Base {
114+
constructor(param1: number) {
115+
super(1);
116+
}
117+
}
118+
119+
const test = (param: number) => number;
120+
121+
class Super extends Base {
122+
constructor(param1: number) {
123+
super(test(param1));
124+
}
125+
}
126+
127+
class Base2 {
128+
constructor(public param1: number, param2: number) {}
129+
}
130+
131+
class Super extends Base2 {
132+
constructor(param1: number, param2: number) {
133+
super(test(param2), param1);
134+
}
135+
}
136+
137+
class Super extends Base2 {
138+
}
139+
140+
class Super extends Base {
141+
public param1: number;
142+
constructor(public param2: number) {
143+
super(param2);
144+
this.param1 = param2;
145+
}
146+
}
147+

0 commit comments

Comments
 (0)