From db770b21158739ebb1a64247c33084e38aab88ec Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sat, 23 Mar 2019 22:28:12 -0300 Subject: [PATCH 01/15] refactor: make use of 'isNotNullOrUndefined' and add tslint rule to enforce file name casing --- src/angularWhitespaceRule.ts | 7 ++++--- src/componentSelectorRule.ts | 2 +- src/directiveClassSuffixRule.ts | 20 ++++++-------------- src/directiveSelectorRule.ts | 2 +- src/selectorPropertyBase.ts | 10 ++++------ src/templateI18nRule.ts | 10 +++++----- src/util/is-not-null-or-undefined.ts | 1 - src/util/isNotNullOrUndefined.ts | 3 +++ tslint.json | 2 +- 9 files changed, 25 insertions(+), 32 deletions(-) delete mode 100644 src/util/is-not-null-or-undefined.ts create mode 100644 src/util/isNotNullOrUndefined.ts diff --git a/src/angularWhitespaceRule.ts b/src/angularWhitespaceRule.ts index 8f764f07d..204f7101e 100644 --- a/src/angularWhitespaceRule.ts +++ b/src/angularWhitespaceRule.ts @@ -6,6 +6,7 @@ import { ExpTypes } from './angular/expressionTypes'; import { NgWalker, NgWalkerConfig } from './angular/ngWalker'; import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; import { RecursiveAngularExpressionVisitor } from './angular/templates/recursiveAngularExpressionVisitor'; +import { isNotNullOrUndefined } from './util/isNotNullOrUndefined'; // Check if ES6 'y' flag is usable. const stickyFlagUsable = (() => { @@ -173,7 +174,7 @@ class TemplateVisitorCtrl extends BasicTemplateAstVisitor { this.visitors .filter(v => options.indexOf(v.getCheckOption()) >= 0) .map(v => v.visitBoundText(text, this)) - .filter(Boolean) + .filter(isNotNullOrUndefined) .forEach(f => this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) ); @@ -185,7 +186,7 @@ class TemplateVisitorCtrl extends BasicTemplateAstVisitor { this.visitors .filter(v => options.indexOf(v.getCheckOption()) >= 0) .map(v => v.visitDirectiveProperty(prop, this)) - .filter(Boolean) + .filter(isNotNullOrUndefined) .forEach(f => this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) ); @@ -270,7 +271,7 @@ class ExpressionVisitorCtrl extends RecursiveAngularExpressionVisitor { .map(v => v.addParentAST(this.parentAST)) .filter(v => options.indexOf(v.getCheckOption()) >= 0) .map(v => v.visitPipe(expr, this)) - .filter(Boolean) + .filter(isNotNullOrUndefined) .forEach(f => this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) ); diff --git a/src/componentSelectorRule.ts b/src/componentSelectorRule.ts index 9fcdf33ea..27ba90947 100644 --- a/src/componentSelectorRule.ts +++ b/src/componentSelectorRule.ts @@ -9,7 +9,7 @@ import { SelectorStyle, SelectorType } from './selectorPropertyBase'; -import { isNotNullOrUndefined } from './util/is-not-null-or-undefined'; +import { isNotNullOrUndefined } from './util/isNotNullOrUndefined'; const STYLE_GUIDE_PREFIX_LINK = 'https://angular.io/guide/styleguide#style-02-07'; const STYLE_GUIDE_STYLE_LINK = 'https://angular.io/guide/styleguide#style-05-02'; diff --git a/src/directiveClassSuffixRule.ts b/src/directiveClassSuffixRule.ts index 3941e52f4..d87fb5b5e 100644 --- a/src/directiveClassSuffixRule.ts +++ b/src/directiveClassSuffixRule.ts @@ -3,7 +3,7 @@ import * as Lint from 'tslint'; import * as ts from 'typescript'; import { DirectiveMetadata } from './angular/metadata'; import { NgWalker } from './angular/ngWalker'; -import { getSymbolName } from './util/utils'; +import { getDeclaredInterfaceNames } from './util/utils'; const ValidatorSuffix = 'Validator'; @@ -40,25 +40,17 @@ export class Rule extends Lint.Rules.AbstractRule { } class Walker extends NgWalker { - protected visitNgDirective(metadata: DirectiveMetadata) { + protected visitNgDirective(metadata: DirectiveMetadata): void { const name = metadata.controller.name!; const className = name.text; const options = this.getOptions(); const suffixes: string[] = options.length ? options : ['Directive']; - const { heritageClauses } = metadata.controller; - if (heritageClauses) { - const i = heritageClauses.filter(h => h.token === ts.SyntaxKind.ImplementsKeyword); + const declaredInterfaceNames = getDeclaredInterfaceNames(metadata.controller); + const hasValidatorInterface = declaredInterfaceNames.some(interfaceName => interfaceName.endsWith(ValidatorSuffix)); - if ( - i.length !== 0 && - i[0].types - .map(getSymbolName) - .filter(Boolean) - .some(x => x.endsWith(ValidatorSuffix)) - ) { - suffixes.push(ValidatorSuffix); - } + if (hasValidatorInterface) { + suffixes.push(ValidatorSuffix); } if (!Rule.validate(className, suffixes)) { diff --git a/src/directiveSelectorRule.ts b/src/directiveSelectorRule.ts index 45eadc221..14fbbab78 100644 --- a/src/directiveSelectorRule.ts +++ b/src/directiveSelectorRule.ts @@ -9,7 +9,7 @@ import { SelectorStyle, SelectorType } from './selectorPropertyBase'; -import { isNotNullOrUndefined } from './util/is-not-null-or-undefined'; +import { isNotNullOrUndefined } from './util/isNotNullOrUndefined'; const STYLE_GUIDE_PREFIX_LINK = 'https://angular.io/guide/styleguide#style-02-08'; const STYLE_GUIDE_STYLE_TYPE_LINK = 'https://angular.io/guide/styleguide#style-02-06'; diff --git a/src/selectorPropertyBase.ts b/src/selectorPropertyBase.ts index ac741be7e..a7a5c300b 100644 --- a/src/selectorPropertyBase.ts +++ b/src/selectorPropertyBase.ts @@ -56,13 +56,11 @@ export abstract class SelectorPropertyBase extends AbstractRule { getValidSelectors(selectors: CssSelector[]): ReadonlyArray { return selectors.reduce>((previousValue, currentValue) => { - const validSelectors = this.types - .reduce>((accumulator, type) => { - const value = currentValue[type]; + const validSelectors = this.types.reduce>((accumulator, type) => { + const value = currentValue[type]; - return value ? accumulator.concat(value) : accumulator; - }, []) - .filter(Boolean); + return value ? accumulator.concat(value) : accumulator; + }, []); return previousValue.concat(validSelectors); }, []); diff --git a/src/templateI18nRule.ts b/src/templateI18nRule.ts index 88623fdf8..719176fb0 100644 --- a/src/templateI18nRule.ts +++ b/src/templateI18nRule.ts @@ -5,7 +5,7 @@ import { arrayify, dedent } from 'tslint/lib/utils'; import { SourceFile } from 'typescript'; import { NgWalker, NgWalkerConfig } from './angular/ngWalker'; import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; -import { isNotNullOrUndefined } from './util/is-not-null-or-undefined'; +import { isNotNullOrUndefined } from './util/isNotNullOrUndefined'; const OPTION_CHECK_ID = 'check-id'; const OPTION_CHECK_TEXT = 'check-text'; @@ -201,7 +201,7 @@ class TemplateVisitorCtrl extends BasicTemplateAstVisitor { this.visitors .filter(v => options.indexOf(v.getCheckOption()) !== -1) .map(v => v.visitAttr(ast, this)) - .filter(Boolean) + .filter(isNotNullOrUndefined) .forEach(f => this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) ); @@ -212,7 +212,7 @@ class TemplateVisitorCtrl extends BasicTemplateAstVisitor { this.visitors .filter(v => options.indexOf(v.getCheckOption()) !== -1) .map(v => v.visitBoundText(text, this)) - .filter(Boolean) + .filter(isNotNullOrUndefined) .forEach(f => this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) ); @@ -223,7 +223,7 @@ class TemplateVisitorCtrl extends BasicTemplateAstVisitor { this.visitors .filter(v => options.indexOf(v.getCheckOption()) !== -1) .map(v => v.visitElement(element, this)) - .filter(Boolean) + .filter(isNotNullOrUndefined) .forEach(f => this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) ); @@ -234,7 +234,7 @@ class TemplateVisitorCtrl extends BasicTemplateAstVisitor { this.visitors .filter(v => options.indexOf(v.getCheckOption()) !== -1) .map(v => v.visitText(text, this)) - .filter(Boolean) + .filter(isNotNullOrUndefined) .forEach(f => this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) ); diff --git a/src/util/is-not-null-or-undefined.ts b/src/util/is-not-null-or-undefined.ts deleted file mode 100644 index 7e131d3e5..000000000 --- a/src/util/is-not-null-or-undefined.ts +++ /dev/null @@ -1 +0,0 @@ -export const isNotNullOrUndefined = (input: null | undefined | T): input is T => input !== null && input !== undefined; diff --git a/src/util/isNotNullOrUndefined.ts b/src/util/isNotNullOrUndefined.ts new file mode 100644 index 000000000..faa2748d1 --- /dev/null +++ b/src/util/isNotNullOrUndefined.ts @@ -0,0 +1,3 @@ +// Needed because in the current Typescript version (TS 3.3.3333), Boolean() cannot be used to perform a null check. +// For more, see: https://github.com/Microsoft/TypeScript/issues/16655 +export const isNotNullOrUndefined = (input: null | undefined | T): input is T => input !== null && input !== undefined; diff --git a/tslint.json b/tslint.json index c068737c6..8725dd64b 100644 --- a/tslint.json +++ b/tslint.json @@ -32,6 +32,6 @@ "no-unused-expression": true, "no-var-keyword": true, "triple-equals": true, - "variable-name": false + "file-name-casing": [true, "camel-case"] } } From b4299f00d1bb835c02eaeb855150baffb734ae9b Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sat, 23 Mar 2019 22:33:48 -0300 Subject: [PATCH 02/15] fix: 'getDecoratorName' not handling decorators without parenthesis --- src/util/utils.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/util/utils.ts b/src/util/utils.ts index 202812f7f..ea3161b99 100644 --- a/src/util/utils.ts +++ b/src/util/utils.ts @@ -123,6 +123,18 @@ export const getDecoratorArgument = (decorator: Decorator): ObjectLiteralExpress return isObjectLiteralExpression(args) && args.properties ? args : undefined; }; +export const getDecoratorName = (decorator: Decorator): string | undefined => { + const { expression } = decorator; + + if (isIdentifier(expression)) return expression.text; + + if (isCallExpression(expression) && isIdentifier(expression.expression)) { + return expression.expression.text; + } + + return undefined; +}; + export const getDecoratorPropertyInitializer = (decorator: Decorator, name: string): Expression | undefined => { const args = getDecoratorArgument(decorator); @@ -136,18 +148,6 @@ export const getDecoratorPropertyInitializer = (decorator: Decorator, name: stri return property.initializer; }; -export const getDecoratorName = (decorator: Decorator): string | undefined => { - const { expression } = decorator; - - if (isIdentifier(expression)) return expression.text; - - if (isCallExpression(expression) && isIdentifier(expression.expression)) { - return expression.expression.text; - } - - return undefined; -}; - export const getNextToLastParentNode = (node: Node): Node => { let currentNode = node; From 0cfb76557b457783c1a6c465aa30a3065f986ba7 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sat, 23 Mar 2019 22:35:46 -0300 Subject: [PATCH 03/15] refactor(utils): add missing decorators and normalize enum/constants/interfaces/types names to Angular* --- src/contextualDecoratorRule.ts | 48 ++++---- src/contextualLifecycleRule.ts | 65 ++++++----- src/noConflictingLifecycleRule.ts | 40 ++++--- src/noHostMetadataPropertyRule.ts | 17 ++- src/noInputsMetadataPropertyRule.ts | 14 ++- src/noLifecycleCallRule.ts | 4 +- src/noOutputsMetadataPropertyRule.ts | 10 +- src/noQueriesMetadataPropertyRule.ts | 23 ++-- src/preferInlineDecoratorRule.ts | 8 +- src/useLifecycleInterfaceRule.ts | 20 ++-- src/usePipeDecoratorRule.ts | 17 ++- src/usePipeTransformInterfaceRule.ts | 14 ++- src/util/objectKeys.ts | 1 + src/util/utils.ts | 149 ++++++++++++++++--------- test/contextualDecoratorRule.spec.ts | 106 +++++++++--------- test/contextualLifecycleRule.spec.ts | 94 ++++++++-------- test/useLifecycleInterfaceRule.spec.ts | 22 ++-- 17 files changed, 373 insertions(+), 279 deletions(-) create mode 100644 src/util/objectKeys.ts diff --git a/src/contextualDecoratorRule.ts b/src/contextualDecoratorRule.ts index b4c0dc3ba..ce7354d7f 100644 --- a/src/contextualDecoratorRule.ts +++ b/src/contextualDecoratorRule.ts @@ -1,36 +1,44 @@ import { sprintf } from 'sprintf-js'; import { IRuleMetadata, RuleFailure } from 'tslint'; import { AbstractRule } from 'tslint/lib/rules'; +import { dedent } from 'tslint/lib/utils'; import { createNodeArray, Decorator, isClassDeclaration, SourceFile } from 'typescript'; import { NgWalker } from './angular/ngWalker'; import { - DecoratorKeys, - Decorators, + ANGULAR_CLASS_DECORATOR_MAPPER, + AngularClassDecoratorKeys, + AngularClassDecorators, + AngularInnerClassDecoratorKeys, + AngularInnerClassDecorators, getDecoratorName, getNextToLastParentNode, - isMetadataType, - isNgDecorator, - METADATA_TYPE_DECORATOR_MAPPER, - MetadataTypes + isAngularClassDecorator, + isAngularInnerClassDecorator } from './util/utils'; interface FailureParameters { readonly className: string; - readonly decoratorName: DecoratorKeys; - readonly metadataType: MetadataTypes; + readonly classDecoratorName: AngularClassDecoratorKeys; + readonly innerClassDecoratorName: AngularInnerClassDecoratorKeys; } export const getFailureMessage = (failureParameters: FailureParameters): string => - sprintf(Rule.FAILURE_STRING, failureParameters.decoratorName, failureParameters.className, failureParameters.metadataType); + sprintf( + Rule.FAILURE_STRING, + failureParameters.innerClassDecoratorName, + failureParameters.className, + failureParameters.classDecoratorName + ); export class Rule extends AbstractRule { static readonly metadata: IRuleMetadata = { description: 'Ensures that classes use allowed decorator in its body.', options: null, optionsDescription: 'Not configurable.', - rationale: `Some decorators can only be used in certain class types. For example, an @${Decorators.Input} should not be used in an @${ - MetadataTypes.Injectable - } class.`, + rationale: dedent`Some decorators can only be used in certain class types. + For example, an @${AngularInnerClassDecorators.Input} should not be used + in an @${AngularClassDecorators.Injectable} class. + `, ruleName: 'contextual-decorator', type: 'functionality', typescriptOnly: true @@ -61,23 +69,23 @@ class Walker extends NgWalker { if (!isClassDeclaration(klass) || !klass.name) return; - const metadataType = createNodeArray(klass.decorators) + const classDecoratorName = createNodeArray(klass.decorators) .map(x => x.expression.getText()) .map(x => x.replace(/[^a-zA-Z]/g, '')) - .find(isMetadataType); + .find(isAngularClassDecorator); - if (!metadataType) return; + if (!classDecoratorName) return; - const decoratorName = getDecoratorName(decorator); + const innerClassDecoratorName = getDecoratorName(decorator); - if (!decoratorName || !isNgDecorator(decoratorName)) return; + if (!innerClassDecoratorName || !isAngularInnerClassDecorator(innerClassDecoratorName)) return; - const allowedDecorators = METADATA_TYPE_DECORATOR_MAPPER[metadataType]; + const allowedDecorators = ANGULAR_CLASS_DECORATOR_MAPPER.get(classDecoratorName); - if (!allowedDecorators || allowedDecorators.has(decoratorName)) return; + if (!allowedDecorators || allowedDecorators.has(innerClassDecoratorName)) return; const className = klass.name.getText(); - const failure = getFailureMessage({ className, decoratorName, metadataType }); + const failure = getFailureMessage({ classDecoratorName, className, innerClassDecoratorName }); this.addFailureAtNode(decorator, failure); } diff --git a/src/contextualLifecycleRule.ts b/src/contextualLifecycleRule.ts index 8335553a8..e7e6af26e 100644 --- a/src/contextualLifecycleRule.ts +++ b/src/contextualLifecycleRule.ts @@ -1,38 +1,41 @@ import { sprintf } from 'sprintf-js'; import { IRuleMetadata, RuleFailure } from 'tslint/lib'; import { AbstractRule } from 'tslint/lib/rules'; +import { dedent } from 'tslint/lib/utils'; import { SourceFile } from 'typescript'; import { InjectableMetadata, ModuleMetadata, PipeMetadata } from './angular'; import { NgWalker } from './angular/ngWalker'; import { + ANGULAR_CLASS_DECORATOR_LIFECYCLE_METHOD_MAPPER, + AngularClassDecoratorKeys, + AngularClassDecorators, + AngularLifecycleMethodKeys, + AngularLifecycleMethods, getClassName, getDecoratorName, - isLifecycleMethod, - isMetadataType, - LifecycleMethodKeys, - LifecycleMethods, - METADATA_TYPE_LIFECYCLE_MAPPER, - MetadataTypeKeys, - MetadataTypes + isAngularClassDecorator, + isAngularLifecycleMethod } from './util/utils'; interface FailureParameters { readonly className: string; - readonly metadataType: MetadataTypeKeys; - readonly methodName: LifecycleMethodKeys; + readonly decoratorName: AngularClassDecoratorKeys; + readonly methodName: AngularLifecycleMethodKeys; } export const getFailureMessage = (failureParameters: FailureParameters): string => - sprintf(Rule.FAILURE_STRING, failureParameters.methodName, failureParameters.className, failureParameters.metadataType); + sprintf(Rule.FAILURE_STRING, failureParameters.methodName, failureParameters.className, failureParameters.decoratorName); export class Rule extends AbstractRule { static readonly metadata: IRuleMetadata = { description: 'Ensures that classes use allowed lifecycle method in its body.', options: null, optionsDescription: 'Not configurable.', - rationale: `Some lifecycle methods can only be used in certain class types. For example, ${ - LifecycleMethods.ngOnInit - }() method should not be used in an @${MetadataTypes.Injectable} class.`, + rationale: dedent` + Some lifecycle methods can only be used in certain class types. + For example, ${AngularLifecycleMethods.ngOnInit}() method should not be used + in an @${AngularClassDecorators.Injectable} class. + `, ruleName: 'contextual-lifecycle', type: 'functionality', typescriptOnly: true @@ -49,26 +52,38 @@ export class Rule extends AbstractRule { class Walker extends NgWalker { protected visitNgInjectable(metadata: InjectableMetadata): void { - this.validateDecorator(metadata, METADATA_TYPE_LIFECYCLE_MAPPER.Injectable); + const injectableAllowedMethods = ANGULAR_CLASS_DECORATOR_LIFECYCLE_METHOD_MAPPER.get(AngularClassDecorators.Injectable); + + if (!injectableAllowedMethods) return; + + this.validateDecorator(metadata, injectableAllowedMethods); super.visitNgInjectable(metadata); } - protected visitNgPipe(metadata: PipeMetadata): void { - this.validateDecorator(metadata, METADATA_TYPE_LIFECYCLE_MAPPER.Pipe); - super.visitNgPipe(metadata); - } + protected visitNgModule(metadata: ModuleMetadata): void { + const ngModuleAllowedMethods = ANGULAR_CLASS_DECORATOR_LIFECYCLE_METHOD_MAPPER.get(AngularClassDecorators.NgModule); + + if (!ngModuleAllowedMethods) return; - protected visitNgModule(metadata: ModuleMetadata) { - this.validateDecorator(metadata, METADATA_TYPE_LIFECYCLE_MAPPER.NgModule); + this.validateDecorator(metadata, ngModuleAllowedMethods); super.visitNgModule(metadata); } - private validateDecorator(metadata: PipeMetadata, allowedMethods: ReadonlySet): void { + protected visitNgPipe(metadata: PipeMetadata): void { + const pipeAllowedMethods = ANGULAR_CLASS_DECORATOR_LIFECYCLE_METHOD_MAPPER.get(AngularClassDecorators.Pipe); + + if (!pipeAllowedMethods) return; + + this.validateDecorator(metadata, pipeAllowedMethods); + super.visitNgPipe(metadata); + } + + private validateDecorator(metadata: PipeMetadata, allowedMethods: ReadonlySet): void { const className = getClassName(metadata.controller)!; - const metadataType = getDecoratorName(metadata.decorator); + const decoratorName = getDecoratorName(metadata.decorator); - if (!metadataType || !isMetadataType(metadataType)) return; + if (!decoratorName || !isAngularClassDecorator(decoratorName)) return; for (const member of metadata.controller.members) { const { name: memberName } = member; @@ -77,9 +92,9 @@ class Walker extends NgWalker { const methodName = memberName.getText(); - if (!isLifecycleMethod(methodName) || allowedMethods.has(methodName)) continue; + if (!isAngularLifecycleMethod(methodName) || allowedMethods.has(methodName)) continue; - const failure = getFailureMessage({ className, metadataType, methodName }); + const failure = getFailureMessage({ className, decoratorName, methodName }); this.addFailureAtNode(member, failure); } diff --git a/src/noConflictingLifecycleRule.ts b/src/noConflictingLifecycleRule.ts index 7478defbf..39761e453 100644 --- a/src/noConflictingLifecycleRule.ts +++ b/src/noConflictingLifecycleRule.ts @@ -4,20 +4,26 @@ import { AbstractRule } from 'tslint/lib/rules'; import { dedent } from 'tslint/lib/utils'; import { ClassDeclaration, forEachChild, isClassDeclaration, Node, SourceFile } from 'typescript'; import { - getDeclaredLifecycleInterfaces, - getDeclaredLifecycleMethods, - LifecycleInterfaceKeys, - LifecycleInterfaces, - LifecycleMethodKeys, - LifecycleMethods + AngularLifecycleInterfaceKeys, + AngularLifecycleInterfaces, + AngularLifecycleMethodKeys, + AngularLifecycleMethods, + getDeclaredAngularLifecycleInterfaces, + getDeclaredAngularLifecycleMethods } from './util/utils'; interface FailureParameters { readonly message: typeof Rule.FAILURE_STRING_INTERFACE_HOOK | typeof Rule.FAILURE_STRING_METHOD_HOOK; } -const LIFECYCLE_INTERFACES: ReadonlyArray = [LifecycleInterfaces.DoCheck, LifecycleInterfaces.OnChanges]; -const LIFECYCLE_METHODS: ReadonlyArray = [LifecycleMethods.ngDoCheck, LifecycleMethods.ngOnChanges]; +const LIFECYCLE_INTERFACES: ReadonlyArray = [ + AngularLifecycleInterfaces.DoCheck, + AngularLifecycleInterfaces.OnChanges +]; +const LIFECYCLE_METHODS: ReadonlyArray = [ + AngularLifecycleMethods.ngDoCheck, + AngularLifecycleMethods.ngOnChanges +]; export const getFailureMessage = (failureParameters: FailureParameters): string => sprintf(failureParameters.message); @@ -28,8 +34,8 @@ export class Rule extends AbstractRule { options: null, optionsDescription: 'Not configurable.', rationale: dedent` - A directive typically should not use both ${LifecycleInterfaces.DoCheck} and ${LifecycleInterfaces.OnChanges} to respond - to changes on the same input, as ${LifecycleMethods.ngOnChanges} will continue to be called when the + A directive typically should not use both ${AngularLifecycleInterfaces.DoCheck} and ${AngularLifecycleInterfaces.OnChanges} to respond + to changes on the same input, as ${AngularLifecycleMethods.ngOnChanges} will continue to be called when the default change detector detects changes. `, ruleName: 'no-conflicting-lifecycle', @@ -38,10 +44,10 @@ export class Rule extends AbstractRule { }; static readonly FAILURE_STRING_INTERFACE_HOOK = dedent` - Implementing ${LifecycleInterfaces.DoCheck} and ${LifecycleInterfaces.OnChanges} in a class is not recommended + Implementing ${AngularLifecycleInterfaces.DoCheck} and ${AngularLifecycleInterfaces.OnChanges} in a class is not recommended `; static readonly FAILURE_STRING_METHOD_HOOK = dedent` - Declaring ${LifecycleMethods.ngDoCheck} and ${LifecycleMethods.ngOnChanges} method in a class is not recommended + Declaring ${AngularLifecycleMethods.ngDoCheck} and ${AngularLifecycleMethods.ngOnChanges} method in a class is not recommended `; apply(sourceFile: SourceFile): RuleFailure[] { @@ -55,9 +61,9 @@ const validateClassDeclaration = (context: WalkContext, node: ClassDeclara }; const validateInterfaces = (context: WalkContext, node: ClassDeclaration): void => { - const declaredLifecycleInterfaces = getDeclaredLifecycleInterfaces(node); - const hasConflictingLifecycle = LIFECYCLE_INTERFACES.every( - lifecycleInterface => declaredLifecycleInterfaces.indexOf(lifecycleInterface) !== -1 + const declaredAngularLifecycleInterfaces = getDeclaredAngularLifecycleInterfaces(node); + const hasConflictingLifecycle = LIFECYCLE_INTERFACES.every(lifecycleInterface => + declaredAngularLifecycleInterfaces.includes(lifecycleInterface) ); if (!hasConflictingLifecycle) return; @@ -70,8 +76,8 @@ const validateInterfaces = (context: WalkContext, node: ClassDeclaration): }; const validateMethods = (context: WalkContext, node: ClassDeclaration): void => { - const declaredLifecycleMethods = getDeclaredLifecycleMethods(node); - const hasConflictingLifecycle = LIFECYCLE_METHODS.every(lifecycleMethod => declaredLifecycleMethods.indexOf(lifecycleMethod) !== -1); + const declaredAngularLifecycleMethods = getDeclaredAngularLifecycleMethods(node); + const hasConflictingLifecycle = LIFECYCLE_METHODS.every(lifecycleMethod => declaredAngularLifecycleMethods.includes(lifecycleMethod)); if (!hasConflictingLifecycle) return; diff --git a/src/noHostMetadataPropertyRule.ts b/src/noHostMetadataPropertyRule.ts index 9421dff86..8751d9886 100644 --- a/src/noHostMetadataPropertyRule.ts +++ b/src/noHostMetadataPropertyRule.ts @@ -1,6 +1,7 @@ import { IOptions, IRuleMetadata } from 'tslint/lib'; +import { dedent } from 'tslint/lib/utils'; import { MetadataPropertyBase } from './metadataPropertyBase'; -import { Decorators } from './util/utils'; +import { AngularInnerClassDecorators } from './util/utils'; const METADATA_PROPERTY_NAME = 'host'; const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-06-03'; @@ -11,16 +12,20 @@ export class Rule extends MetadataPropertyBase { descriptionDetails: `See more at ${STYLE_GUIDE_LINK}.`, options: null, optionsDescription: 'Not configurable.', - rationale: `If you ever need to rename the property associated with @${Decorators.HostBinding} or the method associated with - @${Decorators.HostListener}, you can modify it in a single place.`, + rationale: dedent` + If you ever need to rename the property associated + with @${AngularInnerClassDecorators.HostBinding} or the method associated + with @${AngularInnerClassDecorators.HostListener}, you can modify it in a single place. + `, ruleName: 'no-host-metadata-property', type: 'style', typescriptOnly: true }; - static readonly FAILURE_STRING = `Use @${Decorators.HostBinding} or @${ - Decorators.HostListener - } rather than the \`${METADATA_PROPERTY_NAME}\` metadata property (${STYLE_GUIDE_LINK})`; + static readonly FAILURE_STRING = dedent` + Use @${AngularInnerClassDecorators.HostBinding} or + @${AngularInnerClassDecorators.HostListener} rather than the \`${METADATA_PROPERTY_NAME}\` metadata property (${STYLE_GUIDE_LINK}) + `; constructor(options: IOptions) { super( diff --git a/src/noInputsMetadataPropertyRule.ts b/src/noInputsMetadataPropertyRule.ts index 943268a86..104c1eabc 100644 --- a/src/noInputsMetadataPropertyRule.ts +++ b/src/noInputsMetadataPropertyRule.ts @@ -1,7 +1,7 @@ import { IOptions, IRuleMetadata } from 'tslint/lib'; import { dedent } from 'tslint/lib/utils'; import { MetadataPropertyBase } from './metadataPropertyBase'; -import { Decorators } from './util/utils'; +import { AngularInnerClassDecorators } from './util/utils'; const METADATA_PROPERTY_NAME = 'inputs'; const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-05-12'; @@ -13,8 +13,10 @@ export class Rule extends MetadataPropertyBase { options: null, optionsDescription: 'Not configurable.', rationale: dedent` - * It is easier and more readable to identify which properties in a class are ${METADATA_PROPERTY_NAME}. - * If you ever need to rename the property associated with @${Decorators.Input}, you can modify it in a single place. + * It is easier and more readable to identify which properties in a class + are ${METADATA_PROPERTY_NAME}. + * If you ever need to rename the property associated + with @${AngularInnerClassDecorators.Input}, you can modify it in a single place. * The metadata declaration attached to the directive is shorter and thus more readable. `, ruleName: 'no-inputs-metadata-property', @@ -22,9 +24,9 @@ export class Rule extends MetadataPropertyBase { typescriptOnly: true }; - static readonly FAILURE_STRING = `Use @${ - Decorators.Input - } rather than the \`${METADATA_PROPERTY_NAME}\` metadata property (${STYLE_GUIDE_LINK})`; + static readonly FAILURE_STRING = dedent` + Use @${AngularInnerClassDecorators.Input} rather than the \`${METADATA_PROPERTY_NAME}\` metadata property (${STYLE_GUIDE_LINK}) + `; constructor(options: IOptions) { super( diff --git a/src/noLifecycleCallRule.ts b/src/noLifecycleCallRule.ts index 53bb3ddb9..0fa8d21fb 100644 --- a/src/noLifecycleCallRule.ts +++ b/src/noLifecycleCallRule.ts @@ -2,7 +2,7 @@ import { IRuleMetadata, RuleFailure } from 'tslint'; import { AbstractRule } from 'tslint/lib/rules'; import { CallExpression, isPropertyAccessExpression, SourceFile, SyntaxKind } from 'typescript'; import { NgWalker } from './angular/ngWalker'; -import { isLifecycleMethod } from './util/utils'; +import { isAngularLifecycleMethod } from './util/utils'; export class Rule extends AbstractRule { static readonly metadata: IRuleMetadata = { @@ -39,7 +39,7 @@ class Walker extends NgWalker { expression, name: { text: methodName } } = nodeExpression; - const isLifecycleCall = isLifecycleMethod(methodName); + const isLifecycleCall = isAngularLifecycleMethod(methodName); const isSuperCall = expression.kind === SyntaxKind.SuperKeyword; if (!isLifecycleCall || isSuperCall) return; diff --git a/src/noOutputsMetadataPropertyRule.ts b/src/noOutputsMetadataPropertyRule.ts index 9fa1abd4a..761a122ba 100644 --- a/src/noOutputsMetadataPropertyRule.ts +++ b/src/noOutputsMetadataPropertyRule.ts @@ -1,7 +1,7 @@ import { IOptions, IRuleMetadata } from 'tslint/lib'; import { dedent } from 'tslint/lib/utils'; import { MetadataPropertyBase } from './metadataPropertyBase'; -import { Decorators } from './util/utils'; +import { AngularInnerClassDecorators } from './util/utils'; const METADATA_PROPERTY_NAME = 'outputs'; const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-05-12'; @@ -14,7 +14,7 @@ export class Rule extends MetadataPropertyBase { optionsDescription: 'Not configurable.', rationale: dedent` * It is easier and more readable to identify which properties in a class are events. - * If you ever need to rename the event associated with @${Decorators.Output}, you can modify it in a single place. + * If you ever need to rename the event associated with @${AngularInnerClassDecorators.Output}, you can modify it in a single place. * The metadata declaration attached to the directive is shorter and thus more readable. `, ruleName: 'no-outputs-metadata-property', @@ -22,9 +22,9 @@ export class Rule extends MetadataPropertyBase { typescriptOnly: true }; - static readonly FAILURE_STRING = `Use @${ - Decorators.Output - } rather than the \`${METADATA_PROPERTY_NAME}\` metadata property (${STYLE_GUIDE_LINK})`; + static readonly FAILURE_STRING = dedent` + Use @${AngularInnerClassDecorators.Output} rather than the \`${METADATA_PROPERTY_NAME}\` metadata property (${STYLE_GUIDE_LINK}) + `; constructor(options: IOptions) { super( diff --git a/src/noQueriesMetadataPropertyRule.ts b/src/noQueriesMetadataPropertyRule.ts index 619f62959..e20909f27 100644 --- a/src/noQueriesMetadataPropertyRule.ts +++ b/src/noQueriesMetadataPropertyRule.ts @@ -1,6 +1,7 @@ import { IOptions, IRuleMetadata } from 'tslint/lib'; +import { dedent } from 'tslint/lib/utils'; import { MetadataPropertyBase } from './metadataPropertyBase'; -import { Decorators } from './util/utils'; +import { AngularInnerClassDecorators } from './util/utils'; const METADATA_PROPERTY_NAME = 'queries'; @@ -9,17 +10,25 @@ export class Rule extends MetadataPropertyBase { description: `Disallows usage of the \`${METADATA_PROPERTY_NAME}\` metadata property.`, options: null, optionsDescription: 'Not configurable.', - rationale: `If you ever need to rename the property associated with @${Decorators.ContentChild}, @${Decorators.ContentChildren}, @${ - Decorators.ViewChild - } or @${Decorators.ViewChildren}, you can modify it in a single place.`, + rationale: dedent` + If you ever need to rename the property associated + with @${AngularInnerClassDecorators.ContentChild}, + @${AngularInnerClassDecorators.ContentChildren}, @${AngularInnerClassDecorators.ViewChild} + or @${AngularInnerClassDecorators.ViewChildren}, + you can modify it in a single place. + `, ruleName: 'no-queries-metadata-property', type: 'style', typescriptOnly: true }; - static readonly FAILURE_STRING = `Use @${Decorators.ContentChild}, @${Decorators.ContentChildren}, @${Decorators.ViewChild} or @${ - Decorators.ViewChildren - } rather than the \`${METADATA_PROPERTY_NAME}\` metadata property`; + static readonly FAILURE_STRING = dedent` + Use @${AngularInnerClassDecorators.ContentChild}, + @${AngularInnerClassDecorators.ContentChildren}, + @${AngularInnerClassDecorators.ViewChild} + or @${AngularInnerClassDecorators.ViewChildren} + rather than the \`${METADATA_PROPERTY_NAME}\` metadata property + `; constructor(options: IOptions) { super( diff --git a/src/preferInlineDecoratorRule.ts b/src/preferInlineDecoratorRule.ts index f6dafeddd..bf35d7486 100644 --- a/src/preferInlineDecoratorRule.ts +++ b/src/preferInlineDecoratorRule.ts @@ -20,9 +20,9 @@ import { SetAccessorDeclaration, SourceFile } from 'typescript'; -import { isNotNullOrUndefined } from './util/is-not-null-or-undefined'; +import { isNotNullOrUndefined } from './util/isNotNullOrUndefined'; import { objectKeys } from './util/object-keys'; -import { Decorators, getDecoratorName, isSameLine } from './util/utils'; +import { AngularInnerClassDecorators, getDecoratorName, isSameLine } from './util/utils'; const OPTION_GETTERS = 'getters'; const OPTION_METHODS = 'methods'; @@ -95,13 +95,13 @@ export class Rule extends AbstractRule { true, { [OPTION_GETTERS]: { - [OPTION_SAFELIST]: [Decorators.Input] + [OPTION_SAFELIST]: [AngularInnerClassDecorators.Input] }, [OPTION_METHODS]: true, [OPTION_PARAMETER_PROPERTIES]: false, [OPTION_PARAMETERS]: false, [OPTION_PROPERTIES]: { - [OPTION_SAFELIST]: [Decorators.Output, 'MyCustomDecorator'] + [OPTION_SAFELIST]: [AngularInnerClassDecorators.Output, 'MyCustomDecorator'] }, [OPTION_SETTERS]: true } diff --git a/src/useLifecycleInterfaceRule.ts b/src/useLifecycleInterfaceRule.ts index e95fa61fb..897f2d776 100644 --- a/src/useLifecycleInterfaceRule.ts +++ b/src/useLifecycleInterfaceRule.ts @@ -4,17 +4,17 @@ import { AbstractRule } from 'tslint/lib/rules'; import { ClassDeclaration, forEachChild, isClassDeclaration, Node, SourceFile } from 'typescript'; import { getDeclaredMethods } from './util/classDeclarationUtils'; import { - getDeclaredLifecycleInterfaces, + AngularLifecycleInterfaceKeys, + AngularLifecycleInterfaces, + AngularLifecycleMethodKeys, + getDeclaredAngularLifecycleInterfaces, getLifecycleInterfaceByMethodName, - isLifecycleMethod, - LifecycleInterfaceKeys, - LifecycleInterfaces, - LifecycleMethodKeys + isAngularLifecycleMethod } from './util/utils'; interface FailureParameters { - readonly interfaceName: LifecycleInterfaceKeys; - readonly methodName: LifecycleMethodKeys; + readonly interfaceName: AngularLifecycleInterfaceKeys; + readonly methodName: AngularLifecycleMethodKeys; } const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-09-01'; @@ -42,17 +42,17 @@ export class Rule extends AbstractRule { } const validateClassDeclaration = (context: WalkContext, node: ClassDeclaration): void => { - const declaredLifecycleInterfaces = getDeclaredLifecycleInterfaces(node); + const declaredLifecycleInterfaces = getDeclaredAngularLifecycleInterfaces(node); const declaredMethods = getDeclaredMethods(node); for (const method of declaredMethods) { const { name: methodProperty } = method; const methodName = methodProperty.getText(); - if (!isLifecycleMethod(methodName)) continue; + if (!isAngularLifecycleMethod(methodName)) continue; const interfaceName = getLifecycleInterfaceByMethodName(methodName); - const isMethodImplemented = declaredLifecycleInterfaces.indexOf(LifecycleInterfaces[interfaceName]) !== -1; + const isMethodImplemented = declaredLifecycleInterfaces.includes(AngularLifecycleInterfaces[interfaceName]); if (isMethodImplemented) continue; diff --git a/src/usePipeDecoratorRule.ts b/src/usePipeDecoratorRule.ts index 706df20f4..4bc39b002 100644 --- a/src/usePipeDecoratorRule.ts +++ b/src/usePipeDecoratorRule.ts @@ -1,14 +1,18 @@ import { sprintf } from 'sprintf-js'; import { IRuleMetadata, RuleFailure, WalkContext } from 'tslint/lib'; import { AbstractRule } from 'tslint/lib/rules'; +import { dedent } from 'tslint/lib/utils'; import { ClassDeclaration, forEachChild, isClassDeclaration, Node, SourceFile } from 'typescript/lib/typescript'; -import { getDeclaredInterfaceName, getDecorator, MetadataTypes } from './util/utils'; +import { AngularClassDecorators, getDeclaredInterfaceName, getPipeDecorator } from './util/utils'; const PIPE_TRANSFORM = 'PipeTransform'; export class Rule extends AbstractRule { static readonly metadata: IRuleMetadata = { - description: `Ensures that classes implementing ${PIPE_TRANSFORM} interface use @${MetadataTypes.Pipe} decorator.`, + description: dedent` + Ensures that classes implementing ${PIPE_TRANSFORM} interface + use @${AngularClassDecorators.Pipe} decorator. + `, options: null, optionsDescription: 'Not configurable.', ruleName: 'use-pipe-decorator', @@ -16,19 +20,20 @@ export class Rule extends AbstractRule { typescriptOnly: true }; - static readonly FAILURE_STRING = `Classes that implement the ${PIPE_TRANSFORM} interface should be decorated with @${MetadataTypes.Pipe}`; + static readonly FAILURE_STRING = dedent` + Classes that implement the ${PIPE_TRANSFORM} interface should be decorated + with @${AngularClassDecorators.Pipe} + `; apply(sourceFile: SourceFile): RuleFailure[] { return this.applyWithFunction(sourceFile, walk); } } -const hasPipeDecorator = (node: ClassDeclaration): boolean => !!getDecorator(node, MetadataTypes.Pipe); - const hasPipeTransformInterface = (node: ClassDeclaration): boolean => !!getDeclaredInterfaceName(node, PIPE_TRANSFORM); const validateClassDeclaration = (context: WalkContext, node: ClassDeclaration): void => { - if (!hasPipeTransformInterface(node) || hasPipeDecorator(node)) return; + if (!hasPipeTransformInterface(node) || getPipeDecorator(node)) return; context.addFailureAtNode(node, sprintf(Rule.FAILURE_STRING)); }; diff --git a/src/usePipeTransformInterfaceRule.ts b/src/usePipeTransformInterfaceRule.ts index 11dd751bb..e793d1f84 100644 --- a/src/usePipeTransformInterfaceRule.ts +++ b/src/usePipeTransformInterfaceRule.ts @@ -1,14 +1,15 @@ import { sprintf } from 'sprintf-js'; import { IRuleMetadata, RuleFailure, WalkContext } from 'tslint/lib'; import { AbstractRule } from 'tslint/lib/rules'; +import { dedent } from 'tslint/lib/utils'; import { ClassDeclaration, forEachChild, isClassDeclaration, Node, SourceFile } from 'typescript/lib/typescript'; -import { getDeclaredInterfaceName, getDecorator, MetadataTypes } from './util/utils'; +import { AngularClassDecorators, getDeclaredInterfaceName, getPipeDecorator } from './util/utils'; const PIPE_TRANSFORM = 'PipeTransform'; export class Rule extends AbstractRule { static readonly metadata: IRuleMetadata = { - description: `Ensures tht classes decorated with @${MetadataTypes.Pipe} implement ${PIPE_TRANSFORM} interface.`, + description: `Ensures tht classes decorated with @${AngularClassDecorators.Pipe} implement ${PIPE_TRANSFORM} interface.`, options: null, optionsDescription: 'Not configurable.', rationale: 'Interfaces prescribe typed method signatures. Use those signatures to flag spelling and syntax mistakes.', @@ -17,19 +18,20 @@ export class Rule extends AbstractRule { typescriptOnly: true }; - static readonly FAILURE_STRING = `Classes decorated with @${MetadataTypes.Pipe} decorator should implement ${PIPE_TRANSFORM} interface`; + static readonly FAILURE_STRING = dedent` + Classes decorated with @${AngularClassDecorators.Pipe} decorator should + implement ${PIPE_TRANSFORM} interface + `; apply(sourceFile: SourceFile): RuleFailure[] { return this.applyWithFunction(sourceFile, walk); } } -const hasPipeDecorator = (node: ClassDeclaration): boolean => !!getDecorator(node, MetadataTypes.Pipe); - const hasPipeTransformInterface = (node: ClassDeclaration): boolean => !!getDeclaredInterfaceName(node, PIPE_TRANSFORM); const validateClassDeclaration = (context: WalkContext, node: ClassDeclaration): void => { - if (!hasPipeDecorator(node) || hasPipeTransformInterface(node)) return; + if (!getPipeDecorator(node) || hasPipeTransformInterface(node)) return; context.addFailureAtNode(node, sprintf(Rule.FAILURE_STRING)); }; diff --git a/src/util/objectKeys.ts b/src/util/objectKeys.ts new file mode 100644 index 000000000..faeb007d8 --- /dev/null +++ b/src/util/objectKeys.ts @@ -0,0 +1 @@ +export const objectKeys = Object.keys as (o: T) => ReadonlyArray>; diff --git a/src/util/utils.ts b/src/util/utils.ts index ea3161b99..ca22f6a86 100644 --- a/src/util/utils.ts +++ b/src/util/utils.ts @@ -23,19 +23,46 @@ import { SyntaxKind } from 'typescript'; import { getDeclaredMethods } from './classDeclarationUtils'; +import { objectKeys } from './objectKeys'; -export enum Decorators { +export enum AngularClassDecorators { + Component = 'Component', + Directive = 'Directive', + Injectable = 'Injectable', + NgModule = 'NgModule', + Pipe = 'Pipe' +} + +enum AngularMethodDecorators { + HostListener = 'HostListener' +} + +enum AngularParameterPropertyDecorators { + Attribute = 'Attribute', + Host = 'Host', + Inject = 'Inject', + Optional = 'Optional', + Self = 'Self', + SkipSelf = 'SkipSelf' +} + +enum AngularPropertyAccessorDecorators { ContentChild = 'ContentChild', ContentChildren = 'ContentChildren', HostBinding = 'HostBinding', - HostListener = 'HostListener', Input = 'Input', Output = 'Output', ViewChild = 'ViewChild', ViewChildren = 'ViewChildren' } -export enum LifecycleInterfaces { +export const AngularInnerClassDecorators = { + ...AngularMethodDecorators, + ...AngularParameterPropertyDecorators, + ...AngularPropertyAccessorDecorators +}; + +export enum AngularLifecycleInterfaces { AfterContentChecked = 'AfterContentChecked', AfterContentInit = 'AfterContentInit', AfterViewChecked = 'AfterViewChecked', @@ -46,7 +73,7 @@ export enum LifecycleInterfaces { DoCheck = 'DoCheck' } -export enum LifecycleMethods { +export enum AngularLifecycleMethods { ngAfterContentChecked = 'ngAfterContentChecked', ngAfterContentInit = 'ngAfterContentInit', ngAfterViewChecked = 'ngAfterViewChecked', @@ -57,44 +84,55 @@ export enum LifecycleMethods { ngDoCheck = 'ngDoCheck' } -export enum MetadataTypes { - Component = 'Component', - Directive = 'Directive', - Injectable = 'Injectable', - Pipe = 'Pipe', - NgModule = 'NgModule' -} - -export type DecoratorKeys = keyof typeof Decorators; -export type LifecycleInterfaceKeys = keyof typeof LifecycleInterfaces; -export type LifecycleMethodKeys = keyof typeof LifecycleMethods; -export type MetadataTypeKeys = keyof typeof MetadataTypes; -export type MetadataTypeLifecycleMapper = Readonly>; -export type MetadataTypeDecoratorMapper = Readonly>; - -export const decoratorKeys = Object.keys(Decorators) as ReadonlyArray; -export const lifecycleInterfaceKeys = Object.keys(LifecycleInterfaces) as ReadonlyArray; -export const lifecycleMethodKeys = Object.keys(LifecycleMethods) as ReadonlyArray; -export const metadataTypeKeys = Object.keys(MetadataTypes) as ReadonlyArray; - -export const DECORATORS: ReadonlySet = new Set(decoratorKeys); -export const LIFECYCLE_INTERFACES: ReadonlySet = new Set(lifecycleInterfaceKeys); -export const LIFECYCLE_METHODS: ReadonlySet = new Set(lifecycleMethodKeys); -export const METADATA_TYPES: ReadonlySet = new Set(metadataTypeKeys); -export const METADATA_TYPE_DECORATOR_MAPPER: MetadataTypeDecoratorMapper = { - Component: DECORATORS, - Directive: DECORATORS, - Injectable: 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]), - NgModule: new Set([]) -}; +export type AngularClassDecoratorKeys = keyof typeof AngularClassDecorators; +export type AngularInnerClassDecoratorKeys = Exclude; +export type AngularLifecycleInterfaceKeys = keyof typeof AngularLifecycleInterfaces; +export type AngularLifecycleMethodKeys = keyof typeof AngularLifecycleMethods; + +export const angularClassDecoratorKeys = objectKeys(AngularClassDecorators); +export const angularInnerClassDecoratorKeys = objectKeys(AngularInnerClassDecorators); +export const angularLifecycleInterfaceKeys = objectKeys(AngularLifecycleInterfaces); +export const angularLifecycleMethodKeys = objectKeys(AngularLifecycleMethods); + +export const ANGULAR_INNER_CLASS_DECORATORS: ReadonlySet = new Set(angularInnerClassDecoratorKeys); +export const ANGULAR_CLASS_DECORATORS: ReadonlySet = new Set(angularClassDecoratorKeys); +export const ANGULAR_CLASS_DECORATOR_MAPPER: ReadonlyMap> = new Map([ + [AngularClassDecorators.Component, ANGULAR_INNER_CLASS_DECORATORS], + [AngularClassDecorators.Directive, ANGULAR_INNER_CLASS_DECORATORS], + [ + AngularClassDecorators.Injectable, + new Set([ + AngularInnerClassDecorators.Host, + AngularInnerClassDecorators.Inject, + AngularInnerClassDecorators.Optional, + AngularInnerClassDecorators.Self, + AngularInnerClassDecorators.SkipSelf + ]) + ], + [AngularClassDecorators.NgModule, new Set([])], + [ + AngularClassDecorators.Pipe, + new Set([ + AngularInnerClassDecorators.Host, + AngularInnerClassDecorators.Inject, + AngularInnerClassDecorators.Optional, + AngularInnerClassDecorators.Self, + AngularInnerClassDecorators.SkipSelf + ]) + ] +]); +export const ANGULAR_LIFECYCLE_INTERFACES: ReadonlySet = new Set(angularLifecycleInterfaceKeys); +export const ANGULAR_LIFECYCLE_METHODS: ReadonlySet = new Set(angularLifecycleMethodKeys); +export const ANGULAR_CLASS_DECORATOR_LIFECYCLE_METHOD_MAPPER: ReadonlyMap< + AngularClassDecoratorKeys, + ReadonlySet +> = new Map([ + [AngularClassDecorators.Component, ANGULAR_LIFECYCLE_METHODS], + [AngularClassDecorators.Directive, ANGULAR_LIFECYCLE_METHODS], + [AngularClassDecorators.Injectable, new Set([AngularLifecycleMethods.ngOnDestroy])], + [AngularClassDecorators.NgModule, new Set([])], + [AngularClassDecorators.Pipe, new Set([AngularLifecycleMethods.ngOnDestroy])] +]); export const getClassName = (node: Node): string | undefined => { const klass = getNextToLastParentNode(node); @@ -174,14 +212,17 @@ export const getSymbolName = (expression: ExpressionWithTypeArguments): string = return isPropertyAccessExpression(childExpression) ? childExpression.name.getText() : childExpression.getText(); }; -export const isNgDecorator = (value: string): value is DecoratorKeys => DECORATORS.has(value as DecoratorKeys); +export const isAngularClassDecorator = (value: string): value is AngularClassDecoratorKeys => + ANGULAR_CLASS_DECORATORS.has(value as AngularClassDecoratorKeys); -export const isLifecycleInterface = (value: string): value is LifecycleInterfaceKeys => - LIFECYCLE_INTERFACES.has(value as LifecycleInterfaceKeys); +export const isAngularInnerClassDecorator = (value: string): value is AngularInnerClassDecoratorKeys => + ANGULAR_INNER_CLASS_DECORATORS.has(value as AngularInnerClassDecoratorKeys); -export const isLifecycleMethod = (value: string): value is LifecycleMethodKeys => LIFECYCLE_METHODS.has(value as LifecycleMethodKeys); +export const isAngularLifecycleInterface = (value: string): value is AngularLifecycleInterfaceKeys => + ANGULAR_LIFECYCLE_INTERFACES.has(value as AngularLifecycleInterfaceKeys); -export const isMetadataType = (value: string): value is MetadataTypes => METADATA_TYPES.has(value as MetadataTypes); +export const isAngularLifecycleMethod = (value: string): value is AngularLifecycleMethodKeys => + ANGULAR_LIFECYCLE_METHODS.has(value as AngularLifecycleMethodKeys); export const getDeclaredInterfaces = (node: ClassDeclaration): NodeArray => { const heritageClause = createNodeArray(node.heritageClauses).find(h => h.token === SyntaxKind.ImplementsKeyword); @@ -194,16 +235,16 @@ export const getDeclaredInterfaceNames = (node: ClassDeclaration): string[] => g export const getDeclaredInterfaceName = (node: ClassDeclaration, value: string): string | undefined => getDeclaredInterfaceNames(node).find(interfaceName => interfaceName === value); -export const getDeclaredLifecycleInterfaces = (node: ClassDeclaration): ReadonlyArray => - getDeclaredInterfaceNames(node).filter(isLifecycleInterface) as ReadonlyArray; +export const getDeclaredAngularLifecycleInterfaces = (node: ClassDeclaration): ReadonlyArray => + getDeclaredInterfaceNames(node).filter(isAngularLifecycleInterface) as ReadonlyArray; -export const getLifecycleInterfaceByMethodName = (methodName: LifecycleMethodKeys): LifecycleInterfaceKeys => - methodName.slice(2) as LifecycleInterfaceKeys; +export const getLifecycleInterfaceByMethodName = (methodName: AngularLifecycleMethodKeys): AngularLifecycleInterfaceKeys => + methodName.slice(2) as AngularLifecycleInterfaceKeys; -export const getDeclaredLifecycleMethods = (node: ClassDeclaration): ReadonlyArray => +export const getDeclaredAngularLifecycleMethods = (node: ClassDeclaration): ReadonlyArray => getDeclaredMethods(node) - .map(m => m.name.getText()) - .filter(isLifecycleMethod) as ReadonlyArray; + .map(method => method.name.getText()) + .filter(isAngularLifecycleMethod) as ReadonlyArray; export const kebabToCamelCase = (value: string) => value.replace(/-[a-zA-Z]/g, x => x[1].toUpperCase()); diff --git a/test/contextualDecoratorRule.spec.ts b/test/contextualDecoratorRule.spec.ts index 6d23c3175..5601a516a 100644 --- a/test/contextualDecoratorRule.spec.ts +++ b/test/contextualDecoratorRule.spec.ts @@ -1,5 +1,5 @@ import { getFailureMessage, Rule } from '../src/contextualDecoratorRule'; -import { Decorators, MetadataTypes } from '../src/util/utils'; +import { AngularClassDecorators, AngularInnerClassDecorators } from '../src/util/utils'; import { assertAnnotated, assertSuccess } from './testHelper'; const { @@ -19,9 +19,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Injectable, className: 'Test', - decoratorName: Decorators.ContentChild, - metadataType: MetadataTypes.Injectable + innerClassDecoratorName: AngularInnerClassDecorators.ContentChild }), ruleName, source @@ -38,9 +38,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Injectable, className: 'Test', - decoratorName: Decorators.ContentChildren, - metadataType: MetadataTypes.Injectable + innerClassDecoratorName: AngularInnerClassDecorators.ContentChildren }), ruleName, source @@ -57,9 +57,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Injectable, className: 'Test', - decoratorName: Decorators.HostBinding, - metadataType: MetadataTypes.Injectable + innerClassDecoratorName: AngularInnerClassDecorators.HostBinding }), ruleName, source @@ -79,9 +79,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Injectable, className: 'Test', - decoratorName: Decorators.HostListener, - metadataType: MetadataTypes.Injectable + innerClassDecoratorName: AngularInnerClassDecorators.HostListener }), ruleName, source @@ -98,9 +98,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Injectable, className: 'Test', - decoratorName: Decorators.Input, - metadataType: MetadataTypes.Injectable + innerClassDecoratorName: AngularInnerClassDecorators.Input }), ruleName, source @@ -117,9 +117,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Injectable, className: 'Test', - decoratorName: Decorators.Output, - metadataType: MetadataTypes.Injectable + innerClassDecoratorName: AngularInnerClassDecorators.Output }), ruleName, source @@ -136,9 +136,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Injectable, className: 'Test', - decoratorName: Decorators.ViewChild, - metadataType: MetadataTypes.Injectable + innerClassDecoratorName: AngularInnerClassDecorators.ViewChild }), ruleName, source @@ -155,9 +155,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Injectable, className: 'Test', - decoratorName: Decorators.ViewChildren, - metadataType: MetadataTypes.Injectable + innerClassDecoratorName: AngularInnerClassDecorators.ViewChildren }), ruleName, source @@ -176,9 +176,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.NgModule, className: 'Test', - decoratorName: Decorators.ContentChild, - metadataType: MetadataTypes.NgModule + innerClassDecoratorName: AngularInnerClassDecorators.ContentChild }), ruleName, source @@ -195,9 +195,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.NgModule, className: 'Test', - decoratorName: Decorators.ContentChildren, - metadataType: MetadataTypes.NgModule + innerClassDecoratorName: AngularInnerClassDecorators.ContentChildren }), ruleName, source @@ -214,9 +214,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.NgModule, className: 'Test', - decoratorName: Decorators.HostBinding, - metadataType: MetadataTypes.NgModule + innerClassDecoratorName: AngularInnerClassDecorators.HostBinding }), ruleName, source @@ -236,9 +236,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.NgModule, className: 'Test', - decoratorName: Decorators.HostListener, - metadataType: MetadataTypes.NgModule + innerClassDecoratorName: AngularInnerClassDecorators.HostListener }), ruleName, source @@ -255,9 +255,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.NgModule, className: 'Test', - decoratorName: Decorators.Input, - metadataType: MetadataTypes.NgModule + innerClassDecoratorName: AngularInnerClassDecorators.Input }), ruleName, source @@ -274,9 +274,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.NgModule, className: 'Test', - decoratorName: Decorators.Output, - metadataType: MetadataTypes.NgModule + innerClassDecoratorName: AngularInnerClassDecorators.Output }), ruleName, source @@ -293,9 +293,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.NgModule, className: 'Test', - decoratorName: Decorators.ViewChild, - metadataType: MetadataTypes.NgModule + innerClassDecoratorName: AngularInnerClassDecorators.ViewChild }), ruleName, source @@ -312,9 +312,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.NgModule, className: 'Test', - decoratorName: Decorators.ViewChildren, - metadataType: MetadataTypes.NgModule + innerClassDecoratorName: AngularInnerClassDecorators.ViewChildren }), ruleName, source @@ -333,9 +333,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Pipe, className: 'Test', - decoratorName: Decorators.ContentChild, - metadataType: MetadataTypes.Pipe + innerClassDecoratorName: AngularInnerClassDecorators.ContentChild }), ruleName, source @@ -352,9 +352,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Pipe, className: 'Test', - decoratorName: Decorators.ContentChildren, - metadataType: MetadataTypes.Pipe + innerClassDecoratorName: AngularInnerClassDecorators.ContentChildren }), ruleName, source @@ -371,9 +371,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Pipe, className: 'Test', - decoratorName: Decorators.HostBinding, - metadataType: MetadataTypes.Pipe + innerClassDecoratorName: AngularInnerClassDecorators.HostBinding }), ruleName, source @@ -393,9 +393,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Pipe, className: 'Test', - decoratorName: Decorators.HostListener, - metadataType: MetadataTypes.Pipe + innerClassDecoratorName: AngularInnerClassDecorators.HostListener }), ruleName, source @@ -412,9 +412,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Pipe, className: 'Test', - decoratorName: Decorators.Input, - metadataType: MetadataTypes.Pipe + innerClassDecoratorName: AngularInnerClassDecorators.Input }), ruleName, source @@ -431,9 +431,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Pipe, className: 'Test', - decoratorName: Decorators.Output, - metadataType: MetadataTypes.Pipe + innerClassDecoratorName: AngularInnerClassDecorators.Output }), ruleName, source @@ -450,9 +450,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Pipe, className: 'Test', - decoratorName: Decorators.ViewChild, - metadataType: MetadataTypes.Pipe + innerClassDecoratorName: AngularInnerClassDecorators.ViewChild }), ruleName, source @@ -469,9 +469,9 @@ describe(ruleName, () => { `; assertAnnotated({ message: getFailureMessage({ + classDecoratorName: AngularClassDecorators.Pipe, className: 'Test', - decoratorName: Decorators.ViewChildren, - metadataType: MetadataTypes.Pipe + innerClassDecoratorName: AngularInnerClassDecorators.ViewChildren }), ruleName, source @@ -494,9 +494,9 @@ describe(ruleName, () => { } `; const message = getFailureMessage({ + classDecoratorName: AngularClassDecorators.Pipe, className: 'Test', - decoratorName: Decorators.Input, - metadataType: MetadataTypes.Pipe + innerClassDecoratorName: AngularInnerClassDecorators.Input }); assertAnnotated({ message, @@ -508,9 +508,9 @@ describe(ruleName, () => { }); describe('success', () => { - describe('Component', () => { + describe('', () => { it('should succeed if a property is decorated with @ContentChild() decorator', () => { - const source = ` + const source = `Component @Component() class Test { @ContentChild(Pane) pane: Pane; diff --git a/test/contextualLifecycleRule.spec.ts b/test/contextualLifecycleRule.spec.ts index bc40c305d..ffe271975 100644 --- a/test/contextualLifecycleRule.spec.ts +++ b/test/contextualLifecycleRule.spec.ts @@ -1,5 +1,5 @@ import { getFailureMessage, Rule } from '../src/contextualLifecycleRule'; -import { LifecycleMethods, MetadataTypes } from '../src/util/utils'; +import { AngularClassDecorators, AngularLifecycleMethods } from '../src/util/utils'; import { assertAnnotated, assertSuccess } from './testHelper'; const { @@ -19,8 +19,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Injectable, - methodName: LifecycleMethods.ngAfterContentChecked + decoratorName: AngularClassDecorators.Injectable, + methodName: AngularLifecycleMethods.ngAfterContentChecked }); assertAnnotated({ message, @@ -39,8 +39,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Injectable, - methodName: LifecycleMethods.ngAfterContentInit + decoratorName: AngularClassDecorators.Injectable, + methodName: AngularLifecycleMethods.ngAfterContentInit }); assertAnnotated({ message, @@ -59,8 +59,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Injectable, - methodName: LifecycleMethods.ngAfterViewChecked + decoratorName: AngularClassDecorators.Injectable, + methodName: AngularLifecycleMethods.ngAfterViewChecked }); assertAnnotated({ message, @@ -79,8 +79,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Injectable, - methodName: LifecycleMethods.ngAfterViewInit + decoratorName: AngularClassDecorators.Injectable, + methodName: AngularLifecycleMethods.ngAfterViewInit }); assertAnnotated({ message, @@ -99,8 +99,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Injectable, - methodName: LifecycleMethods.ngDoCheck + decoratorName: AngularClassDecorators.Injectable, + methodName: AngularLifecycleMethods.ngDoCheck }); assertAnnotated({ message, @@ -119,8 +119,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Injectable, - methodName: LifecycleMethods.ngOnChanges + decoratorName: AngularClassDecorators.Injectable, + methodName: AngularLifecycleMethods.ngOnChanges }); assertAnnotated({ message, @@ -139,8 +139,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Injectable, - methodName: LifecycleMethods.ngOnInit + decoratorName: AngularClassDecorators.Injectable, + methodName: AngularLifecycleMethods.ngOnInit }); assertAnnotated({ message, @@ -161,8 +161,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.NgModule, - methodName: LifecycleMethods.ngAfterContentChecked + decoratorName: AngularClassDecorators.NgModule, + methodName: AngularLifecycleMethods.ngAfterContentChecked }); assertAnnotated({ message, @@ -181,8 +181,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.NgModule, - methodName: LifecycleMethods.ngAfterContentInit + decoratorName: AngularClassDecorators.NgModule, + methodName: AngularLifecycleMethods.ngAfterContentInit }); assertAnnotated({ message, @@ -201,8 +201,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.NgModule, - methodName: LifecycleMethods.ngAfterViewChecked + decoratorName: AngularClassDecorators.NgModule, + methodName: AngularLifecycleMethods.ngAfterViewChecked }); assertAnnotated({ message, @@ -221,8 +221,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.NgModule, - methodName: LifecycleMethods.ngAfterViewInit + decoratorName: AngularClassDecorators.NgModule, + methodName: AngularLifecycleMethods.ngAfterViewInit }); assertAnnotated({ message, @@ -241,8 +241,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.NgModule, - methodName: LifecycleMethods.ngDoCheck + decoratorName: AngularClassDecorators.NgModule, + methodName: AngularLifecycleMethods.ngDoCheck }); assertAnnotated({ message, @@ -261,8 +261,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.NgModule, - methodName: LifecycleMethods.ngOnChanges + decoratorName: AngularClassDecorators.NgModule, + methodName: AngularLifecycleMethods.ngOnChanges }); assertAnnotated({ message, @@ -281,8 +281,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.NgModule, - methodName: LifecycleMethods.ngOnInit + decoratorName: AngularClassDecorators.NgModule, + methodName: AngularLifecycleMethods.ngOnInit }); assertAnnotated({ message, @@ -301,8 +301,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.NgModule, - methodName: LifecycleMethods.ngOnDestroy + decoratorName: AngularClassDecorators.NgModule, + methodName: AngularLifecycleMethods.ngOnDestroy }); assertAnnotated({ message, @@ -323,8 +323,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Pipe, - methodName: LifecycleMethods.ngAfterContentChecked + decoratorName: AngularClassDecorators.Pipe, + methodName: AngularLifecycleMethods.ngAfterContentChecked }); assertAnnotated({ message, @@ -343,8 +343,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Pipe, - methodName: LifecycleMethods.ngAfterContentInit + decoratorName: AngularClassDecorators.Pipe, + methodName: AngularLifecycleMethods.ngAfterContentInit }); assertAnnotated({ message, @@ -363,8 +363,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Pipe, - methodName: LifecycleMethods.ngAfterViewChecked + decoratorName: AngularClassDecorators.Pipe, + methodName: AngularLifecycleMethods.ngAfterViewChecked }); assertAnnotated({ message, @@ -383,8 +383,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Pipe, - methodName: LifecycleMethods.ngAfterViewInit + decoratorName: AngularClassDecorators.Pipe, + methodName: AngularLifecycleMethods.ngAfterViewInit }); assertAnnotated({ message, @@ -403,8 +403,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Pipe, - methodName: LifecycleMethods.ngDoCheck + decoratorName: AngularClassDecorators.Pipe, + methodName: AngularLifecycleMethods.ngDoCheck }); assertAnnotated({ message, @@ -423,8 +423,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Pipe, - methodName: LifecycleMethods.ngOnChanges + decoratorName: AngularClassDecorators.Pipe, + methodName: AngularLifecycleMethods.ngOnChanges }); assertAnnotated({ message, @@ -443,8 +443,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Pipe, - methodName: LifecycleMethods.ngOnInit + decoratorName: AngularClassDecorators.Pipe, + methodName: AngularLifecycleMethods.ngOnInit }); assertAnnotated({ message, @@ -475,8 +475,8 @@ describe(ruleName, () => { `; const message = getFailureMessage({ className: 'Test', - metadataType: MetadataTypes.Pipe, - methodName: LifecycleMethods.ngDoCheck + decoratorName: AngularClassDecorators.Pipe, + methodName: AngularLifecycleMethods.ngDoCheck }); assertAnnotated({ message, diff --git a/test/useLifecycleInterfaceRule.spec.ts b/test/useLifecycleInterfaceRule.spec.ts index 043dd1ac8..68a0d25c4 100644 --- a/test/useLifecycleInterfaceRule.spec.ts +++ b/test/useLifecycleInterfaceRule.spec.ts @@ -1,5 +1,5 @@ import { getFailureMessage, Rule } from '../src/useLifecycleInterfaceRule'; -import { LifecycleInterfaces, LifecycleMethods } from '../src/util/utils'; +import { AngularLifecycleInterfaces, AngularLifecycleMethods } from '../src/util/utils'; import { assertAnnotated, assertFailures, assertSuccess } from './testHelper'; const { @@ -17,8 +17,8 @@ describe(ruleName, () => { } `; const message = getFailureMessage({ - interfaceName: LifecycleInterfaces.OnInit, - methodName: LifecycleMethods.ngOnInit + interfaceName: AngularLifecycleInterfaces.OnInit, + methodName: AngularLifecycleMethods.ngOnInit }); assertAnnotated({ message, @@ -38,8 +38,8 @@ describe(ruleName, () => { } `; const message = getFailureMessage({ - interfaceName: LifecycleInterfaces.OnDestroy, - methodName: LifecycleMethods.ngOnDestroy + interfaceName: AngularLifecycleInterfaces.OnDestroy, + methodName: AngularLifecycleMethods.ngOnDestroy }); assertAnnotated({ message, @@ -63,8 +63,8 @@ describe(ruleName, () => { line: 2 }, message: getFailureMessage({ - interfaceName: LifecycleInterfaces.OnInit, - methodName: LifecycleMethods.ngOnInit + interfaceName: AngularLifecycleInterfaces.OnInit, + methodName: AngularLifecycleMethods.ngOnInit }), startPosition: { character: 10, @@ -77,8 +77,8 @@ describe(ruleName, () => { line: 4 }, message: getFailureMessage({ - interfaceName: LifecycleInterfaces.OnDestroy, - methodName: LifecycleMethods.ngOnDestroy + interfaceName: AngularLifecycleInterfaces.OnDestroy, + methodName: AngularLifecycleMethods.ngOnDestroy }), startPosition: { character: 10, @@ -99,8 +99,8 @@ describe(ruleName, () => { } `; const message = getFailureMessage({ - interfaceName: LifecycleInterfaces.OnDestroy, - methodName: LifecycleMethods.ngOnDestroy + interfaceName: AngularLifecycleInterfaces.OnDestroy, + methodName: AngularLifecycleMethods.ngOnDestroy }); assertAnnotated({ message, From 38a8ccf1e4ed905ddad535a00d6e084f5245ebf2 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sat, 23 Mar 2019 22:38:01 -0300 Subject: [PATCH 04/15] refactor: order tslint.json --- tslint.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tslint.json b/tslint.json index 8725dd64b..e72eeed06 100644 --- a/tslint.json +++ b/tslint.json @@ -2,6 +2,7 @@ "rules": { "class-name": true, "deprecation": true, + "file-name-casing": [true, "camel-case"], "no-console": [true, "debug", "log", "info", "time", "timeEnd", "trace"], "member-ordering": [ true, @@ -31,7 +32,6 @@ "no-eval": true, "no-unused-expression": true, "no-var-keyword": true, - "triple-equals": true, - "file-name-casing": [true, "camel-case"] + "triple-equals": true } } From b5518bbf9eafba9f4c7c06bb8d815c741d5f8b05 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sat, 23 Mar 2019 22:52:03 -0300 Subject: [PATCH 05/15] fix: minor fix --- src/contextualDecoratorRule.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/contextualDecoratorRule.ts b/src/contextualDecoratorRule.ts index ce7354d7f..9497449ce 100644 --- a/src/contextualDecoratorRule.ts +++ b/src/contextualDecoratorRule.ts @@ -35,7 +35,8 @@ export class Rule extends AbstractRule { description: 'Ensures that classes use allowed decorator in its body.', options: null, optionsDescription: 'Not configurable.', - rationale: dedent`Some decorators can only be used in certain class types. + rationale: dedent` + Some decorators can only be used in certain class types. For example, an @${AngularInnerClassDecorators.Input} should not be used in an @${AngularClassDecorators.Injectable} class. `, From 2c6a57cb454f06ece3a90d974d0245fd9412f6d8 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sun, 24 Mar 2019 02:29:20 -0300 Subject: [PATCH 06/15] fix: add missing decorators for 'NgModule' --- src/util/utils.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/util/utils.ts b/src/util/utils.ts index ca22f6a86..bb03c98b5 100644 --- a/src/util/utils.ts +++ b/src/util/utils.ts @@ -109,7 +109,16 @@ export const ANGULAR_CLASS_DECORATOR_MAPPER: ReadonlyMap Date: Sun, 24 Mar 2019 02:31:28 -0300 Subject: [PATCH 07/15] fix: parseInvalidSource method not handling other chars --- src/util/escapeRegexp.ts | 1 + test/testHelper.ts | 26 ++++++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) create mode 100644 src/util/escapeRegexp.ts diff --git a/src/util/escapeRegexp.ts b/src/util/escapeRegexp.ts new file mode 100644 index 000000000..2a62bbe4a --- /dev/null +++ b/src/util/escapeRegexp.ts @@ -0,0 +1 @@ +export const escapeRegexp = (value: string) => value.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&'); diff --git a/test/testHelper.ts b/test/testHelper.ts index 111f04aae..077da18df 100644 --- a/test/testHelper.ts +++ b/test/testHelper.ts @@ -1,6 +1,7 @@ import { assert } from 'chai'; import { ILinterOptions, IOptions, IRuleMetadata, Linter, LintResult, RuleFailure } from 'tslint/lib'; import { SourceFile } from 'typescript/lib/typescript'; +import { escapeRegexp } from '../src/util/escapeRegexp'; import { convertRuleOptions, loadRules } from './utils'; interface Failure { @@ -112,17 +113,28 @@ const lint = (ruleName: AssertConfig['ruleName'], source: string | SourceFile, r const parseInvalidSource = ( source: AssertConfig['source'], message: ExpectedFailure['message'], - specialChar = '~' + specialChar = '~', + otherChars: string[] = [] ): { failure: ExpectedFailure; source: AssertConfig['source'] } => { + let replacedSource: string; + + if (otherChars.length > 0) { + const patternAsStr = `[${otherChars.map(escapeRegexp).join('')}]`; + const pattern = new RegExp(patternAsStr, 'g'); + replacedSource = source.replace(pattern, ''); + } else { + replacedSource = source; + } + let startPosition; let line = 0; let col = 0; let lastCol = 0; let lastLine = 0; - for (let i = 0; i < source.length; i += 1) { - const currentSource = source[i]; - const previousSource = source[i - 1]; + for (let i = 0; i < replacedSource.length; i += 1) { + const currentSource = replacedSource[i]; + const previousSource = replacedSource[i - 1]; if (currentSource === specialChar && previousSource !== '/' && startPosition === undefined) { startPosition = { @@ -148,7 +160,7 @@ const parseInvalidSource = ( character: lastCol, line: lastLine }; - const newSource = source.replace(new RegExp(specialChar, 'g'), ''); + const newSource = replacedSource.replace(new RegExp(escapeRegexp(specialChar), 'g'), ''); return { failure: { @@ -186,7 +198,9 @@ export const assertMultipleAnnotated = (configs: AssertMultipleConfigs): RuleFai const { failures, options, ruleName, source } = configs; return failures.reduce((previousValue, currentValue, index) => { - const { failure: parsedFailure, source: parsedSource } = parseInvalidSource(source, currentValue.msg, currentValue.char); + const { msg: currentValueMsg, char: currentValueChar } = currentValue; + const otherCharacters = failures.map(failure => failure.char).filter(char => char !== currentValueChar); + const { failure: parsedFailure, source: parsedSource } = parseInvalidSource(source, currentValueMsg, currentValueChar, otherCharacters); const { character: parsedFailureEndChar, line: parsedFailureEndLine } = parsedFailure.endPosition; const { character: parsedFailureStartChar, line: parsedFailureStartLine } = parsedFailure.startPosition; From 75ab9376945f3414bb69732f23389c3d64647833 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sun, 24 Mar 2019 03:01:47 -0300 Subject: [PATCH 08/15] fixup... --- src/util/escapeRegexp.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/escapeRegexp.ts b/src/util/escapeRegexp.ts index 2a62bbe4a..a001b260b 100644 --- a/src/util/escapeRegexp.ts +++ b/src/util/escapeRegexp.ts @@ -1 +1 @@ -export const escapeRegexp = (value: string) => value.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&'); +export const escapeRegexp = (value: string): string => value.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&'); From cd5460ec9e67019b6fc067551382e8469de4872a Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sun, 24 Mar 2019 03:06:05 -0300 Subject: [PATCH 09/15] fixup... --- test/testHelper.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/testHelper.ts b/test/testHelper.ts index 077da18df..0aacbc63e 100644 --- a/test/testHelper.ts +++ b/test/testHelper.ts @@ -199,8 +199,8 @@ export const assertMultipleAnnotated = (configs: AssertMultipleConfigs): RuleFai return failures.reduce((previousValue, currentValue, index) => { const { msg: currentValueMsg, char: currentValueChar } = currentValue; - const otherCharacters = failures.map(failure => failure.char).filter(char => char !== currentValueChar); - const { failure: parsedFailure, source: parsedSource } = parseInvalidSource(source, currentValueMsg, currentValueChar, otherCharacters); + const otherChars = failures.map(failure => failure.char).filter(char => char !== currentValueChar); + const { failure: parsedFailure, source: parsedSource } = parseInvalidSource(source, currentValueMsg, currentValueChar, otherChars); const { character: parsedFailureEndChar, line: parsedFailureEndLine } = parsedFailure.endPosition; const { character: parsedFailureStartChar, line: parsedFailureStartLine } = parsedFailure.startPosition; From 0006bf90da5f917fd21712d400ff2e1364d743c8 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sun, 24 Mar 2019 19:21:27 -0300 Subject: [PATCH 10/15] fix: remove unnecessary export --- src/templateUseTrackByFunctionRule.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/templateUseTrackByFunctionRule.ts b/src/templateUseTrackByFunctionRule.ts index b76608824..cd3690b9e 100644 --- a/src/templateUseTrackByFunctionRule.ts +++ b/src/templateUseTrackByFunctionRule.ts @@ -5,10 +5,9 @@ import { SourceFile } from 'typescript/lib/typescript'; import { NgWalker, NgWalkerConfig } from './angular/ngWalker'; import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; -// current offset into the template -export let currentOffset = 0; - const PATTERN = /\s*ngFor.*\s*trackBy\s*:|\[ngForTrackBy\]\s*=\s*['"].*['"]/; +// current offset into the template +let currentOffset = 0; export class Rule extends AbstractRule { static readonly metadata: IRuleMetadata = { From fa7d9ba9695fc7ec8b417d17137b746082edc592 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sun, 24 Mar 2019 19:27:27 -0300 Subject: [PATCH 11/15] refactor: improve testHelper --- test/testHelper.ts | 74 ++++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/test/testHelper.ts b/test/testHelper.ts index 0aacbc63e..6e2ad8880 100644 --- a/test/testHelper.ts +++ b/test/testHelper.ts @@ -118,42 +118,42 @@ const parseInvalidSource = ( ): { failure: ExpectedFailure; source: AssertConfig['source'] } => { let replacedSource: string; - if (otherChars.length > 0) { + if (otherChars.length === 0) { + replacedSource = source; + } else { const patternAsStr = `[${otherChars.map(escapeRegexp).join('')}]`; const pattern = new RegExp(patternAsStr, 'g'); - replacedSource = source.replace(pattern, ''); - } else { - replacedSource = source; + + replacedSource = source.replace(pattern, ' '); } - let startPosition; - let line = 0; let col = 0; + let line = 0; let lastCol = 0; let lastLine = 0; + let startPosition; - for (let i = 0; i < replacedSource.length; i += 1) { - const currentSource = replacedSource[i]; - const previousSource = replacedSource[i - 1]; + for (const currentChar of replacedSource) { + if (currentChar === '\n') { + col = 0; + line++; - if (currentSource === specialChar && previousSource !== '/' && startPosition === undefined) { + continue; + } + + col++; + + if (currentChar !== specialChar) continue; + + if (!startPosition) { startPosition = { - character: col, + character: col - 1, line: line - 1 }; } - if (currentSource === '\n') { - col = 0; - line += 1; - } else { - col += 1; - } - - if (currentSource === specialChar && previousSource !== '/') { - lastCol = col; - lastLine = line - 1; - } + lastCol = col; + lastLine = line - 1; } const endPosition: SourcePosition = { @@ -181,13 +181,11 @@ const parseInvalidSource = ( export const assertAnnotated = (config: AssertConfig): RuleFailure[] | void => { const { message, options, ruleName, source: sourceConfig } = config; - if (message) { - const { failure, source } = parseInvalidSource(sourceConfig, message); + if (!message) return assertSuccess(ruleName, sourceConfig, options); - return assertFailure(ruleName, source, failure, options); - } + const { failure, source } = parseInvalidSource(sourceConfig, message); - return assertSuccess(ruleName, sourceConfig, options); + return assertFailure(ruleName, source, failure, options); }; /** @@ -220,6 +218,12 @@ export const assertMultipleAnnotated = (configs: AssertMultipleConfigs): RuleFai }, []); }; +const assertFail = (expectedFailure: ExpectedFailure, ruleFailure: RuleFailure): void => { + assert.equal(expectedFailure.message, ruleFailure.getFailure(), "error messages don't match"); + assert.deepEqual(expectedFailure.startPosition, ruleFailure.getStartPosition().getLineAndCharacter(), "start char doesn't match"); + assert.deepEqual(expectedFailure.endPosition, ruleFailure.getEndPosition().getLineAndCharacter(), "end char doesn't match"); +}; + /** * A helper function used in specs to assert a failure (meaning that the code contains a lint error). * Consider using `assertAnnotated` instead. @@ -235,7 +239,7 @@ export const assertMultipleAnnotated = (configs: AssertMultipleConfigs): RuleFai export const assertFailure = ( ruleName: AssertConfig['ruleName'], source: string | SourceFile, - fail: ExpectedFailure, + expectedFailure: ExpectedFailure, options?: AssertConfig['options'], onlyNthFailure = 0 ): RuleFailure[] => { @@ -243,11 +247,9 @@ export const assertFailure = ( assert(result.failures.length > 0, 'no failures'); - const ruleFail = result.failures[onlyNthFailure]; + const ruleFailure = result.failures[onlyNthFailure]; - assert.equal(fail.message, ruleFail.getFailure(), "error messages don't match"); - assert.deepEqual(fail.startPosition, ruleFail.getStartPosition().getLineAndCharacter(), "start char doesn't match"); - assert.deepEqual(fail.endPosition, ruleFail.getEndPosition().getLineAndCharacter(), "end char doesn't match"); + assertFail(expectedFailure, ruleFailure); return result.failures; }; @@ -264,18 +266,14 @@ export const assertFailure = ( export const assertFailures = ( ruleName: AssertConfig['ruleName'], source: string | SourceFile, - fails: ExpectedFailure[], + expectedFailures: ExpectedFailure[], options?: AssertConfig['options'] ): void => { const result = lint(ruleName, source, options); assert(result.failures.length > 0, 'no failures'); - result.failures.forEach((ruleFail, index) => { - assert.equal(fails[index].message, ruleFail.getFailure(), "error messages don't match"); - assert.deepEqual(fails[index].startPosition, ruleFail.getStartPosition().getLineAndCharacter(), "start char doesn't match"); - assert.deepEqual(fails[index].endPosition, ruleFail.getEndPosition().getLineAndCharacter(), "end char doesn't match"); - }); + result.failures.forEach((ruleFailure, index) => assertFail(expectedFailures[index], ruleFailure)); }; /** From 99bc3aa5c2915b3cb9a97ec295a0e8e7c9c58d12 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Tue, 26 Mar 2019 19:09:17 -0300 Subject: [PATCH 12/15] fix: minor fix --- test/contextualDecoratorRule.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/contextualDecoratorRule.spec.ts b/test/contextualDecoratorRule.spec.ts index 5601a516a..0746beb75 100644 --- a/test/contextualDecoratorRule.spec.ts +++ b/test/contextualDecoratorRule.spec.ts @@ -510,7 +510,7 @@ describe(ruleName, () => { describe('success', () => { describe('', () => { it('should succeed if a property is decorated with @ContentChild() decorator', () => { - const source = `Component + const source = ` @Component() class Test { @ContentChild(Pane) pane: Pane; From 4ce0c44a52737888446d8bd71b13d7ad9957c0f1 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sat, 30 Mar 2019 15:35:19 -0300 Subject: [PATCH 13/15] fix: adjust file-name-casing --- src/preferInlineDecoratorRule.ts | 2 +- src/util/object-keys.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) delete mode 100644 src/util/object-keys.ts diff --git a/src/preferInlineDecoratorRule.ts b/src/preferInlineDecoratorRule.ts index bf35d7486..517a79c15 100644 --- a/src/preferInlineDecoratorRule.ts +++ b/src/preferInlineDecoratorRule.ts @@ -21,7 +21,7 @@ import { SourceFile } from 'typescript'; import { isNotNullOrUndefined } from './util/isNotNullOrUndefined'; -import { objectKeys } from './util/object-keys'; +import { objectKeys } from './util/objectKeys'; import { AngularInnerClassDecorators, getDecoratorName, isSameLine } from './util/utils'; const OPTION_GETTERS = 'getters'; diff --git a/src/util/object-keys.ts b/src/util/object-keys.ts deleted file mode 100644 index faeb007d8..000000000 --- a/src/util/object-keys.ts +++ /dev/null @@ -1 +0,0 @@ -export const objectKeys = Object.keys as (o: T) => ReadonlyArray>; From 3e5bf6e34965e27ea0ac7e643c3f59017024d40d Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sun, 7 Apr 2019 20:56:40 -0300 Subject: [PATCH 14/15] fix: minor fixes --- src/usePipeDecoratorRule.ts | 4 +--- src/usePipeTransformInterfaceRule.ts | 4 +--- test/contextualDecoratorRule.spec.ts | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/usePipeDecoratorRule.ts b/src/usePipeDecoratorRule.ts index 4bc39b002..e92b4ef8c 100644 --- a/src/usePipeDecoratorRule.ts +++ b/src/usePipeDecoratorRule.ts @@ -30,10 +30,8 @@ export class Rule extends AbstractRule { } } -const hasPipeTransformInterface = (node: ClassDeclaration): boolean => !!getDeclaredInterfaceName(node, PIPE_TRANSFORM); - const validateClassDeclaration = (context: WalkContext, node: ClassDeclaration): void => { - if (!hasPipeTransformInterface(node) || getPipeDecorator(node)) return; + if (getPipeDecorator(node) || !getDeclaredInterfaceName(node, PIPE_TRANSFORM)) return; context.addFailureAtNode(node, sprintf(Rule.FAILURE_STRING)); }; diff --git a/src/usePipeTransformInterfaceRule.ts b/src/usePipeTransformInterfaceRule.ts index e793d1f84..5c3a5647e 100644 --- a/src/usePipeTransformInterfaceRule.ts +++ b/src/usePipeTransformInterfaceRule.ts @@ -28,10 +28,8 @@ export class Rule extends AbstractRule { } } -const hasPipeTransformInterface = (node: ClassDeclaration): boolean => !!getDeclaredInterfaceName(node, PIPE_TRANSFORM); - const validateClassDeclaration = (context: WalkContext, node: ClassDeclaration): void => { - if (!getPipeDecorator(node) || hasPipeTransformInterface(node)) return; + if (!getPipeDecorator(node) || getDeclaredInterfaceName(node, PIPE_TRANSFORM)) return; context.addFailureAtNode(node, sprintf(Rule.FAILURE_STRING)); }; diff --git a/test/contextualDecoratorRule.spec.ts b/test/contextualDecoratorRule.spec.ts index 0746beb75..32ab9c798 100644 --- a/test/contextualDecoratorRule.spec.ts +++ b/test/contextualDecoratorRule.spec.ts @@ -508,7 +508,7 @@ describe(ruleName, () => { }); describe('success', () => { - describe('', () => { + describe('Component', () => { it('should succeed if a property is decorated with @ContentChild() decorator', () => { const source = ` @Component() From 0a98bb47f62c2d781d7a4eebb9216ef954d868b1 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Sun, 14 Apr 2019 15:00:37 -0300 Subject: [PATCH 15/15] refactor: change decorator constant name --- src/util/utils.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/utils.ts b/src/util/utils.ts index bb03c98b5..8de69a748 100644 --- a/src/util/utils.ts +++ b/src/util/utils.ts @@ -33,11 +33,7 @@ export enum AngularClassDecorators { Pipe = 'Pipe' } -enum AngularMethodDecorators { - HostListener = 'HostListener' -} - -enum AngularParameterPropertyDecorators { +enum AngularConstructorParameterDecorators { Attribute = 'Attribute', Host = 'Host', Inject = 'Inject', @@ -46,6 +42,10 @@ enum AngularParameterPropertyDecorators { SkipSelf = 'SkipSelf' } +enum AngularMethodDecorators { + HostListener = 'HostListener' +} + enum AngularPropertyAccessorDecorators { ContentChild = 'ContentChild', ContentChildren = 'ContentChildren', @@ -57,8 +57,8 @@ enum AngularPropertyAccessorDecorators { } export const AngularInnerClassDecorators = { + ...AngularConstructorParameterDecorators, ...AngularMethodDecorators, - ...AngularParameterPropertyDecorators, ...AngularPropertyAccessorDecorators };