diff --git a/src/bananaInBoxRule.ts b/src/bananaInBoxRule.ts deleted file mode 100644 index 75d1869f3..000000000 --- a/src/bananaInBoxRule.ts +++ /dev/null @@ -1,70 +0,0 @@ -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import * as ast from '@angular/compiler'; -import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; -import { NgWalker } from './angular/ngWalker'; - -const InvalidSyntaxBoxOpen = '(['; -const InvalidSyntaxBoxClose = '])'; -const ValidSyntaxOpen = '[('; -const ValidSyntaxClose = ')]'; -const InvalidSyntaxBoxRe = /\[(.*?)\]/; - -const getReplacements = (text: ast.BoundEventAst, absolutePosition: number) => { - const expr = text.sourceSpan.toString(); - const internalStart = expr.indexOf(InvalidSyntaxBoxOpen); - const internalEnd = expr.lastIndexOf(InvalidSyntaxBoxClose); - const len = internalEnd - internalStart - InvalidSyntaxBoxClose.length; - const trimmed = expr.substr(internalStart + InvalidSyntaxBoxOpen.length, len).trim(); - - return [ - new Lint.Replacement( - absolutePosition, - internalEnd - internalStart + ValidSyntaxClose.length, - `${ValidSyntaxOpen}${trimmed}${ValidSyntaxClose}` - ) - ]; -}; - -class BananaInBoxTemplateVisitor extends BasicTemplateAstVisitor { - visitEvent(prop: ast.BoundEventAst, context: BasicTemplateAstVisitor): any { - if (prop.name) { - let error: string | null = null; - - if (InvalidSyntaxBoxRe.test(prop.name)) { - error = 'Invalid binding syntax. Use [(expr)] instead'; - } - - if (error) { - const expr = prop.sourceSpan.toString(); - const internalStart = expr.indexOf(InvalidSyntaxBoxOpen) + 1; - const start = prop.sourceSpan.start.offset + internalStart; - const absolutePosition = this.getSourcePosition(start - 1); - - this.addFailureAt(start, expr.trim().length, error, getReplacements(prop, absolutePosition)); - } - } - super.visitEvent(prop, context); - } -} - -export class Rule extends Lint.Rules.AbstractRule { - public static metadata: Lint.IRuleMetadata = { - ruleName: 'banana-in-box', - type: 'functionality', - description: 'Ensure that the two-way data binding syntax is correct.', - rationale: 'The parens "()" should have been inside the brackets "[]".', - options: null, - optionsDescription: 'Not configurable.', - typescriptOnly: true, - hasFix: true - }; - - public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker( - new NgWalker(sourceFile, this.getOptions(), { - templateVisitorCtrl: BananaInBoxTemplateVisitor - }) - ); - } -} diff --git a/src/contextualDecoratorRule.ts b/src/contextualDecoratorRule.ts new file mode 100644 index 000000000..efa3d246e --- /dev/null +++ b/src/contextualDecoratorRule.ts @@ -0,0 +1,82 @@ +import { sprintf } from 'sprintf-js'; +import { IRuleMetadata, RuleFailure } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { createNodeArray, Decorator, isClassDeclaration, SourceFile } from 'typescript'; +import { NgWalker } from './angular/ngWalker'; +import { + DecoratorKeys, + Decorators, + getDecoratorName, + getNextToLastParentNode, + isMetadataType, + isNgDecorator, + METADATA_TYPE_DECORATOR_MAPPER, + MetadataTypes +} from './util/utils'; + +interface FailureParameters { + readonly className: string; + readonly decoratorName: DecoratorKeys; + readonly metadataType: MetadataTypes; +} + +export const getFailureMessage = (failureParameters: FailureParameters): string => + sprintf(Rule.FAILURE_STRING, failureParameters.decoratorName, failureParameters.className, failureParameters.metadataType); + +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.`, + ruleName: 'contextual-decorator', + type: 'functionality', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = 'The decorator "%s" is not allowed for class "%s" because it is decorated with "%s"'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker(new ContextualDecoratorWalker(sourceFile, this.getOptions())); + } +} + +export class ContextualDecoratorWalker extends NgWalker { + protected visitMethodDecorator(decorator: Decorator): void { + this.validateDecorator(decorator); + super.visitMethodDecorator(decorator); + } + + protected visitPropertyDecorator(decorator: Decorator): void { + this.validateDecorator(decorator); + super.visitPropertyDecorator(decorator); + } + + private validateDecorator(decorator: Decorator): void { + const klass = getNextToLastParentNode(decorator); + + if (!isClassDeclaration(klass) || !klass.name) return; + + const metadataType = createNodeArray(klass.decorators) + .map(x => x.expression.getText()) + .map(x => x.replace(/[^a-zA-Z]/g, '')) + .find(isMetadataType); + + if (!metadataType) return; + + const decoratorName = getDecoratorName(decorator); + + if (!decoratorName || !isNgDecorator(decoratorName)) return; + + const allowedDecorators = METADATA_TYPE_DECORATOR_MAPPER[metadataType]; + + if (!allowedDecorators || allowedDecorators.has(decoratorName)) return; + + const className = klass.name.getText(); + const failure = getFailureMessage({ className, decoratorName, metadataType }); + + this.addFailureAtNode(decorator, failure); + } +} diff --git a/src/contextualLifeCycleRule.ts b/src/contextualLifeCycleRule.ts deleted file mode 100644 index cfe72f67e..000000000 --- a/src/contextualLifeCycleRule.ts +++ /dev/null @@ -1,134 +0,0 @@ -import { vsprintf } from 'sprintf-js'; -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { ComponentMetadata, DirectiveMetadata } from './angular/metadata'; -import { NgWalker } from './angular/ngWalker'; - -export class Rule extends Lint.Rules.AbstractRule { - static readonly metadata: Lint.IRuleMetadata = { - description: 'Ensure that classes use allowed life cycle method in its body.', - options: null, - optionsDescription: 'Not configurable.', - rationale: - 'Some life cycle methods can only be used in certain class types.For example, ngOnInit() hook method should not be used in an @Injectable class.', - ruleName: 'contextual-life-cycle', - type: 'functionality', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = 'In the class "%s" which have the "%s" decorator, the "%s" hook method is not allowed. Please, drop it.'; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new ClassMetadataWalker(sourceFile, this.getOptions())); - } -} - -export class ClassMetadataWalker extends NgWalker { - className!: string; - isInjectable = false; - isComponent = false; - isDirective = false; - isPipe = false; - - visitMethodDeclaration(method: ts.MethodDeclaration) { - const methodName = method.name.getText(); - - if (methodName === 'ngOnInit') { - if (this.isInjectable) { - this.generateFailure(method, this.className, '@Injectable', 'ngOnInit()'); - } else if (this.isPipe) { - this.generateFailure(method, this.className, '@Pipe', 'ngOnInit()'); - } - } - - if (methodName === 'ngOnChanges') { - if (this.isInjectable) { - this.generateFailure(method, this.className, '@Injectable', 'ngOnChanges()'); - } else if (this.isPipe) { - this.generateFailure(method, this.className, '@Pipe', 'ngOnChanges()'); - } - } - - if (methodName === 'ngDoCheck') { - if (this.isInjectable) { - this.generateFailure(method, this.className, '@Injectable', 'ngDoCheck()'); - } else if (this.isPipe) { - this.generateFailure(method, this.className, '@Pipe', 'ngDoCheck()'); - } - } - - if (methodName === 'ngAfterContentInit') { - if (this.isInjectable) { - this.generateFailure(method, this.className, '@Injectable', 'ngAfterContentInit()'); - } else if (this.isPipe) { - this.generateFailure(method, this.className, '@Pipe', 'ngAfterContentInit()'); - } - } - - if (methodName === 'ngAfterContentChecked') { - if (this.isInjectable) { - this.generateFailure(method, this.className, '@Injectable', 'ngAfterContentChecked()'); - } else if (this.isPipe) { - this.generateFailure(method, this.className, '@Pipe', 'ngAfterContentChecked()'); - } - } - - if (methodName === 'ngAfterViewInit') { - if (this.isInjectable) { - this.generateFailure(method, this.className, '@Injectable', 'ngAfterViewInit()'); - } else if (this.isPipe) { - this.generateFailure(method, this.className, '@Pipe', 'ngAfterViewInit()'); - } - } - - if (methodName === 'ngAfterViewChecked') { - if (this.isInjectable) { - this.generateFailure(method, this.className, '@Injectable', 'ngAfterViewChecked()'); - } else if (this.isPipe) { - this.generateFailure(method, this.className, '@Pipe', 'ngAfterViewChecked()'); - } - } - - super.visitMethodDeclaration(method); - } - - protected visitNgInjectable(controller: ts.ClassDeclaration, decorator: ts.Decorator) { - this.className = controller.name!.text; - this.isInjectable = true; - this.isComponent = false; - this.isDirective = false; - this.isPipe = false; - super.visitNgInjectable(controller, decorator); - } - - protected visitNgComponent(metadata: ComponentMetadata) { - this.className = metadata.controller.name!.text; - this.isComponent = true; - this.isInjectable = false; - this.isDirective = false; - this.isPipe = false; - super.visitNgComponent(metadata); - } - - protected visitNgDirective(metadata: DirectiveMetadata) { - this.className = metadata.controller.name!.text; - this.isDirective = true; - this.isInjectable = false; - this.isComponent = false; - this.isPipe = false; - super.visitNgDirective(metadata); - } - - protected visitNgPipe(controller: ts.ClassDeclaration, decorator: ts.Decorator) { - this.className = controller.name!.text; - this.isPipe = true; - this.isInjectable = false; - this.isComponent = false; - this.isDirective = false; - super.visitNgPipe(controller, decorator); - } - - private generateFailure(method: ts.MethodDeclaration, ...failureConfig: string[]) { - this.addFailureAtNode(method, vsprintf(Rule.FAILURE_STRING, failureConfig)); - } -} diff --git a/src/contextualLifecycleRule.ts b/src/contextualLifecycleRule.ts new file mode 100644 index 000000000..61c7313e1 --- /dev/null +++ b/src/contextualLifecycleRule.ts @@ -0,0 +1,81 @@ +import { sprintf } from 'sprintf-js'; +import { IRuleMetadata, RuleFailure } from 'tslint/lib'; +import { AbstractRule } from 'tslint/lib/rules'; +import { ClassDeclaration, Decorator, SourceFile } from 'typescript'; +import { NgWalker } from './angular/ngWalker'; +import { + getClassName, + getDecoratorName, + isLifecycleMethod, + isMetadataType, + LifecycleMethodKeys, + LifecycleMethods, + METADATA_TYPE_LIFECYCLE_MAPPER, + MetadataTypeKeys, + MetadataTypes +} from './util/utils'; + +interface FailureParameters { + readonly className: string; + readonly metadataType: MetadataTypeKeys; + readonly methodName: LifecycleMethodKeys; +} + +export const getFailureMessage = (failureParameters: FailureParameters): string => + sprintf(Rule.FAILURE_STRING, failureParameters.methodName, failureParameters.className, failureParameters.metadataType); + +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.`, + ruleName: 'contextual-lifecycle', + type: 'functionality', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = 'The method "%s" is not allowed for class "%s" because it is decorated with "%s"'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker(new ContextualLifecycleWalker(sourceFile, this.getOptions())); + } +} + +class ContextualLifecycleWalker extends NgWalker { + protected visitNgInjectable(controller: ClassDeclaration, decorator: Decorator): void { + this.validateDecorator(controller, decorator, METADATA_TYPE_LIFECYCLE_MAPPER.Injectable); + super.visitNgInjectable(controller, decorator); + } + + protected visitNgPipe(controller: ClassDeclaration, decorator: Decorator): void { + this.validateDecorator(controller, decorator, METADATA_TYPE_LIFECYCLE_MAPPER.Pipe); + super.visitNgPipe(controller, decorator); + } + + private validateDecorator(controller: ClassDeclaration, decorator: Decorator, allowedMethods: ReadonlySet): void { + const className = getClassName(controller); + + if (!className) return; + + const metadataType = getDecoratorName(decorator); + + if (!metadataType || !isMetadataType(metadataType)) return; + + for (const member of controller.members) { + const { name: memberName } = member; + + if (!memberName) continue; + + const methodName = memberName.getText(); + + if (!isLifecycleMethod(methodName) || allowedMethods.has(methodName)) continue; + + const failure = getFailureMessage({ className, metadataType, methodName }); + + this.addFailureAtNode(member, failure); + } + } +} diff --git a/src/decoratorNotAllowedRule.ts b/src/decoratorNotAllowedRule.ts deleted file mode 100644 index 4dd7a68a3..000000000 --- a/src/decoratorNotAllowedRule.ts +++ /dev/null @@ -1,110 +0,0 @@ -import { vsprintf } from 'sprintf-js'; -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { ComponentMetadata, DirectiveMetadata } from './angular/metadata'; -import { NgWalker } from './angular/ngWalker'; - -export class Rule extends Lint.Rules.AbstractRule { - static readonly metadata: Lint.IRuleMetadata = { - description: 'Ensure 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 @Input should not be used in an @Injectable class.', - ruleName: 'decorator-not-allowed', - type: 'functionality', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = 'In the class "%s" which have the "%s" decorator, the "%s" decorator is not allowed. Please, drop it.'; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new ClassMetadataWalker(sourceFile, this.getOptions())); - } -} - -export class ClassMetadataWalker extends NgWalker { - className!: string; - isInjectable = false; - - protected visitNgInjectable(classDeclaration: ts.ClassDeclaration, decorator: ts.Decorator) { - this.className = classDeclaration.name!.text; - this.isInjectable = true; - super.visitNgInjectable(classDeclaration, decorator); - } - - protected visitNgDirective(metadata: DirectiveMetadata) { - this.isInjectable = false; - super.visitNgDirective(metadata); - } - - protected visitNgPipe(controller: ts.ClassDeclaration, decorator: ts.Decorator) { - this.isInjectable = false; - super.visitNgPipe(controller, decorator); - } - - protected visitNgComponent(metadata: ComponentMetadata) { - this.isInjectable = false; - super.visitNgComponent(metadata); - } - - protected visitNgInput(property: ts.PropertyDeclaration, input: ts.Decorator, args: string[]) { - if (this.isInjectable) { - this.generateFailure(property, this.className, '@Injectable', '@Input'); - } - super.visitNgInput(property, input, args); - } - - protected visitNgOutput(property: ts.PropertyDeclaration, input: ts.Decorator, args: string[]) { - if (this.isInjectable) { - this.generateFailure(property, this.className, '@Injectable', '@Output'); - } - super.visitNgInput(property, input, args); - } - - protected visitNgHostBinding(property: ts.PropertyDeclaration, decorator: ts.Decorator, args: string[]) { - if (this.isInjectable) { - this.generateFailure(property, this.className, '@Injectable', '@HostBinding'); - } - super.visitNgHostBinding(property, decorator, args); - } - - protected visitNgHostListener(method: ts.MethodDeclaration, decorator: ts.Decorator, args: string[]) { - if (this.isInjectable) { - this.generateFailure(method, this.className, '@Injectable', '@HostListener'); - } - super.visitNgHostListener(method, decorator, args); - } - - protected visitNgContentChild(property: ts.PropertyDeclaration, input: ts.Decorator, args: string[]) { - if (this.isInjectable) { - this.generateFailure(property, this.className, '@Injectable', '@ContentChild'); - } - super.visitNgContentChild(property, input, args); - } - - protected visitNgContentChildren(property: ts.PropertyDeclaration, input: ts.Decorator, args: string[]) { - if (this.isInjectable) { - this.generateFailure(property, this.className, '@Injectable', '@ContentChildren'); - } - super.visitNgContentChildren(property, input, args); - } - - protected visitNgViewChild(property: ts.PropertyDeclaration, input: ts.Decorator, args: string[]) { - if (this.isInjectable) { - this.generateFailure(property, this.className, '@Injectable', '@ViewChild'); - } - super.visitNgViewChild(property, input, args); - } - - protected visitNgViewChildren(property: ts.PropertyDeclaration, input: ts.Decorator, args: string[]) { - if (this.isInjectable) { - this.generateFailure(property, this.className, '@Injectable', '@ViewChildren'); - } - super.visitNgViewChildren(property, input, args); - } - - private generateFailure(property: ts.Node, ...failureConfig: string[]) { - this.addFailureAtNode(property, vsprintf(Rule.FAILURE_STRING, failureConfig)); - } -} diff --git a/src/enforceComponentSelectorRule.ts b/src/enforceComponentSelectorRule.ts deleted file mode 100644 index e16f68946..000000000 --- a/src/enforceComponentSelectorRule.ts +++ /dev/null @@ -1,33 +0,0 @@ -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { sprintf } from 'sprintf-js'; -import { NgWalker } from './angular/ngWalker'; -import { ComponentMetadata } from './angular/metadata'; - -export class Rule extends Lint.Rules.AbstractRule { - public static metadata: Lint.IRuleMetadata = { - ruleName: 'enforce-component-selector', - type: 'style', - description: 'Component selector must be declared.', - rationale: 'Omit the component selector makes debugging difficult.', - options: null, - optionsDescription: 'Not configurable.', - typescriptOnly: true - }; - - static SELECTOR_FAILURE = 'The selector of the component "%s" is mandatory'; - - public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new EnforceComponentSelectorValidatorWalker(sourceFile, this.getOptions())); - } -} - -export class EnforceComponentSelectorValidatorWalker extends NgWalker { - protected visitNgComponent(metadata: ComponentMetadata) { - if (!metadata.selector) { - this.addFailureAtNode(metadata.decorator, sprintf(Rule.SELECTOR_FAILURE, metadata.controller.name!.text)); - } - - super.visitNgComponent(metadata); - } -} diff --git a/src/i18nRule.ts b/src/i18nRule.ts deleted file mode 100644 index def7f1035..000000000 --- a/src/i18nRule.ts +++ /dev/null @@ -1,206 +0,0 @@ -import * as ast from '@angular/compiler'; -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { NgWalker } from './angular/ngWalker'; -import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; - -const OPTION_CHECK_ID = 'check-id'; -const OPTION_CHECK_TEXT = 'check-text'; - -type CheckOption = typeof OPTION_CHECK_ID | typeof OPTION_CHECK_TEXT; - -interface ConfigurableVisitor { - getCheckOption(): CheckOption; -} - -class I18NAttrVisitor extends BasicTemplateAstVisitor implements ConfigurableVisitor { - visitAttr(attr: ast.AttrAst, context: BasicTemplateAstVisitor) { - if (attr.name === 'i18n') { - const parts = (attr.value || '').split('@@'); - if (parts.length <= 1 || parts[1].length === 0) { - const { - sourceSpan: { - end: { offset: endOffset }, - start: { offset: startOffset } - } - } = attr; - - context.addFailureFromStartToEnd( - startOffset, - endOffset, - 'Missing custom message identifier. For more information visit https://angular.io/guide/i18n' - ); - } - } - super.visitAttr(attr, context); - } - - getCheckOption(): CheckOption { - return 'check-id'; - } -} - -class I18NTextVisitor extends BasicTemplateAstVisitor implements ConfigurableVisitor { - static Error = 'Each element containing text node should have an i18n attribute'; - - private hasI18n = false; - private nestedElements: string[] = []; - private visited = new Set(); - - visitText(text: ast.TextAst, context: BasicTemplateAstVisitor) { - if (!this.visited.has(text)) { - this.visited.add(text); - const textNonEmpty = text.value.trim().length > 0; - if ((!this.hasI18n && textNonEmpty && this.nestedElements.length) || (textNonEmpty && !this.nestedElements.length)) { - const { - sourceSpan: { - end: { offset: endOffset }, - start: { offset: startOffset } - } - } = text; - - context.addFailureFromStartToEnd(startOffset, endOffset, I18NTextVisitor.Error); - } - } - super.visitText(text, context); - } - - visitBoundText(text: ast.BoundTextAst, context: BasicTemplateAstVisitor) { - if (!this.visited.has(text)) { - this.visited.add(text); - const val = text.value; - if (val instanceof ast.ASTWithSource && val.ast instanceof ast.Interpolation && !this.hasI18n) { - const textNonEmpty = val.ast.strings.some(s => /\w+/.test(s)); - if (textNonEmpty) { - const { - sourceSpan: { - end: { offset: endOffset }, - start: { offset: startOffset } - } - } = text; - - context.addFailureFromStartToEnd(startOffset, endOffset, I18NTextVisitor.Error); - } - } - } - - super.visitBoundText(text, context); - } - - visitElement(element: ast.ElementAst, context: BasicTemplateAstVisitor) { - const originalI18n = this.hasI18n; - this.hasI18n = originalI18n || element.attrs.some(e => e.name === 'i18n'); - this.nestedElements.push(element.name); - super.visitElement(element, context); - this.nestedElements.pop(); - this.hasI18n = originalI18n; - super.visitElement(element, context); - } - - getCheckOption(): CheckOption { - return 'check-text'; - } -} - -class I18NTemplateVisitor extends BasicTemplateAstVisitor { - private visitors: (BasicTemplateAstVisitor & ConfigurableVisitor)[] = [ - new I18NAttrVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart), - new I18NTextVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart) - ]; - - visit(node: ast.TemplateAst, context: BasicTemplateAstVisitor) { - super.visit!(node, context); - } - - visitAttr(attr: ast.AttrAst, context: BasicTemplateAstVisitor): any { - const options = this.getOptions(); - this.visitors - .filter(v => options.indexOf(v.getCheckOption()) >= 0) - .map(v => v.visitAttr(attr, this)) - .filter(Boolean) - .forEach(f => - this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) - ); - super.visitAttr(attr, context); - } - - visitElement(element: ast.ElementAst, context: BasicTemplateAstVisitor): any { - const options = this.getOptions(); - this.visitors - .filter(v => options.indexOf(v.getCheckOption()) >= 0) - .map(v => v.visitElement(element, this)) - .filter(Boolean) - .forEach(f => - this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) - ); - super.visitElement(element, context); - } - - visitText(text: ast.TextAst, context: BasicTemplateAstVisitor): any { - const options = this.getOptions(); - this.visitors - .filter(v => options.indexOf(v.getCheckOption()) >= 0) - .map(v => v.visitText(text, this)) - .filter(Boolean) - .forEach(f => - this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) - ); - super.visitText(text, context); - } - - visitBoundText(text: ast.BoundTextAst, context: any): any { - const options = this.getOptions(); - this.visitors - .filter(v => options.indexOf(v.getCheckOption()) >= 0) - .map(v => v.visitBoundText(text, this)) - .filter(Boolean) - .forEach(f => - this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) - ); - super.visitBoundText(text, context); - } -} - -export class Rule extends Lint.Rules.AbstractRule { - static readonly metadata: Lint.IRuleMetadata = { - description: 'Ensures following best practices for i18n.', - optionExamples: [[true, OPTION_CHECK_ID], [true, OPTION_CHECK_TEXT], [true, OPTION_CHECK_ID, OPTION_CHECK_TEXT]], - options: { - items: { - enum: [OPTION_CHECK_ID, OPTION_CHECK_TEXT], - type: 'string' - }, - maxLength: 2, - minLength: 1, - type: 'array' - }, - optionsDescription: Lint.Utils.dedent` - One (or both) of the following arguments must be provided: - * \`${OPTION_CHECK_ID}\` Makes sure i18n attributes have ID specified - * \`${OPTION_CHECK_TEXT}\` Makes sure there are no elements with text content but no i18n attribute - `, - rationale: 'Makes the code more maintainable in i18n sense.', - ruleName: 'i18n', - type: 'maintainability', - typescriptOnly: true - }; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker( - new NgWalker(sourceFile, this.getOptions(), { - templateVisitorCtrl: I18NTemplateVisitor - }) - ); - } - - isEnabled(): boolean { - const { - metadata: { - options: { maxLength, minLength } - } - } = Rule; - const { length } = this.ruleArguments; - - return super.isEnabled() && length >= minLength && length <= maxLength; - } -} diff --git a/src/index.ts b/src/index.ts index be94d9bae..09f7a2b60 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,55 +1,55 @@ export { Rule as AngularWhitespaceRule } from './angularWhitespaceRule'; -export { Rule as BananaInBoxRule } from './bananaInBoxRule'; +export { Rule as ComponentChangeDetectionRule } from './componentChangeDetectionRule'; export { Rule as ComponentClassSuffixRule } from './componentClassSuffixRule'; export { Rule as ComponentSelectorRule } from './componentSelectorRule'; -export { Rule as ContextualLifeCycleRule } from './contextualLifeCycleRule'; -export { Rule as DecoratorNotAllowedRule } from './decoratorNotAllowedRule'; +export { Rule as ContextualDecoratorRule } from './contextualDecoratorRule'; +export { Rule as ContextualLifecycleRule } from './contextualLifecycleRule'; export { Rule as DirectiveClassSuffixRule } from './directiveClassSuffixRule'; export { Rule as DirectiveSelectorRule } from './directiveSelectorRule'; -export { Rule as EnforceComponentSelectorRule } from './enforceComponentSelectorRule'; -export { Rule as I18nRule } from './i18nRule'; export { Rule as ImportDestructuringSpacingRule } from './importDestructuringSpacingRule'; export { Rule as MaxInlineDeclarationsRule } from './maxInlineDeclarationsRule'; export { Rule as NoAttributeParameterDecoratorRule } from './noAttributeParameterDecoratorRule'; -export { Rule as NoConflictingLifeCycleHooksRule } from './noConflictingLifeCycleHooksRule'; +export { Rule as NoConflictingLifecycleRule } from './noConflictingLifecycleRule'; export { Rule as NoForwardRefRule } from './noForwardRefRule'; +export { Rule as NoHostMetadataPropertyRule } from './noHostMetadataPropertyRule'; export { Rule as NoInputPrefixRule } from './noInputPrefixRule'; export { Rule as NoInputRenameRule } from './noInputRenameRule'; -export { Rule as NoLifeCycleCallRule } from './noLifeCycleCallRule'; -export { Rule as NoOutputNamedAfterStandardEventRule } from './noOutputNamedAfterStandardEventRule'; +export { Rule as NoInputsMetadataPropertyRule } from './noInputsMetadataPropertyRule'; +export { Rule as NoLifecycleCallRule } from './noLifecycleCallRule'; +export { Rule as NoOutputNativeRule } from './noOutputNativeRule'; export { Rule as NoOutputOnPrefixRule } from './noOutputOnPrefixRule'; export { Rule as NoOutputRenameRule } from './noOutputRenameRule'; -export { Rule as NoQueriesParameterRule } from './noQueriesParameterRule'; -export { Rule as NoTemplateCallExpressionRule } from './noTemplateCallExpressionRule'; +export { Rule as NoOutputsMetadataPropertyRule } from './noOutputsMetadataPropertyRule'; +export { Rule as NoPipeImpureRule } from './noPipeImpureRule'; +export { Rule as NoQueriesMetadataPropertyRule } from './noQueriesMetadataPropertyRule'; export { Rule as NoUnusedCssRule } from './noUnusedCssRule'; -export { Rule as PipeImpureRule } from './pipeImpureRule'; export { Rule as PipePrefixRule } from './pipePrefixRule'; -export { Rule as PreferInlineDecorator } from './preferInlineDecoratorRule'; +export { Rule as PreferInlineDecoratorRule } from './preferInlineDecoratorRule'; export { Rule as PreferOutputReadonlyRule } from './preferOutputReadonlyRule'; -export { Rule as TemplateConditionalComplexityRule } from './templateConditionalComplexityRule'; -export { Rule as TemplateCyclomaticComplexityRule } from './templateCyclomaticComplexityRule'; -export { Rule as TemplateAccessibilityTabindexNoPositiveRule } from './templateAccessibilityTabindexNoPositiveRule'; -export { Rule as TemplateNoAnyRule } from './templateNoAnyRule'; +export { Rule as RelativeUrlPrefixRule } from './relativeUrlPrefixRule'; +export { Rule as TemplateAccessibilityAltTextRule } from './templateAccessibilityAltTextRule'; export { Rule as TemplateAccessibilityLabelForVisitor } from './templateAccessibilityLabelForRule'; +export { Rule as TemplateAccessibilityTabindexNoPositiveRule } from './templateAccessibilityTabindexNoPositiveRule'; +export { Rule as TemplateAccessibilityTableScopeRule } from './templateAccessibilityTableScopeRule'; export { Rule as TemplateAccessibilityValidAriaRule } from './templateAccessibilityValidAriaRule'; -export { Rule as TemplatesAccessibilityAnchorContentRule } from './templateAccessibilityAnchorContentRule'; +export { Rule as TemplateBananaInBoxRule } from './templateBananaInBoxRule'; export { Rule as TemplateClickEventsHaveKeyEventsRule } from './templateClickEventsHaveKeyEventsRule'; -export { Rule as TemplateAccessibilityAltTextRule } from './templateAccessibilityAltTextRule'; -export { Rule as TemplateAccessibilityTableScopeRule } from './templateAccessibilityTableScopeRule'; -export { Rule as TemplateNoDistractingElementsRule } from './templateNoDistractingElementsRule'; -export { Rule as TemplatesNoNegatedAsync } from './templatesNoNegatedAsyncRule'; -export { Rule as TemplateNoAutofocusRule } from './templateNoAutofocusRule'; +export { Rule as TemplateConditionalComplexityRule } from './templateConditionalComplexityRule'; +export { Rule as TemplateCyclomaticComplexityRule } from './templateCyclomaticComplexityRule'; +export { Rule as TemplateI18nRule } from './templateI18nRule'; export { Rule as TemplateMouseEventsHaveKeyEventsRule } from './templateMouseEventsHaveKeyEventsRule'; -export { Rule as TrackByFunctionRule } from './trackByFunctionRule'; -export { Rule as UseHostPropertyDecoratorRule } from './useHostPropertyDecoratorRule'; -export { Rule as UseInputPropertyDecoratorRule } from './useInputPropertyDecoratorRule'; -export { Rule as UseLifeCycleInterfaceRule } from './useLifeCycleInterfaceRule'; -export { Rule as UseOutputPropertyDecoratorRule } from './useOutputPropertyDecoratorRule'; +export { Rule as TemplateNoAnyRule } from './templateNoAnyRule'; +export { Rule as TemplateNoAutofocusRule } from './templateNoAutofocusRule'; +export { Rule as TemplateNoCallExpressionRule } from './templateNoCallExpressionRule'; +export { Rule as TemplateNoDistractingElementsRule } from './templateNoDistractingElementsRule'; +export { Rule as TemplateNoNegatedAsyncRule } from './templateNoNegatedAsyncRule'; +export { Rule as TemplateUseTrackByFunctionRule } from './templateUseTrackByFunctionRule'; +export { Rule as TemplatesAccessibilityAnchorContentRule } from './templateAccessibilityAnchorContentRule'; +export { Rule as UseComponentSelectorRule } from './useComponentSelectorRule'; +export { Rule as UseComponentViewEncapsulationRule } from './useComponentViewEncapsulationRule'; +export { Rule as UseLifecycleInterfaceRule } from './useLifecycleInterfaceRule'; export { Rule as UsePipeDecoratorRule } from './usePipeDecoratorRule'; export { Rule as UsePipeTransformInterfaceRule } from './usePipeTransformInterfaceRule'; -export { Rule as UseViewEncapsulationRule } from './useViewEncapsulationRule'; -export { Rule as RelativePathExternalResourcesRule } from './relativeUrlPrefixRule'; -export { Rule as ComponentChangeDetectionRule } from './componentChangeDetectionRule'; export * from './angular'; diff --git a/src/noConflictingLifeCycleHooksRule.ts b/src/noConflictingLifeCycleHooksRule.ts deleted file mode 100644 index 6cdc19a0f..000000000 --- a/src/noConflictingLifeCycleHooksRule.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { sprintf } from 'sprintf-js'; -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { getSymbolName } from './util/utils'; - -export class Rule extends Lint.Rules.AbstractRule { - static metadata: Lint.IRuleMetadata = { - description: 'Ensure that directives not implement conflicting life cycle hooks.', - descriptionDetails: 'See more at https://angular.io/api/core/DoCheck#description.', - options: null, - optionsDescription: 'Not configurable.', - rationale: Lint.Utils.dedent` - A directive typically should not use both DoCheck and OnChanges to respond - to changes on the same input, as ngOnChanges will continue to be called when the - default change detector detects changes. - `, - ruleName: 'no-conflicting-life-cycle-hooks', - type: 'maintainability', - typescriptOnly: true - }; - - static FAILURE_STRING = 'Implement DoCheck and OnChanges hooks in class %s is not recommended'; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new ClassMetadataWalker(sourceFile, this.getOptions())); - } -} - -const hooksPrefix = 'ng'; -const lifecycleHooksMethods: string[] = ['DoCheck', 'OnChanges']; - -export class ClassMetadataWalker extends Lint.RuleWalker { - visitClassDeclaration(node: ts.ClassDeclaration) { - this.validateInterfaces(node); - this.validateMethods(node); - super.visitClassDeclaration(node); - } - - private validateInterfaces(node: ts.ClassDeclaration): void { - const { heritageClauses } = node; - - if (!heritageClauses) { - return; - } - - const interfacesClauses = heritageClauses.find(h => h.token === ts.SyntaxKind.ImplementsKeyword); - - if (!interfacesClauses) { - return; - } - - const interfaces = interfacesClauses.types.map(getSymbolName); - const matchesAllHooks = lifecycleHooksMethods.every(l => interfaces.indexOf(l) !== -1); - - if (matchesAllHooks) { - this.addFailureAtNode(node, sprintf(Rule.FAILURE_STRING, node.name!.text)); - } - } - - private validateMethods(node: ts.ClassDeclaration): void { - const methodNames = node.members.filter(ts.isMethodDeclaration).map(m => m.name!.getText()); - const matchesAllHooks = lifecycleHooksMethods.every(l => methodNames.indexOf(`${hooksPrefix}${l}`) !== -1); - - if (matchesAllHooks) { - this.addFailureAtNode(node, sprintf(Rule.FAILURE_STRING, node.name!.text)); - } - } -} diff --git a/src/noConflictingLifecycleRule.ts b/src/noConflictingLifecycleRule.ts new file mode 100644 index 000000000..f8f4e4026 --- /dev/null +++ b/src/noConflictingLifecycleRule.ts @@ -0,0 +1,93 @@ +import { sprintf } from 'sprintf-js'; +import { IRuleMetadata, RuleFailure, RuleWalker } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { dedent } from 'tslint/lib/utils'; +import { ClassDeclaration, SourceFile } from 'typescript'; +import { + getClassName, + getDeclaredLifecycleInterfaces, + getDeclaredLifecycleMethods, + LifecycleInterfaceKeys, + LifecycleInterfaces, + LifecycleMethodKeys, + LifecycleMethods +} from './util/utils'; + +interface FailureParameters { + readonly className: string; + 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]; + +export const getFailureMessage = (failureParameters: FailureParameters): string => + sprintf(failureParameters.message, failureParameters.className); + +export class Rule extends AbstractRule { + static metadata: IRuleMetadata = { + description: 'Ensures that directives not implement conflicting lifecycle interfaces.', + descriptionDetails: 'See more at https://angular.io/api/core/DoCheck#description.', + 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 + default change detector detects changes. + `, + ruleName: 'no-conflicting-lifecycle', + type: 'maintainability', + typescriptOnly: true + }; + + static readonly FAILURE_STRING_INTERFACE_HOOK = dedent` + Implementing ${LifecycleInterfaces.DoCheck} and ${LifecycleInterfaces.OnChanges} in class %s is not recommended + `; + static readonly FAILURE_STRING_METHOD_HOOK = dedent` + Declaring ${LifecycleMethods.ngDoCheck} and ${LifecycleMethods.ngOnChanges} method in class %s is not recommended + `; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker(new ClassMetadataWalker(sourceFile, this.getOptions())); + } +} + +class ClassMetadataWalker extends RuleWalker { + visitClassDeclaration(node: ClassDeclaration): void { + this.validateInterfaces(node); + this.validateMethods(node); + super.visitClassDeclaration(node); + } + + private validateInterfaces(node: ClassDeclaration): void { + const className = getClassName(node); + const declaredLifecycleInterfaces = getDeclaredLifecycleInterfaces(node); + const hasConflictingLifecycle = LIFECYCLE_INTERFACES.every( + lifecycleInterface => declaredLifecycleInterfaces.indexOf(lifecycleInterface) !== -1 + ); + + if (!className || !hasConflictingLifecycle) return; + + const failure = getFailureMessage({ + className, + message: Rule.FAILURE_STRING_INTERFACE_HOOK + }); + + this.addFailureAtNode(node, failure); + } + + private validateMethods(node: ClassDeclaration): void { + const className = getClassName(node); + const declaredLifecycleMethods = getDeclaredLifecycleMethods(node); + const hasConflictingLifecycle = LIFECYCLE_METHODS.every(lifecycleMethod => declaredLifecycleMethods.indexOf(lifecycleMethod) !== -1); + + if (!className || !hasConflictingLifecycle) return; + + const failure = getFailureMessage({ + className, + message: Rule.FAILURE_STRING_METHOD_HOOK + }); + + this.addFailureAtNode(node, failure); + } +} diff --git a/src/noForwardRefRule.ts b/src/noForwardRefRule.ts index d0188c143..db483e94e 100644 --- a/src/noForwardRefRule.ts +++ b/src/noForwardRefRule.ts @@ -1,46 +1,64 @@ import { sprintf } from 'sprintf-js'; -import { IRuleMetadata, RuleFailure, Rules, RuleWalker } from 'tslint/lib'; -import { CallExpression, SourceFile, SyntaxKind } from 'typescript/lib/typescript'; +import { IRuleMetadata, RuleFailure, RuleWalker } from 'tslint/lib'; +import { AbstractRule } from 'tslint/lib/rules'; +import { CallExpression, isClassDeclaration, isVariableStatement, SourceFile } from 'typescript/lib/typescript'; +import { getNextToLastParentNode } from './util/utils'; -export class Rule extends Rules.AbstractRule { +interface FailureParameters { + readonly className: string; + readonly message: typeof Rule.FAILURE_STRING_CLASS | typeof Rule.FAILURE_STRING_VARIABLE; +} + +export const FORWARD_REF = 'forwardRef'; + +export const getFailureMessage = (failureParameters: FailureParameters): string => + sprintf(failureParameters.message, failureParameters.className); + +export class Rule extends AbstractRule { static metadata: IRuleMetadata = { - description: 'Disallows usage of forward references for DI.', + description: `Disallows usage of \`${FORWARD_REF}\` references for DI.`, options: null, optionsDescription: 'Not configurable.', - rationale: 'The flow of DI is disrupted by using `forwardRef` and might make code more difficult to understand.', + rationale: `The flow of DI is disrupted by using \`${FORWARD_REF}\` and might make code more difficult to understand.`, ruleName: 'no-forward-ref', type: 'maintainability', typescriptOnly: true }; - static readonly FAILURE_STRING_CLASS = 'Avoid using forwardRef in class "%s"'; - static readonly FAILURE_STRING_VARIABLE = 'Avoid using forwardRef in variable "%s"'; + static readonly FAILURE_STRING_CLASS = `Avoid using \`${FORWARD_REF}\` in class "%s"`; + static readonly FAILURE_STRING_VARIABLE = `Avoid using \`${FORWARD_REF}\` in variable "%s"`; apply(sourceFile: SourceFile): RuleFailure[] { - return this.applyWithWalker(new ExpressionCallMetadataWalker(sourceFile, this.getOptions())); + return this.applyWithWalker(new NoForwardRefWalker(sourceFile, this.getOptions())); } } -export class ExpressionCallMetadataWalker extends RuleWalker { - visitCallExpression(node: CallExpression) { +export class NoForwardRefWalker extends RuleWalker { + visitCallExpression(node: CallExpression): void { this.validateCallExpression(node); super.visitCallExpression(node); } - private validateCallExpression(callExpression) { - if (callExpression.expression.text === 'forwardRef') { - let currentNode = callExpression; - - while (currentNode.parent.parent) { - currentNode = currentNode.parent; - } + private validateCallExpression(node: CallExpression): void { + if (node.expression.getText() !== FORWARD_REF) return; - const failure = - currentNode.kind === SyntaxKind.VariableStatement - ? sprintf(Rule.FAILURE_STRING_VARIABLE, currentNode.declarationList.declarations[0].name.text) - : sprintf(Rule.FAILURE_STRING_CLASS, currentNode.name.text); + const nextToLastParent = getNextToLastParentNode(node); + let failure: ReturnType; - this.addFailureAtNode(callExpression, failure); + if (isVariableStatement(nextToLastParent)) { + failure = getFailureMessage({ + className: nextToLastParent.declarationList.declarations[0].name.getText(), + message: Rule.FAILURE_STRING_VARIABLE + }); + } else if (isClassDeclaration(nextToLastParent) && nextToLastParent.name) { + failure = getFailureMessage({ + className: nextToLastParent.name.text, + message: Rule.FAILURE_STRING_CLASS + }); + } else { + return; } + + this.addFailureAtNode(node, failure); } } diff --git a/src/noHostMetadataPropertyRule.ts b/src/noHostMetadataPropertyRule.ts new file mode 100644 index 000000000..7bb3d230a --- /dev/null +++ b/src/noHostMetadataPropertyRule.ts @@ -0,0 +1,35 @@ +import { IOptions, IRuleMetadata } from 'tslint/lib'; +import { UsePropertyDecorator } from './propertyDecoratorBase'; +import { Decorators } from './util/utils'; + +const METADATA_PROPERTY_NAME = 'host'; +const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-06-03'; + +export class Rule extends UsePropertyDecorator { + static readonly metadata: IRuleMetadata = { + description: `Disallows usage of the \`${METADATA_PROPERTY_NAME}\` metadata property.`, + 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.`, + 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})`; + + constructor(options: IOptions) { + super( + { + decoratorName: [`${Decorators.HostBinding}s`, `${Decorators.HostListener}s`], + errorMessage: Rule.FAILURE_STRING, + propertyName: METADATA_PROPERTY_NAME + }, + options + ); + } +} diff --git a/src/noInputsMetadataPropertyRule.ts b/src/noInputsMetadataPropertyRule.ts new file mode 100644 index 000000000..f67bd9ff0 --- /dev/null +++ b/src/noInputsMetadataPropertyRule.ts @@ -0,0 +1,39 @@ +import { IOptions, IRuleMetadata } from 'tslint/lib'; +import { dedent } from 'tslint/lib/utils'; +import { UsePropertyDecorator } from './propertyDecoratorBase'; +import { Decorators } from './util/utils'; + +const METADATA_PROPERTY_NAME = 'inputs'; +const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-05-12'; + +export class Rule extends UsePropertyDecorator { + static readonly metadata: IRuleMetadata = { + description: `Disallows usage of the \`${METADATA_PROPERTY_NAME}\` metadata property.`, + descriptionDetails: `See more at ${STYLE_GUIDE_LINK}.`, + 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. + * The metadata declaration attached to the directive is shorter and thus more readable. + `, + ruleName: 'no-inputs-metadata-property', + type: 'style', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = `Use @${ + Decorators.Input + } rather than the \`${METADATA_PROPERTY_NAME}\` metadata property (${STYLE_GUIDE_LINK})`; + + constructor(options: IOptions) { + super( + { + decoratorName: Decorators.Input, + errorMessage: Rule.FAILURE_STRING, + propertyName: METADATA_PROPERTY_NAME + }, + options + ); + } +} diff --git a/src/noLifeCycleCallRule.ts b/src/noLifeCycleCallRule.ts deleted file mode 100644 index 791f4459a..000000000 --- a/src/noLifeCycleCallRule.ts +++ /dev/null @@ -1,61 +0,0 @@ -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { NgWalker } from './angular/ngWalker'; - -export class Rule extends Lint.Rules.AbstractRule { - static readonly metadata: Lint.IRuleMetadata = { - description: 'Disallows explicit calls to life cycle hooks.', - options: null, - optionsDescription: 'Not configurable.', - rationale: 'Explicit calls to life cycle hooks could be confusing. Invoke life cycle hooks is the responsability of Angular.', - ruleName: 'no-life-cycle-call', - type: 'maintainability', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = 'Avoid explicit calls to life cycle hooks.'; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new ExpressionCallMetadataWalker(sourceFile, this.getOptions())); - } -} - -export type LifecycleHooksMethods = - | 'ngAfterContentChecked' - | 'ngAfterContentInit' - | 'ngAfterViewChecked' - | 'ngAfterViewInit' - | 'ngDoCheck' - | 'ngOnChanges' - | 'ngOnDestroy' - | 'ngOnInit'; - -export const lifecycleHooksMethods = new Set([ - 'ngAfterContentChecked', - 'ngAfterContentInit', - 'ngAfterViewChecked', - 'ngAfterViewInit', - 'ngDoCheck', - 'ngOnChanges', - 'ngOnDestroy', - 'ngOnInit' -]); - -export class ExpressionCallMetadataWalker extends NgWalker { - visitCallExpression(node: ts.CallExpression): void { - this.validateCallExpression(node); - super.visitCallExpression(node); - } - - private validateCallExpression(node: ts.CallExpression): void { - const name = ts.isPropertyAccessExpression(node.expression) ? node.expression.name : undefined; - const expression = ts.isPropertyAccessExpression(node.expression) ? node.expression.expression : undefined; - - const isSuperCall = expression && expression.kind === ts.SyntaxKind.SuperKeyword; - const isLifecycleCall = name && ts.isIdentifier(name) && lifecycleHooksMethods.has(name.text as LifecycleHooksMethods); - - if (isLifecycleCall && !isSuperCall) { - this.addFailureAtNode(node, Rule.FAILURE_STRING); - } - } -} diff --git a/src/noLifecycleCallRule.ts b/src/noLifecycleCallRule.ts new file mode 100644 index 000000000..3c150dc1d --- /dev/null +++ b/src/noLifecycleCallRule.ts @@ -0,0 +1,47 @@ +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'; + +export class Rule extends AbstractRule { + static readonly metadata: IRuleMetadata = { + description: 'Disallows explicit calls to lifecycle methods.', + options: null, + optionsDescription: 'Not configurable.', + rationale: "Explicit calls to lifecycle methods could be confusing. Invoke them is an Angular's responsability.", + ruleName: 'no-lifecycle-call', + type: 'functionality', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = 'Avoid explicit calls to lifecycle methods'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker(new ExpressionCallMetadataWalker(sourceFile, this.getOptions())); + } +} + +class ExpressionCallMetadataWalker extends NgWalker { + visitCallExpression(node: CallExpression): void { + this.validateCallExpression(node); + super.visitCallExpression(node); + } + + private validateCallExpression(node: CallExpression): void { + const { expression: nodeExpression } = node; + + if (!isPropertyAccessExpression(nodeExpression)) return; + + const { + expression, + name: { text: methodName } + } = nodeExpression; + const isLifecycleCall = isLifecycleMethod(methodName); + const isSuperCall = expression.kind === SyntaxKind.SuperKeyword; + + if (!isLifecycleCall || isSuperCall) return; + + this.addFailureAtNode(node, Rule.FAILURE_STRING); + } +} diff --git a/src/noOutputNamedAfterStandardEventRule.ts b/src/noOutputNamedAfterStandardEventRule.ts deleted file mode 100644 index affbe070f..000000000 --- a/src/noOutputNamedAfterStandardEventRule.ts +++ /dev/null @@ -1,219 +0,0 @@ -import { sprintf } from 'sprintf-js'; -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { NgWalker } from './angular/ngWalker'; -import { getClassName } from './util/utils'; - -export class Rule extends Lint.Rules.AbstractRule { - static readonly metadata: Lint.IRuleMetadata = { - description: 'Disallows naming directive outputs after a standard DOM event.', - options: null, - optionsDescription: 'Not configurable.', - rationale: 'Listeners subscribed to an output with such a name will also be invoked when the native event is raised.', - ruleName: 'no-output-named-after-standard-event', - type: 'maintainability', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = 'In the class "%s", the output property "%s" should not be named or renamed after a standard event'; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new OutputMetadataWalker(sourceFile, this.getOptions())); - } -} - -export class OutputMetadataWalker extends NgWalker { - // source: https://developer.mozilla.org/en-US/docs/Web/Events - private readonly standardEventNames = new Map([ - ['abort', true], - ['afterprint', true], - ['animationend', true], - ['animationiteration', true], - ['animationstart', true], - ['appinstalled', true], - ['audioprocess', true], - ['audioend', true], - ['audiostart', true], - ['beforeprint', true], - ['beforeunload', true], - ['beginEvent', true], - ['blocked', true], - ['blur', true], - ['boundary', true], - ['cached', true], - ['canplay', true], - ['canplaythrough', true], - ['change', true], - ['chargingchange', true], - ['chargingtimechange', true], - ['checking', true], - ['click', true], - ['close', true], - ['complete', true], - ['compositionend', true], - ['compositionstart', true], - ['compositionupdate', true], - ['contextmenu', true], - ['copy', true], - ['cut', true], - ['dblclick', true], - ['devicechange', true], - ['devicelight', true], - ['devicemotion', true], - ['deviceorientation', true], - ['deviceproximity', true], - ['dischargingtimechange', true], - ['DOMAttributeNameChanged', true], - ['DOMAttrModified', true], - ['DOMCharacterDataModified', true], - ['DOMContentLoaded', true], - ['DOMElementNameChanged', true], - ['focus', true], - ['focusin', true], - ['focusout', true], - ['DOMNodeInserted', true], - ['DOMNodeInsertedIntoDocument', true], - ['DOMNodeRemoved', true], - ['DOMNodeRemovedFromDocument', true], - ['DOMSubtreeModified', true], - ['downloading', true], - ['drag', true], - ['dragend', true], - ['dragenter', true], - ['dragleave', true], - ['dragover', true], - ['dragstart', true], - ['drop', true], - ['durationchange', true], - ['emptied', true], - ['end', true], - ['ended', true], - ['endEvent', true], - ['error', true], - ['fullscreenchange', true], - ['fullscreenerror', true], - ['gamepadconnected', true], - ['gamepaddisconnected', true], - ['gotpointercapture', true], - ['hashchange', true], - ['lostpointercapture', true], - ['input', true], - ['invalid', true], - ['keydown', true], - ['keypress', true], - ['keyup', true], - ['languagechange', true], - ['levelchange', true], - ['load', true], - ['loadeddata', true], - ['loadedmetadata', true], - ['loadend', true], - ['loadstart', true], - ['mark', true], - ['message', true], - ['messageerror', true], - ['mousedown', true], - ['mouseenter', true], - ['mouseleave', true], - ['mousemove', true], - ['mouseout', true], - ['mouseover', true], - ['mouseup', true], - ['nomatch', true], - ['notificationclick', true], - ['noupdate', true], - ['obsolete', true], - ['offline', true], - ['online', true], - ['open', true], - ['orientationchange', true], - ['pagehide', true], - ['pageshow', true], - ['paste', true], - ['pause', true], - ['pointercancel', true], - ['pointerdown', true], - ['pointerenter', true], - ['pointerleave', true], - ['pointerlockchange', true], - ['pointerlockerror', true], - ['pointermove', true], - ['pointerout', true], - ['pointerover', true], - ['pointerup', true], - ['play', true], - ['playing', true], - ['popstate', true], - ['progress', true], - ['push', true], - ['pushsubscriptionchange', true], - ['ratechange', true], - ['readystatechange', true], - ['repeatEvent', true], - ['reset', true], - ['resize', true], - ['resourcetimingbufferfull', true], - ['result', true], - ['resume', true], - ['scroll', true], - ['seeked', true], - ['seeking', true], - ['select', true], - ['selectstart', true], - ['selectionchange', true], - ['show', true], - ['soundend', true], - ['soundstart', true], - ['speechend', true], - ['speechstart', true], - ['stalled', true], - ['start', true], - ['storage', true], - ['submit', true], - ['success', true], - ['suspend', true], - ['SVGAbort', true], - ['SVGError', true], - ['SVGLoad', true], - ['SVGResize', true], - ['SVGScroll', true], - ['SVGUnload', true], - ['SVGZoom', true], - ['timeout', true], - ['timeupdate', true], - ['touchcancel', true], - ['touchend', true], - ['touchmove', true], - ['touchstart', true], - ['transitionend', true], - ['unload', true], - ['updateready', true], - ['upgradeneeded', true], - ['userproximity', true], - ['voiceschanged', true], - ['versionchange', true], - ['visibilitychange', true], - ['volumechange', true], - ['waiting', true], - ['wheel', true] - ]); - - protected visitNgOutput(property: ts.PropertyDeclaration, output: ts.Decorator, args: string[]) { - this.validateOutput(property, args); - super.visitNgOutput(property, output, args); - } - - private validateOutput(property: ts.PropertyDeclaration, args: string[]): void { - const className = getClassName(property); - const memberName = property.name.getText(); - const outputName = args[0] || memberName; - - if (!outputName || !this.standardEventNames.get(outputName)) { - return; - } - - const failure = sprintf(Rule.FAILURE_STRING, className, memberName); - - this.addFailureAtNode(property, failure); - } -} diff --git a/src/noOutputNativeRule.ts b/src/noOutputNativeRule.ts new file mode 100644 index 000000000..57382cf1f --- /dev/null +++ b/src/noOutputNativeRule.ts @@ -0,0 +1,229 @@ +import { sprintf } from 'sprintf-js'; +import { IRuleMetadata, RuleFailure } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { Decorator, PropertyDeclaration, SourceFile } from 'typescript'; +import { NgWalker } from './angular/ngWalker'; +import { getClassName } from './util/utils'; + +interface FailureParameters { + readonly className: string; + readonly propertyName: string; +} + +// source: https://developer.mozilla.org/en-US/docs/Web/Events +const nativeEventNames: ReadonlySet = new Set([ + 'abort', + 'afterprint', + 'animationend', + 'animationiteration', + 'animationstart', + 'appinstalled', + 'audioprocess', + 'audioend', + 'audiostart', + 'beforeprint', + 'beforeunload', + 'beginEvent', + 'blocked', + 'blur', + 'boundary', + 'cached', + 'canplay', + 'canplaythrough', + 'change', + 'chargingchange', + 'chargingtimechange', + 'checking', + 'click', + 'close', + 'complete', + 'compositionend', + 'compositionstart', + 'compositionupdate', + 'contextmenu', + 'copy', + 'cut', + 'dblclick', + 'devicechange', + 'devicelight', + 'devicemotion', + 'deviceorientation', + 'deviceproximity', + 'dischargingtimechange', + 'DOMAttributeNameChanged', + 'DOMAttrModified', + 'DOMCharacterDataModified', + 'DOMContentLoaded', + 'DOMElementNameChanged', + 'focus', + 'focusin', + 'focusout', + 'DOMNodeInserted', + 'DOMNodeInsertedIntoDocument', + 'DOMNodeRemoved', + 'DOMNodeRemovedFromDocument', + 'DOMSubtreeModified', + 'downloading', + 'drag', + 'dragend', + 'dragenter', + 'dragleave', + 'dragover', + 'dragstart', + 'drop', + 'durationchange', + 'emptied', + 'end', + 'ended', + 'endEvent', + 'error', + 'fullscreenchange', + 'fullscreenerror', + 'gamepadconnected', + 'gamepaddisconnected', + 'gotpointercapture', + 'hashchange', + 'lostpointercapture', + 'input', + 'invalid', + 'keydown', + 'keypress', + 'keyup', + 'languagechange', + 'levelchange', + 'load', + 'loadeddata', + 'loadedmetadata', + 'loadend', + 'loadstart', + 'mark', + 'message', + 'messageerror', + 'mousedown', + 'mouseenter', + 'mouseleave', + 'mousemove', + 'mouseout', + 'mouseover', + 'mouseup', + 'nomatch', + 'notificationclick', + 'noupdate', + 'obsolete', + 'offline', + 'online', + 'open', + 'orientationchange', + 'pagehide', + 'pageshow', + 'paste', + 'pause', + 'pointercancel', + 'pointerdown', + 'pointerenter', + 'pointerleave', + 'pointerlockchange', + 'pointerlockerror', + 'pointermove', + 'pointerout', + 'pointerover', + 'pointerup', + 'play', + 'playing', + 'popstate', + 'progress', + 'push', + 'pushsubscriptionchange', + 'ratechange', + 'readystatechange', + 'repeatEvent', + 'reset', + 'resize', + 'resourcetimingbufferfull', + 'result', + 'resume', + 'scroll', + 'seeked', + 'seeking', + 'select', + 'selectstart', + 'selectionchange', + 'show', + 'soundend', + 'soundstart', + 'speechend', + 'speechstart', + 'stalled', + 'start', + 'storage', + 'submit', + 'success', + 'suspend', + 'SVGAbort', + 'SVGError', + 'SVGLoad', + 'SVGResize', + 'SVGScroll', + 'SVGUnload', + 'SVGZoom', + 'timeout', + 'timeupdate', + 'touchcancel', + 'touchend', + 'touchmove', + 'touchstart', + 'transitionend', + 'unload', + 'updateready', + 'upgradeneeded', + 'userproximity', + 'voiceschanged', + 'versionchange', + 'visibilitychange', + 'volumechange', + 'waiting', + 'wheel' +]); + +export const getFailureMessage = (failureParameters: FailureParameters): string => + sprintf(Rule.FAILURE_STRING, failureParameters.className, failureParameters.propertyName); + +export class Rule extends AbstractRule { + static readonly metadata: IRuleMetadata = { + description: 'Disallows naming directive outputs as standard DOM event.', + options: null, + optionsDescription: 'Not configurable.', + rationale: 'Listeners subscribed to an output with such a name will also be invoked when the native event is raised.', + ruleName: 'no-output-native', + type: 'functionality', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = 'In the class "%s", the output property "%s" should not be named or renamed as a native event'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker(new NoOutputNativeWalker(sourceFile, this.getOptions())); + } +} + +class NoOutputNativeWalker extends NgWalker { + protected visitNgOutput(property: PropertyDeclaration, output: Decorator, args: string[]): void { + this.validateOutput(property, args); + super.visitNgOutput(property, output, args); + } + + private validateOutput(property: PropertyDeclaration, args: string[]): void { + const className = getClassName(property); + + if (!className) return; + + const propertyName = property.name.getText(); + const outputName = args[0] || propertyName; + + if (!outputName || !nativeEventNames.has(outputName)) return; + + const failure = getFailureMessage({ className, propertyName }); + + this.addFailureAtNode(property, failure); + } +} diff --git a/src/noOutputsMetadataPropertyRule.ts b/src/noOutputsMetadataPropertyRule.ts new file mode 100644 index 000000000..22309d460 --- /dev/null +++ b/src/noOutputsMetadataPropertyRule.ts @@ -0,0 +1,39 @@ +import { IOptions, IRuleMetadata } from 'tslint/lib'; +import { dedent } from 'tslint/lib/utils'; +import { UsePropertyDecorator } from './propertyDecoratorBase'; +import { Decorators } from './util/utils'; + +const METADATA_PROPERTY_NAME = 'outputs'; +const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-05-12'; + +export class Rule extends UsePropertyDecorator { + static readonly metadata: IRuleMetadata = { + description: `Disallows usage of the \`${METADATA_PROPERTY_NAME}\` metadata property.`, + descriptionDetails: `See more at ${STYLE_GUIDE_LINK}.`, + options: null, + 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. + * The metadata declaration attached to the directive is shorter and thus more readable. + `, + ruleName: 'no-outputs-metadata-property', + type: 'style', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = `Use @${ + Decorators.Output + } rather than the \`${METADATA_PROPERTY_NAME}\` metadata property (${STYLE_GUIDE_LINK})`; + + constructor(options: IOptions) { + super( + { + decoratorName: Decorators.Output, + errorMessage: Rule.FAILURE_STRING, + propertyName: METADATA_PROPERTY_NAME + }, + options + ); + } +} diff --git a/src/noPipeImpureRule.ts b/src/noPipeImpureRule.ts new file mode 100644 index 000000000..47b3419eb --- /dev/null +++ b/src/noPipeImpureRule.ts @@ -0,0 +1,57 @@ +import { sprintf } from 'sprintf-js'; +import { IRuleMetadata, RuleFailure } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { ClassDeclaration, Decorator, SourceFile, SyntaxKind } from 'typescript'; +import { NgWalker } from './angular/ngWalker'; +import { getClassName, getDecoratorPropertyInitializer } from './util/utils'; + +interface FailureParameters { + readonly className: string; +} + +export const getFailureMessage = (failureParameters: FailureParameters): string => + sprintf(Rule.FAILURE_STRING, failureParameters.className); + +export class Rule extends AbstractRule { + static readonly metadata: IRuleMetadata = { + description: 'Disallows the declaration of impure pipes.', + options: null, + optionsDescription: 'Not configurable.', + rationale: 'Impure pipes should be avoided because they are invoked on each change-detection cycle.', + ruleName: 'no-pipe-impure', + type: 'functionality', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = 'Impure pipe declared in class %s'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker(new ClassMetadataWalker(sourceFile, this.getOptions())); + } +} + +export class ClassMetadataWalker extends NgWalker { + protected visitNgPipe(controller: ClassDeclaration, decorator: Decorator): void { + this.validatePipe(controller, decorator); + super.visitNgPipe(controller, decorator); + } + + private validatePipe(controller: ClassDeclaration, decorator: Decorator): void { + const pureExpression = getDecoratorPropertyInitializer(decorator, 'pure'); + + if (!pureExpression) return; + + const { parent: parentExpression } = pureExpression; + const isNotFalseLiteral = pureExpression.kind !== SyntaxKind.FalseKeyword; + + if (!parentExpression || isNotFalseLiteral) return; + + const className = getClassName(controller); + + if (!className) return; + + const failure = getFailureMessage({ className }); + + this.addFailureAtNode(parentExpression, failure); + } +} diff --git a/src/noQueriesMetadataPropertyRule.ts b/src/noQueriesMetadataPropertyRule.ts new file mode 100644 index 000000000..bd9cc9edb --- /dev/null +++ b/src/noQueriesMetadataPropertyRule.ts @@ -0,0 +1,34 @@ +import { IOptions, IRuleMetadata } from 'tslint/lib'; +import { UsePropertyDecorator } from './propertyDecoratorBase'; +import { Decorators } from './util/utils'; + +const METADATA_PROPERTY_NAME = 'queries'; + +export class Rule extends UsePropertyDecorator { + static readonly metadata: IRuleMetadata = { + 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.`, + 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`; + + constructor(options: IOptions) { + super( + { + decoratorName: [Decorators.ContentChild, Decorators.ContentChildren, Decorators.ViewChild, Decorators.ViewChildren], + errorMessage: Rule.FAILURE_STRING, + propertyName: METADATA_PROPERTY_NAME + }, + options + ); + } +} diff --git a/src/noQueriesParameterRule.ts b/src/noQueriesParameterRule.ts deleted file mode 100644 index 14eeaedab..000000000 --- a/src/noQueriesParameterRule.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { IOptions, IRuleMetadata, Utils } from 'tslint/lib'; -import { UsePropertyDecorator } from './propertyDecoratorBase'; - -export class Rule extends UsePropertyDecorator { - static readonly metadata: IRuleMetadata = { - description: - 'Use @ContentChild, @ContentChildren, @ViewChild or @ViewChildren instead of the `queries` property of `@Component` or `@Directive` metadata.', - options: null, - optionsDescription: 'Not configurable.', - rationale: Utils.dedent` - The property associated with @ContentChild, @ContentChildren, @ViewChild or @ViewChildren - can be modified only in a single place: in the directive's class. If you use the queries metadata - property, you must modify both the property declaration inside the controller, and the metadata - associated with the directive. - `, - ruleName: 'no-queries-parameter', - type: 'style', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = 'Use @ContentChild, @ContentChildren, @ViewChild or @ViewChildren instead of the queries property'; - - constructor(options: IOptions) { - super( - { - decoratorName: ['ContentChild', 'ContentChildren', 'ViewChild', 'ViewChildren'], - errorMessage: Rule.FAILURE_STRING, - propertyName: 'queries' - }, - options - ); - } -} diff --git a/src/noTemplateCallExpressionRule.ts b/src/noTemplateCallExpressionRule.ts deleted file mode 100644 index 1fa22bba9..000000000 --- a/src/noTemplateCallExpressionRule.ts +++ /dev/null @@ -1,51 +0,0 @@ -import * as e from '@angular/compiler/src/expression_parser/ast'; -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { NgWalker, NgWalkerConfig } from './angular/ngWalker'; -import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; -import { RecursiveAngularExpressionVisitor } from './angular/templates/recursiveAngularExpressionVisitor'; - -export class Rule extends Lint.Rules.AbstractRule { - static readonly metadata: Lint.IRuleMetadata = { - description: 'Call expressions are not allowed in templates except in output handlers.', - options: null, - optionsDescription: 'Not configurable.', - rationale: 'The change detector will call functions used in templates very often. Use an observable instead.', - ruleName: 'no-template-call-expression', - type: 'maintainability', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = 'Call expressions are not allowed in templates except in output handlers'; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const walkerConfig: NgWalkerConfig = { - expressionVisitorCtrl: ExpressionVisitor, - templateVisitorCtrl: TemplateVisitor - }; - - return this.applyWithWalker(new NgWalker(sourceFile, this.getOptions(), walkerConfig)); - } -} - -class TemplateVisitor extends BasicTemplateAstVisitor { - visitEvent() {} -} - -class ExpressionVisitor extends RecursiveAngularExpressionVisitor { - private allowedMethodNames = ['$any']; - - visitFunctionCall(node: e.FunctionCall, context: any) { - this.addFailureFromStartToEnd(node.span.start, node.span.end, Rule.FAILURE_STRING); - - super.visitFunctionCall(node, context); - } - - visitMethodCall(node: e.MethodCall, context: any) { - if (this.allowedMethodNames.indexOf(node.name) === -1) { - this.addFailureFromStartToEnd(node.span.start, node.span.end, Rule.FAILURE_STRING); - } - - super.visitMethodCall(node, context); - } -} diff --git a/src/pipeImpureRule.ts b/src/pipeImpureRule.ts deleted file mode 100644 index 580cb3b99..000000000 --- a/src/pipeImpureRule.ts +++ /dev/null @@ -1,47 +0,0 @@ -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { sprintf } from 'sprintf-js'; -import { NgWalker } from './angular/ngWalker'; -import { getDecoratorArgument } from './util/utils'; - -export class Rule extends Lint.Rules.AbstractRule { - static readonly metadata: Lint.IRuleMetadata = { - ruleName: 'pipe-impure', - type: 'functionality', - description: 'Pipes cannot be declared as impure.', - rationale: 'Impure pipes do not perform well because they run on every change detection cycle.', - options: null, - optionsDescription: 'Not configurable.', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = 'Warning: impure pipe declared in class %s'; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new ClassMetadataWalker(sourceFile, this.getOptions())); - } -} - -export class ClassMetadataWalker extends NgWalker { - protected visitNgPipe(controller: ts.ClassDeclaration, decorator: ts.Decorator) { - this.validatePipe(controller.name!.text, decorator); - super.visitNgPipe(controller, decorator); - } - - private validatePipe(className: string, decorator: ts.Decorator): void { - const argument = getDecoratorArgument(decorator)!; - const property = argument.properties.find(p => p.name!.getText() === 'pure'); - - if (!property) { - return; - } - - const propValue = ts.isPropertyAssignment(property) ? property.initializer.getText() : undefined; - - if (!propValue || propValue !== 'false') { - return; - } - - this.addFailureAtNode(property, sprintf(Rule.FAILURE_STRING, className)); - } -} diff --git a/src/preferInlineDecoratorRule.ts b/src/preferInlineDecoratorRule.ts index 6bca66db6..de76eebb2 100644 --- a/src/preferInlineDecoratorRule.ts +++ b/src/preferInlineDecoratorRule.ts @@ -1,28 +1,10 @@ -import * as ts from 'typescript'; - -import { IOptions, IRuleMetadata, Replacement, RuleFailure, Rules } from 'tslint/lib'; -import { Decorator, Node, PropertyAccessExpression, SourceFile } from 'typescript'; +import { IOptions, IRuleMetadata, Replacement, RuleFailure } from 'tslint/lib'; +import { AbstractRule } from 'tslint/lib/rules'; +import { Decorator, isPropertyDeclaration, SourceFile } from 'typescript'; import { NgWalker } from './angular/ngWalker'; -import { getDecoratorName, isSameLine } from './util/utils'; - -enum Decorators { - ContentChild = 'ContentChild', - ContentChildren = 'ContentChildren', - HostBinding = 'HostBinding', - HostListener = 'HostListener', - Input = 'Input', - Output = 'Output', - ViewChild = 'ViewChild', - ViewChildren = 'ViewChildren' -} - -type DecoratorKeys = keyof typeof Decorators; +import { decoratorKeys, Decorators, DECORATORS, getDecoratorName, isSameLine } from './util/utils'; -const enumKeys = Object.keys(Decorators); - -export const decoratorKeys: ReadonlySet = new Set(enumKeys); - -export class Rule extends Rules.AbstractRule { +export class Rule extends AbstractRule { static readonly metadata: IRuleMetadata = { description: 'Ensures that decorators are on the same line as the property/method it decorates.', descriptionDetails: 'See more at https://angular.io/guide/styleguide#style-05-12.', @@ -30,10 +12,10 @@ export class Rule extends Rules.AbstractRule { optionExamples: [true, [true, Decorators.HostListener]], options: { items: { - enum: enumKeys, + enum: decoratorKeys, type: 'string' }, - maxLength: decoratorKeys.size, + maxLength: DECORATORS.size, minLength: 0, type: 'array' }, @@ -62,44 +44,44 @@ export class Rule extends Rules.AbstractRule { } } -export const getFailureMessage = (): string => { - return Rule.FAILURE_STRING; -}; - export class PreferInlineDecoratorWalker extends NgWalker { - private readonly blacklistedDecorators: typeof decoratorKeys; + private readonly blacklistedDecorators: ReadonlySet; constructor(source: SourceFile, options: IOptions) { super(source, options); - this.blacklistedDecorators = new Set(options.ruleArguments); + this.blacklistedDecorators = new Set(options.ruleArguments); } - protected visitMethodDecorator(decorator: Decorator) { - this.validateDecorator(decorator, decorator.parent!); + protected visitMethodDecorator(decorator: Decorator): void { + this.validateDecorator(decorator); super.visitMethodDecorator(decorator); } - protected visitPropertyDecorator(decorator: Decorator) { - this.validateDecorator(decorator, decorator.parent!); + protected visitPropertyDecorator(decorator: Decorator): void { + this.validateDecorator(decorator); super.visitPropertyDecorator(decorator); } - private validateDecorator(decorator: Decorator, property: Node) { - const decoratorName = getDecoratorName(decorator) as DecoratorKeys; + private validateDecorator(decorator: Decorator): void { + const decoratorName = getDecoratorName(decorator); + + if (!decoratorName) return; + const isDecoratorBlacklisted = this.blacklistedDecorators.has(decoratorName); - if (isDecoratorBlacklisted) { - return; - } + if (isDecoratorBlacklisted) return; const decoratorStartPos = decorator.getStart(); - const propertyStartPos = (property as PropertyAccessExpression).name.getStart(); + const { parent: property } = decorator; + + if (!property || !isPropertyDeclaration(property)) return; - if (isSameLine(this.getSourceFile(), decoratorStartPos, propertyStartPos)) { - return; - } + const propertyStartPos = property.name.getStart(); + + if (isSameLine(this.getSourceFile(), decoratorStartPos, propertyStartPos)) return; const fix = Replacement.deleteFromTo(decorator.getEnd(), propertyStartPos - 1); - this.addFailureAt(decoratorStartPos, property.getWidth(), getFailureMessage(), fix); + + this.addFailureAt(decoratorStartPos, property.getWidth(), Rule.FAILURE_STRING, fix); } } diff --git a/src/propertyDecoratorBase.ts b/src/propertyDecoratorBase.ts index e9be0ace4..dd5be3387 100644 --- a/src/propertyDecoratorBase.ts +++ b/src/propertyDecoratorBase.ts @@ -1,63 +1,69 @@ import { sprintf } from 'sprintf-js'; -import * as Lint from 'tslint'; -import * as ts from 'typescript'; +import { IOptions, RuleFailure, RuleWalker } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { arrayify } from 'tslint/lib/utils'; +import { ClassDeclaration, createNodeArray, Decorator, isObjectLiteralExpression, ObjectLiteralExpression, SourceFile } from 'typescript'; import { getDecoratorArgument, getDecoratorName } from './util/utils'; -export interface IUsePropertyDecoratorConfig { - propertyName: string; - decoratorName: string | string[]; - errorMessage: string; +export interface PropertyDecoratorConfig { + readonly decoratorName: string | string[]; + readonly errorMessage: string; + readonly propertyName: string; } -export class UsePropertyDecorator extends Lint.Rules.AbstractRule { - public static formatFailureString(config: IUsePropertyDecoratorConfig, decoratorStr: string, className: string) { - const { decoratorName, errorMessage, propertyName } = config; - let decorators: string | string[]; +const formatFailureString = (config: PropertyDecoratorConfig, decoratorStr: string, className: string): string => { + const { decoratorName, errorMessage, propertyName } = config; + const decorators = arrayify(decoratorName) + .map(d => `"@${d}"`) + .join(', '); - if (decoratorName instanceof Array) { - decorators = decoratorName.map(d => `"@${d}"`).join(', '); - } else { - decorators = `"@${decoratorName}"`; - } - - return sprintf(errorMessage, decoratorStr, className, propertyName, decorators); - } + return sprintf(errorMessage, decoratorStr, className, propertyName, decorators); +}; - constructor(private config: IUsePropertyDecoratorConfig, options: Lint.IOptions) { +export class UsePropertyDecorator extends AbstractRule { + constructor(private readonly config: PropertyDecoratorConfig, options: IOptions) { super(options); } - public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + apply(sourceFile: SourceFile): RuleFailure[] { return this.applyWithWalker(new DirectiveMetadataWalker(sourceFile, this.getOptions(), this.config)); } } -class DirectiveMetadataWalker extends Lint.RuleWalker { - constructor(sourceFile: ts.SourceFile, options: Lint.IOptions, private config: IUsePropertyDecoratorConfig) { +class DirectiveMetadataWalker extends RuleWalker { + constructor(sourceFile: SourceFile, options: IOptions, private readonly config: PropertyDecoratorConfig) { super(sourceFile, options); } - visitClassDeclaration(node: ts.ClassDeclaration) { - ts.createNodeArray(node.decorators).forEach(this.validateDecorator.bind(this, node.name!.text)); + protected visitClassDeclaration(node: ClassDeclaration): void { + this.validateClassDeclaration(node); super.visitClassDeclaration(node); } - private validateDecorator(className: string, decorator: ts.Decorator) { - const argument = getDecoratorArgument(decorator)!; - const name = getDecoratorName(decorator); + private validateClassDeclaration(node: ClassDeclaration): void { + const { decorators, name: nodeName } = node; - if (name && argument && /^(Component|Directive)$/.test(name)) { - this.validateProperty(className, name, argument); - } + if (!nodeName) return; + + createNodeArray(decorators).forEach(decorator => this.validateDecorator(nodeName.text, decorator)); } - private validateProperty(className: string, decoratorName: string, arg: ts.ObjectLiteralExpression) { - if (!ts.isObjectLiteralExpression(arg)) { + private validateDecorator(className: string, decorator: Decorator): void { + const decoratorArgument = getDecoratorArgument(decorator); + const decoratorName = getDecoratorName(decorator); + + if (!decoratorName || !decoratorArgument || !/^(Component|Directive)$/.test(decoratorName)) { return; } - arg.properties.filter(prop => prop.name!.getText() === this.config.propertyName).forEach(prop => { - this.addFailureAtNode(prop, UsePropertyDecorator.formatFailureString(this.config, decoratorName, className)); - }); + this.validateProperty(className, decoratorName, decoratorArgument); + } + + private validateProperty(className: string, decoratorName: string, decoratorArgument: ObjectLiteralExpression): void { + if (!isObjectLiteralExpression(decoratorArgument)) return; + + decoratorArgument.properties + .filter(property => property.name && property.name.getText() === this.config.propertyName) + .forEach(property => this.addFailureAtNode(property, formatFailureString(this.config, decoratorName, className))); } } diff --git a/src/relativeUrlPrefixRule.ts b/src/relativeUrlPrefixRule.ts index a43fe1083..792aa647a 100644 --- a/src/relativeUrlPrefixRule.ts +++ b/src/relativeUrlPrefixRule.ts @@ -1,7 +1,7 @@ import { IOptions, IRuleMetadata, RuleFailure, Rules } from 'tslint/lib'; +import * as ts from 'typescript'; import { SourceFile } from 'typescript/lib/typescript'; import { NgWalker } from './angular/ngWalker'; -import * as ts from 'typescript'; export class Rule extends Rules.AbstractRule { static readonly metadata: IRuleMetadata = { @@ -18,11 +18,11 @@ export class Rule extends Rules.AbstractRule { static readonly FAILURE_STRING = 'The ./ prefix is standard syntax for relative URLs. (https://angular.io/styleguide#style-05-04)'; apply(sourceFile: SourceFile): RuleFailure[] { - return this.applyWithWalker(new RelativePathExternalResourcesRuleWalker(sourceFile, this.getOptions())); + return this.applyWithWalker(new RelativeUrlPrefixWalker(sourceFile, this.getOptions())); } } -export class RelativePathExternalResourcesRuleWalker extends NgWalker { +class RelativeUrlPrefixWalker extends NgWalker { constructor(sourceFile: SourceFile, options: IOptions) { super(sourceFile, options); } @@ -38,7 +38,6 @@ export class RelativePathExternalResourcesRuleWalker extends NgWalker { } else if (prop && prop.name.text === 'styleUrls') { if (prop.initializer.elements.length > 0) { prop.initializer.elements.forEach(e => { - const url = e.text; this.checkStyleUrls(e); }); } diff --git a/src/templateBananaInBoxRule.ts b/src/templateBananaInBoxRule.ts new file mode 100644 index 000000000..629890cbe --- /dev/null +++ b/src/templateBananaInBoxRule.ts @@ -0,0 +1,60 @@ +import { BoundEventAst } from '@angular/compiler'; +import { IRuleMetadata, Replacement, RuleFailure } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { SourceFile } from 'typescript'; +import { NgWalker } from './angular/ngWalker'; +import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; + +const INVALID_BOX = /^\[(?!\()(.*)(? { + const { + sourceSpan: { + end: { offset: endOffset }, + start: { offset: startOffset } + } + } = failureParameters.ast; + + failureParameters.context.addFailureFromStartToEnd(startOffset, endOffset, failureParameters.message); +}; + +export class Rule extends AbstractRule { + static readonly metadata: IRuleMetadata = { + description: 'Ensures following best practices for i18n.', + optionExamples: [[true, OPTION_CHECK_ID], [true, OPTION_CHECK_TEXT], [true, OPTION_CHECK_ID, OPTION_CHECK_TEXT]], + options: { + items: { + enum: [OPTION_CHECK_ID, OPTION_CHECK_TEXT], + type: 'string' + }, + maxLength: 2, + minLength: 1, + type: 'array' + }, + optionsDescription: dedent` + One (or both) of the following arguments must be provided: + * \`${OPTION_CHECK_ID}\` Makes sure i18n attributes have ID specified + * \`${OPTION_CHECK_TEXT}\` Makes sure there are no elements with text content but no i18n attribute + `, + rationale: 'Makes the code more maintainable in i18n sense.', + ruleName: 'template-i18n', + type: 'maintainability', + typescriptOnly: true + }; + + static readonly FAILURE_STRING_ATTR = 'Missing custom message identifier. For more information visit https://angular.io/guide/i18n'; + static readonly FAILURE_STRING_TEXT = 'Each element containing text node should have an i18n attribute'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker( + new NgWalker(sourceFile, this.getOptions(), { + templateVisitorCtrl: TemplateI18NVisitor + }) + ); + } + + isEnabled(): boolean { + const { + metadata: { + options: { + items: { enum: enumItems }, + maxLength, + minLength + } + } + } = Rule; + const { length } = this.ruleArguments.filter(ruleArgument => enumItems.indexOf(ruleArgument) !== -1); + + return super.isEnabled() && length >= minLength && length <= maxLength; + } +} + +class TemplateI18NAttrVisitor extends BasicTemplateAstVisitor implements ConfigurableVisitor { + getCheckOption(): CheckOption { + return OPTION_CHECK_ID; + } + + visitAttr(ast: AttrAst, context: BasicTemplateAstVisitor): any { + this.validateAttr(ast, context); + super.visitAttr(ast, context); + } + + private validateAttr(ast: AttrAst, context: BasicTemplateAstVisitor): void { + if (ast.name !== 'i18n') return; + + const parts = ast.value.split('@@'); + + if (parts.length > 1 && parts[1].length !== 0) return; + + generateFailure({ ast, context, message: Rule.FAILURE_STRING_ATTR }); + } +} + +class TemplateI18NTextVisitor extends BasicTemplateAstVisitor implements ConfigurableVisitor { + private hasI18n = false; + private readonly nestedElements: string[] = []; + private readonly visited = new Set(); + + getCheckOption(): CheckOption { + return OPTION_CHECK_TEXT; + } + + visitBoundText(text: BoundTextAst, context: BasicTemplateAstVisitor): any { + this.validateBoundText(text, context); + super.visitBoundText(text, context); + } + + visitElement(element: ElementAst, context: BasicTemplateAstVisitor): any { + const originalI18n = this.hasI18n; + this.hasI18n = originalI18n || element.attrs.some(e => e.name === 'i18n'); + this.nestedElements.push(element.name); + super.visitElement(element, context); + this.nestedElements.pop(); + this.hasI18n = originalI18n; + super.visitElement(element, context); + } + + visitText(text: TextAst, context: BasicTemplateAstVisitor): any { + this.validateText(text, context); + super.visitText(text, context); + } + + private validateBoundText(text: BoundTextAst, context: BasicTemplateAstVisitor): void { + if (this.visited.has(text)) return; + + this.visited.add(text); + + const { value } = text; + + if (!(value instanceof ASTWithSource) || !(value.ast instanceof Interpolation) || this.hasI18n) { + return; + } + + const isTextEmpty = !value.ast.strings.some(s => /\w+/.test(s)); + + if (isTextEmpty) return; + + generateFailure({ ast: text, context, message: Rule.FAILURE_STRING_TEXT }); + } + + private validateText(text: TextAst, context: BasicTemplateAstVisitor): void { + if (this.visited.has(text)) return; + + this.visited.add(text); + const isTextEmpty = text.value.trim().length === 0; + + if (isTextEmpty || (this.hasI18n && this.nestedElements.length > 0)) return; + + generateFailure({ ast: text, context, message: Rule.FAILURE_STRING_TEXT }); + } +} + +class TemplateI18NVisitor extends BasicTemplateAstVisitor { + private readonly visitors: ReadonlyArray = [ + new TemplateI18NAttrVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart), + new TemplateI18NTextVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart) + ]; + + visit(node: TemplateAst, context: BasicTemplateAstVisitor): any { + super.visit!(node, context); + } + + visitAttr(ast: AttrAst, context: BasicTemplateAstVisitor): any { + this.validateAttr(ast); + super.visitAttr(ast, context); + } + + visitBoundText(text: BoundTextAst, context: any): any { + this.validateBoundText(text); + super.visitBoundText(text, context); + } + + visitElement(element: ElementAst, context: BasicTemplateAstVisitor): any { + this.validateElement(element); + super.visitElement(element, context); + } + + visitText(text: TextAst, context: BasicTemplateAstVisitor): any { + this.validateText(text); + super.visitText(text, context); + } + + private validateAttr(ast: AttrAst): void { + const options = this.getOptions(); + this.visitors + .filter(v => options.indexOf(v.getCheckOption()) !== -1) + .map(v => v.visitAttr(ast, this)) + .filter(Boolean) + .forEach(f => + this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) + ); + } + + private validateBoundText(text: BoundTextAst): void { + const options = this.getOptions(); + this.visitors + .filter(v => options.indexOf(v.getCheckOption()) !== -1) + .map(v => v.visitBoundText(text, this)) + .filter(Boolean) + .forEach(f => + this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) + ); + } + + private validateElement(element: ElementAst): void { + const options = this.getOptions(); + this.visitors + .filter(v => options.indexOf(v.getCheckOption()) !== -1) + .map(v => v.visitElement(element, this)) + .filter(Boolean) + .forEach(f => + this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) + ); + } + + private validateText(text: TextAst): void { + const options = this.getOptions(); + this.visitors + .filter(v => options.indexOf(v.getCheckOption()) !== -1) + .map(v => v.visitText(text, this)) + .filter(Boolean) + .forEach(f => + this.addFailureFromStartToEnd(f.getStartPosition().getPosition(), f.getEndPosition().getPosition(), f.getFailure(), f.getFix()) + ); + } +} diff --git a/src/templateNoCallExpressionRule.ts b/src/templateNoCallExpressionRule.ts new file mode 100644 index 000000000..99ee795f3 --- /dev/null +++ b/src/templateNoCallExpressionRule.ts @@ -0,0 +1,59 @@ +import { MethodCall } from '@angular/compiler'; +import { IRuleMetadata, RuleFailure } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { SourceFile } from 'typescript'; +import { NgWalker } from './angular/ngWalker'; +import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; +import { RecursiveAngularExpressionVisitor } from './angular/templates/recursiveAngularExpressionVisitor'; + +const ALLOWED_METHOD_NAMES: ReadonlySet = new Set(['$any']); + +export class Rule extends AbstractRule { + static readonly metadata: IRuleMetadata = { + description: 'Disallows calling expressions in templates, except for output handlers.', + options: null, + optionsDescription: 'Not configurable.', + rationale: 'Calling expressions in templates causes it to run on every change detection cycle and may cause performance issues.', + ruleName: 'template-no-call-expression', + type: 'maintainability', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = 'Avoid calling expressions in templates'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker( + new NgWalker(sourceFile, this.getOptions(), { + expressionVisitorCtrl: ExpressionVisitor, + templateVisitorCtrl: TemplateVisitor + }) + ); + } +} + +class TemplateVisitor extends BasicTemplateAstVisitor { + visitEvent(): any {} +} + +class ExpressionVisitor extends RecursiveAngularExpressionVisitor { + visitMethodCall(ast: MethodCall, context: any): any { + this.validateMethodCall(ast); + super.visitMethodCall(ast, context); + } + + private generateFailure(ast: MethodCall): void { + const { + span: { end: endSpan, start: startSpan } + } = ast; + + this.addFailureFromStartToEnd(startSpan, endSpan, Rule.FAILURE_STRING); + } + + private validateMethodCall(ast: MethodCall): void { + const isMethodAllowed = ALLOWED_METHOD_NAMES.has(ast.name); + + if (isMethodAllowed) return; + + this.generateFailure(ast); + } +} diff --git a/src/templatesNoNegatedAsyncRule.ts b/src/templateNoNegatedAsyncRule.ts similarity index 66% rename from src/templatesNoNegatedAsyncRule.ts rename to src/templateNoNegatedAsyncRule.ts index 0dd2c85b6..7e0189841 100644 --- a/src/templatesNoNegatedAsyncRule.ts +++ b/src/templateNoNegatedAsyncRule.ts @@ -1,15 +1,40 @@ -import { AST, BindingPipe, LiteralPrimitive } from '@angular/compiler'; -import { Binary, PrefixNot } from '@angular/compiler/src/expression_parser/ast'; -import { IRuleMetadata, RuleFailure, Rules, Utils } from 'tslint/lib'; +import { AST, Binary, BindingPipe, LiteralPrimitive, PrefixNot } from '@angular/compiler'; +import { IRuleMetadata, RuleFailure } from 'tslint/lib'; +import { AbstractRule } from 'tslint/lib/rules'; +import { dedent } from 'tslint/lib/utils'; import { SourceFile } from 'typescript'; import { NgWalker } from './angular/ngWalker'; import { RecursiveAngularExpressionVisitor } from './angular/templates/recursiveAngularExpressionVisitor'; const unstrictEqualityOperator = '=='; -const isAsyncBinding = (ast: AST): boolean => { - return ast instanceof BindingPipe && ast.name === 'async'; -}; +const isAsyncBinding = (ast: AST): boolean => ast instanceof BindingPipe && ast.name === 'async'; + +export class Rule extends AbstractRule { + static readonly metadata: IRuleMetadata = { + description: 'Ensures that strict equality is used when evaluating negations on async pipe output.', + options: null, + optionsDescription: 'Not configurable.', + rationale: dedent` + Angular's async pipes emit null initially, prior to the observable emitting any values, or the promise resolving. This can cause negations, like + *ngIf="!(myConditional | async)" to thrash the layout and cause expensive side-effects like firing off XHR requests for a component which should not be shown. + `, + ruleName: 'template-no-negated-async', + type: 'functionality', + typescriptOnly: true + }; + + static readonly FAILURE_STRING_NEGATED_PIPE = 'Async pipes can not be negated, use (observable | async) === false instead'; + static readonly FAILURE_STRING_UNSTRICT_EQUALITY = 'Async pipes must use strict equality `===` when comparing with `false`'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker( + new NgWalker(sourceFile, this.getOptions(), { + expressionVisitorCtrl: TemplateToNgTemplateVisitor + }) + ); + } +} class TemplateToNgTemplateVisitor extends RecursiveAngularExpressionVisitor { visitBinary(ast: Binary, context: any): any { @@ -22,6 +47,17 @@ class TemplateToNgTemplateVisitor extends RecursiveAngularExpressionVisitor { super.visitPrefixNot(ast, context); } + private generateFailure( + ast: Binary | PrefixNot, + errorMessage: typeof Rule.FAILURE_STRING_NEGATED_PIPE | typeof Rule.FAILURE_STRING_UNSTRICT_EQUALITY + ): void { + const { + span: { end: spanEnd, start: spanStart } + } = ast; + + this.addFailureFromStartToEnd(spanStart, spanEnd, errorMessage); + } + private validateBinary(ast: Binary): void { const { left, operation, right } = ast; @@ -35,44 +71,8 @@ class TemplateToNgTemplateVisitor extends RecursiveAngularExpressionVisitor { private validatePrefixNot(ast: PrefixNot): void { const { expression } = ast; - if (!isAsyncBinding(expression)) { - return; - } + if (!isAsyncBinding(expression)) return; this.generateFailure(ast, Rule.FAILURE_STRING_NEGATED_PIPE); } - - private generateFailure(ast: Binary | PrefixNot, errorMessage: string): void { - const { - span: { end: spanEnd, start: spanStart } - } = ast; - - this.addFailureFromStartToEnd(spanStart, spanEnd, errorMessage); - } -} - -export class Rule extends Rules.AbstractRule { - static readonly metadata: IRuleMetadata = { - description: 'Ensures that strict equality is used when evaluating negations on async pipe output.', - options: null, - optionsDescription: 'Not configurable.', - rationale: Utils.dedent` - Angular's async pipes emit null initially, prior to the observable emitting any values, or the promise resolving. This can cause negations, like - *ngIf="!(myConditional | async)" to thrash the layout and cause expensive side-effects like firing off XHR requests for a component which should not be shown. - `, - ruleName: 'templates-no-negated-async', - type: 'functionality', - typescriptOnly: true - }; - - static readonly FAILURE_STRING_NEGATED_PIPE = 'Async pipes can not be negated, use (observable | async) === false instead'; - static readonly FAILURE_STRING_UNSTRICT_EQUALITY = 'Async pipes must use strict equality `===` when comparing with `false`'; - - apply(sourceFile: SourceFile): RuleFailure[] { - return this.applyWithWalker( - new NgWalker(sourceFile, this.getOptions(), { - expressionVisitorCtrl: TemplateToNgTemplateVisitor - }) - ); - } } diff --git a/src/trackByFunctionRule.ts b/src/templateUseTrackByFunctionRule.ts similarity index 59% rename from src/trackByFunctionRule.ts rename to src/templateUseTrackByFunctionRule.ts index 99a9d3d23..aa0b827cb 100644 --- a/src/trackByFunctionRule.ts +++ b/src/templateUseTrackByFunctionRule.ts @@ -1,19 +1,22 @@ import { BoundDirectivePropertyAst } from '@angular/compiler'; -import { IRuleMetadata, RuleFailure, Rules } from 'tslint/lib'; +import { IRuleMetadata, RuleFailure } from 'tslint/lib'; +import { AbstractRule } from 'tslint/lib/rules'; import { SourceFile } from 'typescript/lib/typescript'; import { NgWalker } from './angular/ngWalker'; import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; -//current offSet into the template -export let curOffSet = 0; +// current offset into the template +export let curentOffset = 0; -export class Rule extends Rules.AbstractRule { +const PATTERN = /\s*ngFor.*\s*trackBy\s*:|\[ngForTrackBy\]\s*=\s*['"].*['"]/; + +export class Rule extends AbstractRule { static readonly metadata: IRuleMetadata = { - description: 'Ensures a trackBy function is used.', + description: 'Ensures trackBy function is used.', options: null, optionsDescription: 'Not configurable.', rationale: "The use of 'trackBy' is considered a good practice.", - ruleName: 'trackBy-function', + ruleName: 'template-use-track-by-function', type: 'maintainability', typescriptOnly: true }; @@ -21,16 +24,13 @@ export class Rule extends Rules.AbstractRule { static readonly FAILURE_STRING = 'Missing trackBy function in ngFor directive'; apply(sourceFile: SourceFile): RuleFailure[] { - curOffSet = 0; - return this.applyWithWalker(new NgWalker(sourceFile, this.getOptions(), { templateVisitorCtrl: TrackByTemplateVisitor })); + curentOffset = 0; + + return this.applyWithWalker(new NgWalker(sourceFile, this.getOptions(), { templateVisitorCtrl: TemplateUseTrackByFunctionVisitor })); } } -export const getFailureMessage = (): string => { - return Rule.FAILURE_STRING; -}; - -class TrackByFunctionTemplateVisitor extends BasicTemplateAstVisitor { +class TemplateUseTrackByFunctionTemplateVisitor extends BasicTemplateAstVisitor { visitDirectiveProperty(prop: BoundDirectivePropertyAst, context: BasicTemplateAstVisitor): any { this.validateDirective(prop, context); super.visitDirectiveProperty(prop, context); @@ -39,16 +39,7 @@ class TrackByFunctionTemplateVisitor extends BasicTemplateAstVisitor { private validateDirective(prop: BoundDirectivePropertyAst, context: BasicTemplateAstVisitor): any { const { templateName } = prop; - if (templateName !== 'ngForOf') { - return; - } - - const pattern = /\s*ngFor.*\s*trackBy\s*:|\[ngForTrackBy\]\s*=\s*['"].*['"]/; - - if (pattern.test(context.codeWithMap.source!.substr(curOffSet))) { - curOffSet = prop.sourceSpan.end.offset; - return; - } + if (templateName !== 'ngForOf') return; const { sourceSpan: { @@ -57,18 +48,23 @@ class TrackByFunctionTemplateVisitor extends BasicTemplateAstVisitor { } } = prop; - context.addFailureFromStartToEnd(startOffset, endOffset, getFailureMessage()); + if (PATTERN.test((context.codeWithMap.source || '').substr(curentOffset))) { + curentOffset = endOffset; + + return; + } + + context.addFailureFromStartToEnd(startOffset, endOffset, Rule.FAILURE_STRING); } } -class TrackByTemplateVisitor extends BasicTemplateAstVisitor { +class TemplateUseTrackByFunctionVisitor extends BasicTemplateAstVisitor { private readonly visitors: ReadonlySet = new Set([ - new TrackByFunctionTemplateVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart) + new TemplateUseTrackByFunctionTemplateVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart) ]); visitDirectiveProperty(prop: BoundDirectivePropertyAst, context: BasicTemplateAstVisitor): any { this.visitors.forEach(visitor => visitor.visitDirectiveProperty(prop, this)); - super.visitDirectiveProperty(prop, context); } } diff --git a/src/useComponentSelectorRule.ts b/src/useComponentSelectorRule.ts new file mode 100644 index 000000000..32ecb9609 --- /dev/null +++ b/src/useComponentSelectorRule.ts @@ -0,0 +1,52 @@ +import { sprintf } from 'sprintf-js'; +import { IRuleMetadata, RuleFailure } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { SourceFile } from 'typescript'; +import { ComponentMetadata } from './angular/metadata'; +import { NgWalker } from './angular/ngWalker'; + +interface FailureParameters { + readonly className: string; +} + +export const getFailureMessage = (failureParameters: FailureParameters): string => + sprintf(Rule.FAILURE_STRING, failureParameters.className); + +export class Rule extends AbstractRule { + static readonly metadata: IRuleMetadata = { + description: 'Component selector must be declared.', + options: null, + optionsDescription: 'Not configurable.', + rationale: 'Omit the component selector makes debugging difficult.', + ruleName: 'use-component-selector', + type: 'maintainability', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = 'The selector of the component "%s" is mandatory'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker(new UseComponentSelectorValidatorWalker(sourceFile, this.getOptions())); + } +} + +class UseComponentSelectorValidatorWalker extends NgWalker { + protected visitNgComponent(metadata: ComponentMetadata): void { + this.validateComponent(metadata); + super.visitNgComponent(metadata); + } + + private validateComponent(metadata: ComponentMetadata): void { + const { + decorator: metadataDecorator, + controller: { name: controllerName }, + selector: metadataSelector + } = metadata; + + if (metadataSelector || !controllerName) return; + + const failure = getFailureMessage({ className: controllerName.text }); + + this.addFailureAtNode(metadataDecorator, failure); + } +} diff --git a/src/useViewEncapsulationRule.ts b/src/useComponentViewEncapsulationRule.ts similarity index 80% rename from src/useViewEncapsulationRule.ts rename to src/useComponentViewEncapsulationRule.ts index b0e52c85d..f209caf2e 100644 --- a/src/useViewEncapsulationRule.ts +++ b/src/useComponentViewEncapsulationRule.ts @@ -1,20 +1,21 @@ import { IRuleMetadata, RuleFailure, Rules } from 'tslint/lib'; +import { AbstractRule } from 'tslint/lib/rules'; import { isPropertyAccessExpression, SourceFile } from 'typescript/lib/typescript'; import { ComponentMetadata } from './angular/metadata'; import { NgWalker } from './angular/ngWalker'; import { getDecoratorPropertyInitializer } from './util/utils'; -export class Rule extends Rules.AbstractRule { +export class Rule extends AbstractRule { static readonly metadata: IRuleMetadata = { description: 'Disallows using of ViewEncapsulation.None.', options: null, optionsDescription: 'Not configurable.', - ruleName: 'use-view-encapsulation', + ruleName: 'use-component-view-encapsulation', type: 'maintainability', typescriptOnly: true }; - static readonly FAILURE_STRING = 'Using "ViewEncapsulation.None" will make your styles global which may have unintended effect'; + static readonly FAILURE_STRING = 'Using "ViewEncapsulation.None" makes your styles global, which may have an unintended effect'; apply(sourceFile: SourceFile): RuleFailure[] { return this.applyWithWalker(new ViewEncapsulationWalker(sourceFile, this.getOptions())); @@ -22,7 +23,7 @@ export class Rule extends Rules.AbstractRule { } class ViewEncapsulationWalker extends NgWalker { - protected visitNgComponent(metadata: ComponentMetadata) { + protected visitNgComponent(metadata: ComponentMetadata): void { this.validateComponent(metadata); super.visitNgComponent(metadata); } diff --git a/src/useHostPropertyDecoratorRule.ts b/src/useHostPropertyDecoratorRule.ts deleted file mode 100644 index 2d7954608..000000000 --- a/src/useHostPropertyDecoratorRule.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { IOptions, IRuleMetadata, Utils } from 'tslint/lib'; -import { UsePropertyDecorator } from './propertyDecoratorBase'; - -export class Rule extends UsePropertyDecorator { - static readonly metadata: IRuleMetadata = { - description: 'Use @HostProperty decorator rather than the `host` property of `@Component` and `@Directive` metadata.', - descriptionDetails: 'See more at https://angular.io/styleguide#style-06-03.', - options: null, - optionsDescription: 'Not configurable.', - rationale: Utils.dedent` - The property associated with @HostBinding or the method associated with @HostListener - can be modified only in a single place: in the directive's class. If you use the host metadata - property, you must modify both the property declaration inside the controller, and the metadata - associated with the directive. - `, - ruleName: 'use-host-property-decorator', - type: 'style', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = - 'Use @HostBindings and @HostListeners instead of the host property (https://angular.io/styleguide#style-06-03)'; - - constructor(options: IOptions) { - super( - { - decoratorName: ['HostBindings', 'HostListeners'], - errorMessage: Rule.FAILURE_STRING, - propertyName: 'host' - }, - options - ); - } -} diff --git a/src/useInputPropertyDecoratorRule.ts b/src/useInputPropertyDecoratorRule.ts deleted file mode 100644 index de0c9848c..000000000 --- a/src/useInputPropertyDecoratorRule.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { IOptions, IRuleMetadata, Utils } from 'tslint/lib'; -import { UsePropertyDecorator } from './propertyDecoratorBase'; - -export class Rule extends UsePropertyDecorator { - static readonly metadata: IRuleMetadata = { - description: 'Use `@Input` decorator rather than the `inputs` property of `@Component` and `@Directive` metadata.', - descriptionDetails: 'See more at https://angular.io/styleguide#style-05-12.', - options: null, - optionsDescription: 'Not configurable.', - rationale: Utils.dedent` - * It is easier and more readable to identify which properties in a class are inputs. - * If you ever need to rename the property name associated with @Input, you can modify it in a single place. - * The metadata declaration attached to the directive is shorter and thus more readable. - * Placing the decorator on the same line usually makes for shorter code and still easily identifies the property as an input. - `, - ruleName: 'use-input-property-decorator', - type: 'style', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = - 'Use the @Input property decorator instead of the inputs property (https://angular.io/styleguide#style-05-12)'; - - constructor(options: IOptions) { - super( - { - decoratorName: 'Input', - errorMessage: Rule.FAILURE_STRING, - propertyName: 'inputs' - }, - options - ); - } -} diff --git a/src/useLifeCycleInterfaceRule.ts b/src/useLifeCycleInterfaceRule.ts deleted file mode 100644 index ec35c874f..000000000 --- a/src/useLifeCycleInterfaceRule.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { sprintf } from 'sprintf-js'; -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { getSymbolName } from './util/utils'; - -export class Rule extends Lint.Rules.AbstractRule { - static readonly metadata: Lint.IRuleMetadata = { - description: 'Ensure that components implement life cycle interfaces if they use them.', - descriptionDetails: 'See more at https://angular.io/styleguide#style-09-01.', - options: null, - optionsDescription: 'Not configurable.', - rationale: 'Interfaces prescribe typed method signatures. Use those signatures to flag spelling and syntax mistakes.', - ruleName: 'use-life-cycle-interface', - type: 'maintainability', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = - 'Implement life cycle hook interface %s for method %s in class %s (https://angular.io/styleguide#style-09-01)'; - static readonly HOOKS_PREFIX = 'ng'; - static readonly LIFE_CYCLE_HOOKS_NAMES: string[] = [ - 'OnChanges', - 'OnInit', - 'DoCheck', - 'AfterContentInit', - 'AfterContentChecked', - 'AfterViewInit', - 'AfterViewChecked', - 'OnDestroy' - ]; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new ClassMetadataWalker(sourceFile, this.getOptions())); - } -} - -export class ClassMetadataWalker extends Lint.RuleWalker { - visitClassDeclaration(node: ts.ClassDeclaration) { - const className = node.name!.text; - const interfaces = this.extractInterfaces(node); - const methods = node.members.filter(ts.isMethodDeclaration); - - this.validateMethods(methods, interfaces, className); - super.visitClassDeclaration(node); - } - - private extractInterfaces(node: ts.ClassDeclaration): string[] { - let interfaces: string[] = []; - if (node.heritageClauses) { - const interfacesClause = node.heritageClauses.filter(h => h.token === ts.SyntaxKind.ImplementsKeyword); - if (interfacesClause.length !== 0) { - interfaces = interfacesClause[0].types.map(getSymbolName); - } - } - return interfaces; - } - - private validateMethods(methods: ts.ClassElement[], interfaces: string[], className: string) { - methods.forEach(m => { - const name = m.name!; - const methodName = name.getText(); - - if (methodName && this.isMethodValidHook(methodName, interfaces)) { - const hookName = methodName.slice(2); - this.addFailureAtNode(name, sprintf(Rule.FAILURE_STRING, hookName, `${Rule.HOOKS_PREFIX}${hookName}`, className)); - } - }); - } - - private isMethodValidHook(methodName: string, interfaces: string[]): boolean { - const isNg = methodName.slice(0, 2) === Rule.HOOKS_PREFIX; - const hookName = methodName.slice(2); - const isHook = Rule.LIFE_CYCLE_HOOKS_NAMES.indexOf(hookName) !== -1; - const isNotIn = interfaces.indexOf(hookName) === -1; - - return isNg && isHook && isNotIn; - } -} diff --git a/src/useLifecycleInterfaceRule.ts b/src/useLifecycleInterfaceRule.ts new file mode 100644 index 000000000..b9516c1fa --- /dev/null +++ b/src/useLifecycleInterfaceRule.ts @@ -0,0 +1,75 @@ +import { sprintf } from 'sprintf-js'; +import { IRuleMetadata, RuleFailure, RuleWalker } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { ClassDeclaration, SourceFile } from 'typescript'; +import { getDeclaredMethods } from './util/classDeclarationUtils'; +import { + getClassName, + getDeclaredLifecycleInterfaces, + isLifecycleMethod, + LifecycleInterfaceKeys, + LifecycleInterfaces, + LifecycleMethodKeys +} from './util/utils'; + +interface FailureParameters { + readonly className: string; + readonly interfaceName: LifecycleInterfaceKeys; + readonly methodName: LifecycleMethodKeys; +} + +const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-09-01'; + +export const getFailureMessage = (failureParameters: FailureParameters): string => + sprintf(Rule.FAILURE_STRING, failureParameters.interfaceName, failureParameters.methodName, failureParameters.className); + +export class Rule extends AbstractRule { + static readonly metadata: IRuleMetadata = { + description: 'Ensures classes implement lifecycle interfaces corresponding to the declared lifecycle methods.', + descriptionDetails: `See more at ${STYLE_GUIDE_LINK}`, + options: null, + optionsDescription: 'Not configurable.', + rationale: 'Interfaces prescribe typed method signatures. Use those signatures to flag spelling and syntax mistakes.', + ruleName: 'use-lifecycle-interface', + type: 'functionality', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = `Implement lifecycle interface %s for method %s in class %s (${STYLE_GUIDE_LINK})`; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithWalker(new ClassMetadataWalker(sourceFile, this.getOptions())); + } +} + +class ClassMetadataWalker extends RuleWalker { + protected visitClassDeclaration(node: ClassDeclaration): void { + this.validateClassDeclaration(node); + super.visitClassDeclaration(node); + } + + private validateClassDeclaration(node: ClassDeclaration): void { + const className = getClassName(node); + + if (!className) return; + + const declaredLifecycleInterfaces = getDeclaredLifecycleInterfaces(node); + const declaredMethods = getDeclaredMethods(node); + + for (const method of declaredMethods) { + const { name: methodProperty } = method; + const methodName = methodProperty.getText(); + + if (!isLifecycleMethod(methodName)) continue; + + const interfaceName = methodName.slice(2) as LifecycleInterfaceKeys; + const isMethodImplemented = declaredLifecycleInterfaces.indexOf(LifecycleInterfaces[interfaceName]) !== -1; + + if (isMethodImplemented) continue; + + const failure = getFailureMessage({ className, interfaceName, methodName }); + + this.addFailureAtNode(methodProperty, failure); + } + } +} diff --git a/src/useOutputPropertyDecoratorRule.ts b/src/useOutputPropertyDecoratorRule.ts deleted file mode 100644 index 292ea4385..000000000 --- a/src/useOutputPropertyDecoratorRule.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { IOptions, IRuleMetadata, Utils } from 'tslint/lib'; -import { UsePropertyDecorator } from './propertyDecoratorBase'; - -export class Rule extends UsePropertyDecorator { - static readonly metadata: IRuleMetadata = { - description: 'Use `@Output` decorator rather than the `outputs` property of `@Component` and `@Directive` metadata.', - descriptionDetails: 'See more at https://angular.io/styleguide#style-05-12.', - options: null, - optionsDescription: 'Not configurable.', - rationale: Utils.dedent` - * It is easier and more readable to identify which properties in a class are events. - * If you ever need to rename the event name associated with @Output, you can modify it in a single place. - * The metadata declaration attached to the directive is shorter and thus more readable. - * Placing the decorator on the same line usually makes for shorter code and still easily identifies the property as an output. - `, - ruleName: 'use-output-property-decorator', - type: 'style', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = - 'Use the @Output property decorator instead of the outputs property (https://angular.io/styleguide#style-05-12)'; - - constructor(options: IOptions) { - super( - { - decoratorName: 'Output', - errorMessage: Rule.FAILURE_STRING, - propertyName: 'outputs' - }, - options - ); - } -} diff --git a/src/util/utils.ts b/src/util/utils.ts index 4bf61cf8a..323e361f3 100644 --- a/src/util/utils.ts +++ b/src/util/utils.ts @@ -1,81 +1,185 @@ -import * as ts from 'typescript'; +import { + ClassDeclaration, + createNodeArray, + Decorator, + Expression, + ExpressionWithTypeArguments, + getLineAndCharacterOfPosition, + isCallExpression, + isClassDeclaration, + isIdentifier, + isObjectLiteralExpression, + isPropertyAccessExpression, + isPropertyAssignment, + Node, + NodeArray, + ObjectLiteralExpression, + SourceFile, + SyntaxKind +} from 'typescript'; +import { getDeclaredMethods } from './classDeclarationUtils'; + +export enum Decorators { + ContentChild = 'ContentChild', + ContentChildren = 'ContentChildren', + HostBinding = 'HostBinding', + HostListener = 'HostListener', + Input = 'Input', + Output = 'Output', + ViewChild = 'ViewChild', + ViewChildren = 'ViewChildren' +} + +export enum LifecycleInterfaces { + AfterContentChecked = 'AfterContentChecked', + AfterContentInit = 'AfterContentInit', + AfterViewChecked = 'AfterViewChecked', + AfterViewInit = 'AfterViewInit', + OnChanges = 'OnChanges', + OnDestroy = 'OnDestroy', + OnInit = 'OnInit', + DoCheck = 'DoCheck' +} + +export enum LifecycleMethods { + ngAfterContentChecked = 'ngAfterContentChecked', + ngAfterContentInit = 'ngAfterContentInit', + ngAfterViewChecked = 'ngAfterViewChecked', + ngAfterViewInit = 'ngAfterViewInit', + ngOnChanges = 'ngOnChanges', + ngOnDestroy = 'ngOnDestroy', + ngOnInit = 'ngOnInit', + ngDoCheck = 'ngDoCheck' +} + +export enum MetadataTypes { + Component = 'Component', + Directive = 'Directive', + Injectable = 'Injectable', + Pipe = 'Pipe' +} + +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([]) +}; +export const METADATA_TYPE_LIFECYCLE_MAPPER: MetadataTypeLifecycleMapper = { + Component: LIFECYCLE_METHODS, + Directive: LIFECYCLE_METHODS, + Injectable: new Set([LifecycleMethods.ngOnDestroy]), + Pipe: new Set([LifecycleMethods.ngOnDestroy]) +}; -export const getClassName = (property: ts.PropertyDeclaration): string | undefined => { - const { parent } = property; - const identifier = parent && ts.isClassDeclaration(parent) ? parent.name : undefined; +export const getClassName = (node: Node): string | undefined => { + const klass = getNextToLastParentNode(node); - return identifier && ts.isIdentifier(identifier) ? identifier.text : undefined; + return isClassDeclaration(klass) && klass.name ? klass.name.text : undefined; }; -export const getDecoratorArgument = (decorator: ts.Decorator): ts.ObjectLiteralExpression | undefined => { +export const getDecoratorArgument = (decorator: Decorator): ObjectLiteralExpression | undefined => { const { expression } = decorator; - if (ts.isCallExpression(expression) && expression.arguments && expression.arguments.length > 0) { - const args = expression.arguments[0]; + if (!isCallExpression(expression) || !expression.arguments || expression.arguments.length === 0) return undefined; - if (ts.isObjectLiteralExpression(args) && args.properties) { - return args; - } - } + const args = expression.arguments[0]; - return undefined; + return isObjectLiteralExpression(args) && args.properties ? args : undefined; }; -export const getDecoratorPropertyInitializer = (decorator: ts.Decorator, name: string) => { - const args = ts.isCallExpression(decorator.expression) ? decorator.expression.arguments[0] : undefined; - const properties = ts.createNodeArray(args && ts.isObjectLiteralExpression(args) ? args.properties : undefined); +export const getDecoratorPropertyInitializer = (decorator: Decorator, name: string): Expression | undefined => { + const args = isCallExpression(decorator.expression) ? decorator.expression.arguments[0] : undefined; + const properties = createNodeArray(args && isObjectLiteralExpression(args) ? args.properties : undefined); return properties - .filter(prop => prop.name && ts.isIdentifier(prop.name) && prop.name.text === name) - .map(prop => (ts.isPropertyAssignment(prop) ? prop.initializer : undefined)) + .filter(prop => prop.name && isIdentifier(prop.name) && prop.name.text === name) + .map(prop => (isPropertyAssignment(prop) ? prop.initializer : undefined)) .pop(); }; -export const getDecoratorName = (decorator: ts.Decorator): string | undefined => { - return ts.isCallExpression(decorator.expression) && ts.isIdentifier(decorator.expression.expression) +export const getDecoratorName = (decorator: Decorator): string | undefined => + isCallExpression(decorator.expression) && isIdentifier(decorator.expression.expression) ? decorator.expression.expression.text : undefined; -}; -export const getComponentDecorator = (declaration: ts.ClassDeclaration) => { - return ts.createNodeArray(declaration.decorators).find(d => { - return ( - ts.isCallExpression(d.expression) && - d.expression.arguments && - d.expression.arguments.length > 0 && - getDecoratorName(d) === 'Component' - ); - }); +export const getNextToLastParentNode = (node: Node): Node => { + let currentNode = node; + + while (currentNode.parent && currentNode.parent.parent) { + currentNode = currentNode.parent; + } + + return currentNode; }; -export const getSymbolName = (expression: ts.ExpressionWithTypeArguments): string => { +export const getComponentDecorator = (declaration: ClassDeclaration): Decorator | undefined => + createNodeArray(declaration.decorators).find( + d => + isCallExpression(d.expression) && d.expression.arguments && d.expression.arguments.length > 0 && getDecoratorName(d) === 'Component' + ); + +export const getSymbolName = (expression: ExpressionWithTypeArguments): string => { const { expression: childExpression } = expression; - return ts.isPropertyAccessExpression(childExpression) ? childExpression.name.getText() : childExpression.getText(); + return isPropertyAccessExpression(childExpression) ? childExpression.name.getText() : childExpression.getText(); }; -export const maybeNodeArray = (nodes: ts.NodeArray): ReadonlyArray => { - return nodes || []; -}; +export const isNgDecorator = (value: string): value is DecoratorKeys => DECORATORS.has(value as DecoratorKeys); -export const isSameLine = (sourceFile: ts.SourceFile, pos1: number, pos2: number) => { - return ts.getLineAndCharacterOfPosition(sourceFile, pos1).line === ts.getLineAndCharacterOfPosition(sourceFile, pos2).line; -}; +export const isLifecycleInterface = (value: string): value is LifecycleInterfaceKeys => + LIFECYCLE_INTERFACES.has(value as LifecycleInterfaceKeys); + +export const isLifecycleMethod = (value: string): value is LifecycleMethodKeys => LIFECYCLE_METHODS.has(value as LifecycleMethodKeys); + +export const isMetadataType = (value: string): value is MetadataTypes => METADATA_TYPES.has(value as MetadataTypes); -export const isStringLiteralLike = (node: ts.Node) => { - return node.kind === ts.SyntaxKind.StringLiteral || node.kind === ts.SyntaxKind.NoSubstitutionTemplateLiteral; +export const getDeclaredInterfaces = (node: ClassDeclaration): string[] => { + const heritageClause = createNodeArray(node.heritageClauses).find(h => h.token === SyntaxKind.ImplementsKeyword); + + return heritageClause ? heritageClause.types.map(getSymbolName) : []; }; +export const getDeclaredLifecycleInterfaces = (node: ClassDeclaration): ReadonlyArray => + getDeclaredInterfaces(node).filter(isLifecycleInterface) as ReadonlyArray; + +export const getDeclaredLifecycleMethods = (node: ClassDeclaration): ReadonlyArray => + getDeclaredMethods(node) + .map(m => m.name.getText()) + .filter(isLifecycleMethod) as ReadonlyArray; + +export const maybeNodeArray = (nodes: NodeArray): ReadonlyArray => nodes || []; + +export const isSameLine = (sourceFile: SourceFile, pos1: number, pos2: number) => + getLineAndCharacterOfPosition(sourceFile, pos1).line === getLineAndCharacterOfPosition(sourceFile, pos2).line; + +export const isStringLiteralLike = (node: Node) => + node.kind === SyntaxKind.StringLiteral || node.kind === SyntaxKind.NoSubstitutionTemplateLiteral; + export const kebabToCamelCase = (value: string) => value.replace(/-[a-zA-Z]/g, x => x[1].toUpperCase()); -// // Regex below matches any Unicode word and compatible with ES5. In ES2018 the same result // can be achieved by using /\p{L}\S*/gu and also known as Unicode Property Escapes // (http://2ality.com/2017/07/regexp-unicode-property-escapes.html). Since there is no // transpilation of this functionality down to ES5 without external tool, the only solution is // to use already transpiled form. Example can be found here - // https://mothereff.in/regexpu#input=var+regex+%3D+/%5Cp%7BL%7D/u%3B&unicodePropertyEscape=1 -// const unicodeWordMatch = /(?:[A-Za-z\xAA\xB5\xBA\xC0-\xD6\xD8-\xF6\xF8-\u02C1\u02C6-\u02D1\u02E0-\u02E4\u02EC\u02EE\u0370-\u0374\u0376\u0377\u037A-\u037D\u037F\u0386\u0388-\u038A\u038C\u038E-\u03A1\u03A3-\u03F5\u03F7-\u0481\u048A-\u052F\u0531-\u0556\u0559\u0561-\u0587\u05D0-\u05EA\u05F0-\u05F2\u0620-\u064A\u066E\u066F\u0671-\u06D3\u06D5\u06E5\u06E6\u06EE\u06EF\u06FA-\u06FC\u06FF\u0710\u0712-\u072F\u074D-\u07A5\u07B1\u07CA-\u07EA\u07F4\u07F5\u07FA\u0800-\u0815\u081A\u0824\u0828\u0840-\u0858\u0860-\u086A\u08A0-\u08B4\u08B6-\u08BD\u0904-\u0939\u093D\u0950\u0958-\u0961\u0971-\u0980\u0985-\u098C\u098F\u0990\u0993-\u09A8\u09AA-\u09B0\u09B2\u09B6-\u09B9\u09BD\u09CE\u09DC\u09DD\u09DF-\u09E1\u09F0\u09F1\u09FC\u0A05-\u0A0A\u0A0F\u0A10\u0A13-\u0A28\u0A2A-\u0A30\u0A32\u0A33\u0A35\u0A36\u0A38\u0A39\u0A59-\u0A5C\u0A5E\u0A72-\u0A74\u0A85-\u0A8D\u0A8F-\u0A91\u0A93-\u0AA8\u0AAA-\u0AB0\u0AB2\u0AB3\u0AB5-\u0AB9\u0ABD\u0AD0\u0AE0\u0AE1\u0AF9\u0B05-\u0B0C\u0B0F\u0B10\u0B13-\u0B28\u0B2A-\u0B30\u0B32\u0B33\u0B35-\u0B39\u0B3D\u0B5C\u0B5D\u0B5F-\u0B61\u0B71\u0B83\u0B85-\u0B8A\u0B8E-\u0B90\u0B92-\u0B95\u0B99\u0B9A\u0B9C\u0B9E\u0B9F\u0BA3\u0BA4\u0BA8-\u0BAA\u0BAE-\u0BB9\u0BD0\u0C05-\u0C0C\u0C0E-\u0C10\u0C12-\u0C28\u0C2A-\u0C39\u0C3D\u0C58-\u0C5A\u0C60\u0C61\u0C80\u0C85-\u0C8C\u0C8E-\u0C90\u0C92-\u0CA8\u0CAA-\u0CB3\u0CB5-\u0CB9\u0CBD\u0CDE\u0CE0\u0CE1\u0CF1\u0CF2\u0D05-\u0D0C\u0D0E-\u0D10\u0D12-\u0D3A\u0D3D\u0D4E\u0D54-\u0D56\u0D5F-\u0D61\u0D7A-\u0D7F\u0D85-\u0D96\u0D9A-\u0DB1\u0DB3-\u0DBB\u0DBD\u0DC0-\u0DC6\u0E01-\u0E30\u0E32\u0E33\u0E40-\u0E46\u0E81\u0E82\u0E84\u0E87\u0E88\u0E8A\u0E8D\u0E94-\u0E97\u0E99-\u0E9F\u0EA1-\u0EA3\u0EA5\u0EA7\u0EAA\u0EAB\u0EAD-\u0EB0\u0EB2\u0EB3\u0EBD\u0EC0-\u0EC4\u0EC6\u0EDC-\u0EDF\u0F00\u0F40-\u0F47\u0F49-\u0F6C\u0F88-\u0F8C\u1000-\u102A\u103F\u1050-\u1055\u105A-\u105D\u1061\u1065\u1066\u106E-\u1070\u1075-\u1081\u108E\u10A0-\u10C5\u10C7\u10CD\u10D0-\u10FA\u10FC-\u1248\u124A-\u124D\u1250-\u1256\u1258\u125A-\u125D\u1260-\u1288\u128A-\u128D\u1290-\u12B0\u12B2-\u12B5\u12B8-\u12BE\u12C0\u12C2-\u12C5\u12C8-\u12D6\u12D8-\u1310\u1312-\u1315\u1318-\u135A\u1380-\u138F\u13A0-\u13F5\u13F8-\u13FD\u1401-\u166C\u166F-\u167F\u1681-\u169A\u16A0-\u16EA\u16F1-\u16F8\u1700-\u170C\u170E-\u1711\u1720-\u1731\u1740-\u1751\u1760-\u176C\u176E-\u1770\u1780-\u17B3\u17D7\u17DC\u1820-\u1877\u1880-\u1884\u1887-\u18A8\u18AA\u18B0-\u18F5\u1900-\u191E\u1950-\u196D\u1970-\u1974\u1980-\u19AB\u19B0-\u19C9\u1A00-\u1A16\u1A20-\u1A54\u1AA7\u1B05-\u1B33\u1B45-\u1B4B\u1B83-\u1BA0\u1BAE\u1BAF\u1BBA-\u1BE5\u1C00-\u1C23\u1C4D-\u1C4F\u1C5A-\u1C7D\u1C80-\u1C88\u1CE9-\u1CEC\u1CEE-\u1CF1\u1CF5\u1CF6\u1D00-\u1DBF\u1E00-\u1F15\u1F18-\u1F1D\u1F20-\u1F45\u1F48-\u1F4D\u1F50-\u1F57\u1F59\u1F5B\u1F5D\u1F5F-\u1F7D\u1F80-\u1FB4\u1FB6-\u1FBC\u1FBE\u1FC2-\u1FC4\u1FC6-\u1FCC\u1FD0-\u1FD3\u1FD6-\u1FDB\u1FE0-\u1FEC\u1FF2-\u1FF4\u1FF6-\u1FFC\u2071\u207F\u2090-\u209C\u2102\u2107\u210A-\u2113\u2115\u2119-\u211D\u2124\u2126\u2128\u212A-\u212D\u212F-\u2139\u213C-\u213F\u2145-\u2149\u214E\u2183\u2184\u2C00-\u2C2E\u2C30-\u2C5E\u2C60-\u2CE4\u2CEB-\u2CEE\u2CF2\u2CF3\u2D00-\u2D25\u2D27\u2D2D\u2D30-\u2D67\u2D6F\u2D80-\u2D96\u2DA0-\u2DA6\u2DA8-\u2DAE\u2DB0-\u2DB6\u2DB8-\u2DBE\u2DC0-\u2DC6\u2DC8-\u2DCE\u2DD0-\u2DD6\u2DD8-\u2DDE\u2E2F\u3005\u3006\u3031-\u3035\u303B\u303C\u3041-\u3096\u309D-\u309F\u30A1-\u30FA\u30FC-\u30FF\u3105-\u312E\u3131-\u318E\u31A0-\u31BA\u31F0-\u31FF\u3400-\u4DB5\u4E00-\u9FEA\uA000-\uA48C\uA4D0-\uA4FD\uA500-\uA60C\uA610-\uA61F\uA62A\uA62B\uA640-\uA66E\uA67F-\uA69D\uA6A0-\uA6E5\uA717-\uA71F\uA722-\uA788\uA78B-\uA7AE\uA7B0-\uA7B7\uA7F7-\uA801\uA803-\uA805\uA807-\uA80A\uA80C-\uA822\uA840-\uA873\uA882-\uA8B3\uA8F2-\uA8F7\uA8FB\uA8FD\uA90A-\uA925\uA930-\uA946\uA960-\uA97C\uA984-\uA9B2\uA9CF\uA9E0-\uA9E4\uA9E6-\uA9EF\uA9FA-\uA9FE\uAA00-\uAA28\uAA40-\uAA42\uAA44-\uAA4B\uAA60-\uAA76\uAA7A\uAA7E-\uAAAF\uAAB1\uAAB5\uAAB6\uAAB9-\uAABD\uAAC0\uAAC2\uAADB-\uAADD\uAAE0-\uAAEA\uAAF2-\uAAF4\uAB01-\uAB06\uAB09-\uAB0E\uAB11-\uAB16\uAB20-\uAB26\uAB28-\uAB2E\uAB30-\uAB5A\uAB5C-\uAB65\uAB70-\uABE2\uAC00-\uD7A3\uD7B0-\uD7C6\uD7CB-\uD7FB\uF900-\uFA6D\uFA70-\uFAD9\uFB00-\uFB06\uFB13-\uFB17\uFB1D\uFB1F-\uFB28\uFB2A-\uFB36\uFB38-\uFB3C\uFB3E\uFB40\uFB41\uFB43\uFB44\uFB46-\uFBB1\uFBD3-\uFD3D\uFD50-\uFD8F\uFD92-\uFDC7\uFDF0-\uFDFB\uFE70-\uFE74\uFE76-\uFEFC\uFF21-\uFF3A\uFF41-\uFF5A\uFF66-\uFFBE\uFFC2-\uFFC7\uFFCA-\uFFCF\uFFD2-\uFFD7\uFFDA-\uFFDC]|\uD800[\uDC00-\uDC0B\uDC0D-\uDC26\uDC28-\uDC3A\uDC3C\uDC3D\uDC3F-\uDC4D\uDC50-\uDC5D\uDC80-\uDCFA\uDE80-\uDE9C\uDEA0-\uDED0\uDF00-\uDF1F\uDF2D-\uDF40\uDF42-\uDF49\uDF50-\uDF75\uDF80-\uDF9D\uDFA0-\uDFC3\uDFC8-\uDFCF]|\uD801[\uDC00-\uDC9D\uDCB0-\uDCD3\uDCD8-\uDCFB\uDD00-\uDD27\uDD30-\uDD63\uDE00-\uDF36\uDF40-\uDF55\uDF60-\uDF67]|\uD802[\uDC00-\uDC05\uDC08\uDC0A-\uDC35\uDC37\uDC38\uDC3C\uDC3F-\uDC55\uDC60-\uDC76\uDC80-\uDC9E\uDCE0-\uDCF2\uDCF4\uDCF5\uDD00-\uDD15\uDD20-\uDD39\uDD80-\uDDB7\uDDBE\uDDBF\uDE00\uDE10-\uDE13\uDE15-\uDE17\uDE19-\uDE33\uDE60-\uDE7C\uDE80-\uDE9C\uDEC0-\uDEC7\uDEC9-\uDEE4\uDF00-\uDF35\uDF40-\uDF55\uDF60-\uDF72\uDF80-\uDF91]|\uD803[\uDC00-\uDC48\uDC80-\uDCB2\uDCC0-\uDCF2]|\uD804[\uDC03-\uDC37\uDC83-\uDCAF\uDCD0-\uDCE8\uDD03-\uDD26\uDD50-\uDD72\uDD76\uDD83-\uDDB2\uDDC1-\uDDC4\uDDDA\uDDDC\uDE00-\uDE11\uDE13-\uDE2B\uDE80-\uDE86\uDE88\uDE8A-\uDE8D\uDE8F-\uDE9D\uDE9F-\uDEA8\uDEB0-\uDEDE\uDF05-\uDF0C\uDF0F\uDF10\uDF13-\uDF28\uDF2A-\uDF30\uDF32\uDF33\uDF35-\uDF39\uDF3D\uDF50\uDF5D-\uDF61]|\uD805[\uDC00-\uDC34\uDC47-\uDC4A\uDC80-\uDCAF\uDCC4\uDCC5\uDCC7\uDD80-\uDDAE\uDDD8-\uDDDB\uDE00-\uDE2F\uDE44\uDE80-\uDEAA\uDF00-\uDF19]|\uD806[\uDCA0-\uDCDF\uDCFF\uDE00\uDE0B-\uDE32\uDE3A\uDE50\uDE5C-\uDE83\uDE86-\uDE89\uDEC0-\uDEF8]|\uD807[\uDC00-\uDC08\uDC0A-\uDC2E\uDC40\uDC72-\uDC8F\uDD00-\uDD06\uDD08\uDD09\uDD0B-\uDD30\uDD46]|\uD808[\uDC00-\uDF99]|\uD809[\uDC80-\uDD43]|[\uD80C\uD81C-\uD820\uD840-\uD868\uD86A-\uD86C\uD86F-\uD872\uD874-\uD879][\uDC00-\uDFFF]|\uD80D[\uDC00-\uDC2E]|\uD811[\uDC00-\uDE46]|\uD81A[\uDC00-\uDE38\uDE40-\uDE5E\uDED0-\uDEED\uDF00-\uDF2F\uDF40-\uDF43\uDF63-\uDF77\uDF7D-\uDF8F]|\uD81B[\uDF00-\uDF44\uDF50\uDF93-\uDF9F\uDFE0\uDFE1]|\uD821[\uDC00-\uDFEC]|\uD822[\uDC00-\uDEF2]|\uD82C[\uDC00-\uDD1E\uDD70-\uDEFB]|\uD82F[\uDC00-\uDC6A\uDC70-\uDC7C\uDC80-\uDC88\uDC90-\uDC99]|\uD835[\uDC00-\uDC54\uDC56-\uDC9C\uDC9E\uDC9F\uDCA2\uDCA5\uDCA6\uDCA9-\uDCAC\uDCAE-\uDCB9\uDCBB\uDCBD-\uDCC3\uDCC5-\uDD05\uDD07-\uDD0A\uDD0D-\uDD14\uDD16-\uDD1C\uDD1E-\uDD39\uDD3B-\uDD3E\uDD40-\uDD44\uDD46\uDD4A-\uDD50\uDD52-\uDEA5\uDEA8-\uDEC0\uDEC2-\uDEDA\uDEDC-\uDEFA\uDEFC-\uDF14\uDF16-\uDF34\uDF36-\uDF4E\uDF50-\uDF6E\uDF70-\uDF88\uDF8A-\uDFA8\uDFAA-\uDFC2\uDFC4-\uDFCB]|\uD83A[\uDC00-\uDCC4\uDD00-\uDD43]|\uD83B[\uDE00-\uDE03\uDE05-\uDE1F\uDE21\uDE22\uDE24\uDE27\uDE29-\uDE32\uDE34-\uDE37\uDE39\uDE3B\uDE42\uDE47\uDE49\uDE4B\uDE4D-\uDE4F\uDE51\uDE52\uDE54\uDE57\uDE59\uDE5B\uDE5D\uDE5F\uDE61\uDE62\uDE64\uDE67-\uDE6A\uDE6C-\uDE72\uDE74-\uDE77\uDE79-\uDE7C\uDE7E\uDE80-\uDE89\uDE8B-\uDE9B\uDEA1-\uDEA3\uDEA5-\uDEA9\uDEAB-\uDEBB]|\uD869[\uDC00-\uDED6\uDF00-\uDFFF]|\uD86D[\uDC00-\uDF34\uDF40-\uDFFF]|\uD86E[\uDC00-\uDC1D\uDC20-\uDFFF]|\uD873[\uDC00-\uDEA1\uDEB0-\uDFFF]|\uD87A[\uDC00-\uDFE0]|\uD87E[\uDC00-\uDE1D])\S*/g; export const toTitleCase = (value: string) => value.replace(unicodeWordMatch, val => val[0].toUpperCase() + val.slice(1).toLowerCase()); diff --git a/test/bananaInBoxRule.spec.ts b/test/bananaInBoxRule.spec.ts deleted file mode 100644 index 518dae2fb..000000000 --- a/test/bananaInBoxRule.spec.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { assertSuccess, assertAnnotated } from './testHelper'; -import { Replacement } from 'tslint'; -import { expect } from 'chai'; - -describe('banana-in-box', () => { - describe('success', () => { - it('should work with proper style', () => { - let source = ` - @Component({ - template: \` \` - }) - class Bar {} - `; - assertSuccess('banana-in-box', source); - }); - - it('should work with proper style', () => { - let source = ` - @Component({ - template: \` \` - }) - class Bar {} - `; - assertSuccess('banana-in-box', source); - }); - }); - - describe('failure', () => { - it('should fail when the box is in the banana', () => { - let source = ` - @Component({ - template: \` - ~~~~~~~~~~~~~~~~~ - \` - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'banana-in-box', - message: 'Invalid binding syntax. Use [(expr)] instead', - source - }); - }); - - it('should fail when the box is in the banana', () => { - let source = ` - @Component({ - template: \` - ~~~~~~~~~~~~~ - \` - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'banana-in-box', - message: 'Invalid binding syntax. Use [(expr)] instead', - source - }); - }); - }); - - describe('replacements', () => { - it('should fail when the box is in the banana', () => { - let source = ` - @Component({ - template: \` - ~~~~~~~~~~~~~~~~~ - \` - }) - class Bar {} - `; - const failures = assertAnnotated({ - ruleName: 'banana-in-box', - message: 'Invalid binding syntax. Use [(expr)] instead', - source - }); - - const res = Replacement.applyAll(source, failures[0].getFix()); - expect(res).to.eq(` - @Component({ - template: \` - ~~~~~~~~~~~~~~~~~ - \` - }) - class Bar {} - `); - }); - }); -}); diff --git a/test/contextualDecoratorRule.spec.ts b/test/contextualDecoratorRule.spec.ts new file mode 100644 index 000000000..e9e924ddd --- /dev/null +++ b/test/contextualDecoratorRule.spec.ts @@ -0,0 +1,557 @@ +import { getFailureMessage, Rule } from '../src/contextualDecoratorRule'; +import { Decorators, MetadataTypes } from '../src/util/utils'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + describe('Injectable', () => { + it('should fail if a property is decorated with @ContentChild() decorator', () => { + const source = ` + @Injectable() + class Test { + @ContentChild(Pane) pane: Pane; + ~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ContentChild, + metadataType: MetadataTypes.Injectable + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @ContentChildren() decorator', () => { + const source = ` + @Injectable() + class Test { + @ContentChildren(Pane, { descendants: true }) arbitraryNestedPanes: QueryList; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ContentChildren, + metadataType: MetadataTypes.Injectable + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @HostBinding() decorator', () => { + const source = ` + @Injectable() + class Test { + @HostBinding('class.card-outline') private isCardOutline: boolean; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.HostBinding, + metadataType: MetadataTypes.Injectable + }), + ruleName, + source + }); + }); + + it('should fail if a method is decorated with @HostListener() decorator', () => { + const source = ` + @Injectable() + class Test { + @HostListener('mouseover') + ~~~~~~~~~~~~~~~~~~~~~~~~~~ + mouseOver() { + console.log('mouseOver'); + } + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.HostListener, + metadataType: MetadataTypes.Injectable + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @Input() decorator', () => { + const source = ` + @Injectable() + class Test { + @Input() label: string; + ~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.Input, + metadataType: MetadataTypes.Injectable + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @Output() decorator', () => { + const source = ` + @Injectable() + class Test { + @Output() emitter = new EventEmitter(); + ~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.Output, + metadataType: MetadataTypes.Injectable + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @ViewChild() decorator', () => { + const source = ` + @Injectable() + class Test { + @ViewChild(Pane) pane: Pane; + ~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ViewChild, + metadataType: MetadataTypes.Injectable + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @ViewChildren() decorator', () => { + const source = ` + @Injectable() + class Test { + @ViewChildren(Pane) panes: QueryList; + ~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ViewChildren, + metadataType: MetadataTypes.Injectable + }), + ruleName, + source + }); + }); + }); + + describe('Pipe', () => { + it('should fail if a property is decorated with @ContentChild() decorator', () => { + const source = ` + @Pipe() + class Test { + @ContentChild(Pane) pane: Pane; + ~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ContentChild, + metadataType: MetadataTypes.Pipe + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @ContentChildren() decorator', () => { + const source = ` + @Pipe() + class Test { + @ContentChildren(Pane, { descendants: true }) arbitraryNestedPanes: QueryList; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ContentChildren, + metadataType: MetadataTypes.Pipe + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @HostBinding() decorator', () => { + const source = ` + @Pipe() + class Test { + @HostBinding('class.card-outline') private isCardOutline: boolean; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.HostBinding, + metadataType: MetadataTypes.Pipe + }), + ruleName, + source + }); + }); + + it('should fail if a method is decorated with @HostListener() decorator', () => { + const source = ` + @Pipe() + class Test { + @HostListener('mouseover') + ~~~~~~~~~~~~~~~~~~~~~~~~~~ + mouseOver() { + console.log('mouseOver'); + } + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.HostListener, + metadataType: MetadataTypes.Pipe + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @Input() decorator', () => { + const source = ` + @Pipe() + class Test { + @Input() label: string; + ~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.Input, + metadataType: MetadataTypes.Pipe + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @Output() decorator', () => { + const source = ` + @Pipe() + class Test { + @Output() emitter = new EventEmitter(); + ~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.Output, + metadataType: MetadataTypes.Pipe + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @ViewChild() decorator', () => { + const source = ` + @Pipe() + class Test { + @ViewChild(Pane) pane: Pane; + ~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ViewChild, + metadataType: MetadataTypes.Pipe + }), + ruleName, + source + }); + }); + + it('should fail if a property is decorated with @ViewChildren() decorator', () => { + const source = ` + @Pipe() + class Test { + @ViewChildren(Pane) panes: QueryList; + ~~~~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: getFailureMessage({ + className: 'Test', + decoratorName: Decorators.ViewChildren, + metadataType: MetadataTypes.Pipe + }), + ruleName, + source + }); + }); + }); + + describe('multiple decorators per file', () => { + it('should fail if contains @Directive and @Pipe decorators and the @Pipe contains a not allowed decorator', () => { + const source = ` + @Directive() + class TestDirective { + @Input() label: string; + } + + @Pipe() + class Test { + @Input() label: string; + ~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + decoratorName: Decorators.Input, + metadataType: MetadataTypes.Pipe + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + }); + }); + + describe('success', () => { + describe('Component', () => { + it('should succeed if a property is decorated with @ContentChild() decorator', () => { + const source = ` + @Component() + class Test { + @ContentChild(Pane) pane: Pane; + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @ContentChildren() decorator', () => { + const source = ` + @Component() + class Test { + @ContentChildren(Pane, { descendants: true }) arbitraryNestedPanes: QueryList; + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @HostBinding() decorator', () => { + const source = ` + @Component() + class Test { + @HostBinding('class.card-outline') private isCardOutline: boolean; + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a method is decorated with @HostListener() decorator', () => { + const source = ` + @Component() + class Test { + @HostListener('mouseover') + mouseOver() { + console.log('mouseOver'); + } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @Input() decorator', () => { + const source = ` + @Component() + class Test { + @Input() label: string; + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @Output() decorator', () => { + const source = ` + @Component() + class Test { + @Output() emitter = new EventEmitter(); + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @ViewChild() decorator', () => { + const source = ` + @Component() + class Test { + @ViewChild(Pane) + set pane(value: Pane) { + console.log('panel setter called'); + } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @ViewChildren() decorator', () => { + const source = ` + @Component() + class Test { + @ViewChildren(Pane) panes: QueryList; + } + `; + assertSuccess(ruleName, source); + }); + }); + + describe('Directive', () => { + it('should succeed if a property is decorated with @ContentChild() decorator', () => { + const source = ` + @Directive() + class Test { + @ContentChild(Pane) pane: Pane; + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @ContentChildren() decorator', () => { + const source = ` + @Directive() + class Test { + @ContentChildren(Pane, { descendants: true }) arbitraryNestedPanes: QueryList; + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @HostBinding() decorator', () => { + const source = ` + @Directive() + class Test { + @HostBinding('class.card-outline') private isCardOutline: boolean; + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a method is decorated with @HostListener() decorator', () => { + const source = ` + @Directive() + class Test { + @HostListener('mouseover') + mouseOver() { + console.log('mouseOver'); + } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @Input() decorator', () => { + const source = ` + @Directive() + class Test { + @Input() label: string; + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @Output() decorator', () => { + const source = ` + @Directive() + class Test { + @Output() emitter = new EventEmitter(); + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @ViewChild() decorator', () => { + const source = ` + @Directive() + class Test { + @ViewChild(Pane) + set pane(value: Pane) { + console.log('panel setter called'); + } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is decorated with @ViewChildren() decorator', () => { + const source = ` + @Directive() + class Test { + @ViewChildren(Pane) panes: QueryList; + } + `; + assertSuccess(ruleName, source); + }); + }); + + describe('multiple decorators per file', () => { + it( + 'should succeed if @Component and @Injectable decorators are present on the same file and ' + + 'the @Component contains a non allowed decorator for @Injectable', + () => { + const source = ` + @Injectable() + class TestService { + constructor() {} + } + + @Component({ + selector: 'app-test', + template: '

Hello

', + providers: [TestService] + }) + class TestComponent implements OnChanges { + @Output() emitter = new EventEmitter(); + + constructor(private test: TestService) {} + } + `; + assertSuccess(ruleName, source); + } + ); + }); + }); +}); diff --git a/test/contextualLifeCycleRule.spec.ts b/test/contextualLifeCycleRule.spec.ts deleted file mode 100644 index 4597da196..000000000 --- a/test/contextualLifeCycleRule.spec.ts +++ /dev/null @@ -1,478 +0,0 @@ -import { assertAnnotated, assertSuccess } from './testHelper'; - -describe('contextual-life-cycle', () => { - describe('success', () => { - describe('valid component class', () => { - it('should succeed when is used ngOnInit() method', () => { - let source = ` - @Component() - class TestComponent { - ngOnInit() { this.logger.log('OnInit'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngOnChanges() method', () => { - let source = ` - @Component() - class TestComponent { - ngOnChanges() { this.logger.log('OnChanges'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngDoCheck() method', () => { - let source = ` - @Component() - class TestComponent { - ngDoCheck() { this.logger.log('DoCheck'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngAfterContentInit() method', () => { - let source = ` - @Component() - class TestComponent { - ngAfterContentInit() { this.logger.log('AfterContentInit'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngAfterContentChecked() method', () => { - let source = ` - @Component() - class TestComponent { - ngAfterContentChecked() { this.logger.log('AfterContentChecked'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngAfterViewInit() method', () => { - let source = ` - @Component() - class TestComponent { - ngAfterViewInit() { this.logger.log('AfterViewInit'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngAfterViewChecked() method', () => { - let source = ` - @Component() - class TestComponent { - ngAfterViewChecked() { this.logger.log('AfterViewChecked'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngOnDestroy() method', () => { - let source = ` - @Component() - class TestComponent { - ngOnDestroy() { this.logger.log('OnDestroy'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - }); - - describe('valid directive class', () => { - it('should succeed when is used ngOnInit() method', () => { - let source = ` - @Directive() - class TestDirective { - ngOnInit() { this.logger.log('OnInit'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngOnChanges() method', () => { - let source = ` - @Directive() - class TestDirective { - ngOnChanges() { this.logger.log('OnChanges'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngDoCheck() method', () => { - let source = ` - @Directive() - class TestDirective { - ngDoCheck() { this.logger.log('DoCheck'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngOnDestroy() method', () => { - let source = ` - @Directive() - class TestDirective { - ngOnDestroy() { this.logger.log('OnDestroy'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngAfterContentInit() method', () => { - let source = ` - @Directive() - class TestDirective { - ngAfterContentInit() { this.logger.log('AfterContentInit'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngAfterContentChecked() method', () => { - let source = ` - @Directive() - class TestDirective { - ngAfterContentChecked() { this.logger.log('AfterContentChecked'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngAfterViewInit() method', () => { - let source = ` - @Directive() - class TestDirective { - ngAfterViewInit() { this.logger.log('AfterViewInit'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - - it('should succeed when is used ngAfterViewChecked() method', () => { - let source = ` - @Directive() - class TestDirective { - ngAfterViewChecked() { this.logger.log('AfterViewChecked'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - }); - - describe('valid service class', () => { - it('should succeed when is used ngOnDestroy() method', () => { - let source = ` - @Injectable() - class TestService { - ngOnDestroy() { this.logger.log('OnDestroy'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - }); - - describe('valid pipe class', () => { - it('should succeed when is used ngOnDestroy() method', () => { - let source = ` - @Pipe() - class TestPipe { - ngOnDestroy() { this.logger.log('OnDestroy'); } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - }); - - describe('valid component class', () => { - it('should succeed when an @Injectable is defined before him', () => { - let source = ` - @Injectable() - class Sample { - public constructor() {} - } - - @Component({ - selector: 'sg-hero-cmp', - template: '

Hello {{ hero.name }}

', - providers: [Sample] - }) - class HeroComponent implements OnInit { - public hero: Hero; - - public constructor(private sample: Sample) {} - - ngOnInit() { - console.log('Initialized'); - } - } - `; - assertSuccess('contextual-life-cycle', source); - }); - }); - }); - - describe('failure', () => { - describe('valid service class', () => { - it('should fail when is used ngOnInit() method', () => { - let source = ` - @Injectable() - class TestService { - ngOnInit() { this.logger.log('OnInit'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestService" which have the "@Injectable" decorator, the ' + - '"ngOnInit()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngOnChanges() method', () => { - let source = ` - @Injectable() - class TestService { - ngOnChanges() { this.logger.log('OnChanges'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestService" which have the "@Injectable" decorator, the ' + - '"ngOnChanges()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngDoCheck() method', () => { - let source = ` - @Injectable() - class TestService { - ngDoCheck() { this.logger.log('DoCheck'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestService" which have the "@Injectable" decorator, the ' + - '"ngDoCheck()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngAfterContentInit() method', () => { - let source = ` - @Injectable() - class TestService { - ngAfterContentInit() { this.logger.log('AfterContentInit'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestService" which have the "@Injectable" decorator, the ' + - '"ngAfterContentInit()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngAfterContentChecked() method', () => { - let source = ` - @Injectable() - class TestService { - ngAfterContentChecked() { this.logger.log('AfterContentChecked'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestService" which have the "@Injectable" decorator, the ' + - '"ngAfterContentChecked()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngAfterViewInit() method', () => { - let source = ` - @Injectable() - class TestService { - ngAfterViewInit() { this.logger.log('AfterViewInit'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestService" which have the "@Injectable" decorator, the ' + - '"ngAfterViewInit()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngAfterViewChecked() method', () => { - let source = ` - @Injectable() - class TestService { - ngAfterViewChecked() { this.logger.log('AfterViewChecked'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestService" which have the "@Injectable" decorator, the ' + - '"ngAfterViewChecked()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - }); - - describe('valid pipe class', () => { - it('should fail when is used ngOnInit() method', () => { - let source = ` - @Pipe() - class TestPipe { - ngOnInit() { this.logger.log('OnInit'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestPipe" which have the "@Pipe" decorator, the ' + - '"ngOnInit()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngOnChanges() method', () => { - let source = ` - @Pipe() - class TestPipe { - ngOnChanges() { this.logger.log('OnChanges'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestPipe" which have the "@Pipe" decorator, the ' + - '"ngOnChanges()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngDoCheck() method', () => { - let source = ` - @Pipe() - class TestPipe { - ngDoCheck() { this.logger.log('DoCheck'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestPipe" which have the "@Pipe" decorator, the ' + - '"ngDoCheck()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngAfterContentInit() method', () => { - let source = ` - @Pipe() - class TestPipe { - ngAfterContentInit() { this.logger.log('AfterContentInit'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestPipe" which have the "@Pipe" decorator, the ' + - '"ngAfterContentInit()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngAfterContentChecked() method', () => { - let source = ` - @Pipe() - class TestPipe { - ngAfterContentChecked() { this.logger.log('AfterContentChecked'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestPipe" which have the "@Pipe" decorator, the ' + - '"ngAfterContentChecked()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngAfterViewInit() method', () => { - let source = ` - @Pipe() - class TestPipe { - ngAfterViewInit() { this.logger.log('AfterViewInit'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestPipe" which have the "@Pipe" decorator, the ' + - '"ngAfterViewInit()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - - it('should fail when is used ngAfterViewChecked() method', () => { - let source = ` - @Pipe() - class TestPipe { - ngAfterViewChecked() { this.logger.log('AfterViewChecked'); } - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'contextual-life-cycle', - message: - 'In the class "TestPipe" which have the "@Pipe" decorator, the ' + - '"ngAfterViewChecked()" hook method is not allowed. ' + - 'Please, drop it.', - source - }); - }); - }); - }); -}); diff --git a/test/contextualLifecycleRule.spec.ts b/test/contextualLifecycleRule.spec.ts new file mode 100644 index 000000000..f21ea5dc2 --- /dev/null +++ b/test/contextualLifecycleRule.spec.ts @@ -0,0 +1,547 @@ +import { getFailureMessage, Rule } from '../src/contextualLifecycleRule'; +import { LifecycleMethods, MetadataTypes } from '../src/util/utils'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + describe('Injectable', () => { + it('should fail if ngAfterContentChecked() method is present', () => { + const source = ` + @Injectable() + class Test { + ngAfterContentChecked() { console.log('AfterContentChecked'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Injectable, + methodName: LifecycleMethods.ngAfterContentChecked + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngAfterContentInit() method is present', () => { + const source = ` + @Injectable() + class Test { + ngAfterContentInit() { console.log('AfterContentInit'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Injectable, + methodName: LifecycleMethods.ngAfterContentInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngAfterViewChecked() method is present', () => { + const source = ` + @Injectable() + class Test { + ngAfterViewChecked() { console.log('AfterViewChecked'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Injectable, + methodName: LifecycleMethods.ngAfterViewChecked + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngAfterViewInit() method is present', () => { + const source = ` + @Injectable() + class Test { + ngAfterViewInit() { console.log('AfterViewInit'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Injectable, + methodName: LifecycleMethods.ngAfterViewInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngDoCheck() method is present', () => { + const source = ` + @Injectable() + class Test { + ngDoCheck() { console.log('DoCheck'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Injectable, + methodName: LifecycleMethods.ngDoCheck + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngOnChanges() method is present', () => { + const source = ` + @Injectable() + class Test { + ngOnChanges() { console.log('OnChanges'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Injectable, + methodName: LifecycleMethods.ngOnChanges + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngOnInit() method is present', () => { + const source = ` + @Injectable() + class Test { + ngOnInit() { console.log('OnInit'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Injectable, + methodName: LifecycleMethods.ngOnInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + }); + + describe('Pipe', () => { + it('should fail if ngAfterContentChecked() method is present', () => { + const source = ` + @Pipe() + class Test { + ngAfterContentChecked() { console.log('AfterContentChecked'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Pipe, + methodName: LifecycleMethods.ngAfterContentChecked + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngAfterContentInit() method is present', () => { + const source = ` + @Pipe() + class Test { + ngAfterContentInit() { console.log('AfterContentInit'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Pipe, + methodName: LifecycleMethods.ngAfterContentInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngAfterViewChecked() method is present', () => { + const source = ` + @Pipe() + class Test { + ngAfterViewChecked() { console.log('AfterViewChecked'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Pipe, + methodName: LifecycleMethods.ngAfterViewChecked + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngAfterViewInit() method is present', () => { + const source = ` + @Pipe() + class Test { + ngAfterViewInit() { console.log('AfterViewInit'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Pipe, + methodName: LifecycleMethods.ngAfterViewInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngDoCheck() method is present', () => { + const source = ` + @Pipe() + class Test { + ngDoCheck() { console.log('DoCheck'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Pipe, + methodName: LifecycleMethods.ngDoCheck + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngOnChanges() method is present', () => { + const source = ` + @Pipe() + class Test { + ngOnChanges() { console.log('OnChanges'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Pipe, + methodName: LifecycleMethods.ngOnChanges + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if ngOnInit() method is present', () => { + const source = ` + @Pipe() + class Test { + ngOnInit() { console.log('OnInit'); } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Pipe, + methodName: LifecycleMethods.ngOnInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + }); + + describe('multiple decorators per file', () => { + it( + 'should fail if @Directive and @Pipe decorators are present on the same file and ' + 'the @Pipe contains a non allowed method', + () => { + const source = ` + @Pipe() + class Test implements DoCheck { + constructor() {} + ngDoCheck() {} + ~~~~~~~~~~~~~~ + } + + @Directive() + class TestDirective implements OnInit { + ngOnInit() { + console.log('Initialized'); + } + } + `; + const message = getFailureMessage({ + className: 'Test', + metadataType: MetadataTypes.Pipe, + methodName: LifecycleMethods.ngDoCheck + }); + assertAnnotated({ + message, + ruleName, + source + }); + } + ); + }); + }); + + describe('success', () => { + describe('Component', () => { + it('should succeed if ngAfterContentChecked() method is present', () => { + const source = ` + @Component() + class Test { + ngAfterContentChecked() { console.log('AfterContentChecked'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngAfterContentInit() method is present', () => { + const source = ` + @Component() + class Test { + ngAfterContentInit() { console.log('AfterContentInit'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngAfterViewChecked() method is present', () => { + const source = ` + @Component() + class Test { + ngAfterViewChecked() { console.log('AfterViewChecked'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngAfterViewInit() method is present', () => { + const source = ` + @Component() + class Test { + ngAfterViewInit() { console.log('AfterViewInit'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngDoCheck() method is present', () => { + const source = ` + @Component() + class Test { + ngDoCheck() { console.log('DoCheck'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngOnChanges() method is present', () => { + const source = ` + @Component() + class Test { + ngOnChanges() { console.log('OnChanges'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngOnDestroy() method is present', () => { + const source = ` + @Component() + class Test { + ngOnDestroy() { console.log('OnDestroy'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngOnInit() method is present', () => { + const source = ` + @Component() + class Test { + ngOnInit() { console.log('OnInit'); } + } + `; + assertSuccess(ruleName, source); + }); + }); + + describe('Directive', () => { + it('should succeed if ngAfterContentChecked() method is present', () => { + const source = ` + @Directive() + class Test { + ngAfterContentChecked() { console.log('AfterContentChecked'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngAfterContentInit() method is present', () => { + const source = ` + @Directive() + class Test { + ngAfterContentInit() { console.log('AfterContentInit'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngAfterViewChecked() method is present', () => { + const source = ` + @Directive() + class Test { + ngAfterViewChecked() { console.log('AfterViewChecked'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngAfterViewInit() method is present', () => { + const source = ` + @Directive() + class Test { + ngAfterViewInit() { console.log('AfterViewInit'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngDoCheck() method is present', () => { + const source = ` + @Directive() + class Test { + ngDoCheck() { console.log('DoCheck'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngOnChanges() method is present', () => { + const source = ` + @Directive() + class Test { + ngOnChanges() { console.log('OnChanges'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngOnDestroy() method is present', () => { + const source = ` + @Directive() + class Test { + ngOnDestroy() { console.log('OnDestroy'); } + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ngOnInit() method is present', () => { + const source = ` + @Directive() + class Test { + ngOnInit() { console.log('OnInit'); } + } + `; + assertSuccess(ruleName, source); + }); + }); + + describe('Injectable', () => { + it('should succeed if ngOnDestroy() method is present', () => { + const source = ` + @Injectable() + class Test { + ngOnDestroy() { console.log('OnDestroy'); } + } + `; + assertSuccess(ruleName, source); + }); + }); + + describe('Pipe', () => { + it('should succeed if ngOnDestroy() method is present', () => { + const source = ` + @Pipe() + class Test { + ngOnDestroy() { console.log('OnDestroy'); } + } + `; + assertSuccess(ruleName, source); + }); + }); + + describe('multiple decorators per file', () => { + it( + 'should succeed if @Component and @Injectable decorators are present on the same file and ' + + 'the @Component contains a non allowed method for @Injectable', + () => { + const source = ` + @Injectable() + class TestService { + constructor() {} + } + + @Component({ + selector: 'app-test', + template: '

Hello

', + providers: [TestService] + }) + class TestComponent implements OnChanges { + constructor(private test: TestService) {} + + ngOnChanges() { + console.log('Initialized'); + } + } + `; + assertSuccess(ruleName, source); + } + ); + }); + }); +}); diff --git a/test/decoratorNotAllowedRule.spec.ts b/test/decoratorNotAllowedRule.spec.ts deleted file mode 100644 index 79ae08fb3..000000000 --- a/test/decoratorNotAllowedRule.spec.ts +++ /dev/null @@ -1,311 +0,0 @@ -import { assertAnnotated, assertFailures, assertSuccess } from './testHelper'; - -describe('decorator-not-allowed', () => { - describe('success', () => { - describe('valid directive class', () => { - it('should succeed when is used @Injectable decorator', () => { - let source = ` - @Directive() - class TestDirective { - @Input() label: string - } - `; - assertSuccess('decorator-not-allowed', source); - }); - }); - - describe('valid directive class', () => { - it('should succeed when is used @Injectable decorator', () => { - let source = ` - @Directive() - class TestDirective { - @Output('change') change = new EventEmitter(); - } - `; - assertSuccess('decorator-not-allowed', source); - }); - }); - - describe('valid directive class', () => { - it('should succeed when is used @Injectable decorator', () => { - let source = ` - @Directive() - class TestDirective { - @HostBinding('style.backgroundColor') color = "red"; - @HostListener('mouseenter') onEnter() { - this.color= "blue" ; - } - } - `; - assertSuccess('decorator-not-allowed', source); - }); - }); - - describe('valid directive class', () => { - it('should succeed when is used @Injectable decorator', () => { - let source = ` - @Directive() - class TestDirective { - @ViewChild(ChildDirective) child: ChildDirective; - @ViewChildren(ChildDirective) viewChildren: QueryList; - } - `; - assertSuccess('decorator-not-allowed', source); - }); - }); - - describe('valid service class', () => { - it('should succeed when is used @Injectable decorator', () => { - let source = ` - @Directive() - class TestDirective { - @ContentChildren(Pane) topLevelPanes: QueryList; - @ContentChild(ChildDirective) contentChild: ChildDirective; - } - `; - assertSuccess('decorator-not-allowed', source); - }); - }); - - describe('valid directive class', () => { - it('should succeed when is used @Injectable decorator', () => { - let source = ` - @Directive() - class TestDirective { - @HostBinding('style.backgroundColor') color = "red"; - @HostListener('mouseenter') onEnter() { - this.color= "blue" ; - } - } - `; - assertSuccess('decorator-not-allowed', source); - }); - }); - - describe('valid directive class', () => { - it('should succeed when we have an @Injectable than an @Component decorator with an @Input', () => { - let source = ` - @Injectable() - class MyService { - } - @Component({}) - class MyComponent { - @Input('attribute') label: string; - } - `; - assertSuccess('decorator-not-allowed', source); - }); - }); - }); - - describe('failure', () => { - describe('not allowed input property', () => { - it('should fail, when the class have an @Injectable decorator', () => { - let source = ` - @Injectable() - class MyService { - @Input('attribute') label: string; - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'decorator-not-allowed', - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@Input" decorator is not allowed. ' + - 'Please, drop it.', - source - }); - }); - }); - - describe('not allowed output property', () => { - it('should fail, when the class have an @Injectable decorator', () => { - let source = ` - @Injectable() - class MyService { - @Output('change') change = new EventEmitter(); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'decorator-not-allowed', - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@Output" decorator is not allowed. ' + - 'Please, drop it.', - source - }); - }); - }); - - describe('not allowed input and output properties', () => { - it('should fail, when the class have an @Injectable decorator', () => { - let source = ` - @Injectable() - class MyService { - @Input('attribute') label: string; - @Output('change') change = new EventEmitter(); - } - `; - assertFailures('decorator-not-allowed', source, [ - { - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@Input" decorator is not allowed. ' + - 'Please, drop it.', - startPosition: { - line: 3, - character: 12 - }, - endPosition: { - line: 3, - character: 46 - } - }, - { - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@Output" decorator is not allowed. ' + - 'Please, drop it.', - startPosition: { - line: 4, - character: 12 - }, - endPosition: { - line: 4, - character: 63 - } - } - ]); - }); - }); - - describe('not allowed @HostBinding property', () => { - it('should fail, when the class have an @Injectable decorator', () => { - let source = ` - @Injectable() - class MyService { - @HostBinding('style.backgroundColor') color = "red"; - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'decorator-not-allowed', - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@HostBinding" decorator is not allowed. ' + - 'Please, drop it.', - source - }); - }); - }); - - describe('not allowed @HostListener property', () => { - it('should fail, when the class have an @Injectable decorator', () => { - let source = ` - @Injectable() - class MyService { - @HostListener('mouseenter') onEnter() { - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - this.color= "blue" ; - } - ~ - } - `; - assertAnnotated({ - ruleName: 'decorator-not-allowed', - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@HostListener" decorator is not allowed. ' + - 'Please, drop it.', - source - }); - }); - }); - - describe('not allowed @ViewChild and @ViewChildren properties', () => { - it('should fail, when the class have an @Injectable decorator', () => { - let source = ` - @Injectable() - class MyService { - @ViewChild(ChildDirective) child: ChildDirective; - @ViewChildren(ChildDirective) viewChildren: QueryList; - } - `; - assertFailures('decorator-not-allowed', source, [ - { - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@ViewChild" decorator is not allowed. ' + - 'Please, drop it.', - startPosition: { - line: 3, - character: 12 - }, - endPosition: { - line: 3, - character: 61 - } - }, - { - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@ViewChildren" decorator is not allowed. ' + - 'Please, drop it.', - startPosition: { - line: 4, - character: 12 - }, - endPosition: { - line: 4, - character: 82 - } - } - ]); - }); - }); - - describe('not allowed @ContentChild and @ContentChildren properties', () => { - it('should fail, when the class have an @Injectable decorator', () => { - let source = ` - @Injectable() - class MyService { - @ContentChild(ChildDirective) contentChild: ChildDirective; - @ContentChildren(Pane) topLevelPanes: QueryList; - } - `; - assertFailures('decorator-not-allowed', source, [ - { - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@ContentChild" decorator is not allowed. ' + - 'Please, drop it.', - startPosition: { - line: 3, - character: 12 - }, - endPosition: { - line: 3, - character: 71 - } - }, - { - message: - 'In the class "MyService" which have the "@Injectable" decorator, the ' + - '"@ContentChildren" decorator is not allowed. ' + - 'Please, drop it.', - startPosition: { - line: 4, - character: 12 - }, - endPosition: { - line: 4, - character: 66 - } - } - ]); - }); - }); - }); -}); diff --git a/test/enforceComponentSelectorRule.spec.ts b/test/enforceComponentSelectorRule.spec.ts deleted file mode 100644 index 67d5cde05..000000000 --- a/test/enforceComponentSelectorRule.spec.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { assertAnnotated, assertSuccess } from './testHelper'; - -describe('enforceComoponentSelectorRule', () => { - it('should fail when selector is not given in @Component', () => { - let source = ` - @Component() - ~~~~~~~~~~~~ - class Test {}`; - assertAnnotated({ - ruleName: 'enforce-component-selector', - message: 'The selector of the component "Test" is mandatory', - source - }); - }); - - it('should fail when selector is empty in @Component', () => { - let source = ` - @Component({ - ~~~~~~~~~~~~ - selector: '' - }) - ~~ - class Test {}`; - assertAnnotated({ - ruleName: 'enforce-component-selector', - message: 'The selector of the component "Test" is mandatory', - source - }); - }); - - it('should fail when selector equals 0 in @Component', () => { - let source = ` - @Component({ - ~~~~~~~~~~~~ - selector: 0 - }) - ~~ - class Test {}`; - assertAnnotated({ - ruleName: 'enforce-component-selector', - message: 'The selector of the component "Test" is mandatory', - source - }); - }); - - it('should fail when selector equals null in @Component', () => { - let source = ` - @Component({ - ~~~~~~~~~~~~ - selector: null - }) - ~~ - class Test {}`; - assertAnnotated({ - ruleName: 'enforce-component-selector', - message: 'The selector of the component "Test" is mandatory', - source - }); - }); - - it('should succeed when selector is given in @Component', () => { - let source = ` - @Component({ - selector: 'sg-bar-foo' - }) - class Test {}`; - assertSuccess('enforce-component-selector', source); - }); -}); diff --git a/test/i18nRule.spec.ts b/test/i18nRule.spec.ts deleted file mode 100644 index dde565f13..000000000 --- a/test/i18nRule.spec.ts +++ /dev/null @@ -1,382 +0,0 @@ -import { assertSuccess, assertAnnotated } from './testHelper'; -import { assertFailure } from './testHelper'; - -describe('i18n', () => { - describe('check-id', () => { - it('should work with proper id', () => { - const source = ` - @Component({ - template: \` -
Text
- \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-id']); - }); - - it('should work with proper id on ng-container', () => { - const source = ` - @Component({ - template: \` - Text - \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-id']); - }); - - it('should work with proper i18n attribute', () => { - const source = ` - @Component({ - template: \` -
- Use at least {{ minLength }} characters -
- \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-text']); - }); - - it('should work with proper id', () => { - const source = ` - @Component({ - template: \` -
Text
- \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-id']); - }); - - it('should work with proper id', () => { - const source = ` - @Component({ - template: \` -
Text
- \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-id']); - }); - - it('should fail with missing id string', () => { - const source = ` - @Component({ - template: \` -
Text
- ~~~~~~~~~~~~ - \` - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'i18n', - options: ['check-id'], - source, - message: 'Missing custom message identifier. For more information visit https://angular.io/guide/i18n' - }); - }); - - it('should fail with missing id', () => { - const source = ` - @Component({ - template: \` -
Text
- ~~~~~~~~~~ - \` - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'i18n', - options: ['check-id'], - source, - message: 'Missing custom message identifier. For more information visit https://angular.io/guide/i18n' - }); - }); - - it('should fail with missing id', () => { - const source = ` - @Component({ - template: \` -
Text
- ~~~~ - \` - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'i18n', - options: ['check-id'], - source, - message: 'Missing custom message identifier. For more information visit https://angular.io/guide/i18n' - }); - }); - }); - - describe('check-text', () => { - it('should work with i18n attribute', () => { - const source = ` - @Component({ - template: \` -
Text
- \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-text']); - }); - - it('should work with i18n attribute on ng-container', () => { - const source = ` - @Component({ - template: \` - Text - \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-text']); - }); - - it('should work with input and text', () => { - const source = ` - @Component({ - template: \` - - \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-text']); - }); - - it('should work with plural', () => { - const source = ` - @Component({ - template: \` - Updated {minutes, plural, =0 {just now} =1 {one minute ago} other {{{minutes}} minutes ago}} - \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-text']); - }); - - it('should work without i18n attribute & interpolation', () => { - const source = ` - @Component({ - template: \` -
{{text}}
- \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-text']); - }); - - it('should work with multiple valid elements', () => { - const source = ` - @Component({ - template: \` -
{{text}}
-
- Text - foo -
- \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-text']); - }); - - it('should work with multiple nested elements', () => { - const source = ` - @Component({ - template: \` -
    -
  • ItemA
    • -
  • ItemB
    • -
  • ItemC
    • -
- \` - }) - class Bar {} - `; - assertSuccess('i18n', source, ['check-text']); - }); - - it('should fail with missing id string', () => { - const source = ` - @Component({ - template: \` -
Text
- ~~~~ - \` - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'i18n', - options: ['check-text'], - source, - message: 'Each element containing text node should have an i18n attribute' - }); - }); - - it('should fail with missing id string in nested elements', () => { - const source = ` - @Component({ - template: \` -
- foo -
Text
- ~~~~ -
- \` - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'i18n', - options: ['check-text'], - source, - message: 'Each element containing text node should have an i18n attribute' - }); - }); - - it('should fail with text outside element with i18n attribute', () => { - const source = ` - @Component({ - template: \` -
Text
- foo - \` - }) - class Bar {} - `; - assertFailure( - 'i18n', - source, - { - message: 'Each element containing text node should have an i18n attribute', - startPosition: { - line: 3, - character: 32 - }, - endPosition: { - line: 5, - character: 10 - } - }, - ['check-text'] - ); - }); - - it('should fail with missing id string', () => { - const source = ` - @Component({ - template: \` -
Text {{ foo }}
- ~~~~~~~~~~~~~~ - \` - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'i18n', - options: ['check-text'], - source, - message: 'Each element containing text node should have an i18n attribute' - }); - }); - - it('should fail with missing id string', () => { - const source = ` - @Component({ - template: \` -
{{ foo }} text
- ~~~~~~~~~~~~~~ - \` - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'i18n', - options: ['check-text'], - source, - message: 'Each element containing text node should have an i18n attribute' - }); - }); - - it('should fail with text outside element with i18n attribute', () => { - const source = ` - @Component({ - template: \` -
Text
- {{foo}} text - \` - }) - class Bar {} - `; - assertFailure( - 'i18n', - source, - { - message: 'Each element containing text node should have an i18n attribute', - startPosition: { - line: 3, - character: 32 - }, - endPosition: { - line: 5, - character: 10 - } - }, - ['check-text'] - ); - }); - - it('should fail with text outside multiple nested elements', () => { - const source = ` - @Component({ - template: \` -
    -
  • ItemA
    • -
  • ItemB
    • -
  • ItemC
    • -
- Text - \` - }) - class Bar {} - `; - assertFailure( - 'i18n', - source, - { - message: 'Each element containing text node should have an i18n attribute', - startPosition: { - line: 7, - character: 17 - }, - endPosition: { - line: 9, - character: 10 - } - }, - ['check-text'] - ); - }); - }); -}); diff --git a/test/noConflictingLifeCycleHooksRule.spec.ts b/test/noConflictingLifeCycleHooksRule.spec.ts deleted file mode 100644 index 549c3b2f5..000000000 --- a/test/noConflictingLifeCycleHooksRule.spec.ts +++ /dev/null @@ -1,102 +0,0 @@ -import { assertFailures, assertSuccess, IExpectedFailure } from './testHelper'; - -const ruleName = 'no-conflicting-life-cycle-hooks'; -const fails: IExpectedFailure[] = [ - { - endPosition: { - line: 4, - character: 9 - }, - message: 'Implement DoCheck and OnChanges hooks in class Test is not recommended', - startPosition: { - line: 1, - character: 8 - } - } -]; - -describe(ruleName, () => { - describe('failure', () => { - it('should fail when implements both DoCheck and OnChanges hooks', () => { - const source = ` - class Test implements DoCheck, OnChanges { - test() {} - test1() {} - } - `; - assertFailures(ruleName, source, fails); - }); - - it('should fail when implements both DoCheck and OnChanges hooks/methods', () => { - const source = ` - class Test implements DoCheck, OnChanges { - ngDoCheck() {} - ngOnChanges() {} - } - `; - assertFailures(ruleName, source, fails.concat(fails)); - }); - - it('should fail have both ngDoCheck and ngOnChanges methods exist', () => { - const source = ` - class Test { - ngDoCheck() {} - ngOnChanges() {} - } - `; - assertFailures(ruleName, source, fails); - }); - }); - - describe('success', () => { - it('should pass when contain ngDoCheck, but not ngOnChanges method', () => { - const source = ` - class Test { - ngDoCheck() {} - } - `; - assertSuccess(ruleName, source); - }); - - it('should pass when implements DoCheck, but not OnChanges hook', () => { - const source = ` - class Test implements DoCheck {} - `; - assertSuccess(ruleName, source); - }); - - it('should pass when implementing and contain DoCheck hook/method, but not OnChanges hook/method', () => { - const source = ` - class Test implements DoCheck { - ngDoCheck() {} - } - `; - assertSuccess(ruleName, source); - }); - - it('should pass when contain ngOnChanges, but not ngDoCheck method', () => { - const source = ` - class Test { - ngOnChanges() {} - } - `; - assertSuccess(ruleName, source); - }); - - it('should pass when implements OnChanges, but not DoCheck hook', () => { - const source = ` - class Test implements OnChanges {} - `; - assertSuccess(ruleName, source); - }); - - it('should pass when implementing and contain OnChanges hook/method, but not DoCheck hook/method', () => { - const source = ` - class Test implements OnChanges { - ngOnChanges() {} - } - `; - assertSuccess(ruleName, source); - }); - }); -}); diff --git a/test/noConflictingLifecycleRule.spec.ts b/test/noConflictingLifecycleRule.spec.ts new file mode 100644 index 000000000..9c2d1bc64 --- /dev/null +++ b/test/noConflictingLifecycleRule.spec.ts @@ -0,0 +1,123 @@ +import { getFailureMessage, Rule } from '../src/noConflictingLifecycleRule'; +import { assertFailures, assertSuccess, IExpectedFailure } from './testHelper'; + +const { + FAILURE_STRING_INTERFACE_HOOK, + FAILURE_STRING_METHOD_HOOK, + metadata: { ruleName } +} = Rule; +const failures: IExpectedFailure = { + endPosition: { + character: 9, + line: 4 + }, + message: '', + startPosition: { + character: 8, + line: 1 + } +}; +const interfaceFailures: IExpectedFailure[] = [ + { + ...failures, + message: getFailureMessage({ + className: 'Test', + message: FAILURE_STRING_INTERFACE_HOOK + }) + } +]; +const methodFailures: IExpectedFailure[] = [ + { + ...failures, + message: getFailureMessage({ + className: 'Test', + message: FAILURE_STRING_METHOD_HOOK + }) + } +]; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail if implement DoCheck and OnChanges', () => { + const source = ` + class Test implements DoCheck, OnChanges { + test() {} + test1() {} + } + `; + assertFailures(ruleName, source, interfaceFailures); + }); + + it('should fail if implement DoCheck and OnChanges and contain the ngDoCheck and ngOnChanges methods', () => { + const source = ` + class Test implements DoCheck, OnChanges { + ngDoCheck() {} + ngOnChanges() {} + } + `; + assertFailures(ruleName, source, interfaceFailures.concat(methodFailures)); + }); + + it('should fail if the ngDoCheck and ngOnChanges methods exist', () => { + const source = ` + class Test { + ngDoCheck() {} + ngOnChanges() {} + } + `; + assertFailures(ruleName, source, methodFailures); + }); + }); + + describe('success', () => { + it('should pass if implements DoCheck, but not OnChanges', () => { + const source = ` + class Test implements DoCheck {} + `; + assertSuccess(ruleName, source); + }); + + it('should pass if contains ngDoCheck method, but not ngOnChanges', () => { + const source = ` + class Test { + ngDoCheck() {} + } + `; + assertSuccess(ruleName, source); + }); + + it('should pass if implements DoCheck and contains ngDoCheck method, but does not implement OnChanges and does not contain ngOnChanges method', () => { + const source = ` + class Test implements DoCheck { + ngDoCheck() {} + } + `; + assertSuccess(ruleName, source); + }); + + it('should pass if implements OnChanges, but not DoCheck', () => { + const source = ` + class Test implements OnChanges {} + `; + assertSuccess(ruleName, source); + }); + + it('should pass if contains ngOnChanges method, but not ngDoCheck', () => { + const source = ` + class Test { + ngOnChanges() {} + } + `; + assertSuccess(ruleName, source); + }); + + it('should pass if implements OnChanges and contains ngOnChanges method, but does not implement DoCheck and does not contain ngDoCheck method', () => { + const source = ` + class Test implements OnChanges { + ngOnChanges() {} + } + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/noForwardRefRule.spec.ts b/test/noForwardRefRule.spec.ts index c91ac9626..0d02a2824 100644 --- a/test/noForwardRefRule.spec.ts +++ b/test/noForwardRefRule.spec.ts @@ -1,81 +1,109 @@ +import { getFailureMessage, Rule } from '../src/noForwardRefRule'; import { assertAnnotated, assertSuccess } from './testHelper'; -describe('no-forward-ref', () => { - describe('invalid function call', () => { - it('should fail when we are calling forwardRef in constructor', () => { - let source = ` - class Test { - constructor(@Inject(forwardRef(()=>NameService)) nameService) {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - class NameService {} - `; - assertAnnotated({ - ruleName: 'no-forward-ref', - message: 'Avoid using forwardRef in class "Test"', - source - }); - }); +const { + FAILURE_STRING_CLASS, + FAILURE_STRING_VARIABLE, + metadata: { ruleName } +} = Rule; - it('should fail when we are calling forwardRef in Component directives array', () => { - let source = ` - @Component({ - directives: [forwardRef(()=>NameService)] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~ - }) - class Test {} - class NameService {} - `; - assertAnnotated({ - ruleName: 'no-forward-ref', - message: 'Avoid using forwardRef in class "Test"', - source +describe(ruleName, () => { + describe('failure', () => { + describe('class', () => { + it('should fail if forwardRef is called in constructor', () => { + const source = ` + @Component({ + selector: 'test', + template: '' + }) + export class Test { + constructor(@Inject(forwardRef(() => TestService)) testService) {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + export class TestService {} + `; + const message = getFailureMessage({ + className: 'Test', + message: FAILURE_STRING_CLASS + }); + assertAnnotated({ + message, + ruleName, + source + }); }); - }); - it('should fail when we are calling forwardRef in variable', () => { - let source = ` - const TAGS_INPUT_CONTROL_VALUE_ACCESSOR = new Provider( - NG_VALUE_ACCESSOR, { - useExisting: forwardRef(() => TagsInput), - ~~~~~~~~~~~~~~~~~~~~~~~~~~~ - multi: true - } - ); - `; - assertAnnotated({ - ruleName: 'no-forward-ref', - message: 'Avoid using forwardRef in variable "TAGS_INPUT_CONTROL_VALUE_ACCESSOR"', - source + it('should fail if forwardRef is called in a metadata property', () => { + const source = ` + @Component({ + providers: [ + { + multi: true, + provide: NG_VALUE_ACCESSOR, + useExisting: forwardRef(() => TagsValueAccessor) + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + ], + selector: '[tags]', + }) + export class TagsValueAccessor {} + `; + const message = getFailureMessage({ + className: 'TagsValueAccessor', + message: FAILURE_STRING_CLASS + }); + assertAnnotated({ + message, + ruleName, + source + }); }); }); - }); - it('should work with NG_VALUE_ACCESSORs', () => { - let source = ` - const CUSTOM_VALUE_ACCESSOR = new Provider( - NG_VALUE_ACCESSOR, { useExisting: forwardRef(() => CountryListValueAccessor)}); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - `; - assertAnnotated({ - ruleName: 'no-forward-ref', - message: 'Avoid using forwardRef in variable "CUSTOM_VALUE_ACCESSOR"', - source + describe('variable', () => { + it('should fail if forwardRef is called in a variable declaration', () => { + const source = ` + const TAGS_VALUE_ACCESSOR: StaticProvider = { + multi: true, + provide: NG_VALUE_ACCESSOR, + useExisting: forwardRef(() => TagsValueAccessor) + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }; + @Directive({ + providers: [TAGS_VALUE_ACCESSOR], + selector: '[tags]' + }) + export class TagsValueAccessor {} + `; + const message = getFailureMessage({ + className: 'TAGS_VALUE_ACCESSOR', + message: FAILURE_STRING_VARIABLE + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); }); }); - describe('valid function call', () => { - it('should succeed, when we are not calling forwardRef', () => { - let source = ` - class Test { + describe('success', () => { + it('should succeed if forwardRef is not called', () => { + const source = ` + @Component({ + selector: 'test', + template: '' + }) + export class Test { constructor() { this.test(); } - test() { - } + + test() {} } `; - assertSuccess('no-forward-ref', source); + assertSuccess(ruleName, source); }); }); }); diff --git a/test/noHostMetadataPropertyRule.spec.ts b/test/noHostMetadataPropertyRule.spec.ts new file mode 100644 index 000000000..12c65e170 --- /dev/null +++ b/test/noHostMetadataPropertyRule.spec.ts @@ -0,0 +1,76 @@ +import { Rule } from '../src/noHostMetadataPropertyRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + FAILURE_STRING, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail if "host" metadata property is used in @Component', () => { + const source = ` + @Component({ + host: [ + ~~~~~~~ + class: 'my-class', + [type]: 'test', + '(click)': 'bar()' + ], + ~ + selector: 'app-test' + }) + class TestComponent {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail if "host" metadata property is used in @Directive', () => { + const source = ` + @Directive({ + host: [ + ~~~~~~~ + class: 'my-class', + [type]: 'test', + '(click)': 'bar()' + ], + ~ + selector: 'app-test' + }) + class TestDirective {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed if "host" metadata property is not used in @Component', () => { + const source = ` + @Component({ + selector: 'app-test', + template: 'Hello' + }) + class TestComponent {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if "host" metadata property is not used in Directive', () => { + const source = ` + @Directive({ + selector: 'app-test' + }) + class TestDirective {} + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/noInputsMetadataPropertyRule.spec.ts b/test/noInputsMetadataPropertyRule.spec.ts new file mode 100644 index 000000000..9b00fff8a --- /dev/null +++ b/test/noInputsMetadataPropertyRule.spec.ts @@ -0,0 +1,72 @@ +import { Rule } from '../src/noInputsMetadataPropertyRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + FAILURE_STRING, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail if "inputs" metadata property is used in @Component', () => { + const source = ` + @Component({ + inputs: [ + ~~~~~~~~~ + 'id: foo' + ], + ~ + selector: 'app-test' + }) + class TestComponent {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail if "inputs" metadata property is used in @Directive', () => { + const source = ` + @Directive({ + inputs: [ + ~~~~~~~~~ + 'id: foo' + ], + ~ + selector: 'app-test' + }) + class TestDirective {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed if "inputs" metadata property is not used in @Component', () => { + const source = ` + @Component({ + selector: 'app-test', + template: 'Hello' + }) + class TestComponent {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if "inputs" metadata property is not used in Directive', () => { + const source = ` + @Directive({ + selector: 'app-test' + }) + class TestDirective {} + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/noLifeCycleCallRule.spec.ts b/test/noLifeCycleCallRule.spec.ts deleted file mode 100644 index 3dfa96ae0..000000000 --- a/test/noLifeCycleCallRule.spec.ts +++ /dev/null @@ -1,134 +0,0 @@ -import { lifecycleHooksMethods, LifecycleHooksMethods, Rule } from '../src/noLifeCycleCallRule'; -import { assertAnnotated, assertSuccess } from './testHelper'; - -type Metadata = 'Component' | 'Directive' | 'Injectable' | 'Pipe'; - -type MetadataPair = { [key in Metadata]: typeof lifecycleHooksMethods }; - -type MetadataFakePair = { [key in Metadata]?: Set }; - -const { - FAILURE_STRING, - metadata: { ruleName } -} = Rule; -const className = 'Test'; -const metadataPairs: MetadataPair = { - Component: lifecycleHooksMethods, - Directive: lifecycleHooksMethods, - Injectable: new Set(['ngOnDestroy']), - Pipe: new Set(['ngOnDestroy']) -}; -const metadataKeys = Object.keys(metadataPairs); -const prefix = 'prefix'; -const suffix = 'Suffix'; -const metadataFakePairs: MetadataFakePair = {}; - -for (const metadataKey of metadataKeys) { - metadataFakePairs[metadataKey] = new Set(); - - metadataPairs[metadataKey].forEach(lifecycleHookMethod => { - metadataFakePairs[metadataKey] - .add(`${prefix}${lifecycleHookMethod}`) - .add(`${lifecycleHookMethod}${suffix}`) - .add(`${prefix}${lifecycleHookMethod}${suffix}`); - }); -} - -const getFailureAnnotations = (num: number): string => { - return '~'.repeat(num); -}; - -describe(ruleName, () => { - describe('failure', () => { - for (const metadataKey of metadataKeys) { - describe(metadataKey, () => { - metadataPairs[metadataKey].forEach(lifecycleHookMethod => { - const lifecycleHookMethodCall = `this.${lifecycleHookMethod}()`; - const source = ` - @${metadataKey}() - class ${className} implements ${lifecycleHookMethod.slice(2)} { - ${lifecycleHookMethod}() {} - - ${className.toLowerCase()}() { - ${lifecycleHookMethodCall} - ${getFailureAnnotations(lifecycleHookMethodCall.length)} - } - } - `; - - it(`should fail when explicitly calling ${lifecycleHookMethodCall}`, () => { - assertAnnotated({ - message: FAILURE_STRING, - ruleName, - source - }); - }); - }); - }); - } - - describe('outside of a class', () => { - lifecycleHooksMethods.forEach(lifecycleHookMethod => { - const lifecycleHookMethodCall = `fixture.componentInstance.${lifecycleHookMethod}()`; - const source = ` - it('should work', () => { - // test code... - - ${lifecycleHookMethodCall} - ${getFailureAnnotations(lifecycleHookMethodCall.length)} - - // more test code... - }) - `; - - it(`should fail when explicitly calling ${lifecycleHookMethodCall}`, () => { - assertAnnotated({ - message: FAILURE_STRING, - ruleName, - source - }); - }); - }); - }); - }); - - describe('success', () => { - for (const metadataKey of metadataKeys) { - describe(metadataKey, () => { - metadataFakePairs[metadataKey].forEach(fakeMethod => { - const source = ` - @${metadataKey}() - class ${className} { - ${fakeMethod}() {} - - ${className.toLowerCase()}() { - this.${fakeMethod}(); - } - } - `; - - it(`should pass when calling ${fakeMethod} method`, () => { - assertSuccess(ruleName, source); - }); - }); - - // call super lifecycle hook - metadataPairs[metadataKey].forEach(lifecycleHookMethod => { - const lifecycleHookMethodCall = `super.${lifecycleHookMethod}()`; - const source = ` - @${metadataKey}() - class ${className} implements ${lifecycleHookMethod.slice(2)} { - ${lifecycleHookMethod}() { - ${lifecycleHookMethodCall} - } - } - `; - - it(`should pass when explicitly calling ${lifecycleHookMethodCall}`, () => { - assertSuccess(ruleName, source); - }); - }); - }); - } - }); -}); diff --git a/test/noLifecycleCallRule.spec.ts b/test/noLifecycleCallRule.spec.ts new file mode 100644 index 000000000..af3a72771 --- /dev/null +++ b/test/noLifecycleCallRule.spec.ts @@ -0,0 +1,566 @@ +import { Rule } from '../src/noLifecycleCallRule'; +import { assertAnnotated, assertMultipleAnnotated, assertSuccess } from './testHelper'; + +const { + FAILURE_STRING, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + describe('Component', () => { + it('should fail when explicitly calling ngAfterContentChecked()', () => { + const source = ` + @Component() + class Test { + test() { + this.ngAfterContentChecked(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterContentInit()', () => { + const source = ` + @Component() + class Test { + test() { + this.ngAfterContentInit(); + ~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterViewChecked()', () => { + const source = ` + @Component() + class Test { + test() { + this.ngAfterViewChecked(); + ~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterViewInit()', () => { + const source = ` + @Component() + class Test { + test() { + this.ngAfterViewInit(); + ~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngDoCheck()', () => { + const source = ` + @Component() + class Test { + test() { + this.ngDoCheck(); + ~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngOnChanges()', () => { + const source = ` + @Component() + class Test { + test() { + this.ngOnChanges(); + ~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngOnInit()', () => { + const source = ` + @Component() + class Test { + test() { + this.ngOnInit(); + ~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + }); + + describe('Directive', () => { + it('should fail when explicitly calling ngAfterContentChecked()', () => { + const source = ` + @Directive() + class Test { + test() { + this.ngAfterContentChecked(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterContentInit()', () => { + const source = ` + @Directive() + class Test { + test() { + this.ngAfterContentInit(); + ~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterViewChecked()', () => { + const source = ` + @Directive() + class Test { + test() { + this.ngAfterViewChecked(); + ~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterViewInit()', () => { + const source = ` + @Directive() + class Test { + test() { + this.ngAfterViewInit(); + ~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngDoCheck()', () => { + const source = ` + @Directive() + class Test { + test() { + this.ngDoCheck(); + ~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngOnChanges()', () => { + const source = ` + @Directive() + class Test { + test() { + this.ngOnChanges(); + ~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngOnInit()', () => { + const source = ` + @Directive() + class Test { + test() { + this.ngOnInit(); + ~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + }); + + describe('Injectable', () => { + it('should fail when explicitly calling ngAfterContentChecked()', () => { + const source = ` + @Injectable() + class Test { + test() { + this.ngAfterContentChecked(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterContentInit()', () => { + const source = ` + @Injectable() + class Test { + test() { + this.ngAfterContentInit(); + ~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterViewChecked()', () => { + const source = ` + @Injectable() + class Test { + test() { + this.ngAfterViewChecked(); + ~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterViewInit()', () => { + const source = ` + @Injectable() + class Test { + test() { + this.ngAfterViewInit(); + ~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngDoCheck()', () => { + const source = ` + @Injectable() + class Test { + test() { + this.ngDoCheck(); + ~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngOnChanges()', () => { + const source = ` + @Injectable() + class Test { + test() { + this.ngOnChanges(); + ~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngOnInit()', () => { + const source = ` + @Injectable() + class Test { + test() { + this.ngOnInit(); + ~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + }); + + describe('Pipe', () => { + it('should fail when explicitly calling ngAfterContentChecked()', () => { + const source = ` + @Pipe() + class Test { + test() { + this.ngAfterContentChecked(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterContentInit()', () => { + const source = ` + @Pipe() + class Test { + test() { + this.ngAfterContentInit(); + ~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterViewChecked()', () => { + const source = ` + @Pipe() + class Test { + test() { + this.ngAfterViewChecked(); + ~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngAfterViewInit()', () => { + const source = ` + @Pipe() + class Test { + test() { + this.ngAfterViewInit(); + ~~~~~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngDoCheck()', () => { + const source = ` + @Pipe() + class Test { + test() { + this.ngDoCheck(); + ~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngOnChanges()', () => { + const source = ` + @Pipe() + class Test { + test() { + this.ngOnChanges(); + ~~~~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail when explicitly calling ngOnInit()', () => { + const source = ` + @Pipe() + class Test { + test() { + this.ngOnInit(); + ~~~~~~~~~~~~~~~ + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + }); + + describe('multiple calls per file', () => { + it('should fail when explicitly calling multiple lifecycle methods', () => { + const source = ` + @Component() + class Test { + test(): void { + this.ngDoCheck(); + ~~~~~~~~~~~~~~~~ + this.ngOnChanges(); + ^^^^^^^^^^^^^^^^^^ + } + } + + @Directive() + class TestDirective { + test2(): void { + this.ngOnDestroy(); + ################## + this.ngOnInit(); + %%%%%%%%%%%%%%% + } + } + `; + assertMultipleAnnotated({ + failures: [ + { + char: '~', + msg: FAILURE_STRING + }, + { + char: '^', + msg: FAILURE_STRING + }, + { + char: '#', + msg: FAILURE_STRING + }, + { + char: '%', + msg: FAILURE_STRING + } + ], + ruleName, + source + }); + }); + }); + }); + + describe('success', () => { + it('should succeed when explicitly calling multiple non lifecycle methods', () => { + const source = ` + @Component() + class Test { + test(): void { + this.ng4fterViewChecked(); + this.ngOnChange$(); + } + } + + @Directive() + class TestDirective { + test2(): void { + this.ngOnD3stroy(); + this.ngOn1nit(); + } + } + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/noOutputNamedAfterStandardEventRule.spec.ts b/test/noOutputNamedAfterStandardEventRule.spec.ts deleted file mode 100644 index 0168ee833..000000000 --- a/test/noOutputNamedAfterStandardEventRule.spec.ts +++ /dev/null @@ -1,107 +0,0 @@ -import { assertSuccess, assertAnnotated } from './testHelper'; - -describe('no-output-named-after-standard-event', () => { - describe('invalid directive output property', () => { - it('should fail, when component output property is named "change"', () => { - const source = ` - @Component() - class ButtonComponent { - @Output() change = new EventEmitter(); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'no-output-named-after-standard-event', - message: 'In the class "ButtonComponent", the output property "change" should not be named or renamed after a standard event', - source - }); - }); - - it('should fail, when directive output property is named "change"', () => { - const source = ` - @Directive() - class ButtonDirective { - @Output() change = new EventEmitter(); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'no-output-named-after-standard-event', - message: 'In the class "ButtonDirective", the output property "change" should not be named or renamed after a standard event', - source - }); - }); - - it('should fail, when component output property is renamed to "change"', () => { - const source = ` - @Component() - class ButtonComponent { - @Output("change") _change = new EventEmitter(); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'no-output-named-after-standard-event', - message: 'In the class "ButtonComponent", the output property "_change" should not be named or renamed after a standard event', - source - }); - }); - - it('should fail, when directive output property is renamed to "change"', () => { - const source = ` - @Directive() - class ButtonDirective { - @Output("change") _change = new EventEmitter(); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - } - `; - assertAnnotated({ - ruleName: 'no-output-named-after-standard-event', - message: 'In the class "ButtonDirective", the output property "_change" should not be named or renamed after a standard event', - source - }); - }); - }); - - describe('valid directive output property', () => { - it('should succeed, when a component output property is properly named', () => { - const source = ` - @Component() - class ButtonComponent { - @Output() buttonChange = new EventEmitter(); - } - `; - assertSuccess('no-output-named-after-standard-event', source); - }); - - it('should succeed, when a directive output property is properly named', () => { - const source = ` - @Directive() - class ButtonDirective { - @Output() buttonChange = new EventEmitter(); - } - `; - assertSuccess('no-output-named-after-standard-event', source); - }); - - it('should succeed, when a component output property is properly renamed', () => { - const source = ` - @Component() - class ButtonComponent { - @Output("buttonChange") _buttonChange = new EventEmitter(); - } - `; - assertSuccess('no-output-named-after-standard-event', source); - }); - - it('should succeed, when a directive output property is properly renamed', () => { - const source = ` - @Directive() - class ButtonDirective { - @Output("buttonChange") _buttonChange = new EventEmitter(); - } - `; - assertSuccess('no-output-named-after-standard-event', source); - }); - }); -}); diff --git a/test/noOutputNativeRule.spec.ts b/test/noOutputNativeRule.spec.ts new file mode 100644 index 000000000..b6d12af66 --- /dev/null +++ b/test/noOutputNativeRule.spec.ts @@ -0,0 +1,136 @@ +import { getFailureMessage, Rule } from '../src/noOutputNativeRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + describe('Component', () => { + it('should fail if a property is named "change"', () => { + const source = ` + @Component() + class Test { + @Output() change = new EventEmitter(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + propertyName: 'change' + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if a property is renamed to "change"', () => { + const source = ` + @Component() + class Test { + @Output('change') _change = new EventEmitter(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + propertyName: '_change' + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + }); + + describe('Directive', () => { + it('should fail if a property is named "change"', () => { + const source = ` + @Directive() + class Test { + @Output() change = new EventEmitter(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + propertyName: 'change' + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if a property is renamed to "change"', () => { + const source = ` + @Directive() + class Test { + @Output('change') _change = new EventEmitter(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const message = getFailureMessage({ + className: 'Test', + propertyName: '_change' + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + }); + }); + + describe('success', () => { + describe('Component', () => { + it('should succeed if a property is properly named', () => { + const source = ` + @Component() + class Test { + @Output() buttonChange = new EventEmitter(); + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is properly renamed', () => { + const source = ` + @Component() + class Test { + @Output('buttonChange') _buttonChange = new EventEmitter(); + } + `; + assertSuccess(ruleName, source); + }); + }); + + describe('Directive', () => { + it('should succeed if a property is properly named', () => { + const source = ` + @Directive() + class Test { + @Output() buttonChange = new EventEmitter(); + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property is properly renamed', () => { + const source = ` + @Directive() + class Test { + @Output('buttonChange') _buttonChange = new EventEmitter(); + } + `; + assertSuccess(ruleName, source); + }); + }); + }); +}); diff --git a/test/noOutputsPropertyMetadataRule.spec.ts b/test/noOutputsPropertyMetadataRule.spec.ts new file mode 100644 index 000000000..7e5e2dbb6 --- /dev/null +++ b/test/noOutputsPropertyMetadataRule.spec.ts @@ -0,0 +1,72 @@ +import { Rule } from '../src/noOutputsMetadataPropertyRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + FAILURE_STRING, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail if "outputs" metadata property is used in @Component', () => { + const source = ` + @Component({ + outputs: [ + ~~~~~~~~~~ + 'id: foo' + ], + ~ + selector: 'app-test' + }) + class TestComponent {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail if "outputs" metadata property is used in @Directive', () => { + const source = ` + @Directive({ + outputs: [ + ~~~~~~~~~~ + 'id: foo' + ], + ~ + selector: 'app-test' + }) + class TestDirective {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed if "outputs" metadata property is not used in @Component', () => { + const source = ` + @Component({ + selector: 'app-test', + template: 'Hello' + }) + class TestComponent {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if "outputs" metadata property is not used in Directive', () => { + const source = ` + @Directive({ + selector: 'app-test' + }) + class TestDirective {} + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/noPipeImpureRule.spec.ts b/test/noPipeImpureRule.spec.ts new file mode 100644 index 000000000..6c11bb6bd --- /dev/null +++ b/test/noPipeImpureRule.spec.ts @@ -0,0 +1,52 @@ +import { getFailureMessage, Rule } from '../src/noPipeImpureRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failures', () => { + it('should fail if pure property is set to false', () => { + const source = ` + @Pipe({ + name: 'test', + pure: false + ~~~~~~~~~~~ + }) + class Test {} + `; + const message = getFailureMessage({ + className: 'Test' + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed if pure property is set to true', () => { + const source = ` + @Pipe({ + name: 'test', + pure: true + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if pure property is not set', () => { + const source = ` + @Pipe({ + name: 'test' + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/noQueriesMetadataPropertyRule.spec.ts b/test/noQueriesMetadataPropertyRule.spec.ts new file mode 100644 index 000000000..95e2cd382 --- /dev/null +++ b/test/noQueriesMetadataPropertyRule.spec.ts @@ -0,0 +1,78 @@ +import { Rule } from '../src/noQueriesMetadataPropertyRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + FAILURE_STRING, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail if "queries" metadata property is used in @Component', () => { + const source = ` + @Component({ + queries: [ + ~~~~~~~~~~ + contentChild: new ContentChild(ChildDirective), + contentChildren: new ContentChildren(ChildDirective), + viewChild: new ViewChild(ChildDirective), + viewChildren: new ViewChildren(ChildDirective) + ], + ~ + selector: 'app-test' + }) + class TestComponent {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail if "queries" metadata property is used in @Directive', () => { + const source = ` + @Directive({ + queries: [ + ~~~~~~~~~ + contentChild: new ContentChild(ChildDirective), + contentChildren: new ContentChildren(ChildDirective), + viewChild: new ViewChild(ChildDirective), + viewChildren: new ViewChildren(ChildDirective) + ], + ~ + selector: 'app-test' + }) + class TestDirective {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed if "queries" metadata property is not used in @Component', () => { + const source = ` + @Component({ + selector: 'app-test', + template: 'Hello' + }) + class TestComponent {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if "queries" metadata property is not used in Directive', () => { + const source = ` + @Directive({ + selector: 'app-test' + }) + class TestDirective {} + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/noQueriesParameterRule.spec.ts b/test/noQueriesParameterRule.spec.ts deleted file mode 100644 index a4871f405..000000000 --- a/test/noQueriesParameterRule.spec.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { Rule } from '../src/noQueriesParameterRule'; -import { assertAnnotated, assertSuccess } from './testHelper'; - -type MetadataType = 'Component' | 'Directive'; - -const { - FAILURE_STRING, - metadata: { ruleName } -} = Rule; -const metadataTypes = new Set(['Component', 'Directive']); - -describe(ruleName, () => { - metadataTypes.forEach(metadataType => { - describe(metadataType, () => { - describe('failure', () => { - it('should fail when "queries" is used', () => { - const source = ` - @${metadataType}({ - queries: { - ~~~~~~~~ - contentChildren: new ContentChildren(ChildDirective), - viewChildren: new ViewChildren(ChildDirective) - } - ~ - }) - class Test {} - `; - assertAnnotated({ - message: FAILURE_STRING, - ruleName, - source - }); - }); - - it('should fail when "queries" is used', () => { - const source = ` - @${metadataType}({ - queries: { - ~~~~~~~~ - contentChild: new ContentChild(ChildDirective), - viewChild: new ViewChild(ChildDirective) - } - ~ - }) - class Test {} - `; - assertAnnotated({ message: FAILURE_STRING, ruleName, source }); - }); - }); - describe('success', () => { - it('should succeed when "queries" is not used', () => { - const source = ` - @${metadataType}({}) - class Test {} - `; - assertSuccess(ruleName, source); - }); - - it('should succeed when content decorators are used', () => { - const source = ` - @${metadataType}({}) - class Test { - @ContentChild(ChildDirective) contentChild: ChildDirective; - } - `; - assertSuccess(ruleName, source); - }); - }); - }); - }); -}); diff --git a/test/pipeImpureRule.spec.ts b/test/pipeImpureRule.spec.ts deleted file mode 100644 index 4f6ff71df..000000000 --- a/test/pipeImpureRule.spec.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { assertSuccess, assertAnnotated } from './testHelper'; - -describe('pipe-impure', () => { - describe('impure pipe', () => { - it('should fail when Pipe is impure', () => { - let source = ` - @Pipe({ - pure: false - ~~~~~~~~~~~ - }) - class Test {} - `; - assertAnnotated({ - ruleName: 'pipe-impure', - message: 'Warning: impure pipe declared in class Test', - source - }); - }); - }); - - describe('pure pipe', () => { - it('should succeed when Pipe is pure', () => { - let source = ` - @Pipe({ - pure: true - }) - class Test {} - `; - assertSuccess('pipe-impure', source); - }); - }); - - describe('empty pipe', () => { - it('should not fail', () => { - let source = ` - @Pipe - class Test {} - `; - assertSuccess('pipe-impure', source); - }); - }); - - describe('default pipe', () => { - it('should succeed when Pipe pure property is not set', () => { - let source = ` - @Pipe({ - name: 'testPipe' - }) - class Test {} - `; - assertSuccess('pipe-impure', source); - }); - }); -}); diff --git a/test/preferInlineDecoratorRule.spec.ts b/test/preferInlineDecoratorRule.spec.ts index eb2901321..31f59cd43 100644 --- a/test/preferInlineDecoratorRule.spec.ts +++ b/test/preferInlineDecoratorRule.spec.ts @@ -1,163 +1,137 @@ import { expect } from 'chai'; import { Replacement } from 'tslint/lib'; -import { decoratorKeys, getFailureMessage, Rule } from '../src/preferInlineDecoratorRule'; -import { assertFailure, assertFailures, assertSuccess, IExpectedFailure } from './testHelper'; +import { Rule } from '../src/preferInlineDecoratorRule'; +import { Decorators } from '../src/util/utils'; +import { assertAnnotated, assertFailures, assertSuccess } from './testHelper'; const { + FAILURE_STRING, metadata: { ruleName } } = Rule; -const className = 'Test'; describe(ruleName, () => { describe('failure', () => { - const expectedFailure: IExpectedFailure = { - endPosition: { - character: 35, - line: 3 - }, - message: getFailureMessage(), - startPosition: { - character: 14, - line: 2 - } - }; - - decoratorKeys.forEach(decoratorKey => { - describe(decoratorKey, () => { - it('should fail when a property does not start on the same line as the decoratorKey', () => { - const source = ` - class ${className} { - @${decoratorKey}('childTest') - childTest: ChildTest; - } - `; - assertFailure(ruleName, source, expectedFailure); - }); - - it('should fail and apply proper replacements when a property does not start on the same line as the decoratorKey', () => { - const source = ` - class ${className} { - @${decoratorKey}('childTest') - childTest: ChildTest; - } - `; - const failures = assertFailure(ruleName, source, expectedFailure)!; - const replacement = Replacement.applyFixes(source, failures.map(f => f.getFix()!)); - - expect(replacement).to.eq(` - class ${className} { - @${decoratorKey}('childTest') childTest: ChildTest; - } - `); + describe('common cases', () => { + it('should fail if a property is not on the same line as its decorator', () => { + const source = ` + class Test { + @Input('test') + ~~~~~~~~~~~~~~ + testVar: string; + ~~~~~~~~~~~~~~~~ + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source }); }); - }); - describe('blacklist', () => { - it('should fail when a property does not start on the same line as the decoratorKey and is not present on blacklist options', () => { - const [firstDecorator, ...restDecorators] = Array.from(decoratorKeys); + it('should fail if multiple properties are not on the same line as their decorators', () => { const source = ` - class ${className} { - @${firstDecorator}() - test = new EventEmitter(); + class Test { + @Input('test1') + testVar1: string; + @Input('test2') + testVar2: string; } `; - assertFailure( - ruleName, - source, + assertFailures(ruleName, source, [ { endPosition: { - character: 44, + character: 29, line: 3 }, - message: getFailureMessage(), + message: FAILURE_STRING, startPosition: { character: 12, line: 2 } }, - restDecorators - ); + { + endPosition: { + character: 29, + line: 5 + }, + message: FAILURE_STRING, + startPosition: { + character: 12, + line: 4 + } + } + ]); }); }); - it('should fail when there are multiple properties that does not start on the same line as the decoratorKey', () => { - const source = ` - class ${className} { - @Input('childTest') - childTest: ChildTest; - @Input('foo') - foo: Foo; - } - `; - assertFailures(ruleName, source, [ - { - endPosition: { - character: 31, - line: 3 - }, - message: getFailureMessage(), - startPosition: { - character: 10, - line: 2 - } - }, - { - endPosition: { - character: 19, - line: 5 - }, - message: getFailureMessage(), - startPosition: { - character: 10, - line: 4 + describe('blacklist', () => { + it('should fail if a property is not on the same line as its decorator, which is not blacklisted', () => { + const source = ` + class Test { + @Input('test') + ~~~~~~~~~~~~~~ + testVar: string; + ~~~~~~~~~~~~~~~~ } - } - ]); + `; + assertAnnotated({ + message: FAILURE_STRING, + options: [Decorators.Output], + ruleName, + source + }); + }); }); }); describe('success', () => { - decoratorKeys.forEach(decoratorKey => { - describe(decoratorKey, () => { - it('should succeed when a property starts and ends on the same line as the decoratorKey', () => { - const source = ` - class ${className} { - @${decoratorKey}('childTest') childTest123: ChildTest; - } - `; - assertSuccess(ruleName, source); - }); + describe('common cases', () => { + it('should succeed if a property is on the same line as its decorator', () => { + const source = ` + class Test { + @Input('test') testVar: string; + } + `; + assertSuccess(ruleName, source); + }); - it('should succeed when a property starts on the same line as the decoratorKey and ends on another line', () => { - const source = ` - class ${className} { - @${decoratorKey}('childTest') childTest123: ChildTest = - veryVeryVeryVeryVeryVeryVeryLongDefaultVariable; - } - `; - assertSuccess(ruleName, source); - }); + it('should succeed if multiple properties are on the same line as their decorators', () => { + const source = ` + class Test { + @Input('test1') testVar1: string; + @Input('test2') testVar2: string; + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if a property starts on the same line as its decorator and ends on the next line', () => { + const source = ` + class Test { + @Input('test') testVar: string = + veryVeryVeryVeryVeryVeryVeryLongDefaultVariable; + } + `; + assertSuccess(ruleName, source); }); }); describe('blacklist', () => { - it('should succeed when a property starts on another line and is present on blacklist options', () => { - const [firstDecorator] = Array.from(decoratorKeys); + it('should succeed if a property is not on the same line as its decorator, which is blacklisted', () => { const source = ` - class ${className} { - @${firstDecorator}() + class Test { + @Output() test = new EventEmitter(); } `; - assertSuccess(ruleName, source, [firstDecorator]); + assertSuccess(ruleName, source, [Decorators.Output]); }); }); describe('special cases', () => { - it('should succeed when getters starts on the same line as the decoratorKey and ends on another line', () => { + it('should succeed if getter accessor starts on the same line as its decorator and ends on the next line', () => { const source = ` - class ${className} { + class Test { @Input() get test(): string { return this._test; } @@ -167,9 +141,9 @@ describe(ruleName, () => { assertSuccess(ruleName, source); }); - it('should succeed when setters starts on the same line as the decoratorKey and ends on another line', () => { + it('should succeed if setter accessor starts on the same line as its decorator and ends on the next line', () => { const source = ` - class ${className} { + class Test { @Input() set test(value: string) { this._test = value; } @@ -179,9 +153,9 @@ describe(ruleName, () => { assertSuccess(ruleName, source); }); - it('should succeed for getters and setters on another line', () => { + it('should succeed if getters/setters accessors are not on the same line as their decorators', () => { const source = ` - class ${className} { + class Test { @Input() get test(): string { return this._test; @@ -196,4 +170,33 @@ describe(ruleName, () => { }); }); }); + + describe('replacements', () => { + it('should fail if a property is not on the same line as its decorator', () => { + const source = ` + class Test { + @Output() + ~~~~~~~~~ + test = new EventEmitter(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `; + const failures = assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + + if (!Array.isArray(failures)) return; + + const replacement = Replacement.applyFixes(source, failures.map(x => x.getFix()!)); + + expect(replacement).to.eq(` + class Test { + @Output() test = new EventEmitter(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + `); + }); + }); }); diff --git a/test/templateBananaInBoxRule.spec.ts b/test/templateBananaInBoxRule.spec.ts new file mode 100644 index 000000000..4f80a8981 --- /dev/null +++ b/test/templateBananaInBoxRule.spec.ts @@ -0,0 +1,183 @@ +import { expect } from 'chai'; +import { Replacement } from 'tslint'; +import { Rule } from '../src/templateBananaInBoxRule'; +import { assertAnnotated, assertMultipleAnnotated, assertSuccess } from './testHelper'; + +const { + FAILURE_STRING, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail if the brackets are inside the parentheses', () => { + const source = ` + @Component({ + template: \` + + ~~~~~~~~~~~~~~~~~ + \` + }) + class Test {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail if there are multiple brackets inside the parentheses', () => { + const source = ` + @Component({ + template: \` + + ~~~~~~~~~~~~~ ^^^^^^^^^^^^^^^ + +
+ %%%%%%%%%%%%%%%%%%% + \` + }) + class Test {} + `; + assertMultipleAnnotated({ + failures: [ + { + char: '~', + msg: FAILURE_STRING + }, + { + char: '^', + msg: FAILURE_STRING + }, + { + char: '%', + msg: FAILURE_STRING + } + ], + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed if one-way binding is used', () => { + const source = ` + @Component({ + template: \` + + \` + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if the parentheses are inside the brackets', () => { + const source = ` + @Component({ + template: \` + + \` + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if the brackets are inside the parentheses in an output event', () => { + const source = ` + @Component({ + template: \` + + \` + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + }); + + describe('replacements', () => { + it('should fail if the box is in the banana', () => { + const source = ` + @Component({ + template: \` + + ~~~~~~~~~~~~~~~~~ + \` + }) + class Test {} + `; + const failures = assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + + if (!Array.isArray(failures)) return; + + const replacement = Replacement.applyFixes(source, failures.map(f => f.getFix()!)); + + expect(replacement).to.eq(` + @Component({ + template: \` + + ~~~~~~~~~~~~~~~~~ + \` + }) + class Test {} + `); + }); + + it('should fail if there are multiple brackets inside the parentheses', () => { + const source = ` + @Component({ + template: \` + + ~~~~~~~~~~~~~ ^^^^^^^^^^^^^^^ + +
+ %%%%%%%%%%%%%%%%%%% + \` + }) + class Test {} + `; + const failures = assertMultipleAnnotated({ + failures: [ + { + char: '~', + msg: FAILURE_STRING + }, + { + char: '^', + msg: FAILURE_STRING + }, + { + char: '%', + msg: FAILURE_STRING + } + ], + ruleName, + source + }); + const replacement = Replacement.applyFixes(source, failures.map(f => f.getFix()!)); + + expect(replacement).to.eq(` + @Component({ + template: \` + + ~~~~~~~~~~~~~ ^^^^^^^^^^^^^^^ + +
+ %%%%%%%%%%%%%%%%%%% + \` + }) + class Test {} + `); + }); + }); +}); diff --git a/test/templateI18nRule.spec.ts b/test/templateI18nRule.spec.ts new file mode 100644 index 000000000..27403ebe3 --- /dev/null +++ b/test/templateI18nRule.spec.ts @@ -0,0 +1,403 @@ +import { Rule } from '../src/templateI18nRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; +import { assertFailure } from './testHelper'; + +const { + FAILURE_STRING_ATTR, + FAILURE_STRING_TEXT, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + describe('check-id', () => { + it('should fail with missing id string', () => { + const source = ` + @Component({ + template: \` +
Text
+ ~~~~~~~~~~~~ + \` + }) + class Test {} + `; + assertAnnotated({ + message: FAILURE_STRING_ATTR, + options: ['check-id'], + ruleName, + source + }); + }); + + it('should fail with missing id', () => { + const source = ` + @Component({ + template: \` +
Text
+ ~~~~~~~~~~ + \` + }) + class Test {} + `; + assertAnnotated({ + message: FAILURE_STRING_ATTR, + options: ['check-id'], + ruleName, + source + }); + }); + + it('should fail with missing id', () => { + const source = ` + @Component({ + template: \` +
Text
+ ~~~~ + \` + }) + class Test {} + `; + assertAnnotated({ + message: FAILURE_STRING_ATTR, + options: ['check-id'], + ruleName, + source + }); + }); + }); + + describe('check-text', () => { + it('should fail with missing id string', () => { + const source = ` + @Component({ + template: \` +
Text
+ ~~~~ + \` + }) + class Test {} + `; + assertAnnotated({ + message: FAILURE_STRING_TEXT, + options: ['check-text'], + ruleName, + source + }); + }); + + it('should fail with missing id string in nested elements', () => { + const source = ` + @Component({ + template: \` +
+ foo +
Text
+ ~~~~ +
+ \` + }) + class Test {} + `; + assertAnnotated({ + message: FAILURE_STRING_TEXT, + options: ['check-text'], + ruleName, + source + }); + }); + + it('should fail with text outside element with i18n attribute', () => { + const source = ` + @Component({ + template: \` +
Text
+ foo + \` + }) + class Test {} + `; + assertFailure( + ruleName, + source, + { + endPosition: { + character: 12, + line: 5 + }, + message: FAILURE_STRING_TEXT, + startPosition: { + character: 34, + line: 3 + } + }, + ['check-text'] + ); + }); + + it('should fail with missing id string', () => { + const source = ` + @Component({ + template: \` +
Text {{ foo }}
+ ~~~~~~~~~~~~~~ + \` + }) + class Test {} + `; + assertAnnotated({ + message: FAILURE_STRING_TEXT, + options: ['check-text'], + ruleName, + source + }); + }); + + it('should fail with missing id string', () => { + const source = ` + @Component({ + template: \` +
{{ foo }} text
+ ~~~~~~~~~~~~~~ + \` + }) + class Test {} + `; + assertAnnotated({ + message: FAILURE_STRING_TEXT, + options: ['check-text'], + ruleName, + source + }); + }); + + it('should fail with text outside element with i18n attribute', () => { + const source = ` + @Component({ + template: \` +
Text
+ {{ foo }} text + \` + }) + class Test {} + `; + assertFailure( + ruleName, + source, + { + endPosition: { + character: 12, + line: 5 + }, + message: FAILURE_STRING_TEXT, + startPosition: { + character: 34, + line: 3 + } + }, + ['check-text'] + ); + }); + + it('should fail with text outside multiple nested elements', () => { + const source = ` + @Component({ + template: \` +
    +
  • ItemA
    • +
  • ItemB
    • +
  • ItemC
    • +
+ Text + \` + }) + class Test {} + `; + assertFailure( + ruleName, + source, + { + endPosition: { + character: 12, + line: 9 + }, + message: FAILURE_STRING_TEXT, + startPosition: { + character: 19, + line: 7 + } + }, + ['check-text'] + ); + }); + }); + }); + + describe('success', () => { + describe('check-id', () => { + it('should work with proper id', () => { + const source = ` + @Component({ + template: \` +
Text
+ \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-id']); + }); + + it('should work with proper id on ng-container', () => { + const source = ` + @Component({ + template: \` + Text + \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-id']); + }); + + it('should work with proper id', () => { + const source = ` + @Component({ + template: \` +
Text
+ \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-id']); + }); + + it('should work with proper id', () => { + const source = ` + @Component({ + template: \` +
Text
+ \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-id']); + }); + }); + + describe('check-text', () => { + it('should work with i18n attribute', () => { + const source = ` + @Component({ + template: \` +
Text
+ \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-text']); + }); + + it('should work with i18n attribute on ng-container', () => { + const source = ` + @Component({ + template: \` + Text + \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-text']); + }); + + it('should work with proper i18n attribute', () => { + const source = ` + @Component({ + template: \` +
+ Use at least {{ minLength }} characters +
+ \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-text']); + }); + + it('should work with input and text', () => { + const source = ` + @Component({ + template: \` + + \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-text']); + }); + + it('should work with plural', () => { + const source = ` + @Component({ + template: \` + + Updated {minutes, plural, =0 {just now} =1 {one minute ago} other {{ {minutes}} minutes ago }} + + \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-text']); + }); + + it('should work without i18n attribute and interpolation', () => { + const source = ` + @Component({ + template: \` +
{{ text }}
+ \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-text']); + }); + + it('should work with multiple valid elements', () => { + const source = ` + @Component({ + template: \` +
{{ text }}
+
+ Text + foo +
+ \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-text']); + }); + + it('should work with multiple nested elements', () => { + const source = ` + @Component({ + template: \` +
    +
  • ItemA
    • +
  • ItemB
    • +
  • ItemC
    • +
+ \` + }) + class Test {} + `; + assertSuccess(ruleName, source, ['check-text']); + }); + }); + }); +}); diff --git a/test/noTemplateCallExpressionRule.spec.ts b/test/templateNoCallExpressionRule.spec.ts similarity index 56% rename from test/noTemplateCallExpressionRule.spec.ts rename to test/templateNoCallExpressionRule.spec.ts index 05cc9494b..3de5af0fd 100644 --- a/test/noTemplateCallExpressionRule.spec.ts +++ b/test/templateNoCallExpressionRule.spec.ts @@ -1,82 +1,88 @@ -import { assertSuccess, assertAnnotated } from './testHelper'; +import { Rule } from '../src/templateNoCallExpressionRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; -describe('no-template-call-expression', () => { - describe('success', () => { - it('should pass with no call expression', () => { - let source = ` +const { + FAILURE_STRING, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail with call expression in expression binding', () => { + const source = ` @Component({ - template: '{{info}}' + template: '{{ getInfo() }}' + ~~~~~~~~~~ }) class Bar {} `; - assertSuccess('no-template-call-expression', source); + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); }); - it('should pass with call expression in an output handler', () => { - let source = ` + it('should fail with call expression in property binding', () => { + const source = ` @Component({ - template: '' + template: 'info' + ~~~~~~~~ }) class Bar {} `; - assertSuccess('no-template-call-expression', source); + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); }); - it('should allow $any to wrap unknown variables', () => { - let source = ` + it('should fail with a property access call expression', () => { + const source = ` @Component({ - template: '{{$any(info)}}' + template: 'info' + ~~~~~~~~~~~~~~~ }) class Bar {} `; - assertSuccess('no-template-call-expression', source); + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); }); }); - describe('failure', () => { - it('should fail with call expression in expression binding', () => { - let source = ` + describe('success', () => { + it('should pass with no call expression', () => { + const source = ` @Component({ - template: '{{getInfo()}}' - ~~~~~~~~~ + template: '{{ info }}' }) class Bar {} `; - assertAnnotated({ - ruleName: 'no-template-call-expression', - message: 'Call expressions are not allowed in templates except in output handlers', - source - }); + assertSuccess(ruleName, source); }); - it('should fail with call expression in property binding', () => { - let source = ` + it('should pass with call expression in an output handler', () => { + const source = ` @Component({ - template: 'info' - ~~~~~~~~ + template: '' }) class Bar {} `; - assertAnnotated({ - ruleName: 'no-template-call-expression', - message: 'Call expressions are not allowed in templates except in output handlers', - source - }); + assertSuccess(ruleName, source); }); - it('should fail with a property access call expression', () => { - let source = ` + it('should allow $any to wrap unknown variables', () => { + const source = ` @Component({ - template: 'info' - ~~~~~~~~~~~~~~~ + template: '{{ $any(info) }}' }) class Bar {} `; - assertAnnotated({ - ruleName: 'no-template-call-expression', - message: 'Call expressions are not allowed in templates except in output handlers', - source - }); + assertSuccess(ruleName, source); }); }); }); diff --git a/test/templatesNoNegatedAsyncRule.spec.ts b/test/templateNoNegatedAsyncRule.spec.ts similarity index 96% rename from test/templatesNoNegatedAsyncRule.spec.ts rename to test/templateNoNegatedAsyncRule.spec.ts index 8d7770700..e8861936c 100644 --- a/test/templatesNoNegatedAsyncRule.spec.ts +++ b/test/templateNoNegatedAsyncRule.spec.ts @@ -1,5 +1,5 @@ +import { Rule } from '../src/templateNoNegatedAsyncRule'; import { assertAnnotated, assertMultipleAnnotated, assertSuccess } from './testHelper'; -import { Rule } from '../src/templatesNoNegatedAsyncRule'; const { FAILURE_STRING_NEGATED_PIPE, @@ -22,7 +22,7 @@ describe(ruleName, () => { `; assertAnnotated({ message: FAILURE_STRING_NEGATED_PIPE, - ruleName: ruleName, + ruleName, source }); }); @@ -40,7 +40,7 @@ describe(ruleName, () => { `; assertAnnotated({ message: FAILURE_STRING_NEGATED_PIPE, - ruleName: ruleName, + ruleName, source }); }); @@ -58,7 +58,7 @@ describe(ruleName, () => { `; assertAnnotated({ message: FAILURE_STRING_UNSTRICT_EQUALITY, - ruleName: ruleName, + ruleName, source }); }); @@ -76,7 +76,7 @@ describe(ruleName, () => { `; assertAnnotated({ message: FAILURE_STRING_NEGATED_PIPE, - ruleName: ruleName, + ruleName, source }); }); diff --git a/test/trackByFunctionRule.spec.ts b/test/templateUseTrackByFunctionRule.spec.ts similarity index 84% rename from test/trackByFunctionRule.spec.ts rename to test/templateUseTrackByFunctionRule.spec.ts index 3f59f55fc..37917812b 100644 --- a/test/trackByFunctionRule.spec.ts +++ b/test/templateUseTrackByFunctionRule.spec.ts @@ -1,7 +1,8 @@ -import { getFailureMessage, Rule } from '../src/trackByFunctionRule'; +import { Rule } from '../src/templateUseTrackByFunctionRule'; import { assertAnnotated, assertMultipleAnnotated, assertSuccess } from './testHelper'; const { + FAILURE_STRING, metadata: { ruleName } } = Rule; @@ -21,7 +22,7 @@ describe(ruleName, () => { }) class Bar {} `; - assertAnnotated({ message: getFailureMessage(), ruleName, source }); + assertAnnotated({ message: FAILURE_STRING, ruleName, source }); }); it('should fail when trackBy is missing colon', () => { @@ -36,7 +37,7 @@ describe(ruleName, () => { }) class Bar {} `; - assertAnnotated({ message: getFailureMessage(), ruleName, source }); + assertAnnotated({ message: FAILURE_STRING, ruleName, source }); }); it('should fail when [ngForTrackBy] is missing in ng-template', () => { @@ -51,27 +52,27 @@ describe(ruleName, () => { }) class Bar {} `; - assertAnnotated({ message: getFailureMessage(), ruleName, source }); + assertAnnotated({ message: FAILURE_STRING, ruleName, source }); }); - it('should fail when we have two ngFor and the second trackBy function is not present', () => { + it('should fail when there are two ngFor and for the second, trackBy function is not present', () => { const source = ` - @Component({ + @Component({ template: \` -
- {{ item }} -
-
    -
  • - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - {{ item }} -
    • -
- \` - }) - class Bar {} - `; - assertAnnotated({ message: getFailureMessage(), ruleName, source }); +
+ {{ item }} +
+
    +
  • + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + {{ item }} +
    • +
+ \` + }) + class Bar {} + `; + assertAnnotated({ message: FAILURE_STRING, ruleName, source }); }); it('should fail when trackBy function is missing in multiple *ngFor', () => { @@ -95,11 +96,11 @@ describe(ruleName, () => { failures: [ { char: '~', - msg: getFailureMessage() + msg: FAILURE_STRING }, { char: '^', - msg: getFailureMessage() + msg: FAILURE_STRING } ], ruleName, diff --git a/test/useComponentSelectorRule.spec.ts b/test/useComponentSelectorRule.spec.ts new file mode 100644 index 000000000..48a419bac --- /dev/null +++ b/test/useComponentSelectorRule.spec.ts @@ -0,0 +1,95 @@ +import { getFailureMessage, Rule } from '../src/useComponentSelectorRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail when selector is not given in @Component', () => { + const source = ` + @Component() + ~~~~~~~~~~~~ + class Test {} + `; + const message = getFailureMessage({ + className: 'Test' + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail when selector is empty in @Component', () => { + const source = ` + @Component({ + ~~~~~~~~~~~~ + selector: '' + }) + ~~ + class Test {} + `; + const message = getFailureMessage({ + className: 'Test' + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail when selector equals 0 in @Component', () => { + const source = ` + @Component({ + ~~~~~~~~~~~~ + selector: 0 + }) + ~~ + class Test {} + `; + const message = getFailureMessage({ + className: 'Test' + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail when selector equals null in @Component', () => { + const source = ` + @Component({ + ~~~~~~~~~~~~ + selector: null + }) + ~~ + class Test {} + `; + const message = getFailureMessage({ + className: 'Test' + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed when selector is given in @Component', () => { + const source = ` + @Component({ + selector: 'sg-bar-foo' + }) + class Test {} + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/useComponentViewEncapsulationRule.spec.ts b/test/useComponentViewEncapsulationRule.spec.ts new file mode 100644 index 000000000..a4e06a66e --- /dev/null +++ b/test/useComponentViewEncapsulationRule.spec.ts @@ -0,0 +1,71 @@ +import { Rule } from '../src/useComponentViewEncapsulationRule'; +import { assertAnnotated, assertSuccess } from './testHelper'; + +const { + FAILURE_STRING, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail if ViewEncapsulation.None is set', () => { + const source = ` + @Component({ + encapsulation: ViewEncapsulation.None, + ~~~~~~~~~~~~~~~~~~~~~~ + selector: 'sg-foo-bar', + }) + export class Test {} + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed if ViewEncapsulation.Native is set', () => { + const source = ` + @Component({ + encapsulation: ViewEncapsulation.Native, + selector: 'sg-foo-bar' + }) + export class Test {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if ViewEncapsulation.Emulated is set', () => { + const source = ` + @Component({ + encapsulation: ViewEncapsulation.Emulated, + selector: 'sg-foo-bar' + }) + export class Test {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if no ViewEncapsulation is explicitly set', () => { + const source = ` + @Component({ + selector: 'sg-foo-bar' + }) + export class Test {} + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if no component is defined', () => { + const source = ` + @NgModule({ + bootstrap: [Foo] + }) + export class Test {} + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/useHostPropertyDecoratorRule.spec.ts b/test/useHostPropertyDecoratorRule.spec.ts deleted file mode 100644 index f723a0892..000000000 --- a/test/useHostPropertyDecoratorRule.spec.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { assertSuccess, assertAnnotated } from './testHelper'; - -describe('use-host-property-decorator', () => { - it('should fail when "host" is used in @Component', () => { - let source = ` - @Component({ - host: { - ~~~~~~~ - '(click)': 'bar()' - } - ~ - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'use-host-property-decorator', - message: 'Use @HostBindings and @HostListeners instead of the host property (https://angular.io/styleguide#style-06-03)', - source - }); - }); - - it('should succeed when "host" is not used', () => { - let source = ` - @Component({ - selector: 'baz' - }) - class Bar {} - `; - assertSuccess('use-host-property-decorator', source); - }); - - it('should fail when "host" is used in @Directive', () => { - let source = ` - @Directive({ - host: { - ~~~~~~~ - '(click)': 'bar()' - } - ~ - }) - class Baz {} - `; - assertAnnotated({ - ruleName: 'use-host-property-decorator', - message: 'Use @HostBindings and @HostListeners instead of the host property (https://angular.io/styleguide#style-06-03)', - source - }); - }); -}); diff --git a/test/useInputPropertyDecoratorRule.spec.ts b/test/useInputPropertyDecoratorRule.spec.ts deleted file mode 100644 index 21a1e9cb1..000000000 --- a/test/useInputPropertyDecoratorRule.spec.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { assertSuccess, assertAnnotated } from './testHelper'; - -describe('use-input-property-decorator', () => { - it('should fail when "inputs" is used in @Component', () => { - let source = ` - @Component({ - inputs: [ - ~~~~~~~~~ - 'id: foo' - ] - ~ - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'use-input-property-decorator', - message: 'Use the @Input property decorator instead of the inputs property (https://angular.io/styleguide#style-05-12)', - source - }); - }); - - it('should succeed when "inputs" is not used', () => { - let source = ` - @Component({ - selector: 'baz' - }) - class Bar {} - `; - assertSuccess('use-input-property-decorator', source); - }); - - it('should fail when "inputs" is used in @Directive', () => { - let source = ` - @Directive({ - inputs: [ - ~~~~~~~~~ - 'id: foo' - ] - ~ - }) - class Baz {} - `; - assertAnnotated({ - ruleName: 'use-input-property-decorator', - message: 'Use the @Input property decorator instead of the inputs property (https://angular.io/styleguide#style-05-12)', - source - }); - }); -}); diff --git a/test/useLifeCycleInterfaceRule.spec.ts b/test/useLifeCycleInterfaceRule.spec.ts deleted file mode 100644 index fb6d923a3..000000000 --- a/test/useLifeCycleInterfaceRule.spec.ts +++ /dev/null @@ -1,205 +0,0 @@ -import { sprintf } from 'sprintf-js'; -import { Rule } from '../src/useLifeCycleInterfaceRule'; -import { assertAnnotated, assertFailures, assertSuccess } from './testHelper'; - -const { - FAILURE_STRING, - metadata: { ruleName } -} = Rule; -const className = 'Test'; - -const getErrorMessage = (interfaceName: string, methodName): string => { - return sprintf(FAILURE_STRING, interfaceName, methodName, className); -}; - -describe(ruleName, () => { - describe('invalid declaration of life hook', () => { - it("should fail, when a life cycle hook is used without implementing it's interface", () => { - let source = ` - class ${className} { - ngOnInit() { - ~~~~~~~~ - } - } - `; - assertAnnotated({ - message: getErrorMessage('OnInit', 'ngOnInit'), - ruleName, - source - }); - }); - - it('should fail, when life cycle hooks are used without implementing their interfaces', () => { - let source = ` - class ${className} { - ngOnInit() { - } - ngOnDestroy() { - } - } - `; - assertFailures(ruleName, source, [ - { - endPosition: { - character: 18, - line: 2 - }, - message: getErrorMessage('OnInit', 'ngOnInit'), - startPosition: { - character: 10, - line: 2 - } - }, - { - endPosition: { - character: 21, - line: 4 - }, - message: getErrorMessage('OnDestroy', 'ngOnDestroy'), - startPosition: { - character: 10, - line: 4 - } - } - ]); - }); - - it('should fail, when some of the life cycle hooks are used without implementing their interfaces', () => { - let source = ` - class ${className} extends Component implements OnInit { - ngOnInit() { - } - ngOnDestroy() { - ~~~~~~~~~~~ - } - } - `; - assertAnnotated({ - ruleName, - message: getErrorMessage('OnDestroy', 'ngOnDestroy'), - source - }); - }); - }); - - describe('invalid declaration of life hooks, using ng.hookName', () => { - it('should fail, when life cycle hooks are used without implementing all interfaces, using ng.hookName', () => { - let source = ` - class ${className} extends Component implements ng.OnInit { - ngOnInit() { - } - ngOnDestroy() { - ~~~~~~~~~~~ - } - } - `; - assertAnnotated({ - ruleName, - message: getErrorMessage('OnDestroy', 'ngOnDestroy'), - source - }); - }); - }); - - describe('valid declaration of life hook', () => { - it("should succeed, when life cycle hook is used with it's corresponding interface", () => { - let source = ` - class ${className} implements OnInit { - ngOnInit() { - } - } - `; - assertSuccess(ruleName, source); - }); - - it('should succeed, when life cycle hooks are used with their corresponding interfaces', () => { - let source = ` - class ${className} extends Component implements OnInit, OnDestroy { - ngOnInit() { - } - - private ngOnChanges: string=""; - - ngOnDestroy() { - } - - ngOnSmth { - } - } - `; - assertSuccess(ruleName, source); - }); - }); - - describe('valid declaration of life hooks, using ng.hookName', () => { - it("should succeed, when life cycle hook is used with it's interface", () => { - let source = ` - class ${className} implements ng.OnInit { - ngOnInit() { - } - } - `; - assertSuccess(ruleName, source); - }); - - it('should succeed when life cycle hook is implemented by using any prefix', () => { - let source = ` - class ${className} implements bar.OnInit { - ngOnInit() { - } - } - `; - assertSuccess(ruleName, source); - }); - - it('should succeed when life cycle hook is implemented by using nested interfaces', () => { - let source = ` - class ${className} implements bar.foo.baz.OnInit { - ngOnInit() { - } - } - `; - assertSuccess(ruleName, source); - }); - - it('should succeed, when life cycle hooks are used with their corresponding interfaces', () => { - let source = ` - class ${className} extends Component implements ng.OnInit, ng.OnDestroy { - ngOnInit() { - } - - private ngOnChanges: string=""; - - ngOnDestroy() { - } - - ngOnSmth { - } - } - `; - assertSuccess(ruleName, source); - }); - }); - - describe('valid use of class without interfaces and life cycle hooks', () => { - it('should succeed when life cycle hooks are not used', () => { - let source = ` - class ${className} {} - `; - assertSuccess(ruleName, source); - }); - }); - - describe('valid declaration of class using Iterator', () => { - it('should succeed, when is used iterator', () => { - let source = ` - export class Heroes implements Iterable { - [Symbol.iterator]() { - return this.currentHeroes.values(); - } - } - `; - assertSuccess(ruleName, source); - }); - }); -}); diff --git a/test/useLifecycleInterfaceRule.spec.ts b/test/useLifecycleInterfaceRule.spec.ts new file mode 100644 index 000000000..57a0d1edd --- /dev/null +++ b/test/useLifecycleInterfaceRule.spec.ts @@ -0,0 +1,165 @@ +import { getFailureMessage, Rule } from '../src/useLifecycleInterfaceRule'; +import { LifecycleInterfaces, LifecycleMethods } from '../src/util/utils'; +import { assertAnnotated, assertFailures, assertSuccess } from './testHelper'; + +const { + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail if lifecycle method is declared without implementing its interface', () => { + const source = ` + class Test { + ngOnInit() { + ~~~~~~~~ + } + } + `; + const message = getFailureMessage({ + className: 'Test', + interfaceName: LifecycleInterfaces.OnInit, + methodName: LifecycleMethods.ngOnInit + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if one of the lifecycle methods is declared without implementing its interface', () => { + const source = ` + class Test extends Component implements OnInit { + ngOnInit() {} + + ngOnDestroy() { + ~~~~~~~~~~~ + } + } + `; + const message = getFailureMessage({ + className: 'Test', + interfaceName: LifecycleInterfaces.OnDestroy, + methodName: LifecycleMethods.ngOnDestroy + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + + it('should fail if lifecycle methods are declared without implementing their interfaces', () => { + const source = ` + class Test { + ngOnInit() {} + + ngOnDestroy() {} + } + `; + assertFailures(ruleName, source, [ + { + endPosition: { + character: 18, + line: 2 + }, + message: getFailureMessage({ + className: 'Test', + interfaceName: LifecycleInterfaces.OnInit, + methodName: LifecycleMethods.ngOnInit + }), + startPosition: { + character: 10, + line: 2 + } + }, + { + endPosition: { + character: 21, + line: 4 + }, + message: getFailureMessage({ + className: 'Test', + interfaceName: LifecycleInterfaces.OnDestroy, + methodName: LifecycleMethods.ngOnDestroy + }), + startPosition: { + character: 10, + line: 4 + } + } + ]); + }); + + it('should fail if lifecycle methods are declared without implementing their interfaces, using namespace', () => { + const source = ` + class Test extends Component implements ng.OnInit { + ngOnInit() {} + + ngOnDestroy() { + ~~~~~~~~~~~ + } + } + `; + const message = getFailureMessage({ + className: 'Test', + interfaceName: LifecycleInterfaces.OnDestroy, + methodName: LifecycleMethods.ngOnDestroy + }); + assertAnnotated({ + message, + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed if lifecycle method is declared implementing its interface', () => { + const source = ` + class Test implements OnInit { + ngOnInit() {} + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if lifecycle methods are declared implementing their interfaces', () => { + const source = ` + class Test extends Component implements OnInit, OnDestroy { + ngOnInit() {} + + private ngOnChanges = ''; + + ngOnDestroy() {} + + ngOnSmth {} + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if lifecycle methods are declared implementing their interfaces, using namespaces', () => { + const source = ` + class Test extends Component implements ng.OnInit, ng.OnDestroy { + ngOnInit() {} + + private ngOnChanges = ''; + + ngOnDestroy() {} + + ngOnSmth {} + } + `; + assertSuccess(ruleName, source); + }); + + it('should succeed if no lifecycle methods are declared', () => { + const source = ` + class Test {} + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/useOutputPropertyDecoratorRule.spec.ts b/test/useOutputPropertyDecoratorRule.spec.ts deleted file mode 100644 index d0c4c91ae..000000000 --- a/test/useOutputPropertyDecoratorRule.spec.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { assertSuccess, assertAnnotated } from './testHelper'; - -describe('use-output-property-decorator', () => { - it('should fail when "outputs" is used in @Component', () => { - let source = ` - @Component({ - outputs: [ - ~~~~~~~~~~ - 'id: foo' - ] - ~ - }) - class Bar {} - `; - assertAnnotated({ - ruleName: 'use-output-property-decorator', - message: 'Use the @Output property decorator instead of the outputs property (https://angular.io/styleguide#style-05-12)', - source - }); - }); - - it('should succeed when "outputs" is not used', () => { - let source = ` - @Component({ - selector: 'baz' - }) - class Bar {} - `; - assertSuccess('use-output-property-decorator', source); - }); - - it('should fail when "outputs" is used in @Directive', () => { - let source = ` - @Directive({ - outputs: [ - ~~~~~~~~~~ - 'id: foo' - ] - ~ - }) - class Baz {} - `; - assertAnnotated({ - ruleName: 'use-output-property-decorator', - message: 'Use the @Output property decorator instead of the outputs property (https://angular.io/styleguide#style-05-12)', - source - }); - }); -}); diff --git a/test/useViewEncapsulationRule.spec.ts b/test/useViewEncapsulationRule.spec.ts deleted file mode 100644 index 4ddad467d..000000000 --- a/test/useViewEncapsulationRule.spec.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { assertAnnotated, assertSuccess } from './testHelper'; - -describe('use-view-encapsulation', () => { - describe('invalid view encapsulation', () => { - it('should fail if ViewEncapsulation.None is set', () => { - const source = ` - @Component({ - selector: 'sg-foo-bar', - encapsulation: ViewEncapsulation.None - ~~~~~~~~~~~~~~~~~~~~~~ - }) - export class TestComponent {} - `; - assertAnnotated({ - ruleName: 'use-view-encapsulation', - message: 'Using "ViewEncapsulation.None" will make your styles global which may have unintended effect', - source - }); - }); - }); - - describe('valid view encapsulation', () => { - it('should succeed if ViewEncapsulation.Native is set', () => { - const source = ` - @Component({ - selector: 'sg-foo-bar', - encapsulation: ViewEncapsulation.Native - }) - export class TestComponent {} - `; - assertSuccess('use-view-encapsulation', source); - }); - - it('should succeed if ViewEncapsulation.Emulated is set', () => { - const source = ` - @Component({ - selector: 'sg-foo-bar', - encapsulation: ViewEncapsulation.Emulated - }) - export class TestComponent {} - `; - assertSuccess('use-view-encapsulation', source); - }); - - it('should succeed if no ViewEncapsulation is set explicitly', () => { - const source = ` - @Component({ - selector: 'sg-foo-bar', - }) - export class TestComponent {} - `; - assertSuccess('use-view-encapsulation', source); - }); - - it('should succeed if a class is not a component', () => { - const source = ` - @NgModule({ - bootstrap: [Foo] - }) - export class TestModule {} - `; - assertSuccess('use-view-encapsulation', source); - }); - }); -});