diff --git a/src/useLifeCycleInterfaceRule.ts b/src/useLifeCycleInterfaceRule.ts index 8fb49dbb5..0ff1893b1 100644 --- a/src/useLifeCycleInterfaceRule.ts +++ b/src/useLifeCycleInterfaceRule.ts @@ -4,9 +4,8 @@ import {sprintf} from 'sprintf-js'; import SyntaxKind = require('./util/syntaxKind'); export class Rule extends Lint.Rules.AbstractRule { - static FAILURE_SINGLE:string = 'Implement lifecycle hook interfaces ($$09-01$$)'; - static FAILURE_MANY = 'Implement lifecycle hook interfaces ($$09-01$$)'; + static FAILURE:string = 'Implement lifecycle hook interface %s for method %s in class %s ($$09-01$$)'; static HOOKS_PREFIX = 'ng'; @@ -30,51 +29,54 @@ export class Rule extends Lint.Rules.AbstractRule { export class ClassMetadataWalker extends Lint.RuleWalker { - visitClassDeclaration(node:ts.ClassDeclaration) { - let syntaxKind = SyntaxKind.current(); - let className = node.name.text; - - let interfaces = []; - if (node.heritageClauses) { - let interfacesClause = node.heritageClauses.filter(h=>h.token === syntaxKind.ImplementsKeyword); - if (interfacesClause.length !== 0) { - interfaces = interfacesClause[0].types.map(t=>(t.expression).text); - } + visitClassDeclaration(node:ts.ClassDeclaration) { + let syntaxKind = SyntaxKind.current(); + let className = node.name.text; + let interfaces = this.extractInterfaces(node,syntaxKind); + let methods = node.members.filter(m=>m.kind === syntaxKind.MethodDeclaration); + this.validateMethods(methods,interfaces,className); + super.visitClassDeclaration(node); } - let missing:Array = this.extractMissing(node.members, syntaxKind, interfaces); - - if (missing.length !== 0) { - this.addFailure( - this.createFailure( - node.getStart(), - node.getWidth(), - sprintf.apply(this, this.formatFailure(className, missing)))); + 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); + if (interfacesClause.length !== 0) { + interfaces = interfacesClause[0].types.map(t=>{ + let expr =(t.expression); + if(expr.expression && expr.expression.text == Rule.HOOKS_PREFIX){ + return expr.name.text; + } else { + return expr.text; + } + }); + } + } + return interfaces; } - super.visitClassDeclaration(node); - } - - private extractMissing(members:ts.NodeArray, - syntaxKind:SyntaxKind.SyntaxKind, - interfaces:Array):Array { - let ngMembers = members.filter(m=>m.kind === syntaxKind.MethodDeclaration) - .map(m=>(m.name).text) - .filter(n=>(n && n.substr(0, 2) === Rule.HOOKS_PREFIX)) - .map(n=>n.substr(2, n.lenght)) - .filter(n=>Rule.LIFE_CYCLE_HOOKS_NAMES.indexOf(n) !== -1); - return ngMembers.filter(m=>interfaces.indexOf(m) === -1); - } + private validateMethods( methods:any[],interfaces:string[],className:string){ + methods.forEach(m=>{ + let n = (m.name).text; + 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]))); + } + }); + } - private formatFailure(className:string, missing:Array):Array { - let failureConfig:Array; - if (missing.length === 1) { - failureConfig = [Rule.FAILURE_SINGLE, className, Rule.HOOKS_PREFIX + missing[0], missing[0]]; - } else { - let joinedNgMissing:string = missing.map(m=>Rule.HOOKS_PREFIX + m).join(', '); - let joinedMissingInterfaces = missing.join(', '); - failureConfig = [Rule.FAILURE_MANY, className, joinedNgMissing, joinedMissingInterfaces]; + private isMethodValidHook(m:any,interfaces:string[]):boolean{ + let n = (m.name).text; + 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; + return isNg && isHook && isNotIn; } - return failureConfig; - } + } diff --git a/test/testHelper.ts b/test/testHelper.ts index e22922c2c..7be289d4a 100644 --- a/test/testHelper.ts +++ b/test/testHelper.ts @@ -47,6 +47,21 @@ export function assertFailure(ruleName: string, source: string, fail: IExpectedF }); }; +export function assertFailures(ruleName: string, source: string, fails: IExpectedFailure[], options = null) { + let result; + try { + result = lint(ruleName, source, options); + } catch (e) { + console.log(e.stack); + } + chai.assert(result.failureCount > 0, 'no failures'); + result.failures.forEach((ruleFail,index) => { + chai.assert.equal(fails[index].message, ruleFail.getFailure(), 'error messages dont\'t match'); + chai.assert.deepEqual(fails[index].startPosition, ruleFail.getStartPosition().getLineAndCharacter(), 'start char doesn\'t match'); + chai.assert.deepEqual(fails[index].endPosition, ruleFail.getEndPosition().getLineAndCharacter(), 'end char doesn\'t match'); + }); +}; + export function assertSuccess(ruleName: string, source: string, options = null) { chai.assert.equal(lint(ruleName, source, options).failureCount, 0); }; diff --git a/test/useLifeCycleInterfaceRule.spec.ts b/test/useLifeCycleInterfaceRule.spec.ts index 3448a9cac..be455639a 100644 --- a/test/useLifeCycleInterfaceRule.spec.ts +++ b/test/useLifeCycleInterfaceRule.spec.ts @@ -1,4 +1,4 @@ -import {assertFailure, assertSuccess} from './testHelper'; +import {assertFailure, assertSuccess, assertFailures} from './testHelper'; describe('use-life-cycle-interface', () => { describe('invalid declaration of life hook', () => { @@ -9,20 +9,17 @@ describe('use-life-cycle-interface', () => { } }`; assertFailure('use-life-cycle-interface', source, { - message: 'Implement lifecycle hook interfaces ($$09-01$$)', + message: 'Implement lifecycle hook interface OnInit for method ngOnInit in class App ($$09-01$$)', startPosition: { - line: 1, - character: 12 + line: 2, + character: 16 }, endPosition: { - line: 4, - character: 13 + line: 2, + character: 24 } }); }); - }); - - describe('invalid declaration of life hooks', () => { it(`should fail, when life cycle hooks are used without implementing their interfaces`, () => { let source = ` class App { @@ -31,43 +28,72 @@ describe('use-life-cycle-interface', () => { ngOnDestroy(){ } }`; + assertFailures('use-life-cycle-interface', source, [{ + message: 'Implement lifecycle hook interface OnInit for method ngOnInit in class App ($$09-01$$)', + startPosition: { + line: 2, + character: 16 + }, + endPosition: { + line: 2, + character: 24 + } + }, { + message: 'Implement lifecycle hook interface OnDestroy for method ngOnDestroy in class App ($$09-01$$)', + startPosition: { + line: 4, + character: 16 + }, + endPosition: { + line: 4, + character: 27 + } + } + ]); + }); + it(`should fail, when some of the life cycle hooks are used without implementing their interfaces`, () => { + let source = ` + class App extends Component implements OnInit{ + ngOnInit(){ + } + ngOnDestroy(){ + } + }`; assertFailure('use-life-cycle-interface', source, { - message: 'Implement lifecycle hook interfaces ($$09-01$$)', + message: 'Implement lifecycle hook interface OnDestroy for method ngOnDestroy in class App ($$09-01$$)', startPosition: { - line: 1, - character: 12 + line: 4, + character: 16 }, endPosition: { - line: 6, - character: 13 + line: 4, + character: 27 } }); }); }); - - describe('invalid declaration of life hooks', () => { - it(`should fail, when some of the life cycle hooks are used without implementing their interfaces`, () => { + 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 App extends Component implements OnInit{ + class App extends Component implements ng.OnInit{ ngOnInit(){ } ngOnDestroy(){ } }`; assertFailure('use-life-cycle-interface', source, { - message: 'Implement lifecycle hook interfaces ($$09-01$$)', + message: 'Implement lifecycle hook interface OnDestroy for method ngOnDestroy in class App ($$09-01$$)', startPosition: { - line: 1, - character: 12 + line: 4, + character: 16 }, endPosition: { - line: 6, - character: 13 + line: 4, + character: 27 } }); }); }); - describe('valid declaration of life hook', () => { it(`should succeed, when life cycle hook is used with it's corresponding interface`, () => { let source = ` @@ -77,9 +103,6 @@ describe('use-life-cycle-interface', () => { }`; assertSuccess('use-life-cycle-interface', source); }); - }); - - describe('valid declaration of life hooks', () => { it(`should succeed, when life cycle hooks are used with their corresponding interfaces`, () => { let source = ` class App extends Component implements OnInit,OnDestroy { @@ -97,7 +120,33 @@ describe('use-life-cycle-interface', () => { assertSuccess('use-life-cycle-interface', 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 App implements ng.OnInit { + ngOnInit(){ + } + }`; + assertSuccess('use-life-cycle-interface', source); + }); + it(`should succeed, when life cycle hooks are used with their corresponding interfaces`, () => { + let source = ` + class App extends Component implements ng.OnInit, ng.OnDestroy { + ngOnInit(){ + } + + private ngOnChanges:string=""; + + ngOnDestroy(){ + } + + ngOnSmth{ + } + }`; + assertSuccess('use-life-cycle-interface', source); + }); + }); describe('valid use of class without interfaces and life cycle hooks', () => { it(`should succeed when life cycle hooks are not used`, () => { let source = ` @@ -105,7 +154,6 @@ describe('use-life-cycle-interface', () => { assertSuccess('use-life-cycle-interface', source); }); }); - describe('valid declaration of class using Iterator', () => { it(`should succeed, when is used iterator`, () => { let source = `