-
-
Notifications
You must be signed in to change notification settings - Fork 784
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
Add RedundantHigherOrderMapUsage rule #3182
Conversation
8194ca6
to
55d4af7
Compare
Codecov Report
@@ Coverage Diff @@
## master #3182 +/- ##
============================================
- Coverage 79.58% 79.53% -0.05%
- Complexity 2603 2631 +28
============================================
Files 439 440 +1
Lines 7919 7966 +47
Branches 1502 1521 +19
============================================
+ Hits 6302 6336 +34
Misses 820 820
- Partials 797 810 +13
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.
I really like this rule. The tests are really good and you are checking a lot of edge cases. Great work 👏.
assertThat(findings).hasSize(1) | ||
assertThat(findings).hasSourceLocation(5, 10) | ||
assertThat(findings[0]).hasMessage("This 'map' call can be replaced with 'onEach' or 'forEach'.") | ||
} |
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.
This! 👏👏👏👏👏 I saw it SO MANY TIMES! A map that should be replaced with an onEach
. And it slow down the reading of the code a lot.
import org.jetbrains.kotlin.types.KotlinType | ||
|
||
/** | ||
* Turn on this rule to flag redundant 'map' calls. |
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 would change the description with something like:
Redundant maps add complexity to the code and accomplish nothing. They should be removed or replaced with the proper operator.
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 👍
val message = if (lambdaStatements.size == 1) { | ||
if (receiverIsSet) { | ||
"This 'map' call can be replaced with 'toList'." | ||
} else { | ||
"This 'map' call can be removed." | ||
} | ||
} else { | ||
"This 'map' call can be replaced with 'onEach' or 'forEach'." | ||
} |
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.
Can you convert this to a when
?
7835d1a
to
460da97
Compare
460da97
to
c8e57dd
Compare
* Add RedundantHigherOrderMapUsage rule * Fix description * Convert `if` to `when`
* Add RedundantHigherOrderMapUsage rule * Fix description * Convert `if` to `when`
* Add RedundantHigherOrderMapUsage rule * Fix description * Convert `if` to `when`
Fixes #2975