diff options
author | nickreid <nickreid@google.com> | 2022-03-21 06:37:13 -0700 |
---|---|---|
committer | Facebook GitHub Bot <facebook-github-bot@users.noreply.github.com> | 2022-03-21 06:37:13 -0700 |
commit | a66fbc41530271027b139277f72e8bc2182a0ba5 (patch) | |
tree | 83925fa15b878e9e3183c11caac8814fc3a56697 /core | |
parent | f3c2964c4607b4d899cf6aabf5285b9b291e1aed (diff) | |
download | ktfmt-a66fbc41530271027b139277f72e8bc2182a0ba5.tar.gz |
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
Diffstat (limited to 'core')
-rw-r--r-- | core/src/main/java/com/facebook/ktfmt/format/RedundantSemicolonDetector.kt | 15 | ||||
-rw-r--r-- | core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt | 31 |
2 files changed, 43 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 diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt index 3abf188..59213e4 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt @@ -3183,6 +3183,19 @@ class FormatterTest { | foo(0) { trailing -> lambda }; { dead -> lambda } | | foo { trailing -> lambda }; { dead -> lambda } + | + | val x = foo(); { dead -> lambda } + | + | val x = bar() && foo(); { dead -> lambda } + | + | // `z` has a property and a method both named `bar` + | val x = z.bar; { dead -> lambda } + | + | // `this` has a property and a method both named `bar` + | val x = bar; { dead -> lambda } + | + | // Literally any callable expression is dangerous + | val x = (if (cond) x::foo else x::bar); { dead -> lambda } |} |""".trimMargin() val expected = @@ -3205,6 +3218,24 @@ class FormatterTest { | | foo { trailing -> lambda }; | { dead -> lambda } + | + | val x = foo(); + | { dead -> lambda } + | + | val x = bar() && foo(); + | { dead -> lambda } + | + | // `z` has a property and a method both named `bar` + | val x = z.bar; + | { dead -> lambda } + | + | // `this` has a property and a method both named `bar` + | val x = bar; + | { dead -> lambda } + | + | // Literally any callable expression is dangerous + | val x = (if (cond) x::foo else x::bar); + | { dead -> lambda } |} |""".trimMargin() assertThatFormatting(code).isEqualTo(expected) |