Repository: cqfn/diKTat
Pull request #932 (2021-06-17 11:27:21)

Bugfix of NPE in LongLine rule on Binary Expressions
### What's done:
* Fix bug with NPE: we didn't consider, that offset changes in course of fixes, so it's break the logic sometimes
* Add additional check for long comments: if there is only one white space, which is located after `//` - we shouldn't fix split it
* Add tests

codecov[bot]: # [Codecov](https://codecov.io/gh/cqfn/diKTat/pull/932?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn) Report
> Merging [#932](https://codecov.io/gh/cqfn/diKTat/pull/932?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn) (144af71) into [master](https://codecov.io/gh/cqfn/diKTat/commit/94a4d1c707052e7e827a441897ccb681b60cc52b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn) (94a4d1c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/cqfn/diKTat/pull/932/graphs/tree.svg?width=650&height=150&src=pr&token=P2quRDffpt&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn)](https://codecov.io/gh/cqfn/diKTat/pull/932?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn)

```diff
@@            Coverage Diff            @@
##             master     #932   +/-   ##
=========================================
  Coverage     84.15%   84.15%           
  Complexity     2374     2374           
=========================================
  Files           101      101           
  Lines          6008     6010    +2     
  Branches       1773     1773           
=========================================
+ Hits           5056     5058    +2     
  Misses          263      263           
  Partials        689      689           
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `84.15% <100.00%> (+<0.01%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/cqfn/diKTat/pull/932?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn) | Coverage Δ | |
|---|---|---|
| [...g/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt](https://codecov.io/gh/cqfn/diKTat/pull/932/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn#diff-ZGlrdGF0LXJ1bGVzL3NyYy9tYWluL2tvdGxpbi9vcmcvY3Fmbi9kaWt0YXQvcnVsZXNldC9ydWxlcy9jaGFwdGVyMy9MaW5lTGVuZ3RoLmt0) | `91.86% <100.00%> (+0.07%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/cqfn/diKTat/pull/932?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/cqfn/diKTat/pull/932?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn). Last update [94a4d1c...144af71](https://codecov.io/gh/cqfn/diKTat/pull/932?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cqfn).

kgevorkyan: Please take a look for a [explanation](https://github.com/cqfn/diKTat/issues/920#issuecomment-863198257), if changes are unclear

Patch:
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt
index 5ea3e440..0981c113 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/LineLength.kt
@@ -91,7 +91,12 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
                     LONG_LINE.warnAndFix(configRules, emitWarn, isFixMode,
                         "max line length ${configuration.lineLength}, but was ${line.length}",
                         offset + node.startOffset, node, fixableType != LongLineFixableCases.None) {
+                        // we should keep in mind, that in the course of fixing we change the offset
+                        val textLenBeforeFix = node.textLength
                         fixError(fixableType)
+                        val textLenAfterFix = node.textLength
+                        // offset for all next nodes changed to this delta
+                        offset += (textLenAfterFix - textLenBeforeFix)
                     }
                 }
             }
@@ -137,13 +142,14 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
      */
     private fun checkStringTemplate(node: ASTNode, configuration: LineLengthConfiguration): LongLineFixableCases {
         val leftOffset = positionByOffset(node.startOffset).second
-        val difference = configuration.lineLength.toInt() - leftOffset
-        // case when new line should be inserted after `+`. Example: "first" + "second"
-        if (difference > node.text.length) {
-            return LongLineFixableCases.BinaryExpression(node.treeParent)
-        }
         val delimiterIndex = node.text.substring(0, configuration.lineLength.toInt() - leftOffset).lastIndexOf(' ')
         if (delimiterIndex == -1) {
+            // we can't split this string, however may be we can move it entirely:
+            // case when new line should be inserted after `+`. Example: "first" + "second"
+            node.treeParent.findChildByType(OPERATION_REFERENCE)?.let {
+                return LongLineFixableCases.BinaryExpression(node.treeParent)
+            }
+            // can't fix this case
             return LongLineFixableCases.None
         }
         // minus 2 here as we are inserting ` +` and we don't want it to exceed line length
@@ -161,7 +167,9 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
     private fun checkComment(wrongNode: ASTNode, configuration: LineLengthConfiguration): LongLineFixableCases {
         val leftOffset = positionByOffset(wrongNode.startOffset).second
         val indexLastSpace = wrongNode.text.substring(0, configuration.lineLength.toInt() - leftOffset).lastIndexOf(' ')
-        if (indexLastSpace == -1) {
+        // index == -1 indicates, that we didn't find any possible way to split this comment
+        // index == 2 indicates, that we found the white space after `//`, and shouldn't fix it
+        if (indexLastSpace == -1 || indexLastSpace == 2) {
             return LongLineFixableCases.None
         }
         return LongLineFixableCases.Comment(wrongNode, indexLastSpace)


Test patch:
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/LineLengthFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/LineLengthFixTest.kt
index e0f317ae..6325ed13 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/LineLengthFixTest.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/LineLengthFixTest.kt
@@ -30,6 +30,11 @@ class LineLengthFixTest : FixTestBase("test/paragraph3/long_line", ::LineLength)
         fixAndCompare("LongLineCommentExpected2.kt", "LongLineCommentTest2.kt", rulesConfigListDefaultLineLength)
     }
 
+    @Test
+    fun `should fix long string template while some fix is already done`() {
+        fixAndCompare("LongStringTemplateExpected.kt", "LongStringTemplateTest.kt", rulesConfigListDefaultLineLength)
+    }
+
     @Test
     fun `should fix long binary expression`() {
         fixAndCompare("LongLineExpressionExpected.kt", "LongLineExpressionTest.kt", rulesConfigListLineLength)
diff --git a/diktat-rules/src/test/resources/test/paragraph3/long_line/LongExpressionNoFixExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/long_line/LongExpressionNoFixExpected.kt
index e6b32e1a..ddc30f06 100644
--- a/diktat-rules/src/test/resources/test/paragraph3/long_line/LongExpressionNoFixExpected.kt
+++ b/diktat-rules/src/test/resources/test/paragraph3/long_line/LongExpressionNoFixExpected.kt
@@ -1,6 +1,8 @@
 package org.cqfn.diktat.resources.paragraph3.longline
 
 class veryLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong {
+    // looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongggggggggggggggggggggggggg
+    //looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongggggggggggggggggggggggggg
     val s = "d" +
 " s d d d d ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
 }
diff --git a/diktat-rules/src/test/resources/test/paragraph3/long_line/LongExpressionNoFixTest.kt b/diktat-rules/src/test/resources/test/paragraph3/long_line/LongExpressionNoFixTest.kt
index 407a2a61..286bc4e9 100644
--- a/diktat-rules/src/test/resources/test/paragraph3/long_line/LongExpressionNoFixTest.kt
+++ b/diktat-rules/src/test/resources/test/paragraph3/long_line/LongExpressionNoFixTest.kt
@@ -1,5 +1,7 @@
 package org.cqfn.diktat.resources.paragraph3.longline
 
 class veryLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong {
+    // looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongggggggggggggggggggggggggg
+    //looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongggggggggggggggggggggggggg
     val s = "d s d d d d ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
 }
diff --git a/diktat-rules/src/test/resources/test/paragraph3/long_line/LongStringTemplateExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/long_line/LongStringTemplateExpected.kt
new file mode 100644
index 00000000..b15925bf
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/paragraph3/long_line/LongStringTemplateExpected.kt
@@ -0,0 +1,12 @@
+object Observables {
+    val someCode = 15 // looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongggggggggggggggggggggggggg
+    // Some
+// looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong line
+    @Deprecated("New type inference algorithm in Kotlin 1.4 makes this method obsolete. Method will be removed in" +
+" future RxKotlin release.",
+        replaceWith = ReplaceWith("Observable.combineLatest(source1, source2, source3, source4, combineFunction)", "io.reactivex.Observable"),
+        level = DeprecationLevel.WARNING)
+    @CheckReturnValue
+    @SchedulerSupport(SchedulerSupport.NONE)
+    inline fun <T1 : Any, T2 : Any, T3 : Any, T4 : Any, R : Any> combineLatest() {}
+}
diff --git a/diktat-rules/src/test/resources/test/paragraph3/long_line/LongStringTemplateTest.kt b/diktat-rules/src/test/resources/test/paragraph3/long_line/LongStringTemplateTest.kt
new file mode 100644
index 00000000..bff4d2ad
--- /dev/null
+++ b/diktat-rules/src/test/resources/test/paragraph3/long_line/LongStringTemplateTest.kt
@@ -0,0 +1,10 @@
+object Observables {
+    val someCode = 15 // looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongggggggggggggggggggggggggg
+    // Some looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong line
+    @Deprecated("New type inference algorithm in Kotlin 1.4 makes this method obsolete. Method will be removed in future RxKotlin release.",
+        replaceWith = ReplaceWith("Observable.combineLatest(source1, source2, source3, source4, combineFunction)", "io.reactivex.Observable"),
+        level = DeprecationLevel.WARNING)
+    @CheckReturnValue
+    @SchedulerSupport(SchedulerSupport.NONE)
+    inline fun <T1 : Any, T2 : Any, T3 : Any, T4 : Any, R : Any> combineLatest() {}
+}