-
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
Wrong newlines in functional style #346
Conversation
### What's done: * Fixed bugs
### What's done: * Fixed bugs
### What's done: * Fixed bugs
Codecov Report
@@ Coverage Diff @@
## master #346 +/- ##
============================================
- Coverage 82.92% 82.90% -0.02%
- Complexity 1224 1239 +15
============================================
Files 60 60
Lines 3027 3047 +20
Branches 968 973 +5
============================================
+ Hits 2510 2526 +16
Misses 170 170
- Partials 347 351 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
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.
Take a look at comments. Also, there is another issue in this very logic: #326 . Will it be automatically fixed by change from this PR?
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
### What's done: * Fixed bugs
### What's done: * Fixed bugs
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
### What's done: * Fixed bugs
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/files/NewlinesRuleWarnTest.kt
Show resolved
Hide resolved
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
@@ -330,6 +332,10 @@ class NewlinesRule(private val configRules: List<RulesConfig>) : Rule("newlines" | |||
private fun ASTNode.getParentExpressions() = | |||
parents().takeWhile { it.elementType in chainExpressionTypes && it.elementType != LAMBDA_ARGUMENT } | |||
|
|||
private fun isMultilineLambda(node: ASTNode): Boolean = | |||
node.findAllNodesWithSpecificType(LAMBDA_ARGUMENT).firstOrNull()?.text?.count { it == '\n' } ?: -1 > 0 |
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.
Won't findAllNodesWithSpecificType
trigger on some nested lambdas or lambdas further in a call chain?
### What's done: * Fixed bugs * Call chains logic remade
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/NewlinesRule.kt
Outdated
Show resolved
Hide resolved
### What's done: * Fixed bugs
### What's done: * Fixed bugs
} | ||
} | ||
?.dropWhile { !it.treeParent.textContains('(') && !it.treeParent.textContains('{') } | ||
// fixme: we can't distinguish fully qualified names from chain of calls for now |
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.
// fixme: we can't distinguish fully qualified names from chain of calls for now | |
// fixme: we can't distinguish fully qualified names (like java.lang) from chain of property accesses (like list.size) for now |
Just to be sure we don't forget what this was about
|
||
@Test | ||
@Tag(WarningNames.WRONG_NEWLINES) | ||
fun `should trigger on non-multiline lambdas`() { |
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 example like
list.filter {
}
.map(::foo).filter {
bar()
}
It should be considered incorrect, when multiline lambda is not the first call.
### What's done: * Fixed bugs
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.
LGTM
What's done:
This pull request closes #321 #326
Actions checklist