Skip to content

Commit

Permalink
Make sure not to remove valid semicolons
Browse files Browse the repository at this point in the history
Summary:
Fixes #459, which solves this code here being turned invalid by ktfmt

```
fun doSearch() {
    _results.postValue(Resource.Loading());

    {
        performSearch(searchParams)
    }
    .multiCatch(
        SocketException::class,
        SocketTimeoutException::class,
        SSLException::class
    ) {
        // Activity suspended while parsing stream
        _results.postValue(Resource.Error(it, null))
    }
}
```

Reviewed By: strulovich

Differential Revision: D58465670

fbshipit-source-id: ac2c5ea174f79d21bab5ca0ef62eb3bf69848b95
  • Loading branch information
Nivaldo Bondança authored and facebook-github-bot committed Jun 12, 2024
1 parent e90d4fc commit 5d5b0f7
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,10 @@

package com.facebook.ktfmt.format

import org.jetbrains.kotlin.com.intellij.psi.PsiComment
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.KtClassBody
import org.jetbrains.kotlin.psi.KtContainerNodeForControlStructureBody
import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtEnumEntry
import org.jetbrains.kotlin.psi.KtIfExpression
import org.jetbrains.kotlin.psi.KtLambdaExpression
Expand All @@ -31,7 +29,6 @@ import org.jetbrains.kotlin.psi.KtWhileExpression
import org.jetbrains.kotlin.psi.psiUtil.getNextSiblingIgnoringWhitespaceAndComments
import org.jetbrains.kotlin.psi.psiUtil.getPrevSiblingIgnoringWhitespaceAndComments
import org.jetbrains.kotlin.psi.psiUtil.prevLeaf
import org.jetbrains.kotlin.psi.psiUtil.siblings

internal class RedundantSemicolonDetector {
private val extraSemicolons = mutableListOf<PsiElement>()
Expand Down Expand Up @@ -66,20 +63,6 @@ internal class RedundantSemicolonDetector {
return element != enumEntryList.terminatingSemicolon || parent.children.isEmpty()
}

if (parent is KtClassBody) {
val grandParent = parent.parent
if (grandParent is KtClass && grandParent.isEnum()) {
// Don't remove the first semicolon on non-empty enum.
if (element.getPrevSiblingIgnoringWhitespaceAndComments()?.text == "{" &&
element
.siblings(forward = true, withItself = false)
.filter { it !is PsiWhiteSpace && it !is PsiComment && it.text != ";" }
.firstOrNull()
?.text != "}")
return false
}
}

val prevLeaf = element.prevLeaf(false)
val prevConcreteSibling = element.getPrevSiblingIgnoringWhitespaceAndComments()
if ((prevConcreteSibling is KtIfExpression || prevConcreteSibling is KtWhileExpression) &&
Expand All @@ -89,17 +72,23 @@ internal class RedundantSemicolonDetector {
}

val nextConcreteSibling = element.getNextSiblingIgnoringWhitespaceAndComments()
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
/**
* Examples:
* ```
* val x = foo(0) ; { dead -> lambda }
* val y = foo(1) ; { dead -> lambda }.bar()
* ```
*
* 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.
*/
val nextSiblingIsLambda =
(nextConcreteSibling is KtLambdaExpression ||
(nextConcreteSibling is KtDotQualifiedExpression &&
nextConcreteSibling.receiverExpression is KtLambdaExpression))

return !nextSiblingIsLambda
}
}
5 changes: 5 additions & 0 deletions core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4164,6 +4164,8 @@ class FormatterTest {
|
| // Literally any callable expression is dangerous
| val x = (if (cond) x::foo else x::bar); { dead -> lambda }
|
| funcCall(); { dead -> lambda }.withChained(call)
|}
|"""
.trimMargin()
Expand Down Expand Up @@ -4205,6 +4207,9 @@ class FormatterTest {
| // Literally any callable expression is dangerous
| val x = (if (cond) x::foo else x::bar);
| { dead -> lambda }
|
| funcCall();
| { dead -> lambda }.withChained(call)
|}
|"""
.trimMargin()
Expand Down

0 comments on commit 5d5b0f7

Please sign in to comment.