Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor ComplexMethod #2090

Merged
merged 9 commits into from
Nov 19, 2019
Merged

Refactor ComplexMethod #2090

merged 9 commits into from
Nov 19, 2019

Conversation

arturbosch
Copy link
Member

@arturbosch arturbosch commented Nov 16, 2019

Now considers

  • &&, || and ?: everywhere
  • break, continue (throw is explicitly not counted like in ndepend)
  • functions used to replace if or for are now configurable
  • threshold increased to 15 like ndepend does - also linked their page

Closes #1921

@arturbosch arturbosch added this to the 1.2.0 milestone Nov 16, 2019
Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

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

I particularly like the config implementation for the CyclomaticComplexity class.
TIL that you can now use Kotlin functions inside Spec's context() functions. Cool!
As I commented, the ComplexMethod case file is very hard to understand and maintain.
Hopefully, we can also approach at some point in the future.
For now, this PR looks very fine! 👍

@codecov-io
Copy link

codecov-io commented Nov 18, 2019

Codecov Report

Merging #2090 into master will increase coverage by 0.11%.
The diff coverage is 94.11%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2090      +/-   ##
============================================
+ Coverage     80.84%   80.95%   +0.11%     
- Complexity     2008     2020      +12     
============================================
  Files           336      336              
  Lines          5784     5813      +29     
  Branches       1059     1058       -1     
============================================
+ Hits           4676     4706      +30     
  Misses          546      546              
+ Partials        562      561       -1
Impacted Files Coverage Δ Complexity Δ
...tekt/core/processors/ProjectComplexityProcessor.kt 100% <100%> (ø) 3 <0> (ø) ⬇️
...rturbosch/detekt/rules/complexity/ComplexMethod.kt 94.44% <92.59%> (-2.11%) 16 <10> (+4)
...rbosch/detekt/api/internal/CyclomaticComplexity.kt 94.54% <94.54%> (ø) 23 <23> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acc7d14...15fff9b. Read the comment docs.

arturbosch and others added 3 commits November 18, 2019 20:24
…/complexity/ComplexMethod.kt

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>
…/complexity/ComplexMethod.kt

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>
…ernal/CyclomaticComplexity.kt

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>
@arturbosch
Copy link
Member Author

I particularly like the config implementation for the CyclomaticComplexity class.
TIL that you can now use Kotlin functions inside Spec's context() functions. Cool!
As I commented, the ComplexMethod case file is very hard to understand and maintain.
Hopefully, we can also approach at some point in the future.
For now, this PR looks very fine!

Thanks! I've refactored some test cases and removed that ComplexMethod case file :)

Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

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

LGTM!!

@arturbosch arturbosch merged commit 6bcb423 into master Nov 19, 2019
@arturbosch arturbosch deleted the refactor-cyclo-visitor branch November 19, 2019 07:02
smyachenkov pushed a commit to smyachenkov/detekt that referenced this pull request Dec 9, 2019
* Simplify cyclomatic complexity visitor logic - detekt#1921

* Support couting ?:, &&, ||, continue and break expressions - detekt#1921

* Update documentation and increase threshold for ComplexMethod based on ndepend's implementation of cyclomatic complexity - detekt#1921

* Make nesting functions configurable - detekt#1921

* Split ComplexMethod test case to many simpler test cases

* Update detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/ComplexMethod.kt

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>

* Update detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/ComplexMethod.kt

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>

* Update detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/CyclomaticComplexity.kt

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>

* Remove semicolon from split delimiters
smyachenkov pushed a commit to smyachenkov/detekt that referenced this pull request Dec 9, 2019
* Simplify cyclomatic complexity visitor logic - detekt#1921

* Support couting ?:, &&, ||, continue and break expressions - detekt#1921

* Update documentation and increase threshold for ComplexMethod based on ndepend's implementation of cyclomatic complexity - detekt#1921

* Make nesting functions configurable - detekt#1921

* Split ComplexMethod test case to many simpler test cases

* Update detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/ComplexMethod.kt

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>

* Update detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/ComplexMethod.kt

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>

* Update detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/CyclomaticComplexity.kt

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>

* Remove semicolon from split delimiters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calculate MCC (McCabe Complexity) accordingly
3 participants