Add Cognitive Complexity Rule
#5838
Conversation
|
what needs to happen to get this merged? Is thing we can do to help on this issue. It would be amazing to have cognitive complexity rule enforcement in this library. |
|
also why are these warnings being generated here when none of these projects have anything to do with this project? |
So it will need to be code reviewed and approved, and then merged. Everyone who contributes to SwiftLint works on a volunteer basis, so it can take a while for people to find time to come back to it. If you look at previously merged PRs, you'll see that it can take a while. I do have some comments on the PR, but I don't have time right now to take a proper look. Generally if you can keep the branch clean and without merge conflicts that is good. The changelog can be a bit of a hotspot. |
So PRs that change any code in SwiftLint (some PRs are excluded but I can't remember the exact basis offhand) are run against a set of popular open source projects, and their violations compared against what This gives a kind of sanity check across a much wider codebase, and inspecting the new violations can be a big clue as to where we might be picking up false alarms, and "fixed" violations can be inspected to see if they really should have been fixed by the changes. In this case, it's a new rule, so you'd only expect new violations. |
| @@ -16,6 +16,10 @@ | |||
| [Jordan Rose](https://github.com/jrose-signal) | |||
| [SimplyDanny](https://github.com/SimplyDanny) | |||
|
|
|||
| * Add `Cognitive Complexity Rule` | |||
There was a problem hiding this comment.
If you look at previous rule addition changelog entries, they tend to be a bit wordier, and to specify the rule by its identifier. e.g.
* Add new `contrasted_opening_brace` rule that enforces opening
braces to be on a separate line after the preceding declaration.
[SimplyDanny](https://github.com/SimplyDanny)
| } | ||
|
|
||
| override func visit(_: IfExprSyntax) -> SyntaxVisitorContinueKind { | ||
| nesting += 1 |
There was a problem hiding this comment.
It feels like there's quite a lot of repetition in the methods - it would be nice to abstract that way a bit if possible
There was a problem hiding this comment.
Thank you for bringing up this rule, @lorwe!
Evaluating proper implementation of all the rules stated in the white paper was much easier if ComplexityVisitor would be defined in its own file accessible for dedicated testing. In other words, we should first test the complexity calculation on a set of code snippets. Once we are happy with it and find the rules describing Cognitive Complexity being considered, we may embed the visitor in the new SwiftLint rule where we only check for violations according to the computed complexity and the configured limits.
Resolves #3335
Reference: https://www.sonarsource.com/resources/cognitive-complexity/