Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rule): fix bugs in no-life-cycle-call rule #575

Merged
merged 2 commits into from Apr 25, 2018
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
19 changes: 7 additions & 12 deletions src/noLifeCycleCallRule.ts
Expand Up @@ -14,7 +14,7 @@ export class Rule extends Lint.Rules.AbstractRule {
typescriptOnly: true,
};

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

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

private validateCallExpression(node: ts.CallExpression): void {
const name = (node.expression as any).name;
const name = ts.isPropertyAccessExpression(node.expression) ? node.expression.name : undefined;
const expression = ts.isPropertyAccessExpression(node.expression) ? node.expression.expression : undefined;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: extra new line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intentionally separated these variable declarations for readability. Source node info vs. rule logic checks.

if (!name || !lifecycleHooksMethods.has(name.text)) {
return;
}

let currentNode = node as any;
const isSuperCall = expression && expression.kind === ts.SyntaxKind.SuperKeyword;
const isLifecycleCall = name && ts.isIdentifier(name) && lifecycleHooksMethods.has(name.text as any);

while (currentNode.parent.parent) {
currentNode = currentNode.parent;
if (isLifecycleCall && !isSuperCall) {
this.addFailureAtNode(node, Rule.FAILURE_STRING);
}

const failureConfig = [Rule.FAILURE_STRING, currentNode.name.text];
this.addFailureAtNode(node, sprintf.apply(this, failureConfig));
}
}
50 changes: 47 additions & 3 deletions test/noLifeCycleCallRule.spec.ts
Expand Up @@ -58,16 +58,39 @@ describe(ruleName, () => {
}
`;

it(`should fail when explicitly call ${lifecycleHookMethod}`, () => {
it(`should fail when explicitly calling ${lifecycleHookMethodCall}`, () => {
assertAnnotated({
ruleName,
message: `Avoid explicitly calls to lifecycle hooks in class "${className}"`,
message: 'Avoid explicit calls to lifecycle hooks.',
source
});
});
});
});
}

lifecycleHooksMethods.forEach(lifecycleHookMethod => {
const lifecycleHookMethodCall = `fixture.componentInstance.${lifecycleHookMethod}()`;
const totalTildes = '~'.repeat(lifecycleHookMethodCall.length);
const source = `
it('should work', () => {
// test code...

${lifecycleHookMethodCall}
${totalTildes}

// more test code...
})
`;

it(`should fail when explicitly calling ${lifecycleHookMethod} outside of a class`, () => {
assertAnnotated({
ruleName,
message: `Avoid explicit calls to lifecycle hooks.`,
source
});
});
});
});

describe('success', () => {
Expand All @@ -85,7 +108,28 @@ describe(ruleName, () => {
}
`;

it(`should pass when call ${fakeMethod} method`, () => {
it(`should pass when calling ${fakeMethod} method`, () => {
assertSuccess(ruleName, source);
});
});
});
}

// call super lifecycle hook
for (const metadataKey of metadataKeys) {
describe(metadataKey, () => {
metadataPairs[metadataKey].forEach(lifecycleHookMethod => {
const lifecycleHookMethodCall = `super.${lifecycleHookMethod}()`;
const source = `
@${metadataKey}()
class ${className} implements ${lifecycleHookMethod.slice(2)} {
${lifecycleHookMethod}() {
${lifecycleHookMethodCall}
}
}
`;

it(`should pass when explicitly calling ${lifecycleHookMethodCall}`, () => {
assertSuccess(ruleName, source);
});
});
Expand Down