From a23ccde36de2b76ea7aa218458fd17a03d4f4a92 Mon Sep 17 00:00:00 2001 From: Matt Lewis Date: Wed, 6 Feb 2019 04:44:55 +0700 Subject: [PATCH] feat(component-change-detection): add change detection strategy rule (#737) Closes #135 --- src/componentChangeDetectionRule.ts | 79 +++++++++++++++++++++++ src/index.ts | 1 + test/componentChangeDetectionRule.spec.ts | 47 ++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 src/componentChangeDetectionRule.ts create mode 100644 test/componentChangeDetectionRule.spec.ts diff --git a/src/componentChangeDetectionRule.ts b/src/componentChangeDetectionRule.ts new file mode 100644 index 000000000..ad5316cf5 --- /dev/null +++ b/src/componentChangeDetectionRule.ts @@ -0,0 +1,79 @@ +import { Utils } from 'tslint/lib'; +import * as Lint from 'tslint'; +import * as ts from 'typescript'; +import { getDecoratorArgument, getDecoratorName } from './util/utils'; +import { sprintf } from 'sprintf-js'; +import { NgWalker } from './angular'; + +export class Rule extends Lint.Rules.AbstractRule { + public static metadata: Lint.IRuleMetadata = { + ruleName: 'component-change-detection', + type: 'functionality', + description: 'Enforce the preferred component change detection type as ChangeDetectionStrategy.OnPush.', + descriptionDetails: Utils.dedent` + See more at https://angular.io/api/core/ChangeDetectionStrategy + `, + options: null, + optionsDescription: 'Not configurable.', + rationale: Utils.dedent` + By using OnPush for change detection, Angular will only run a change detection cycle when that + components inputs or outputs change + `, + typescriptOnly: true + }; + + static CHANGE_DETECTION_INVALID_FAILURE = + 'The changeDetection value of the component "%s" should be set to ChangeDetectionStrategy.OnPush'; + + apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithWalker(new ComponentChangeDetectionValidatorWalker(sourceFile, this)); + } +} + +export class ComponentChangeDetectionValidatorWalker extends NgWalker { + constructor(sourceFile: ts.SourceFile, private rule: Rule) { + super(sourceFile, rule.getOptions()); + } + + visitClassDeclaration(node: ts.ClassDeclaration) { + ts.createNodeArray(node.decorators).forEach(this.validateDecorator.bind(this, node.name!.text)); + super.visitClassDeclaration(node); + } + + private validateDecorator(className: string, decorator: ts.Decorator) { + const argument = getDecoratorArgument(decorator)!; + const name = getDecoratorName(decorator); + + // Run only for Components + if (name === 'Component') { + this.validateComponentChangeDetection(className, decorator, argument); + } + } + + private validateComponentChangeDetection(className: string, decorator: ts.Decorator, arg: ts.Node) { + if (!ts.isObjectLiteralExpression(arg)) { + return; + } + + const changeDetectionAssignment = arg.properties + .filter(prop => ts.isPropertyAssignment(prop) && this.validateProperty(prop)) + .map(prop => (ts.isPropertyAssignment(prop) ? prop.initializer : undefined)) + .filter(Boolean)[0] as ts.PropertyAccessExpression; + + if (!changeDetectionAssignment) { + this.addFailureAtNode(decorator, sprintf(Rule.CHANGE_DETECTION_INVALID_FAILURE, className)); + } else { + if (!this.validateChangeDetectionType(changeDetectionAssignment!.name.escapedText as string)) { + this.addFailureAtNode(changeDetectionAssignment, sprintf(Rule.CHANGE_DETECTION_INVALID_FAILURE, className)); + } + } + } + + private validateProperty(p: ts.PropertyAssignment): boolean { + return ts.isIdentifier(p.name) && p.name.text === 'changeDetection'; + } + + private validateChangeDetectionType(changeDetectionValue: string): boolean { + return changeDetectionValue === 'OnPush'; + } +} diff --git a/src/index.ts b/src/index.ts index d9dd0f6cd..c3e49e849 100644 --- a/src/index.ts +++ b/src/index.ts @@ -39,6 +39,7 @@ export { Rule as UsePipeDecoratorRule } from './usePipeDecoratorRule'; export { Rule as UsePipeTransformInterfaceRule } from './usePipeTransformInterfaceRule'; export { Rule as UseViewEncapsulationRule } from './useViewEncapsulationRule'; export { Rule as RelativePathExternalResourcesRule } from './relativeUrlPrefixRule'; +export { Rule as ComponentChangeDetectionRule } from './componentChangeDetectionRule'; export * from './angular'; diff --git a/test/componentChangeDetectionRule.spec.ts b/test/componentChangeDetectionRule.spec.ts new file mode 100644 index 000000000..862592453 --- /dev/null +++ b/test/componentChangeDetectionRule.spec.ts @@ -0,0 +1,47 @@ +import { assertSuccess, assertAnnotated } from './testHelper'; + +describe('component-change-detection', () => { + describe('invalid component change detection', () => { + it('should fail when component used without preferred change detection type', () => { + let source = ` + @Component({ + changeDetection: ChangeDetectionStrategy.Default + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `; + assertAnnotated({ + ruleName: 'component-change-detection', + message: 'The changeDetection value of the component "Test" should be set to ChangeDetectionStrategy.OnPush', + source + }); + }); + + it('should fail when component change detection is not set', () => { + let source = ` + @Component({ + ~~~~~~~~~~~~ + selector: 'foo' + }) + ~~ + class Test {}`; + assertAnnotated({ + ruleName: 'component-change-detection', + message: 'The changeDetection value of the component "Test" should be set to ChangeDetectionStrategy.OnPush', + source + }); + }); + }); + + describe('valid component selector', () => { + it('should succeed when a valid change detection strategy is set on @Component', () => { + let source = ` + @Component({ + changeDetection: ChangeDetectionStrategy.OnPush + }) + class Test {} + `; + assertSuccess('component-change-detection', source); + }); + }); +});