Skip to content

Commit 4415cc2

Browse files
kevinphelpsmgechev
authored andcommitted
fix(rule): fix bugs in no-life-cycle-call rule (#575)
* fix(rule): allow super lifecycle calls fixes #573 * fix(rule): prevent error when parent class node does not exist
1 parent 2184b91 commit 4415cc2

File tree

2 files changed

+54
-15
lines changed

2 files changed

+54
-15
lines changed

src/noLifeCycleCallRule.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export class Rule extends Lint.Rules.AbstractRule {
1414
typescriptOnly: true
1515
};
1616

17-
static FAILURE_STRING: string = 'Avoid explicitly calls to lifecycle hooks in class "%s"';
17+
static FAILURE_STRING: string = 'Avoid explicit calls to lifecycle hooks.';
1818

1919
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
2020
return this.applyWithWalker(new ExpressionCallMetadataWalker(sourceFile, this.getOptions()));
@@ -49,19 +49,14 @@ export class ExpressionCallMetadataWalker extends NgWalker {
4949
}
5050

5151
private validateCallExpression(node: ts.CallExpression): void {
52-
const name = (node.expression as any).name;
52+
const name = ts.isPropertyAccessExpression(node.expression) ? node.expression.name : undefined;
53+
const expression = ts.isPropertyAccessExpression(node.expression) ? node.expression.expression : undefined;
5354

54-
if (!name || !lifecycleHooksMethods.has(name.text)) {
55-
return;
56-
}
57-
58-
let currentNode = node as any;
55+
const isSuperCall = expression && expression.kind === ts.SyntaxKind.SuperKeyword;
56+
const isLifecycleCall = name && ts.isIdentifier(name) && lifecycleHooksMethods.has(name.text as any);
5957

60-
while (currentNode.parent.parent) {
61-
currentNode = currentNode.parent;
58+
if (isLifecycleCall && !isSuperCall) {
59+
this.addFailureAtNode(node, Rule.FAILURE_STRING);
6260
}
63-
64-
const failureConfig = [Rule.FAILURE_STRING, currentNode.name.text];
65-
this.addFailureAtNode(node, sprintf.apply(this, failureConfig));
6661
}
6762
}

test/noLifeCycleCallRule.spec.ts

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,39 @@ describe(ruleName, () => {
5050
}
5151
`;
5252

53-
it(`should fail when explicitly call ${lifecycleHookMethod}`, () => {
53+
it(`should fail when explicitly calling ${lifecycleHookMethodCall}`, () => {
5454
assertAnnotated({
5555
ruleName,
56-
message: `Avoid explicitly calls to lifecycle hooks in class "${className}"`,
56+
message: 'Avoid explicit calls to lifecycle hooks.',
5757
source
5858
});
5959
});
6060
});
6161
});
6262
}
63+
64+
lifecycleHooksMethods.forEach(lifecycleHookMethod => {
65+
const lifecycleHookMethodCall = `fixture.componentInstance.${lifecycleHookMethod}()`;
66+
const totalTildes = '~'.repeat(lifecycleHookMethodCall.length);
67+
const source = `
68+
it('should work', () => {
69+
// test code...
70+
71+
${lifecycleHookMethodCall}
72+
${totalTildes}
73+
74+
// more test code...
75+
})
76+
`;
77+
78+
it(`should fail when explicitly calling ${lifecycleHookMethod} outside of a class`, () => {
79+
assertAnnotated({
80+
ruleName,
81+
message: `Avoid explicit calls to lifecycle hooks.`,
82+
source
83+
});
84+
});
85+
});
6386
});
6487

6588
describe('success', () => {
@@ -77,7 +100,28 @@ describe(ruleName, () => {
77100
}
78101
`;
79102

80-
it(`should pass when call ${fakeMethod} method`, () => {
103+
it(`should pass when calling ${fakeMethod} method`, () => {
104+
assertSuccess(ruleName, source);
105+
});
106+
});
107+
});
108+
}
109+
110+
// call super lifecycle hook
111+
for (const metadataKey of metadataKeys) {
112+
describe(metadataKey, () => {
113+
metadataPairs[metadataKey].forEach(lifecycleHookMethod => {
114+
const lifecycleHookMethodCall = `super.${lifecycleHookMethod}()`;
115+
const source = `
116+
@${metadataKey}()
117+
class ${className} implements ${lifecycleHookMethod.slice(2)} {
118+
${lifecycleHookMethod}() {
119+
${lifecycleHookMethodCall}
120+
}
121+
}
122+
`;
123+
124+
it(`should pass when explicitly calling ${lifecycleHookMethodCall}`, () => {
81125
assertSuccess(ruleName, source);
82126
});
83127
});

0 commit comments

Comments
 (0)