Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

cyclomatic-complexity: Don't count empty switch cases #2743

Merged
merged 3 commits into from
May 31, 2017
Merged

cyclomatic-complexity: Don't count empty switch cases #2743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
527a362
cyclomatic-complexity: Don't count empty switch cases, and enable rule
andy-hanson May 12, 2017
229e98b
Disable cyclomatic-complexity for good
andy-hanson May 25, 2017
ae61488
Merge branch 'master' into cyclomatic-complexity-switch
andy-hanson May 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/rules/cyclomaticComplexityRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ function walk(ctx: Lint.WalkContext<{ threshold: number }>): void {
function increasesComplexity(node: ts.Node): boolean {
switch (node.kind) {
case ts.SyntaxKind.CaseClause:
return (node as ts.CaseClause).statements.length > 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not entirely right. consider the following code which does not increase complexity with this change:

switch (foo) {
    case 0:
    default:
        return bar();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's only one possible path through that code, so I don't think it should increase complexity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (foo === 0 || foo === 1) {bar()} increases complexity twice. The will increase complexity only once with this change:

switch (foo) {
    case 0:
    case 1:
        bar();
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw. if revert your change here to make them equal again, you probably need to disable the rule in almost all ts.forEachChild callbacks in this repo

Copy link
Contributor Author

@andy-hanson andy-hanson May 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

foo === 0 || foo === 1 is semantically equivalent to the switch statement, but syntactically it's treated the same as any other || expression, which can contain arbitrarily complex behavior.
There's some debate about this issue here: http://stackoverflow.com/questions/30240236/cyclomatic-complexity-of-switch-case-statement
In my opinion, treating multiple identical switch cases as each contributing to more complexity just makes this rule unusable, as you just pointed out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related: why do else branches or default clauses not increase complexity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, these two code snippets seem equally complex to me:

if (foo) {
    return 0;
}
return 1;
if (foo) {
    return 0;
} else {
    return 1;
}

case ts.SyntaxKind.CatchClause:
case ts.SyntaxKind.ConditionalExpression:
case ts.SyntaxKind.DoStatement:
Expand Down
6 changes: 4 additions & 2 deletions test/rules/cyclomatic-complexity/specifiedThreshold/test.ts.lint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,11 @@ function invalidBinaryExpression() {
function validSwitch() {
switch(5) {
case 0:
return 0;
case 2:
return "even";
case 1:
return 1;
case 3:
return "odd";
default:
return -1;
}
Expand Down
2 changes: 1 addition & 1 deletion tslint.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"prefer-switch": [true, { "min-cases": 3 }],

// Don't want these
"cyclomatic-complexity": false,
"newline-before-return": false,
"no-parameter-properties": false,
"no-unused-variable": false,
Expand All @@ -34,7 +35,6 @@

// TODO: Enable these
"completed-docs": false,
"cyclomatic-complexity": false,
"no-any": false,
"no-magic-numbers": false,
"no-non-null-assertion": false,
Expand Down