From c346638a15356589b4ecc896c2bb6171bc5d6b4c Mon Sep 17 00:00:00 2001 From: PreskoIsTheGreatest Date: Sun, 19 Mar 2017 19:38:38 +0200 Subject: [PATCH] add usePipeDecoratorRule, refactored linting erros, updated to rc5, refactored linting rc5 errors --- package.json | 4 +- src/angular/styles/chars.ts | 2 +- src/angular/styles/parseUtil.ts | 8 +-- .../templates/basicTemplateAstVisitor.ts | 2 +- .../recursiveAngularExpressionVisitor.ts | 2 +- .../templates/referenceCollectorVisitor.ts | 2 +- src/angular/templates/templateParser.ts | 5 +- src/componentClassSuffixRule.ts | 8 +-- src/componentSelectorRule.ts | 22 +++---- src/directiveSelectorRule.ts | 22 +++---- src/importDestructuringSpacingRule.ts | 2 +- src/noAccessMissingMemberRule.ts | 2 +- src/noAttributeParameterDecoratorRule.ts | 4 +- src/noUnusedCssRule.ts | 2 +- src/pipeImpureRule.ts | 2 +- src/pipeNamingRule.ts | 10 +-- src/templatesUsePublicRule.ts | 2 +- src/useLifeCycleInterfaceRule.ts | 16 ++--- src/usePipeDecoratorRule.ts | 66 +++++++++++++++++++ src/usePipeTransformInterfaceRule.ts | 4 +- test/testHelper.ts | 2 +- test/usePipeDecoratorRule.spec.ts | 64 ++++++++++++++++++ 22 files changed, 192 insertions(+), 61 deletions(-) create mode 100644 src/usePipeDecoratorRule.ts create mode 100644 test/usePipeDecoratorRule.spec.ts diff --git a/package.json b/package.json index 6ba39fc42..58ddd8f7a 100644 --- a/package.json +++ b/package.json @@ -44,8 +44,8 @@ }, "homepage": "https://github.com/mgechev/codelyzer#readme", "devDependencies": { - "@angular/compiler": "^4.0.0-rc.2", - "@angular/core": "^4.0.0-rc.2", + "@angular/compiler": "^4.0.0-rc.5", + "@angular/core": "^4.0.0-rc.5", "@types/chai": "^3.4.33", "@types/less": "0.0.31", "@types/mocha": "^2.2.32", diff --git a/src/angular/styles/chars.ts b/src/angular/styles/chars.ts index 4e510a2d5..83c975867 100644 --- a/src/angular/styles/chars.ts +++ b/src/angular/styles/chars.ts @@ -73,7 +73,7 @@ export const $AT = 64; export const $BT = 96; export function isWhitespace(code: number): boolean { - return (code >= $TAB && code <= $SPACE) || (code == $NBSP); + return (code >= $TAB && code <= $SPACE) || (code === $NBSP); } export function isDigit(code: number): boolean { diff --git a/src/angular/styles/parseUtil.ts b/src/angular/styles/parseUtil.ts index fe9086ce4..8bfb5a822 100644 --- a/src/angular/styles/parseUtil.ts +++ b/src/angular/styles/parseUtil.ts @@ -47,8 +47,8 @@ export abstract class ParseError { while (ctxLen < 100 && ctxStart > 0) { ctxStart--; ctxLen++; - if (source[ctxStart] == '\n') { - if (++ctxLines == 3) { + if (source[ctxStart] === '\n') { + if (++ctxLines === 3) { break; } } @@ -59,8 +59,8 @@ export abstract class ParseError { while (ctxLen < 100 && ctxEnd < source.length - 1) { ctxEnd++; ctxLen++; - if (source[ctxEnd] == '\n') { - if (++ctxLines == 3) { + if (source[ctxEnd] === '\n') { + if (++ctxLines === 3) { break; } } diff --git a/src/angular/templates/basicTemplateAstVisitor.ts b/src/angular/templates/basicTemplateAstVisitor.ts index dd9bdb1bd..d5e475d7f 100644 --- a/src/angular/templates/basicTemplateAstVisitor.ts +++ b/src/angular/templates/basicTemplateAstVisitor.ts @@ -1,7 +1,7 @@ import * as ast from '@angular/compiler'; import * as ts from 'typescript'; import * as Lint from 'tslint'; -import * as e from '@angular/compiler/typings/src/expression_parser/ast'; +import * as e from '@angular/compiler/src/expression_parser/ast'; import { ExpTypes } from '../expressionTypes'; import { ComponentMetadata } from '../metadata'; diff --git a/src/angular/templates/recursiveAngularExpressionVisitor.ts b/src/angular/templates/recursiveAngularExpressionVisitor.ts index 6e7d195d3..702eea9de 100644 --- a/src/angular/templates/recursiveAngularExpressionVisitor.ts +++ b/src/angular/templates/recursiveAngularExpressionVisitor.ts @@ -1,6 +1,6 @@ import * as Lint from 'tslint'; import * as ts from 'typescript'; -import * as e from '@angular/compiler/typings/src/expression_parser/ast'; +import * as e from '@angular/compiler/src/expression_parser/ast'; import {SourceMappingVisitor} from '../sourceMappingVisitor'; import {ComponentMetadata} from '../metadata'; diff --git a/src/angular/templates/referenceCollectorVisitor.ts b/src/angular/templates/referenceCollectorVisitor.ts index dcd64d8fc..7507310e9 100644 --- a/src/angular/templates/referenceCollectorVisitor.ts +++ b/src/angular/templates/referenceCollectorVisitor.ts @@ -39,7 +39,7 @@ export class ReferenceCollectorVisitor implements ast.TemplateAstVisitor { element.children.forEach(e => this.visit(e, context)); this._variables = this._variables.concat(references); } - + get variables() { return this._variables; } diff --git a/src/angular/templates/templateParser.ts b/src/angular/templates/templateParser.ts index b05884995..a0083d890 100644 --- a/src/angular/templates/templateParser.ts +++ b/src/angular/templates/templateParser.ts @@ -2,8 +2,8 @@ import * as compiler from '@angular/compiler'; import { Config, DirectiveDeclaration } from '../config'; import { SemVerDSL } from '../../util/ngVersion'; -import { NO_ERRORS_SCHEMA } from "@angular/core"; -import { ɵConsole } from "@angular/core"; +import { NO_ERRORS_SCHEMA } from '@angular/core'; +import { ɵConsole } from '@angular/core'; let refId = 0; const dummyMetadataFactory = (declaration: DirectiveDeclaration) => { @@ -72,6 +72,7 @@ export const parseTemplate = (template: string, directives: DirectiveDeclaration const templateMetadata: compiler.CompileTemplateMetadata = { encapsulation: 0, template: template, + isInline: false, templateUrl: '', styles: [], styleUrls: [], diff --git a/src/componentClassSuffixRule.ts b/src/componentClassSuffixRule.ts index 0c4666649..d891be957 100644 --- a/src/componentClassSuffixRule.ts +++ b/src/componentClassSuffixRule.ts @@ -32,10 +32,6 @@ export class Rule extends Lint.Rules.AbstractRule { static FAILURE: string = 'The name of the class %s should end with the suffix %s ($$02-03$$)'; - static validate(className: string, suffixList: string[]): boolean { - return suffixList.some(suffix => className.endsWith(suffix)); - } - static walkerBuilder: F2 = all( validateComponent((meta: ComponentMetadata, suffixList?: string[]) => @@ -50,6 +46,10 @@ export class Rule extends Lint.Rules.AbstractRule { }) )); + static validate(className: string, suffixList: string[]): boolean { + return suffixList.some(suffix => className.endsWith(suffix)); + } + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { return this.applyWithWalker( Rule.walkerBuilder(sourceFile, this.getOptions()) diff --git a/src/componentSelectorRule.ts b/src/componentSelectorRule.ts index ead42b59f..0ff6a56c9 100644 --- a/src/componentSelectorRule.ts +++ b/src/componentSelectorRule.ts @@ -19,30 +19,30 @@ export class Rule extends SelectorRule { * It is easier to recognize that a symbol is a component by looking at the template's HTML. `, options: { - "type": "array", - "items": [ + 'type': 'array', + 'items': [ { - "enum": ["element", "attribute"] + 'enum': ['element', 'attribute'] }, { - "oneOf": [ + 'oneOf': [ { - "type": "array", - "items": { - "type": "string" + 'type': 'array', + 'items': { + 'type': 'string' } }, { - "type": "string" + 'type': 'string' } ] }, { - "enum": ["kebab-case", "camelCase"] + 'enum': ['kebab-case', 'camelCase'] } ], - "minItems": 3, - "maxItems": 3 + 'minItems': 3, + 'maxItems': 3 }, optionExamples: [ `["element", "my-prefix", "kebab-case"]`, diff --git a/src/directiveSelectorRule.ts b/src/directiveSelectorRule.ts index f47d48c61..cc3735ea7 100644 --- a/src/directiveSelectorRule.ts +++ b/src/directiveSelectorRule.ts @@ -16,30 +16,30 @@ export class Rule extends SelectorRule { * It is easier to recognize that a symbol is a directive by looking at the template's HTML. `, options: { - "type": "array", - "items": [ + 'type': 'array', + 'items': [ { - "enum": ["element", "attribute"] + 'enum': ['element', 'attribute'] }, { - "oneOf": [ + 'oneOf': [ { - "type": "array", - "items": { - "type": "string" + 'type': 'array', + 'items': { + 'type': 'string' } }, { - "type": "string" + 'type': 'string' } ] }, { - "enum": ["kebab-case", "camelCase"] + 'enum': ['kebab-case', 'camelCase'] } ], - "minItems": 3, - "maxItems": 3 + 'minItems': 3, + 'maxItems': 3 }, optionExamples: [ `["element", "my-prefix", "kebab-case"]`, diff --git a/src/importDestructuringSpacingRule.ts b/src/importDestructuringSpacingRule.ts index 3c68326c3..51ffc3034 100644 --- a/src/importDestructuringSpacingRule.ts +++ b/src/importDestructuringSpacingRule.ts @@ -30,7 +30,7 @@ class ImportDestructuringSpacingWalker extends Lint.SkippableTokenAwareRuleWalke public visitImportDeclaration(node: ts.ImportDeclaration) { const importClause = node.importClause; - if (importClause != null && importClause.namedBindings != null) { + if (importClause !== null && importClause.namedBindings !== null) { const text = importClause.namedBindings.getText(); if (!this.checkForWhiteSpace(text)) { diff --git a/src/noAccessMissingMemberRule.ts b/src/noAccessMissingMemberRule.ts index 163275042..c245318f4 100644 --- a/src/noAccessMissingMemberRule.ts +++ b/src/noAccessMissingMemberRule.ts @@ -6,7 +6,7 @@ import {Ng2Walker} from './angular/ng2Walker'; import {RecursiveAngularExpressionVisitor} from './angular/templates/recursiveAngularExpressionVisitor'; import {ExpTypes} from './angular/expressionTypes'; import {getDeclaredMethodNames, getDeclaredPropertyNames} from './util/classDeclarationUtils'; -import * as e from '@angular/compiler/typings/src/expression_parser/ast'; +import * as e from '@angular/compiler/src/expression_parser/ast'; import {Config} from './angular/config'; diff --git a/src/noAttributeParameterDecoratorRule.ts b/src/noAttributeParameterDecoratorRule.ts index 41f0f5e30..5ab1ae6a3 100644 --- a/src/noAttributeParameterDecoratorRule.ts +++ b/src/noAttributeParameterDecoratorRule.ts @@ -46,10 +46,10 @@ export class Rule extends Lint.Rules.AbstractRule { // We only care about 1 since we highlight the whole 'parameter' return decoratorsFailed.fmap(() => new Failure(p, sprintf(Rule.FAILURE_STRING, - parentName, (p.name).text, (p.name).text))) + parentName, (p.name).text, (p.name).text))); }) ); - return listToMaybe(failures) + return listToMaybe(failures); }); }) ); diff --git a/src/noUnusedCssRule.ts b/src/noUnusedCssRule.ts index ccaa9d2ae..64a1eeae7 100644 --- a/src/noUnusedCssRule.ts +++ b/src/noUnusedCssRule.ts @@ -133,7 +133,7 @@ class ElementFilterVisitor extends BasicTemplateAstVisitor { return !selectorTypes[s] || !strategy(ast); }) && (ast.children || []) .every(c => ast instanceof ElementAst && this.shouldVisit(c, strategies, selectorTypes) - || ast instanceof EmbeddedTemplateAst && + || ast instanceof EmbeddedTemplateAst && (ast.children || []).every(c => this.shouldVisit(c, strategies, selectorTypes))); } } diff --git a/src/pipeImpureRule.ts b/src/pipeImpureRule.ts index 8b94ac0b7..a18762080 100644 --- a/src/pipeImpureRule.ts +++ b/src/pipeImpureRule.ts @@ -38,7 +38,7 @@ export class ClassMetadataWalker extends Ng2Walker { let argument = this.extractArgument(pipe); if (argument.kind === SyntaxKind.current().ObjectLiteralExpression) { argument.properties.filter(n => n.name.text === 'pure') - .forEach(this.validateProperty.bind(this, className)) + .forEach(this.validateProperty.bind(this, className)); } } diff --git a/src/pipeNamingRule.ts b/src/pipeNamingRule.ts index 64378ca65..be34fbd94 100644 --- a/src/pipeNamingRule.ts +++ b/src/pipeNamingRule.ts @@ -12,12 +12,12 @@ export class Rule extends Lint.Rules.AbstractRule { description: `Enforce consistent case and prefix for pipes.`, rationale: `Consistent conventions make it easy to quickly identify and reference assets of different types.`, options: { - "type": "array", - "items": [ - {"enum": ["kebab-case", "attribute"]}, - {"type": "string"} + 'type': 'array', + 'items': [ + {'enum': ['kebab-case', 'attribute']}, + {'type': 'string'} ], - "minItems": 1 + 'minItems': 1 }, optionExamples: [ `["camelCase", "myPrefix"]`, diff --git a/src/templatesUsePublicRule.ts b/src/templatesUsePublicRule.ts index 3a211f2dc..593203671 100644 --- a/src/templatesUsePublicRule.ts +++ b/src/templatesUsePublicRule.ts @@ -4,7 +4,7 @@ import {stringDistance} from './util/utils'; import {getDeclaredProperties, getDeclaredMethods} from './util/classDeclarationUtils'; import {Ng2Walker} from './angular/ng2Walker'; import {RecursiveAngularExpressionVisitor} from './angular/templates/recursiveAngularExpressionVisitor'; -import * as e from '@angular/compiler/typings/src/expression_parser/ast'; +import * as e from '@angular/compiler/src/expression_parser/ast'; import SyntaxKind = require('./util/syntaxKind'); enum DeclarationType { diff --git a/src/useLifeCycleInterfaceRule.ts b/src/useLifeCycleInterfaceRule.ts index 2dfffe7f5..0d02accaf 100644 --- a/src/useLifeCycleInterfaceRule.ts +++ b/src/useLifeCycleInterfaceRule.ts @@ -55,7 +55,7 @@ export class ClassMetadataWalker extends Lint.RuleWalker { super.visitClassDeclaration(node); } - private extractInterfaces(node:ts.ClassDeclaration,syntaxKind:SyntaxKind.SyntaxKind):string[]{ + private extractInterfaces(node:ts.ClassDeclaration,syntaxKind:SyntaxKind.SyntaxKind): string[] { let interfaces:string[] = []; if (node.heritageClauses) { let interfacesClause = node.heritageClauses.filter(h=>h.token === syntaxKind.ImplementsKeyword); @@ -66,26 +66,26 @@ export class ClassMetadataWalker extends Lint.RuleWalker { return interfaces; } - private validateMethods( methods:any[],interfaces:string[],className:string){ - methods.forEach(m => { + private validateMethods( methods: any[], interfaces: string[], className: string) { + methods.forEach( m => { let n = (m.name).text; - if(n && this.isMethodValidHook(m,interfaces)){ + if(n && this.isMethodValidHook(m, interfaces)) { let hookName = n.substr(2, n.lenght); this.addFailure( this.createFailure( m.name.getStart(), m.name.getWidth(), - sprintf.apply(this, [Rule.FAILURE, hookName,Rule.HOOKS_PREFIX + hookName, className]))); + sprintf.apply(this, [Rule.FAILURE, hookName, Rule.HOOKS_PREFIX + hookName, className]))); } }); } - private isMethodValidHook(m:any,interfaces:string[]):boolean{ + private isMethodValidHook(m:any,interfaces:string[]): boolean { let n = (m.name).text; - let isNg:boolean = n.substr(0, 2) === Rule.HOOKS_PREFIX; + let isNg: boolean = n.substr(0, 2) === Rule.HOOKS_PREFIX; let hookName = n.substr(2, n.lenght); let isHook = Rule.LIFE_CYCLE_HOOKS_NAMES.indexOf(hookName) !== -1; - let isNotIn:boolean = interfaces.indexOf(hookName) === -1; + let isNotIn: boolean = interfaces.indexOf(hookName) === -1; return isNg && isHook && isNotIn; } diff --git a/src/usePipeDecoratorRule.ts b/src/usePipeDecoratorRule.ts new file mode 100644 index 000000000..c7c99ecbb --- /dev/null +++ b/src/usePipeDecoratorRule.ts @@ -0,0 +1,66 @@ +import * as Lint from 'tslint'; +import * as ts from 'typescript'; +import {sprintf} from 'sprintf-js'; +import SyntaxKind = require('./util/syntaxKind'); + +const getInterfaceName = (t: any) => { + if (t.expression && t.expression.name) { + return t.expression.name.text; + } + return t.expression.text; +}; + +export class Rule extends Lint.Rules.AbstractRule { + public static metadata: Lint.IRuleMetadata = { + ruleName: 'use-pipe-decorator', + type: 'maintainability', + description: `Ensure that classes implementing PipeTransform interface, use Pipe decorator`, + rationale: `Interfaces prescribe typed method signatures. Use those signatures to flag spelling and syntax mistakes.`, + options: null, + optionsDescription: `Not configurable.`, + typescriptOnly: true, + }; + + + static FAILURE: string = 'The %s class implements the PipeTransform interface, so it should use the @Pipe decorator'; + static PIPE_INTERFACE_NAME = 'PipeTransform'; + + public apply(sourceFile:ts.SourceFile):Lint.RuleFailure[] { + return this.applyWithWalker( + new ClassMetadataWalker(sourceFile, + this.getOptions())); + } +} + +export class ClassMetadataWalker extends Lint.RuleWalker { + + visitClassDeclaration(node:ts.ClassDeclaration) { + if (this.hasIPipeTransform(node)) { + let decorators = node.decorators || []; + let className:string = node.name.text; + let pipes:Array = decorators.map(d => + (d.expression).text || + ((d.expression).expression || {}).text).filter( t=> t === 'Pipe'); + if (pipes.length === 0) { + this.addFailure( + this.createFailure( + node.getStart(), + node.getWidth(), + sprintf.apply(this, [Rule.FAILURE, className]))); + } + } + super.visitClassDeclaration(node); + } + + private hasIPipeTransform(node: ts.ClassDeclaration): boolean { + let interfaces = []; + if (node.heritageClauses) { + let interfacesClause = node.heritageClauses + .filter(h => h.token === SyntaxKind.current().ImplementsKeyword); + if (interfacesClause.length !== 0) { + interfaces = interfacesClause[0].types.map(getInterfaceName); + } + } + return interfaces.indexOf(Rule.PIPE_INTERFACE_NAME) !== -1; + } +} diff --git a/src/usePipeTransformInterfaceRule.ts b/src/usePipeTransformInterfaceRule.ts index 5b3b09f06..b65925de1 100644 --- a/src/usePipeTransformInterfaceRule.ts +++ b/src/usePipeTransformInterfaceRule.ts @@ -41,7 +41,7 @@ export class ClassMetadataWalker extends Lint.RuleWalker { ((d.expression).expression || {}).text).filter(t=> t === 'Pipe'); if (pipes.length !== 0) { let className:string = node.name.text; - if(!this.hasIPipeTransform(node)){ + if (!this.hasIPipeTransform(node)) { this.addFailure( this.createFailure( node.getStart(), @@ -53,7 +53,7 @@ export class ClassMetadataWalker extends Lint.RuleWalker { super.visitClassDeclaration(node); } - private hasIPipeTransform(node:ts.ClassDeclaration):boolean{ + private hasIPipeTransform(node:ts.ClassDeclaration): boolean { let interfaces = []; if (node.heritageClauses) { let interfacesClause = node.heritageClauses diff --git a/test/testHelper.ts b/test/testHelper.ts index f59328fc1..e2ef68620 100644 --- a/test/testHelper.ts +++ b/test/testHelper.ts @@ -158,7 +158,7 @@ export function assertMultipleAnnotated(configs: AssertMultipleConfigs): void { } else { assertSuccess(configs.ruleName, configs.source, configs.options); } - }) + }); } /** diff --git a/test/usePipeDecoratorRule.spec.ts b/test/usePipeDecoratorRule.spec.ts new file mode 100644 index 000000000..3f631a9d3 --- /dev/null +++ b/test/usePipeDecoratorRule.spec.ts @@ -0,0 +1,64 @@ +import {assertSuccess, assertAnnotated} from './testHelper'; + +describe('use-pipe-decorator', () => { + describe('invalid use of pipe transform interface', () => { + it(`should fail, when PipeTransform interface is used, without @Pipe or any decorators`, () => { + let source = ` + export class NewPipe implements PipeTransform { + ~~~~~~~~~~~~~~~~~~~~ + transform(url:string):any { + } + } + ~`; + assertAnnotated({ + ruleName: 'use-pipe-decorator', + message: 'The NewPipe class implements the PipeTransform interface, so it should use the @Pipe decorator', + source + }); + }); + it(`should fail, when PipeTransform interface is used, without @Pipe decorator, but using other`, () => { + let source = ` + @Test() + ~~~~~~~ + export class Test implements PipeTransform { + } + ~`; + assertAnnotated({ + ruleName: 'use-pipe-decorator', + message: 'The Test class implements the PipeTransform interface, so it should use the @Pipe decorator', + source + }); + }); + it(`should fail, when PipeTransform interface is used, without @Pipe decorator, but using multiple others`, () => { + let source = ` + @Test() + ~~~~~~~ + @Test2() + export class Test implements PipeTransform { + } + ~`; + assertAnnotated({ + ruleName: 'use-pipe-decorator', + message: 'The Test class implements the PipeTransform interface, so it should use the @Pipe decorator', + source + }); + }); + }); + describe('valid use of pipe transform interface', () => { + it(`should succeed, when PipeTransform interface is used, with Pipe decorator`, () => { + let source = ` + @Pipe({name: 'fetch'}) + export class NewPipe implements PipeTransform{ + transform(url:string):any { + } + }`; + assertSuccess('use-pipe-decorator', source); + }); + }); + describe('valid use of empty class', () => { + it(`should succeed, when PipeTransform interface is not used`, () => { + let source = `class App{}`; + assertSuccess('use-pipe-decorator', source); + }); + }); +});