-
-
Notifications
You must be signed in to change notification settings - Fork 793
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
New rule: ReplaceSafeCallChainWithRun #3089
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3089 +/- ##
============================================
+ Coverage 79.28% 79.41% +0.13%
- Complexity 2577 2584 +7
============================================
Files 435 436 +1
Lines 7766 7791 +25
Branches 1482 1481 -1
============================================
+ Hits 6157 6187 +30
Misses 819 819
+ Partials 790 785 -5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, neat trick. Those endless safe chains are always a problem for me. They are hard to understand what can really be null. Even for one is worth it. 👏🏻👏🏻 Nice one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. This rule is a nice addition. Great idea! 👍 🥇
.../src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/ReplaceSafeCallChainWithRun.kt
Show resolved
Hide resolved
.../src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/ReplaceSafeCallChainWithRun.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/ReplaceSafeCallChainWithRun.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great addition 👍
I saw this neat trick while browsing Kotlin compiler source.
Instead of having a long chain of safe calls, when many of those safe calls are redundant (because the type returned by the preceding call is non-nullable), that chain of safe calls can be surrounded with
run {}
and the redundant safe calls can be removed.This rule only checks from the end of a chain and works backwards, so it won't recommend inserting
run
blocks in the middle of a safe call chain which I think could be very confusing to read, especially since it could mean multiplerun
blocks in a very long chain.The rule also checks for any opportunity to replace a chain by surrounding with
run
, so any redundant safe calls at the end of a chain will be reported, even if there's only one. I personally think this is fine because it still reduces complexity which I see as being the main benefit of this rule.I'm interested in feedback on this one!