From c3847e5eb2538f2bd45979c737094098d0467069 Mon Sep 17 00:00:00 2001 From: WilliamKoza Date: Sun, 17 Mar 2019 18:20:00 +0100 Subject: [PATCH] fix(rule): handle NgModule with both contextual-lifecycle and contextual-decorators rules --- src/contextualLifecycleRule.ts | 7 +- src/util/utils.ts | 9 +- test/contextualDecoratorRule.spec.ts | 157 ++++++++++++++++++++++++++ test/contextualLifecycleRule.spec.ts | 162 +++++++++++++++++++++++++++ 4 files changed, 331 insertions(+), 4 deletions(-) diff --git a/src/contextualLifecycleRule.ts b/src/contextualLifecycleRule.ts index 15bb4e55b..a845b96ef 100644 --- a/src/contextualLifecycleRule.ts +++ b/src/contextualLifecycleRule.ts @@ -14,7 +14,7 @@ import { MetadataTypeKeys, MetadataTypes } from './util/utils'; -import { InjectableMetadata, PipeMetadata } from './angular'; +import { InjectableMetadata, ModuleMetadata, PipeMetadata } from './angular'; interface FailureParameters { readonly className: string; @@ -56,6 +56,11 @@ class ContextualLifecycleWalker extends NgWalker { super.visitNgPipe(metadata); } + protected visitNgModule(metadata: ModuleMetadata) { + this.validateDecorator(metadata, METADATA_TYPE_LIFECYCLE_MAPPER.NgModule); + super.visitNgModule(metadata); + } + private validateDecorator(metadata: PipeMetadata, allowedMethods: ReadonlySet): void { const className = getClassName(metadata.controller)!; diff --git a/src/util/utils.ts b/src/util/utils.ts index 655001783..3387927c8 100644 --- a/src/util/utils.ts +++ b/src/util/utils.ts @@ -61,7 +61,8 @@ export enum MetadataTypes { Component = 'Component', Directive = 'Directive', Injectable = 'Injectable', - Pipe = 'Pipe' + Pipe = 'Pipe', + NgModule = 'NgModule' } export type DecoratorKeys = keyof typeof Decorators; @@ -84,13 +85,15 @@ export const METADATA_TYPE_DECORATOR_MAPPER: MetadataTypeDecoratorMapper = { Component: DECORATORS, Directive: DECORATORS, Injectable: new Set([]), - Pipe: new Set([]) + Pipe: new Set([]), + NgModule: new Set([]) }; export const METADATA_TYPE_LIFECYCLE_MAPPER: MetadataTypeLifecycleMapper = { Component: LIFECYCLE_METHODS, Directive: LIFECYCLE_METHODS, Injectable: new Set([LifecycleMethods.ngOnDestroy]), - Pipe: new Set([LifecycleMethods.ngOnDestroy]) + Pipe: new Set([LifecycleMethods.ngOnDestroy]), + NgModule: new Set([]) }; export const getClassName = (node: Node): string | undefined => { diff --git a/test/contextualDecoratorRule.spec.ts b/test/contextualDecoratorRule.spec.ts index e9e924ddd..6d23c3175 100644 --- a/test/contextualDecoratorRule.spec.ts +++ b/test/contextualDecoratorRule.spec.ts @@ -165,6 +165,163 @@ describe(ruleName, () => { }); }); + describe('NgModule', () => { + it('should fail if a property is decorated with @ContentChild() decorator', () => { + const source = ` + @NgModule() + class Test { + @ContentChild(Pane) pane: Pane; + ~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ContentChild, + metadataType: MetadataTypes.NgModule + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @ContentChildren() decorator', () => { + const source = ` + @NgModule() + class Test { + @ContentChildren(Pane, { descendants: true }) arbitraryNestedPanes: QueryList; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ContentChildren, + metadataType: MetadataTypes.NgModule + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @HostBinding() decorator', () => { + const source = ` + @NgModule() + class Test { + @HostBinding('class.card-outline') private isCardOutline: boolean; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.HostBinding, + metadataType: MetadataTypes.NgModule + }), + ruleName, + source + }); + }); + + it('should fail if a method is decorated with @HostListener() decorator', () => { + const source = ` + @NgModule() + class Test { + @HostListener('mouseover') + ~~~~~~~~~~~~~~~~~~~~~~~~~~ + mouseOver() { + console.log('mouseOver'); + } + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.HostListener, + metadataType: MetadataTypes.NgModule + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @Input() decorator', () => { + const source = ` + @NgModule() + class Test { + @Input() label: string; + ~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.Input, + metadataType: MetadataTypes.NgModule + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @Output() decorator', () => { + const source = ` + @NgModule() + class Test { + @Output() emitter = new EventEmitter(); + ~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.Output, + metadataType: MetadataTypes.NgModule + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @ViewChild() decorator', () => { + const source = ` + @NgModule() + class Test { + @ViewChild(Pane) pane: Pane; + ~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ViewChild, + metadataType: MetadataTypes.NgModule + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @ViewChildren() decorator', () => { + const source = ` + @NgModule() + class Test { + @ViewChildren(Pane) panes: QueryList; + ~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ViewChildren, + metadataType: MetadataTypes.NgModule + }), + ruleName, + source + }); + }); + }); + describe('Pipe', () => { it('should fail if a property is decorated with @ContentChild() decorator', () => { const source = ` diff --git a/test/contextualLifecycleRule.spec.ts b/test/contextualLifecycleRule.spec.ts index f21ea5dc2..bc40c305d 100644 --- a/test/contextualLifecycleRule.spec.ts +++ b/test/contextualLifecycleRule.spec.ts @@ -150,6 +150,168 @@ describe(ruleName, () => { }); }); + describe('NgModule', () => { + it('should fail if ngAfterContentChecked() method is present', () => { + const source = ` + @NgModule() + class Test { + ngAfterContentChecked() { console.log('AfterContentChecked'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.NgModule, + methodName: LifecycleMethods.ngAfterContentChecked + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngAfterContentInit() method is present', () => { + const source = ` + @NgModule() + class Test { + ngAfterContentInit() { console.log('AfterContentInit'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.NgModule, + methodName: LifecycleMethods.ngAfterContentInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngAfterViewChecked() method is present', () => { + const source = ` + @NgModule() + class Test { + ngAfterViewChecked() { console.log('AfterViewChecked'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.NgModule, + methodName: LifecycleMethods.ngAfterViewChecked + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngAfterViewInit() method is present', () => { + const source = ` + @NgModule() + class Test { + ngAfterViewInit() { console.log('AfterViewInit'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.NgModule, + methodName: LifecycleMethods.ngAfterViewInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngDoCheck() method is present', () => { + const source = ` + @NgModule() + class Test { + ngDoCheck() { console.log('DoCheck'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.NgModule, + methodName: LifecycleMethods.ngDoCheck + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngOnChanges() method is present', () => { + const source = ` + @NgModule() + class Test { + ngOnChanges() { console.log('OnChanges'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.NgModule, + methodName: LifecycleMethods.ngOnChanges + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngOnInit() method is present', () => { + const source = ` + @NgModule() + class Test { + ngOnInit() { console.log('OnInit'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.NgModule, + methodName: LifecycleMethods.ngOnInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngOnDestroy() method is present', () => { + const source = ` + @NgModule() + class Test { + ngOnDestroy() { console.log('OnDestroy'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.NgModule, + methodName: LifecycleMethods.ngOnDestroy + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + }); + describe('Pipe', () => { it('should fail if ngAfterContentChecked() method is present', () => { const source = `