Add cognitive complexity rule #378
Conversation
ghost
assigned 
README.md
Outdated
| @@ -49,6 +49,7 @@ How does it work? | |||
| * The output of functions that don't return anything should not be used ([`no-use-of-empty-return-value`]) ([`requires type-check`]) | |||
| * Variables should be declared before they are used ([`no-variable-usage-before-declaration`]) ([`requires type-check`]) | |||
| * Type aliases should be used ([`use-type-alias`]) ([`requires type-check`]) | |||
| * Cognitive Complexity of functions should not be too high ([`cognitive-complexity`]) | |||
There was a problem hiding this comment.
@m-g-sonar we sort rules alphabetically by rule key
README.md
Outdated
| @@ -84,6 +85,7 @@ How does it work? | |||
| [`no-variable-usage-before-declaration`]: ./sonarts-core/docs/rules/no-variable-usage-before-declaration.md | |||
| [`use-type-alias`]: ./sonarts-core/docs/rules/use-type-alias.md | |||
| [`requires type-check`]: https://palantir.github.io/tslint/usage/type-checking/ | |||
| [`cognitive-complexity`]: ./sonarts-core/docs/rules/cognitive-complexity.md | |||
There was a problem hiding this comment.
@m-g-sonar same here
| @@ -0,0 +1,252 @@ | |||
| // tslint:disable | |||
There was a problem hiding this comment.
@m-g-sonar you can drop this line, it's leftover
| if (condition) { } | ||
| } | ||
|
|
||
| get field(): string { |
There was a problem hiding this comment.
@m-g-sonar field -> property or getter (in JS they don't use term field)
sonarts-core/tests/runRule.ts
Outdated
| */ | ||
| export function runRuleWithLintFile( | ||
| Rule: any, | ||
| testDirName: string, |
There was a problem hiding this comment.
@m-g-sonar it's dir here, and ruleName in deeper call. Let's keep one abstraction
|
|
||
| ## See | ||
|
|
||
| * [Cognitive Complexity](http://redirect.sonarsource.com/doc/cognitive-complexity.html) |
There was a problem hiding this comment.
@m-g-sonar add configuration section (like I did for mccabe-complexity)
|
|
||
| public visitNode(node: ts.Node): void { | ||
| if (is(node, ...Rule.TARGETED_KINDS)) { | ||
| const body = (node as ts.FunctionLikeDeclarationBase).body as ts.Node; |
There was a problem hiding this comment.
@m-g-sonar what for this cast as ts.Node? I think you can just drop it
| const complexity = this.getComplexity(body); | ||
| if (complexity > this.threshold) { | ||
| this.addFailureAtNode( | ||
| FunctionWalker.reportNode(node), |
There was a problem hiding this comment.
@m-g-sonar why didn't you use functionLikeMainToken from navigation?
| } | ||
| } | ||
|
|
||
| class ComplexityWalker extends tslint.RuleWalker { |
There was a problem hiding this comment.
@m-g-sonar use SyntaxWalker instead
| class ComplexityWalker extends tslint.RuleWalker { | ||
| private complexity = 0; | ||
| private nesting = 0; | ||
| private logicalOperationsToIgnore: ts.Node[] = []; |
There was a problem hiding this comment.
@m-g-sonar can we use a more precise type here?
There was a problem hiding this comment.
Yup, we only add ts.BinaryExpression here.
| } | ||
| } | ||
|
|
||
| private isEleseIf(node: ts.IfStatement): boolean { |
There was a problem hiding this comment.
@m-g-sonar typo in name
There was a problem hiding this comment.
I think it was sounding nice. but I'll fix :)
| } | ||
|
|
||
| private isEleseIf(node: ts.IfStatement): boolean { | ||
| return ( |
There was a problem hiding this comment.
@m-g-sonar what for this parentheses?
There was a problem hiding this comment.
That's not from me. It's Prettier which add it.
| } | ||
|
|
||
| public visitBinaryExpression(node: ts.BinaryExpression): void { | ||
| const opertator = node.operatorToken; |
| return false; | ||
| } | ||
|
|
||
| private static skipParentheses(node: ts.Node): ts.Node { |
There was a problem hiding this comment.
@m-g-sonar there is drillDownThroughParenthesis in navigation
vilchik-elena
changed the title
Fixes #162
I executed current implementation against ruling to see repartition of cognitive complexity by function/method, with the following results. From my point of view, 15 seems to be a good starting value.