feat(rule): add no-conflicting-life-cycle-hooks rule (#563)

Author: rafaelss95

src/index.ts

@@ -11,6 +11,7 @@ export { Rule as I18nRule } from './i18nRule';
 export { Rule as ImportDestructuringSpacingRule } from './importDestructuringSpacingRule';
 export { Rule as MaxInlineDeclarationsRule } from './maxInlineDeclarationsRule';
 export { Rule as NoAttributeParameterDecoratorRule } from './noAttributeParameterDecoratorRule';
+export { Rule as NoConflictingLifeCycleHooksRule } from './noConflictingLifeCycleHooksRule';
 export { Rule as NoForwardRefRule } from './noForwardRefRule';
 export { Rule as NoInputPrefixRule } from './noInputPrefixRule';
 export { Rule as NoInputRenameRule } from './noInputRenameRule';

src/noConflictingLifeCycleHooksRule.ts

@@ -0,0 +1,72 @@
+import { sprintf } from 'sprintf-js';
+import * as Lint from 'tslint';
+import * as ts from 'typescript';
+
+export class Rule extends Lint.Rules.AbstractRule {
+  public static metadata: Lint.IRuleMetadata = {
+    ruleName: 'no-conflicting-life-cycle-hooks',
+    type: 'maintainability',
+    description: 'Ensure that directives not implement conflicting lifecycle hooks.',
+    descriptionDetails: 'See more at https://angular.io/api/core/DoCheck#description.',
+    rationale: 'A directive typically should not use both DoCheck and OnChanges to respond ' +
+      'to changes on the same input, as ngOnChanges will continue to be called when the ' +
+      'default change detector detects changes.',
+    options: null,
+    optionsDescription: 'Not configurable.',
+    typescriptOnly: true,
+  };
+
+  static FAILURE_STRING = 'Implement DoCheck and OnChanges hooks in class %s is not recommended';
+
+  public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+    return this.applyWithWalker(new ClassMetadataWalker(sourceFile, this.getOptions()));
+  }
+}
+
+const hooksPrefix = 'ng';
+const lifecycleHooksMethods: string[] = [
+  'DoCheck',
+  'OnChanges'
+];
+
+export class ClassMetadataWalker extends Lint.RuleWalker {
+  visitClassDeclaration(node: ts.ClassDeclaration) {
+    this.validateInterfaces(node);
+    this.validateMethods(node);
+    super.visitClassDeclaration(node);
+  }
+
+  private validateInterfaces(node: ts.ClassDeclaration): void {
+    if (!node.heritageClauses) {
+      return;
+    }
+
+    const interfacesClause = node.heritageClauses.find(h => h.token === ts.SyntaxKind.ImplementsKeyword);
+
+    if (!interfacesClause) {
+      return;
+    }
+
+    const interfaces = interfacesClause.types.map<string>((t: any) => {
+      return t.expression.name ? t.expression.name.text : t.expression.text;
+    });
+    const matchesAllHooks = lifecycleHooksMethods.every(l => interfaces.indexOf(l) !== -1);
+
+    if (matchesAllHooks) {
+      this.addFailureAtNode(node, sprintf.apply(this, [Rule.FAILURE_STRING, node.name.text]));
+    }
+  }
+
+  private validateMethods(node: ts.ClassDeclaration): void {
+    const methodNames = node.members
+        .filter(m => m.kind === ts.SyntaxKind.MethodDeclaration)
+        .map<string>(m => (m.name as any).text);
+    const matchesAllHooks = lifecycleHooksMethods.every(l => {
+      return methodNames.indexOf(`${hooksPrefix}${l}`) !== -1;
+    });
+
+    if (matchesAllHooks) {
+      this.addFailureAtNode(node, sprintf.apply(this, [Rule.FAILURE_STRING, node.name.text]));
+    }
+  }
+}

test/noConflictingLifeCycleHooksRule.spec.ts

@@ -0,0 +1,102 @@
+import { assertFailures, assertSuccess, IExpectedFailure } from './testHelper';
+
+const ruleName = 'no-conflicting-life-cycle-hooks';
+const fails: IExpectedFailure[] = [
+  {
+    endPosition: {
+      line: 4,
+      character: 9
+    },
+    message: 'Implement DoCheck and OnChanges hooks in class Test is not recommended',
+    startPosition: {
+      line: 1,
+      character: 8
+    }
+  }
+];
+
+describe(ruleName, () => {
+  describe('failure', () => {
+    it('should fail when implements both DoCheck and OnChanges hooks', () => {
+      const source = `
+        class Test implements DoCheck, OnChanges {
+          test() {}
+          test1() {}
+        }
+      `;
+      assertFailures(ruleName, source, fails);
+    });
+
+    it('should fail when implements both DoCheck and OnChanges hooks/methods', () => {
+      const source = `
+        class Test implements DoCheck, OnChanges {
+          ngDoCheck() {}
+          ngOnChanges() {}
+        }
+      `;
+      assertFailures(ruleName, source, fails.concat(fails));
+    });
+
+    it('should fail have both ngDoCheck and ngOnChanges methods exist', () => {
+      const source = `
+        class Test {
+          ngDoCheck() {}
+          ngOnChanges() {}
+        }
+      `;
+      assertFailures(ruleName, source, fails);
+    });
+  });
+
+  describe('success', () => {
+    it('should pass when contain ngDoCheck, but not ngOnChanges method', () => {
+      const source = `
+        class Test {
+          ngDoCheck() {}
+        }
+      `;
+      assertSuccess(ruleName, source);
+    });
+
+    it('should pass when implements DoCheck, but not OnChanges hook', () => {
+      const source = `
+        class Test implements DoCheck {}
+      `;
+      assertSuccess(ruleName, source);
+    });
+
+    it('should pass when implementing and contain DoCheck hook/method, but not OnChanges hook/method', () => {
+      const source = `
+        class Test implements DoCheck {
+          ngDoCheck() {}
+        }
+      `;
+      assertSuccess(ruleName, source);
+    });
+
+    it('should pass when contain ngOnChanges, but not ngDoCheck method', () => {
+      const source = `
+        class Test {
+          ngOnChanges() {}
+        }
+      `;
+      assertSuccess(ruleName, source);
+    });
+
+    it('should pass when implements OnChanges, but not DoCheck hook', () => {
+      const source = `
+        class Test implements OnChanges {}
+      `;
+      assertSuccess(ruleName, source);
+    });
+
+    it('should pass when implementing and contain OnChanges hook/method, but not DoCheck hook/method', () => {
+      const source = `
+        class Test implements OnChanges {
+          ngOnChanges() {}
+        }
+      `;
+      assertSuccess(ruleName, source);
+    });
+  });
+});