Correct fix with string concat #946
Conversation
### What's done: Fixed bug Added comment and tests
| wrongStringTemplate.node.treeParent.replaceChild(wrongStringTemplate.node, KotlinParser().createNode("$firstPart\" +\n\"$secondPart")) | ||
| val correctNode = | ||
| try { | ||
| KotlinParser().createNode("$firstPart\" +\n\"$secondPart") |
There was a problem hiding this comment.
I think we need to just not put any pluses in ${} block, no?
Need to check if we are in ${} write now or not. If yes - add a new line
There was a problem hiding this comment.
I am just afraid that simple parsing of code can be inaccurate, but not sure, may be other reviewers will be ok
There was a problem hiding this comment.
I wanted to suggest the same. We can find the current node by offset, and if it is text - add concatenation, else - simply add a line break
Codecov Report
@@ Coverage Diff @@
## master #946 +/- ##
============================================
+ Coverage 83.85% 83.93% +0.08%
- Complexity 2380 2392 +12
============================================
Files 101 101
Lines 6025 6050 +25
Branches 1777 1785 +8
============================================
+ Hits 5052 5078 +26
Misses 264 264
+ Partials 709 708 -1
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: Fixed logic to resolve bug
### What's done: Fix to build
### What's done: Fix to build
### What's done: Updated
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt
Outdated
Show resolved
Hide resolved
| @@ -457,7 +480,7 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule( | |||
| val hasNewLineBefore: Boolean, | |||
| val indexLastSpace: Int = 0) : LongLineFixableCases() | |||
|
|
|||
| class StringTemplate(val node: ASTNode, val delimiterIndex: Int) : LongLineFixableCases() | |||
| class StringTemplate(val node: ASTNode, val delimiterIndex: Int, val multiLineOffset: Int) : LongLineFixableCases() | |||
There was a problem hiding this comment.
Maybe, make multiLineOffset Int? to better indicate, that string is single line if it is null instead of 0?
There was a problem hiding this comment.
I think it will be better when we check if multiLineOffset == 0, but when we calculate this variable, it will be worse
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt
Outdated
Show resolved
Hide resolved
### What's done: Fixed after review
| @@ -293,6 +294,7 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule( | |||
| val incorrectText = wrongStringTemplate.node.text | |||
| val firstPart = incorrectText.substring(0, wrongStringTemplate.delimiterIndex) | |||
| val secondPart = incorrectText.substring(wrongStringTemplate.delimiterIndex, incorrectText.length) | |||
| // wrongStringTemplate.multiLineOffset equals zero if string in one line | |||
There was a problem hiding this comment.
That's not what I meant :) Why do you check if split is at white space only for single-line string?
### What's done: Fixed after review
| private fun fixStringTemplate(wrongStringTemplate: LongLineFixableCases.StringTemplate) { | ||
| val incorrectText = wrongStringTemplate.node.text | ||
| val firstPart = incorrectText.substring(0, wrongStringTemplate.delimiterIndex) | ||
| val secondPart = incorrectText.substring(wrongStringTemplate.delimiterIndex, incorrectText.length) | ||
| wrongStringTemplate.node.treeParent.replaceChild(wrongStringTemplate.node, KotlinParser().createNode("$firstPart\" +\n\"$secondPart")) | ||
| // wrongStringTemplate.multiLineOffset equals zero if string in one line | ||
| // if wrongStringTemplate.multiLineOffset isn't equal to 0, it means that string is multiline, which means that split should be without quote |
There was a problem hiding this comment.
With or without? If we are inside a multiline string, we should split it with quote and concatenation, shouldn't we? Also, for multiline strings we are not checking any methods like trimIndent():
val string = """
first line
veryy looooooooooooooooong second line
""".trimIndent()how will we split it?
### What's done: Changed logic
### What's done: Fixed to pass all tests
### What's done: Fixed according to our code style
### What's done: Fixed according to our code style
### What's done: Fixed according to our code style
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt
Outdated
Show resolved
Hide resolved
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt
Show resolved
Hide resolved
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt
Show resolved
Hide resolved
### What's done: Fixed after review
What's done: