From 28ecbdda3adfe8022cea330cd643c4e741270afa Mon Sep 17 00:00:00 2001 From: mgechev Date: Fri, 8 Sep 2017 08:57:41 +0200 Subject: [PATCH 1/2] fix: support validation directives and extra suffixes - Allow `Validator` suffix if any of the implemented interfaces by a directive ends with `Validator`. - Allow multiple suffixes for a directive. Fix #397 --- src/directiveClassSuffixRule.ts | 27 ++++- test/directiveClassSuffix.spec.ts | 158 ++++++++++++++++++------------ 2 files changed, 118 insertions(+), 67 deletions(-) diff --git a/src/directiveClassSuffixRule.ts b/src/directiveClassSuffixRule.ts index 25f92c466..f23deeda0 100644 --- a/src/directiveClassSuffixRule.ts +++ b/src/directiveClassSuffixRule.ts @@ -5,6 +5,15 @@ import { NgWalker } from './angular/ngWalker'; import { DirectiveMetadata } from './angular/metadata'; +const getInterfaceName = (t: any): string => { + if (t.expression && t.expression.name) { + return t.expression.name.text; + } + return t.expression.text; +}; + +const ValidatorSuffix = 'Validator'; + export class Rule extends Lint.Rules.AbstractRule { public static metadata: Lint.IRuleMetadata = { @@ -29,8 +38,8 @@ export class Rule extends Lint.Rules.AbstractRule { static FAILURE: string = 'The name of the class %s should end with the suffix %s (https://angular.io/styleguide#style-02-03)'; - static validate(className: string, suffix: string): boolean { - return className.endsWith(suffix); + static validate(className: string, suffixes: string[]): boolean { + return suffixes.some(s => className.endsWith(s)); } public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { @@ -44,13 +53,21 @@ export class ClassMetadataWalker extends NgWalker { visitNgDirective(meta: DirectiveMetadata) { let name = meta.controller.name; let className: string = name.text; - const suffix = this.getOptions()[0] || 'Directive'; - if (!Rule.validate(className, suffix)) { + const options = this.getOptions(); + const suffixes: string[] = options.length ? options : ['Directive']; + const heritageClauses = meta.controller.heritageClauses; + if (heritageClauses) { + const i = heritageClauses.filter(h => h.token === ts.SyntaxKind.ImplementsKeyword); + if (i.length !== 0 && i[0].types.map(getInterfaceName).some(name => name.endsWith(ValidatorSuffix))) { + suffixes.push(ValidatorSuffix); + } + } + if (!Rule.validate(className, suffixes)) { this.addFailure( this.createFailure( name.getStart(), name.getWidth(), - sprintf.apply(this, [Rule.FAILURE, className, suffix]))); + sprintf.apply(this, [Rule.FAILURE, className, suffixes.join(', ')]))); } } } diff --git a/test/directiveClassSuffix.spec.ts b/test/directiveClassSuffix.spec.ts index da953e7e8..740f0196a 100644 --- a/test/directiveClassSuffix.spec.ts +++ b/test/directiveClassSuffix.spec.ts @@ -1,122 +1,156 @@ import { assertAnnotated, assertSuccess } from './testHelper'; describe('directive-class-suffix', () => { - describe('invalid directive class suffix', () => { - it('should fail when directive class is with the wrong suffix', () => { - let source = ` + describe('invalid directive class suffix', () => { + it('should fail when directive class is with the wrong suffix', () => { + let source = ` @Directive({ selector: 'sgBarFoo' }) class Test {} ~~~~ `; - assertAnnotated({ - ruleName: 'directive-class-suffix', - message: 'The name of the class Test should end with the suffix Directive (https://angular.io/styleguide#style-02-03)', - source - }); - }); + assertAnnotated({ + ruleName: 'directive-class-suffix', + message: 'The name of the class Test should end with the suffix Directive (https://angular.io/styleguide#style-02-03)', + source + }); }); - describe('valid directive class name', () => { - it('should succeed when the directive class name ends with Directive', () => { - let source = ` + it('should fail when directive class is with the wrong suffix', () => { + let source = ` + @Directive({ + selector: 'sgBarFoo' + }) + class Test {} + ~~~~ + `; + assertAnnotated({ + ruleName: 'directive-class-suffix', + message: 'The name of the class Test should end with the suffix Directive, Page, Validator (https://angular.io/styleguide#style-02-03)', + source, + options: ['Directive', 'Page', 'Validator'] + }); + }); + }); + + describe('valid directive class name', () => { + it('should succeed when the directive class name ends with Directive', () => { + let source = ` @Directive({ selector: 'sgBarFoo' }) class TestDirective {}`; - assertSuccess('directive-class-suffix', source); - }); + assertSuccess('directive-class-suffix', source); + }); + + it('should succeed when the directive class name ends with Validator and implements Validator', () => { + const source = ` + @Directive({ + selector: 'sgBarFoo' + }) + class TestValidator implements Validator {}`; + assertSuccess('directive-class-suffix', source); }); - describe('not called decorator', () => { - it('should not fail when @Directive is not called', () => { - let source = ` + it('should succeed when the directive class name ends with Validator and implements AsyncValidator', () => { + const source = ` + @Directive({ + selector: 'sgBarFoo' + }) + class TestValidator implements AsyncValidator {}`; + assertSuccess('directive-class-suffix', source); + }); + }); + + describe('not called decorator', () => { + it('should not fail when @Directive is not called', () => { + let source = ` @Directive class TestDirective {}`; - assertSuccess('directive-class-suffix', source); - }); + assertSuccess('directive-class-suffix', source); }); + }); - describe('valid directive class', () => { - it('should succeed when is used @Component decorator', () => { - let source = ` + describe('valid directive class', () => { + it('should succeed when is used @Component decorator', () => { + let source = ` @Component({ selector: 'sg-foo-bar' }) class TestComponent {}`; - assertSuccess('directive-class-suffix', source); - }); + assertSuccess('directive-class-suffix', source); }); + }); - describe('valid pipe class', () => { - it('should succeed when is used @Pipe decorator', () => { - let source = ` + describe('valid pipe class', () => { + it('should succeed when is used @Pipe decorator', () => { + let source = ` @Pipe({ selector: 'sg-test-pipe' }) class TestPipe {}`; - assertSuccess('directive-class-suffix', source); - }); + assertSuccess('directive-class-suffix', source); }); + }); - describe('valid service class', () => { - it('should succeed when is used @Injectable decorator', () => { - let source = ` + describe('valid service class', () => { + it('should succeed when is used @Injectable decorator', () => { + let source = ` @Injectable() class TestService {}`; - assertSuccess('directive-class-suffix', source); - }); + assertSuccess('directive-class-suffix', source); }); + }); - describe('valid empty class', () => { - it('should succeed when the class is empty', () => { - let source = ` + describe('valid empty class', () => { + it('should succeed when the class is empty', () => { + let source = ` class TestEmpty {}`; - assertSuccess('directive-class-suffix', source); - }); + assertSuccess('directive-class-suffix', source); }); + }); - describe('changed suffix', () => { - it('should suceed when different sufix is set', () => { - let source = ` + describe('changed suffix', () => { + it('should suceed when different sufix is set', () => { + let source = ` @Directive({ selector: 'sgBarFoo' }) class TestPage {}`; - assertSuccess('directive-class-suffix', source, ['Page']); - }); + assertSuccess('directive-class-suffix', source, ['Page']); + }); - it('should fail when different sufix is set and doesnt match', () => { - let source = ` + it('should fail when different sufix is set and doesnt match', () => { + let source = ` @Directive({ selector: 'sgBarFoo' }) class TestPage {} ~~~~~~~~ `; - assertAnnotated({ - ruleName: 'directive-class-suffix', - message: 'The name of the class TestPage should end with the suffix Directive (https://angular.io/styleguide#style-02-03)', - source, - options: ['Directive'] - }); - }); + assertAnnotated({ + ruleName: 'directive-class-suffix', + message: 'The name of the class TestPage should end with the suffix Directive (https://angular.io/styleguide#style-02-03)', + source, + options: ['Directive'] + }); + }); - it('should fail when different sufix is set and doesnt match', () => { - let source = ` + it('should fail when different sufix is set and doesnt match', () => { + let source = ` @Directive({ selector: 'sgBarFoo' }) class TestDirective {} ~~~~~~~~~~~~~ `; - assertAnnotated({ - ruleName: 'directive-class-suffix', - message: 'The name of the class TestDirective should end with the suffix Page (https://angular.io/styleguide#style-02-03)', - source, - options: ['Page'] - }); - }); + assertAnnotated({ + ruleName: 'directive-class-suffix', + message: 'The name of the class TestDirective should end with the suffix Page (https://angular.io/styleguide#style-02-03)', + source, + options: ['Page'] + }); }); + }); }); From cdfd3d0bbbfbfa17b90ba62c2271cad9cbfa8900 Mon Sep 17 00:00:00 2001 From: mgechev Date: Mon, 11 Sep 2017 11:49:41 -0700 Subject: [PATCH 2/2] fix: proper check for interface name --- src/directiveClassSuffixRule.ts | 10 ++++++++-- test/directiveClassSuffix.spec.ts | 3 ++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/directiveClassSuffixRule.ts b/src/directiveClassSuffixRule.ts index f23deeda0..6d15a1f8e 100644 --- a/src/directiveClassSuffixRule.ts +++ b/src/directiveClassSuffixRule.ts @@ -6,7 +6,10 @@ import { NgWalker } from './angular/ngWalker'; import { DirectiveMetadata } from './angular/metadata'; const getInterfaceName = (t: any): string => { - if (t.expression && t.expression.name) { + if (!t.expression) { + return ''; + } + if (t.expression.name) { return t.expression.name.text; } return t.expression.text; @@ -58,7 +61,10 @@ export class ClassMetadataWalker extends NgWalker { const heritageClauses = meta.controller.heritageClauses; if (heritageClauses) { const i = heritageClauses.filter(h => h.token === ts.SyntaxKind.ImplementsKeyword); - if (i.length !== 0 && i[0].types.map(getInterfaceName).some(name => name.endsWith(ValidatorSuffix))) { + if (i.length !== 0 && + i[0].types.map(getInterfaceName) + .filter(name => !!name) + .some(name => name.endsWith(ValidatorSuffix))) { suffixes.push(ValidatorSuffix); } } diff --git a/test/directiveClassSuffix.spec.ts b/test/directiveClassSuffix.spec.ts index 740f0196a..3424ad6b4 100644 --- a/test/directiveClassSuffix.spec.ts +++ b/test/directiveClassSuffix.spec.ts @@ -27,7 +27,8 @@ describe('directive-class-suffix', () => { `; assertAnnotated({ ruleName: 'directive-class-suffix', - message: 'The name of the class Test should end with the suffix Directive, Page, Validator (https://angular.io/styleguide#style-02-03)', + message: 'The name of the class Test should end with the suffix ' + + 'Directive, Page, Validator (https://angular.io/styleguide#style-02-03)', source, options: ['Directive', 'Page', 'Validator'] });