-
Notifications
You must be signed in to change notification settings - Fork 39
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
Fix: AVOID_NULL_CHECKS conflicts with NO_BRACES_IN_CONDITIONALS_AND_LOOPS #1191
Conversation
…o bugfix/braces_rule
Codecov Report
@@ Coverage Diff @@
## master #1191 +/- ##
============================================
+ Coverage 84.49% 84.51% +0.02%
- Complexity 2511 2518 +7
============================================
Files 103 103
Lines 7054 7065 +11
Branches 1902 1909 +7
============================================
+ Hits 5960 5971 +11
Misses 306 306
Partials 788 788
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
### What's done: * Code style
@@ -90,6 +92,13 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR | |||
} | |||
|
|||
if (elseKeyword != null && elseNode?.elementType != IF && elseNode?.elementType != BLOCK) { | |||
val referenceExpressionChildren = elseNode?.findAllDescendantsWithSpecificType(REFERENCE_EXPRESSION) |
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.
Shouldn't we check only top-level children? IIRC, findAllDescendantsWithSpecificType
will return even deeply nested nodes.
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.
Yeah, they reason, why it can be dangerous, that in theory, we can take some wrong node, from the nested ones
Initially I can't imagine some proper case, and thought that it will worked anyway, however now I did this, add more tests and little bit changed the logic, now we looking into the direct children
if (a) { | ||
bar() | ||
} else b?.let { |
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.
Please add it to the smoke test too.
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.
Added
…nto bugfix/braces_rule
What's done: