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.

Sign up for GitHub

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

Fixing issue in WhiteSpaceRule #412

Merged
merged 4 commits into from
Oct 16, 2020
Merged

Fixing issue in WhiteSpaceRule #412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ba8d705
Fixing issue in WhiteSpaceRule
petertrr Oct 16, 2020
00962ed
Merge branch 'master' into bugfix/whitespacerule#405
petertrr Oct 16, 2020
216d621
Merge remote-tracking branch 'origin/master' into bugfix/whitespaceru…
petertrr Oct 16, 2020
47d77c6
Fixing issue in WhiteSpaceRule
petertrr Oct 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 25 additions & 16 deletions diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/WhiteSpaceRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ class WhiteSpaceRule(private val configRules: List<RulesConfig>) : Rule("horizon
*/
@Suppress("UnsafeCallOnNullableType")
private fun handleLbrace(node: ASTNode) {
val whitespaceOrPrevNode = node.parent({ it.treePrev != null }, strict = false)!!.treePrev
// `{` can't be the very first symbol in the file, so `!!` should be safe
val whitespaceOrPrevNode = node.selfOrParentsTreePrev()!!
val isFromLambdaAsArgument = node
.parents()
.take(numParentsForLambda)
Expand All @@ -181,12 +182,15 @@ class WhiteSpaceRule(private val configRules: List<RulesConfig>) : Rule("horizon
it[0].elementType == FUNCTION_LITERAL &&
it[1].elementType == LAMBDA_EXPRESSION &&
it[2].elementType == VALUE_ARGUMENT &&
// lambda is not passed as a named argument
!it[2].hasChildOfType(EQ) &&
// lambda is the first argument in the list
it[2].prevSibling { prevNode -> prevNode.elementType == COMMA } == null
}
?: false
val prevNode = whitespaceOrPrevNode.let { if (it.elementType == WHITE_SPACE) it.treePrev else it }
val numWhiteSpace = whitespaceOrPrevNode.numWhiteSpaces()
// note: the conditions in the following `if`s cannot be collapsed into simple conjunctions
if (isFromLambdaAsArgument) {
if (numWhiteSpace != 0) {
WRONG_WHITESPACE.warnAndFix(configRules, emitWarn, isFixMode, "there should be no whitespace before '{' of lambda" +
Expand Down Expand Up @@ -239,8 +243,11 @@ class WhiteSpaceRule(private val configRules: List<RulesConfig>) : Rule("horizon

@Suppress("UnsafeCallOnNullableType")
private fun handleToken(node: ASTNode, requiredSpacesBefore: Int?, requiredSpacesAfter: Int?) {
require(requiredSpacesBefore != null || requiredSpacesAfter != null)
val spacesBefore = node.parent({ it.treePrev != null }, strict = false)!!.treePrev.numWhiteSpaces()
require(requiredSpacesBefore != null || requiredSpacesAfter != null) {
"requiredSpacesBefore=$requiredSpacesBefore and requiredSpacesAfter=$requiredSpacesAfter, but at least one should not be null"
}
val spacesBefore = node.selfOrParentsTreePrev()!!.numWhiteSpaces()
// calculate actual spaces after but only if requiredSpacesAfter is not null, otherwise we won't check it
val spacesAfter = requiredSpacesAfter?.let { _ ->
// for `!!` and possibly other postfix expressions treeNext and treeParent.treeNext can be null
// upper levels are already outside of the expression this token belongs to, so we won't check them
Expand Down Expand Up @@ -309,23 +316,25 @@ class WhiteSpaceRule(private val configRules: List<RulesConfig>) : Rule("horizon
}

private fun ASTNode.fixSpaceAround(requiredSpacesBefore: Int?, requiredSpacesAfter: Int?) {
if (requiredSpacesBefore != null) {
if (requiredSpacesBefore == 1) {
(if (treePrev.elementType == WHITE_SPACE) treePrev.treePrev else treePrev).leaveSingleWhiteSpace()
} else if (requiredSpacesBefore == 0) {
treePrev.removeIfWhiteSpace()
}
if (requiredSpacesBefore == 1) {
selfOrParentsTreePrev()?.let { if (it.elementType == WHITE_SPACE) it.treePrev else it }?.leaveSingleWhiteSpace()
} else if (requiredSpacesBefore == 0) {
selfOrParentsTreePrev()?.removeIfWhiteSpace()
}
if (requiredSpacesAfter != null) {
if (requiredSpacesAfter == 1) {
leaveSingleWhiteSpace()
} else if (requiredSpacesAfter == 0) {
// for `!!` and possibly other postfix expressions treeNext can be null
(treeNext ?: treeParent.treeNext).removeIfWhiteSpace()
}

if (requiredSpacesAfter == 1) {
leaveSingleWhiteSpace()
} else if (requiredSpacesAfter == 0) {
// for `!!` and possibly other postfix expressions treeNext can be null
(treeNext ?: treeParent.treeNext).removeIfWhiteSpace()
}
}

/**
* Function that returns `treePrev` of this node, or if this.treePrev is null, `treePrev` of first parent node that has it
*/
private fun ASTNode.selfOrParentsTreePrev() = parent({ it.treePrev != null }, strict = false)?.treePrev

/**
* This method counts spaces in this node. Null is returned in following cases:
* * if it is WHITE_SPACE with a newline
Expand Down
5 changes: 5 additions & 0 deletions diktat-rules/src/test/resources/test/paragraph3/spaces/WhiteSpaceBeforeLParExpected.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,8 @@ class Example : SuperExample {
when (expression) { }
}
}

data class Example(
Copy link
Member

Choose a reason for hiding this comment

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

same bug was with functions?
now it fixes foo ()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, this case was missing in tests. Added.
But diktat fixes it already.

val foo: Foo,
val bar: Bar
)
7 changes: 6 additions & 1 deletion diktat-rules/src/test/resources/test/paragraph3/spaces/WhiteSpaceBeforeLParTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ class Example : SuperExample {
when(expression) { }
}

fun bar() {
fun bar () {
if (condition) { }
for (i in 1..100) { }
when (expression) { }
}
}

data class Example (
val foo: Foo,
val bar: Bar
)