Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(rule): add templateConditionalComplexityRule (#509)
* feat(rule): add complexityRule * feat(rule): rename complexity to template-conditional-complexity * feat(rule): fix bug * fix(rule): switch to 2 spaces instead of 4 * feat(rule): review * feat(rule): use Angular expression parser
- Loading branch information
Showing
3 changed files
with
229 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
import * as Lint from 'tslint'; | ||
import * as ts from 'typescript'; | ||
import * as ast from '@angular/compiler'; | ||
import { sprintf } from 'sprintf-js'; | ||
import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor'; | ||
import { NgWalker } from './angular/ngWalker'; | ||
import * as compiler from '@angular/compiler'; | ||
import { Binary } from '@angular/compiler'; | ||
|
||
export class Rule extends Lint.Rules.AbstractRule { | ||
public static metadata: Lint.IRuleMetadata = { | ||
ruleName: 'template-conditional-complexity', | ||
type: 'functionality', | ||
description: 'The condition complexity shouldn\'t exceed a rational limit in a template.', | ||
rationale: 'An important complexity complicates the tests and the maintenance.', | ||
options: { | ||
type: 'array', | ||
items: { | ||
type: 'string' | ||
}, | ||
minLength: 0, | ||
maxLength: 2, | ||
}, | ||
optionExamples: [ | ||
'true', | ||
'[true, 4]' | ||
], | ||
optionsDescription: 'Determine the maximum number of Boolean operators allowed.', | ||
typescriptOnly: true, | ||
hasFix: false | ||
}; | ||
|
||
// tslint:disable-next-line:max-line-length | ||
static COMPLEXITY_FAILURE_STRING = 'The condition complexity (cost \'%s\') exceeded the defined limit (cost \'%s\'). The conditional expression should be move in the component\'s template.'; | ||
|
||
static COMPLEXITY_MAX = 3; | ||
|
||
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { | ||
|
||
return this.applyWithWalker( | ||
new NgWalker(sourceFile, | ||
this.getOptions(), { | ||
templateVisitorCtrl: TemplateConditionalComplexityVisitor, | ||
})); | ||
} | ||
} | ||
|
||
class TemplateConditionalComplexityVisitor extends BasicTemplateAstVisitor { | ||
|
||
visitDirectiveProperty(prop: ast.BoundDirectivePropertyAst, context: BasicTemplateAstVisitor): any { | ||
|
||
if (prop.sourceSpan) { | ||
const directive = (<any>prop.sourceSpan).toString(); | ||
|
||
if (directive.startsWith('*ngIf')) { | ||
// extract expression and drop characters new line and quotes | ||
const expr = directive.split(/\*ngIf\s*=\s*/)[1].slice(1, -1).replace(/[\n\r]/g, ''); | ||
|
||
const expressionParser = new compiler.Parser(new compiler.Lexer()); | ||
const ast = expressionParser.parseAction(expr, null); | ||
|
||
let complexity = 0; | ||
let conditions: Array<Binary> = []; | ||
let condition = ast.ast as Binary; | ||
if (condition.operation) { | ||
complexity++; | ||
conditions.push(condition); | ||
} | ||
|
||
while (conditions.length > 0) { | ||
condition = conditions.pop(); | ||
if (condition.operation) { | ||
if (condition.left instanceof Binary) { | ||
complexity++; | ||
conditions.push(condition.left as Binary); | ||
} | ||
|
||
if (condition.right instanceof Binary) { | ||
conditions.push(condition.right as Binary); | ||
} | ||
} | ||
|
||
} | ||
const options = this.getOptions(); | ||
const complexityMax: number = options.length ? options[0] : Rule.COMPLEXITY_MAX; | ||
|
||
if (complexity > complexityMax) { | ||
const span = prop.sourceSpan; | ||
let failureConfig: string[] = [String(complexity), String(Rule.COMPLEXITY_MAX)]; | ||
failureConfig.unshift(Rule.COMPLEXITY_FAILURE_STRING); | ||
this.addFailure(this.createFailure(span.start.offset, span.end.offset - span.start.offset, | ||
sprintf.apply(this, failureConfig)) | ||
); | ||
} | ||
} | ||
} | ||
super.visitDirectiveProperty(prop, context); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
// tslint:disable:max-line-length | ||
import { assertSuccess, assertAnnotated } from './testHelper'; | ||
import { Replacement } from 'tslint'; | ||
import { expect } from 'chai'; | ||
|
||
describe('complexity', () => { | ||
describe('success', () => { | ||
it('should work with a lower level of complexity', () => { | ||
let source = ` | ||
@Component({ | ||
template: \` | ||
<div *ngIf="a === '1' || (b === '2' && c.d !== e)"> | ||
Enter your card details | ||
</div> | ||
\` | ||
}) | ||
class Bar {} | ||
`; | ||
assertSuccess('template-conditional-complexity', source); | ||
}); | ||
|
||
|
||
it('should work with a lower level of complexity', () => { | ||
let source = ` | ||
@Component({ | ||
template: \` | ||
<div *ngIf="a === '1' || b === '2' && c.d !== e"> | ||
Enter your card details | ||
</div> | ||
\` | ||
}) | ||
class Bar {} | ||
`; | ||
assertSuccess('template-conditional-complexity', source); | ||
}); | ||
|
||
it('should work with a level of complexity customisable', () => { | ||
let source = ` | ||
@Component({ | ||
template: \` | ||
<div *ngIf="a === '3' || (b === '3' && c.d !== '1' && e.f !== '6' && q !== g)"> | ||
Enter your card details | ||
</div> | ||
\` | ||
}) | ||
class Bar {} | ||
`; | ||
assertSuccess('template-conditional-complexity', source, [5]); | ||
}); | ||
|
||
it('should work with a level of complexity customisable', () => { | ||
let source = ` | ||
@Component({ | ||
template: \` | ||
<div *ngIf="(b === '3' && c.d !== '1' && e.f !== '6' && q !== g) || a === '3'"> | ||
Enter your card details | ||
</div> | ||
\` | ||
}) | ||
class Bar {} | ||
`; | ||
assertSuccess('template-conditional-complexity', source, [5]); | ||
}); | ||
|
||
it('should work with something else', () => { | ||
let source = ` | ||
@Component({ | ||
template: \` | ||
<div *ngIf="isValid;then content else other_content"> | ||
Enter your card details | ||
</div> | ||
\` | ||
}) | ||
class Bar {} | ||
`; | ||
assertSuccess('template-conditional-complexity', source, [5]); | ||
}); | ||
|
||
}); | ||
|
||
|
||
describe('failure', () => { | ||
it('should fail with a higher level of complexity', () => { | ||
let source = ` | ||
@Component({ | ||
template: \` | ||
<div *ngIf="a === '3' || (b === '3' && c.d !== '1' && e.f !== '6' && q !== g)"> | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
Enter your card details | ||
</div> | ||
\` | ||
}) | ||
class Bar {} | ||
`; | ||
assertAnnotated({ | ||
ruleName: 'template-conditional-complexity', | ||
message: 'The condition complexity (cost \'5\') exceeded the defined limit (cost \'3\'). The conditional expression should be move in the component\'s template.', | ||
source | ||
}); | ||
}); | ||
|
||
}); | ||
|
||
describe('failure', () => { | ||
it('should fail with a higher level of complexity and a carrier return', () => { | ||
let source = ` | ||
@Component({ | ||
template: \` | ||
<div *ngIf="a === '3' || (b === '3' && c.d !== '1' | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
&& e.f !== '6' && q !== g)"> | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
Enter your card details | ||
</div> | ||
\` | ||
}) | ||
class Bar {} | ||
`; | ||
assertAnnotated({ | ||
ruleName: 'template-conditional-complexity', | ||
message: 'The condition complexity (cost \'5\') exceeded the defined limit (cost \'3\'). The conditional expression should be move in the component\'s template.', | ||
source | ||
}); | ||
}); | ||
|
||
}); | ||
|
||
|
||
}); |