Always preserve semicolons preceding lambda expressions, even when they might be meaningless (#301)

Summary:
There are a huge number of cases where removal might change behaviour, because the trailing
lambda syntax is so flexible. Therefore, we just assume that all semicolons followed by
lambdas are meaningful. The cases where they could be removed are too rare to justify the
risk of changing behaviour.

https://github.com/facebookincubator/ktfmt/pull/278 tried to fix this earlier, but missed
a lot of cases. Some examples are added to the tests.

Pull Request resolved: https://github.com/facebookincubator/ktfmt/pull/301

Reviewed By: strulovich

Differential Revision: D34999205

Pulled By: cgrushko

fbshipit-source-id: 7f1bc75ba450e1e98d7b764f93bc613188dfa5cd

1 files changed, 12 insertions, 3 deletions

diff --git a/core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt b/core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt
index 344a467..3f6601b 100644
--- a/core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt
+++ b/core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt
@@ -17,7 +17,6 @@
 package com.facebook.ktfmt.format
 
 import org.jetbrains.kotlin.com.intellij.psi.PsiElement
-import org.jetbrains.kotlin.psi.KtCallExpression
 import org.jetbrains.kotlin.psi.KtContainerNodeForControlStructureBody
 import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.psi.KtEnumEntry
@@ -49,6 +48,7 @@ internal class RedundantSemicolonDetector {
     if (element.text != ";") {
       return false
     }
+
     val parent = element.parent
     if (parent is KtStringTemplateExpression || parent is KtStringTemplateEntry) {
       return false
@@ -57,6 +57,7 @@ internal class RedundantSemicolonDetector {
         parent.siblings(forward = true, withItself = false).any { it is KtDeclaration }) {
       return false
     }
+
     val prevLeaf = element.prevLeaf(false)
     val prevConcreteSibling = element.getPrevSiblingIgnoringWhitespaceAndComments()
     if ((prevConcreteSibling is KtIfExpression || prevConcreteSibling is KtWhileExpression) &&
@@ -64,9 +65,17 @@ internal class RedundantSemicolonDetector {
         prevLeaf.text.isEmpty()) {
       return false
     }
+
     val nextConcreteSibling = element.getNextSiblingIgnoringWhitespaceAndComments()
-    if (prevConcreteSibling is KtCallExpression && nextConcreteSibling is KtLambdaExpression) {
-      return false // Example: `foo(0); { dead -> lambda }`
+    if (nextConcreteSibling is KtLambdaExpression) {
+      /**
+       * Example: `val x = foo(0) ; { dead -> lambda }`
+       *
+       * There are a huge number of cases here because the trailing lambda syntax is so flexible.
+       * Therefore, we just assume that all semicolons followed by lambdas are meaningful. The cases
+       * where they could be removed are too rare to justify the risk of changing behaviour.
+       */
+      return false
     }
 
     return true