Skip to content

Commit

Permalink
feat(rule): cyclomatic complexity rule (#512)
Browse files Browse the repository at this point in the history
* feat(rule): add templateCyclomaticComplexityRule (#509)

* feat(rule): review + rebase
  • Loading branch information
wKoza authored and mgechev committed Feb 11, 2018
1 parent f1e0286 commit 3221330
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/angular/config.ts
Expand Up @@ -69,6 +69,8 @@ export const Config: Config = {
{ selector: '[ngIf]', exportAs: 'ngIf', inputs: ['ngIf'] },
{ selector: '[ngFor][ngForOf]', exportAs: 'ngFor', inputs: ['ngForTemplate', 'ngForOf'] },
{ selector: '[ngSwitch]', exportAs: 'ngSwitch', inputs: ['ngSwitch'] },
{ selector: '[ngSwitchCase]', exportAs: 'ngSwitchCase', inputs: ['ngSwitchCase'] },
{ selector: '[ngSwitchDefault]', exportAs: 'ngSwitchDefault', inputs: ['ngSwitchDefault'] },

// @angular/material
{ selector: 'mat-autocomplete', exportAs: 'matAutocomplete' },
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Expand Up @@ -7,6 +7,7 @@ export { Rule as DecoratorNotAllowedRule } from './decoratorNotAllowedRule';
export { Rule as DirectiveClassSuffixRule } from './directiveClassSuffixRule';
export { Rule as DirectiveSelectorRule } from './directiveSelectorRule';
export { Rule as I18nRule } from './i18nRule';
export { Rule as TemplateCyclomaticComplexityRule } from './templateCyclomaticComplexityRule';
export { Rule as TemplateConditionalComplexityRule } from './templateConditionalComplexityRule';
export { Rule as ImportDestructuringSpacingRule } from './importDestructuringSpacingRule';
export { Rule as NoAttributeParameterDecoratorRule } from './noAttributeParameterDecoratorRule';
Expand Down
75 changes: 75 additions & 0 deletions src/templateCyclomaticComplexityRule.ts
@@ -0,0 +1,75 @@
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';

export class Rule extends Lint.Rules.AbstractRule {
public static metadata: Lint.IRuleMetadata = {
ruleName: 'template-cyclomatic-complexity',
type: 'functionality',
// tslint:disable-next-line:max-line-length
description: 'Checks cyclomatic complexity against a specified limit. It is a quantitative measure of the number of linearly independent paths through a program\'s source code',
rationale: 'Cyclomatic complexity over some threshold indicates that the logic should be moved outside the template.',
options: {
type: 'array',
items: {
type: 'string'
},
minLength: 0,
maxLength: 2,
},
optionExamples: [
'true',
'[true, 6]'
],
optionsDescription: 'Determine the maximum number of the cyclomatic complexity.',
typescriptOnly: true,
hasFix: false
};

// tslint:disable-next-line:max-line-length
static COMPLEXITY_FAILURE_STRING = 'The cyclomatic complexity exceeded the defined limit (cost \'%s\'). Your template should be refactored.';

static COMPLEXITY_MAX = 5;

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {

return this.applyWithWalker(
new NgWalker(sourceFile,
this.getOptions(), {
templateVisitorCtrl: TemplateConditionalComplexityVisitor,
}));
}
}

class TemplateConditionalComplexityVisitor extends BasicTemplateAstVisitor {

complexity = 0;

visitDirectiveProperty(prop: ast.BoundDirectivePropertyAst, context: BasicTemplateAstVisitor): any {
if (prop.sourceSpan) {
const directive = (<any>prop.sourceSpan).toString();

if (directive.startsWith('*ngFor') || directive.startsWith('*ngIf') ||
directive.startsWith('*ngSwitchCase') || directive.startsWith('*ngSwitchDefault')) {
this.complexity++;
}
}

const options = this.getOptions();
const complexityMax: number = options.length ? options[0] : Rule.COMPLEXITY_MAX;

if (this.complexity > complexityMax) {
const span = prop.sourceSpan;
let failureConfig: string[] = [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);
}
}
84 changes: 84 additions & 0 deletions test/templateCyclomaticComplexityRule.spec.ts
@@ -0,0 +1,84 @@
// tslint:disable:max-line-length
import { assertSuccess, assertAnnotated } from './testHelper';
import { Replacement } from 'tslint';
import { expect } from 'chai';

describe('cyclomatic complexity', () => {
describe('success', () => {
it('should work with a lower level of complexity', () => {
let source = `
@Component({
template: \`
<div *ngIf="a === '1'">
<li *ngFor="let person of persons; trackBy: trackByFn">
{{ person.name }}
<div [ngSwitch]="person.emotion">
<app-happy-hero *ngSwitchCase="'happy'" [hero]="currentHero"></app-happy-hero>
<app-sad-hero *ngSwitchCase="'sad'" [hero]="currentHero"></app-sad-hero>
<app-unknown-hero *ngSwitchDefault [hero]="currentHero"></app-unknown-hero>
</div>
</li>
</div>
\`
})
class Bar {}
`;
assertSuccess('template-cyclomatic-complexity', source);
});

it('should work with a higher level of complexity', () => {
let source = `
@Component({
template: \`
<div *ngIf="a === '1'">
<li *ngFor="let person of persons; trackBy: trackByFn">
<div *ngIf="a === '1'">{{ person.name }}</div>
<div [ngSwitch]="person.emotion">
<app-happy-hero *ngSwitchCase="'happy'" [hero]="currentHero"></app-happy-hero>
<app-sad-hero *ngSwitchCase="'sad'" [hero]="currentHero"></app-sad-hero>
<app-confused-hero *ngSwitchCase="'confused'" [hero]="currentHero"></app-confused-hero>
<app-unknown-hero *ngSwitchDefault [hero]="currentHero"></app-unknown-hero>
</div>
</li>
</div>
\`
})
class Bar {}
`;
assertSuccess('template-cyclomatic-complexity', source, [7]);
});

});


describe('failure', () => {
it('should fail with a higher level of complexity', () => {
let source = `
@Component({
template: \`
<div *ngIf="a === '1'">
<li *ngFor="let person of persons; trackBy: trackByFn">
<div *ngIf="a === '1'">{{ person.name }}</div>
<div [ngSwitch]="person.emotion">
<app-happy-hero *ngSwitchCase="'happy'" [hero]="currentHero"></app-happy-hero>
<app-sad-hero *ngSwitchCase="'sad'" [hero]="currentHero"></app-sad-hero>
<app-confused-hero *ngSwitchCase="'confused'" [hero]="currentHero"></app-confused-hero>
~~~~~~~~~~~~~~~~~~~~~~~~~~
<app-unknown-hero *ngSwitchDefault [hero]="currentHero"></app-unknown-hero>
</div>
</li>
</div>
\`
})
class Bar {}
`;
assertAnnotated({
ruleName: 'template-cyclomatic-complexity',
message: 'The cyclomatic complexity exceeded the defined limit (cost \'5\'). Your template should be refactored.',
source
});
});

});

});

0 comments on commit 3221330

Please sign in to comment.