refactor(rule): rename/clean up 'component-change-detection' (#782)

Author: rafaelss95

src/componentChangeDetectionRule.ts

@@ -1,5 +1,4 @@
 export { Rule as AngularWhitespaceRule } from './angularWhitespaceRule';
-export { Rule as ComponentChangeDetectionRule } from './componentChangeDetectionRule';
 export { Rule as ComponentClassSuffixRule } from './componentClassSuffixRule';
 export { Rule as ComponentSelectorRule } from './componentSelectorRule';
 export { Rule as ContextualDecoratorRule } from './contextualDecoratorRule';
@@ -25,6 +24,7 @@ export { Rule as NoQueriesMetadataPropertyRule } from './noQueriesMetadataProper
 export { Rule as NoUnusedCssRule } from './noUnusedCssRule';
 export { Rule as PipePrefixRule } from './pipePrefixRule';
 export { Rule as PreferInlineDecoratorRule } from './preferInlineDecoratorRule';
+export { Rule as PreferOnPushComponentChangeDetectionRule } from './preferOnPushComponentChangeDetectionRule';
 export { Rule as PreferOutputReadonlyRule } from './preferOutputReadonlyRule';
 export { Rule as RelativeUrlPrefixRule } from './relativeUrlPrefixRule';
 export { Rule as TemplateAccessibilityAltTextRule } from './templateAccessibilityAltTextRule';

src/index.ts

@@ -0,0 +1,54 @@
+import { IRuleMetadata, RuleFailure } from 'tslint/lib';
+import { AbstractRule } from 'tslint/lib/rules';
+import { dedent } from 'tslint/lib/utils';
+import { isPropertyAccessExpression, SourceFile } from 'typescript/lib/typescript';
+import { ComponentMetadata } from './angular/metadata';
+import { NgWalker } from './angular/ngWalker';
+import { getDecoratorPropertyInitializer } from './util/utils';
+
+const ON_PUSH = 'OnPush';
+
+export class Rule extends AbstractRule {
+  static readonly metadata: IRuleMetadata = {
+    description: `Enforces component's change detection to ChangeDetectionStrategy.${ON_PUSH}.`,
+    options: null,
+    optionsDescription: 'Not configurable.',
+    rationale: dedent`
+      By default Angular uses the ChangeDetectionStrategy.Default.
+
+      This strategy doesn’t assume anything about the application, therefore every time something changes in our application, as a result of various user events, timers, XHR, promises, etc., a change detection will run on all components.
+
+      By using ChangeDetectionStrategy.${ON_PUSH}, Angular will only run the change detection cycle in the following cases:
+      * Inputs references change.
+      * An event originated from the component or one of its children.
+      * If manually called.
+    `,
+    ruleName: 'prefer-on-push-component-change-detection',
+    type: 'functionality',
+    typescriptOnly: true
+  };
+
+  static readonly FAILURE_STRING = `The changeDetection value of a component should be set to ChangeDetectionStrategy.${ON_PUSH}`;
+
+  apply(sourceFile: SourceFile): RuleFailure[] {
+    return this.applyWithWalker(new PreferOnPushComponentChangeDetectionWalker(sourceFile, this.getOptions()));
+  }
+}
+
+class PreferOnPushComponentChangeDetectionWalker extends NgWalker {
+  protected visitNgComponent(metadata: ComponentMetadata): void {
+    this.validateComponent(metadata);
+    super.visitNgComponent(metadata);
+  }
+
+  private validateComponent(metadata: ComponentMetadata): void {
+    const { decorator: metadataDecorator } = metadata;
+    const changeDetectionExpression = getDecoratorPropertyInitializer(metadataDecorator, 'changeDetection');
+
+    if (!changeDetectionExpression) {
+      this.addFailureAtNode(metadataDecorator, Rule.FAILURE_STRING);
+    } else if (isPropertyAccessExpression(changeDetectionExpression) && changeDetectionExpression.name.text !== ON_PUSH) {
+      this.addFailureAtNode(changeDetectionExpression, Rule.FAILURE_STRING);
+    }
+  }
+}

src/preferOnPushComponentChangeDetectionRule.ts

@@ -1,21 +1,23 @@
-import { IRuleMetadata, RuleFailure, Rules } from 'tslint/lib';
+import { IRuleMetadata, RuleFailure } from 'tslint/lib';
 import { AbstractRule } from 'tslint/lib/rules';
 import { isPropertyAccessExpression, SourceFile } from 'typescript/lib/typescript';
 import { ComponentMetadata } from './angular/metadata';
 import { NgWalker } from './angular/ngWalker';
 import { getDecoratorPropertyInitializer } from './util/utils';
 
+const NONE = 'None';
+
 export class Rule extends AbstractRule {
   static readonly metadata: IRuleMetadata = {
-    description: 'Disallows using of ViewEncapsulation.None.',
+    description: `Disallows using ViewEncapsulation.${NONE}.`,
     options: null,
     optionsDescription: 'Not configurable.',
     ruleName: 'use-component-view-encapsulation',
     type: 'maintainability',
     typescriptOnly: true
   };
 
-  static readonly FAILURE_STRING = 'Using "ViewEncapsulation.None" makes your styles global, which may have an unintended effect';
+  static readonly FAILURE_STRING = `Using ViewEncapsulation.${NONE} makes your styles global, which may have an unintended effect`;
 
   apply(sourceFile: SourceFile): RuleFailure[] {
     return this.applyWithWalker(new ViewEncapsulationWalker(sourceFile, this.getOptions()));
@@ -29,12 +31,12 @@ class ViewEncapsulationWalker extends NgWalker {
   }
 
   private validateComponent(metadata: ComponentMetadata): void {
-    const encapsulation = getDecoratorPropertyInitializer(metadata.decorator, 'encapsulation');
+    const encapsulationExpression = getDecoratorPropertyInitializer(metadata.decorator, 'encapsulation');
 
-    if (!encapsulation || (isPropertyAccessExpression(encapsulation) && encapsulation.name.text !== 'None')) {
+    if (!encapsulationExpression || (isPropertyAccessExpression(encapsulationExpression) && encapsulationExpression.name.text !== NONE)) {
       return;
     }
 
-    this.addFailureAtNode(encapsulation, Rule.FAILURE_STRING);
+    this.addFailureAtNode(encapsulationExpression, Rule.FAILURE_STRING);
   }
 }

src/useComponentViewEncapsulationRule.ts

@@ -105,13 +105,16 @@ export const getDecoratorArgument = (decorator: Decorator): ObjectLiteralExpress
 };
 
 export const getDecoratorPropertyInitializer = (decorator: Decorator, name: string): Expression | undefined => {
-  const args = isCallExpression(decorator.expression) ? decorator.expression.arguments[0] : undefined;
-  const properties = createNodeArray(args && isObjectLiteralExpression(args) ? args.properties : undefined);
+  const args = getDecoratorArgument(decorator);
 
-  return properties
-    .filter(prop => prop.name && isIdentifier(prop.name) && prop.name.text === name)
-    .map(prop => (isPropertyAssignment(prop) ? prop.initializer : undefined))
-    .pop();
+  if (!args || !isObjectLiteralExpression(args)) return undefined;
+
+  const properties = createNodeArray(args.properties);
+  const property = properties.find(prop => !!(prop.name && prop.name.getText() === name));
+
+  if (!property || !isPropertyAssignment(property)) return undefined;
+
+  return property.initializer;
 };
 
 export const getDecoratorName = (decorator: Decorator): string | undefined =>

src/util/utils.ts

@@ -0,0 +1,54 @@
+import { Rule } from '../src/preferOnPushComponentChangeDetectionRule';
+import { assertAnnotated, assertSuccess } from './testHelper';
+
+const {
+  FAILURE_STRING,
+  metadata: { ruleName }
+} = Rule;
+
+describe(ruleName, () => {
+  describe('failure', () => {
+    it('should fail if ChangeDetectionStrategy.Default is set', () => {
+      const source = `
+        @Component({
+          changeDetection: ChangeDetectionStrategy.Default
+                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+        })
+        class Test {}
+      `;
+      assertAnnotated({
+        message: FAILURE_STRING,
+        ruleName,
+        source
+      });
+    });
+
+    it('should fail if no ChangeDetectionStrategy is explicitly set', () => {
+      const source = `
+        @Component({
+        ~~~~~~~~~~~~
+          selector: 'foo'
+        })
+        ~~
+        class Test {}
+      `;
+      assertAnnotated({
+        message: FAILURE_STRING,
+        ruleName,
+        source
+      });
+    });
+  });
+
+  describe('success', () => {
+    it('should succeed if ChangeDetectionStrategy.OnPush is set', () => {
+      const source = `
+        @Component({
+          changeDetection: ChangeDetectionStrategy.OnPush
+        })
+        class Test {}
+      `;
+      assertSuccess(ruleName, source);
+    });
+  });
+});