From 3e1001385a1b140633f35b2a1234032b8b9c41a2 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Wed, 18 Apr 2018 21:02:18 -0300 Subject: [PATCH] feat(rule): add no-life-cycle-call rule (#559) --- src/index.ts | 13 +++-- src/noLifeCycleCallRule.ts | 67 ++++++++++++++++++++++ test/noLifeCycleCallRule.spec.ts | 95 ++++++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 6 deletions(-) create mode 100644 src/noLifeCycleCallRule.ts create mode 100644 test/noLifeCycleCallRule.spec.ts diff --git a/src/index.ts b/src/index.ts index ecf4c0d38..c07434c6a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,29 +7,30 @@ export { Rule as DecoratorNotAllowedRule } from './decoratorNotAllowedRule'; export { Rule as DirectiveClassSuffixRule } from './directiveClassSuffixRule'; export { Rule as DirectiveSelectorRule } from './directiveSelectorRule'; export { Rule as I18nRule } from './i18nRule'; -export { Rule as TemplateCyclomaticComplexityRule } from './templateCyclomaticComplexityRule'; -export { Rule as TemplateConditionalComplexityRule } from './templateConditionalComplexityRule'; export { Rule as ImportDestructuringSpacingRule } from './importDestructuringSpacingRule'; export { Rule as MaxInlineDeclarationsRule } from './maxInlineDeclarationsRule'; export { Rule as NoAttributeParameterDecoratorRule } from './noAttributeParameterDecoratorRule'; export { Rule as NoForwardRefRule } from './noForwardRefRule'; +export { Rule as NoInputPrefixRule } from './noInputPrefixRule'; export { Rule as NoInputRenameRule } from './noInputRenameRule'; +export { Rule as NoLifeCycleCallRule } from './noLifeCycleCallRule'; export { Rule as NoOutputNamedAfterStandardEventRule } from './noOutputNamedAfterStandardEventRule'; export { Rule as NoOutputOnPrefixRule } from './noOutputOnPrefixRule'; -export { Rule as NoInputPrefixRule } from './noInputPrefixRule'; export { Rule as NoOutputRenameRule } from './noOutputRenameRule'; export { Rule as NoUnusedCssRule } from './noUnusedCssRule'; export { Rule as PipeImpureRule } from './pipeImpureRule'; export { Rule as PipeNamingRule } from './pipeNamingRule'; +export { Rule as TemplateConditionalComplexityRule } from './templateConditionalComplexityRule'; +export { Rule as TemplateCyclomaticComplexityRule } from './templateCyclomaticComplexityRule'; +export { Rule as TemplatesNoNegatedAsync } from './templatesNoNegatedAsyncRule'; +export { Rule as TrackByFunctionRule } from './trackByFunctionRule'; export { Rule as UseHostPropertyDecoratorRule } from './useHostPropertyDecoratorRule'; export { Rule as UseInputPropertyDecoratorRule } from './useInputPropertyDecoratorRule'; export { Rule as UseLifeCycleInterfaceRule } from './useLifeCycleInterfaceRule'; export { Rule as UseOutputPropertyDecoratorRule } from './useOutputPropertyDecoratorRule'; -export { Rule as UsePipeTransformInterfaceRule } from './usePipeTransformInterfaceRule'; export { Rule as UsePipeDecoratorRule } from './usePipeDecoratorRule'; +export { Rule as UsePipeTransformInterfaceRule } from './usePipeTransformInterfaceRule'; export { Rule as UseViewEncapsulationRule } from './useViewEncapsulationRule'; -export { Rule as TemplatesNoNegatedAsync } from './templatesNoNegatedAsyncRule'; -export { Rule as TrackByFunctionRule } from './trackByFunctionRule'; export * from './angular/config'; // this file exists for tslint to resolve the rules directory diff --git a/src/noLifeCycleCallRule.ts b/src/noLifeCycleCallRule.ts new file mode 100644 index 000000000..a9ef60bc0 --- /dev/null +++ b/src/noLifeCycleCallRule.ts @@ -0,0 +1,67 @@ +import { sprintf } from 'sprintf-js'; +import * as Lint from 'tslint'; +import * as ts from 'typescript'; +import { NgWalker } from './angular/ngWalker'; + +export class Rule extends Lint.Rules.AbstractRule { + public static metadata: Lint.IRuleMetadata = { + ruleName: 'no-life-cycle-call', + type: 'maintainability', + description: 'Disallows explicit calls to lifecycle hooks.', + rationale: 'Explicit calls to lifecycle hooks could be confusing. Invoke lifecycle hooks is the responsability of Angular.', + options: null, + optionsDescription: 'Not configurable.', + typescriptOnly: true, + }; + + static FAILURE_STRING: string = 'Avoid explicitly calls to lifecycle hooks in class "%s"'; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithWalker(new ExpressionCallMetadataWalker(sourceFile, this.getOptions())); + } +} + +export type LifecycleHooksMethods = + 'ngAfterContentChecked' | + 'ngAfterContentInit' | + 'ngAfterViewChecked' | + 'ngAfterViewInit' | + 'ngDoCheck' | + 'ngOnChanges' | + 'ngOnDestroy' | + 'ngOnInit'; + +export const lifecycleHooksMethods = new Set([ + 'ngAfterContentChecked', + 'ngAfterContentInit', + 'ngAfterViewChecked', + 'ngAfterViewInit', + 'ngDoCheck', + 'ngOnChanges', + 'ngOnDestroy', + 'ngOnInit' +]); + +export class ExpressionCallMetadataWalker extends NgWalker { + visitCallExpression(node: ts.CallExpression): void { + this.validateCallExpression(node); + super.visitCallExpression(node); + } + + private validateCallExpression(node: ts.CallExpression): void { + const name = (node.expression as any).name; + + if (!name || !lifecycleHooksMethods.has(name.text)) { + return; + } + + let currentNode = node as any; + + while (currentNode.parent.parent) { + currentNode = currentNode.parent; + } + + const failureConfig = [Rule.FAILURE_STRING, currentNode.name.text]; + this.addFailureAtNode(node, sprintf.apply(this, failureConfig)); + } +} diff --git a/test/noLifeCycleCallRule.spec.ts b/test/noLifeCycleCallRule.spec.ts new file mode 100644 index 000000000..fc3182125 --- /dev/null +++ b/test/noLifeCycleCallRule.spec.ts @@ -0,0 +1,95 @@ +import { lifecycleHooksMethods, LifecycleHooksMethods } from '../src/noLifeCycleCallRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +type Metadata = 'Component' | 'Directive' | 'Injectable' | 'Pipe'; + +type MetadataPair = { + [key in Metadata]: typeof lifecycleHooksMethods +}; + +type MetadataFakePair = { + [key in Metadata]?: Set +}; + +const className = 'Test'; +const ruleName = 'no-life-cycle-call'; +const metadataPairs: MetadataPair = { + Component: lifecycleHooksMethods, + Directive: lifecycleHooksMethods, + Injectable: new Set([ + 'ngOnDestroy' + ]), + Pipe: new Set([ + 'ngOnDestroy' + ]) +}; + +const metadataKeys = Object.keys(metadataPairs); +const prefix = 'prefix'; +const suffix = 'Suffix'; +const metadataFakePairs: MetadataFakePair = {}; +for (const metadataKey of metadataKeys) { + metadataFakePairs[metadataKey] = new Set(); + + metadataPairs[metadataKey].forEach(lifecycleHookMethod => { + metadataFakePairs[metadataKey] + .add(`${prefix}${lifecycleHookMethod}`) + .add(`${lifecycleHookMethod}${suffix}`) + .add(`${prefix}${lifecycleHookMethod}${suffix}`); + }); +} + +describe(ruleName, () => { + describe('failure', () => { + for (const metadataKey of metadataKeys) { + describe(metadataKey, () => { + metadataPairs[metadataKey].forEach(lifecycleHookMethod => { + const lifecycleHookMethodCall = `this.${lifecycleHookMethod}()`; + const totalTildes = '~'.repeat(lifecycleHookMethodCall.length); + const source = ` + @${metadataKey}() + class ${className} implements ${lifecycleHookMethod.slice(2)} { + ${lifecycleHookMethod}() { } + + ${className.toLowerCase()}() { + ${lifecycleHookMethodCall} + ${totalTildes} + } + } + `; + + it(`should fail when explicitly call ${lifecycleHookMethod}`, () => { + assertAnnotated({ + ruleName, + message: `Avoid explicitly calls to lifecycle hooks in class "${className}"`, + source + }); + }); + }); + }); + } + }); + + describe('success', () => { + for (const metadataKey of metadataKeys) { + describe(metadataKey, () => { + metadataFakePairs[metadataKey].forEach(fakeMethod => { + const source = ` + @${metadataKey}() + class ${className} { + ${fakeMethod}() { } + + ${className.toLowerCase()}() { + this.${fakeMethod}(); + } + } + `; + + it(`should pass when call ${fakeMethod} method`, () => { + assertSuccess(ruleName, source); + }); + }); + }); + } + }); +});