Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not insert a whitespace element as first or last child inside a composite element #2715

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Do not insert a whitespace element as first or last child inside a composite element #2715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a93fdc
Prevent inserting whitespace element as first or last element in a co…
paul-dingemans Jun 25, 2024
39ebb36
Prevent inserting whitespace element as first or last element in a co…
paul-dingemans Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions ...engine-core/src/main/kotlin/com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtension.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,20 @@ public fun ASTNode.upsertWhitespaceBeforeMe(text: String) {
return replaceWhitespaceWith(text)
}
val previous = treePrev ?: prevLeaf()
if (previous != null && previous.elementType == WHITE_SPACE) {
previous.replaceWhitespaceWith(text)
} else {
PsiWhiteSpaceImpl(text).also { psiWhiteSpace ->
(psi as LeafElement).rawInsertBeforeMe(psiWhiteSpace)
when {
previous?.elementType == WHITE_SPACE -> {
previous.replaceWhitespaceWith(text)
}

treeParent.firstChildNode == this -> {
// Never insert a whitespace node as first node in a composite node
treeParent.upsertWhitespaceBeforeMe(text)
}

else -> {
PsiWhiteSpaceImpl(text).also { psiWhiteSpace ->
(psi as LeafElement).rawInsertBeforeMe(psiWhiteSpace)
}
}
}
} else {
Expand Down Expand Up @@ -284,18 +293,27 @@ public fun ASTNode.upsertWhitespaceAfterMe(text: String) {
return replaceWhitespaceWith(text)
}
val next = treeNext ?: nextLeaf()
if (next != null && next.elementType == WHITE_SPACE) {
next.replaceWhitespaceWith(text)
} else {
PsiWhiteSpaceImpl(text).also { psiWhiteSpace ->
(psi as LeafElement).rawInsertAfterMe(psiWhiteSpace)
when {
next?.elementType == WHITE_SPACE -> {
next.replaceWhitespaceWith(text)
}

treeParent.lastChildNode == this -> {
// Never insert a whitespace as last node in a composite node
treeParent.upsertWhitespaceAfterMe(text)
}

else -> {
PsiWhiteSpaceImpl(text).also { psiWhiteSpace ->
(psi as LeafElement).rawInsertAfterMe(psiWhiteSpace)
}
}
}
} else {
when (val nextSibling = nextSibling()) {
null -> {
// Never insert a whitespace element as last child node in a composite node. Instead, upsert just after the composite node
treeParent.upsertWhitespaceAfterMe(text)
treeParent?.upsertWhitespaceAfterMe(text)
}

is LeafElement -> {
Expand Down
51 changes: 51 additions & 0 deletions ...ne-core/src/test/kotlin/com/pinterest/ktlint/rule/engine/core/api/ASTNodeExtensionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.FILE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN
import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN_KEYWORD
import com.pinterest.ktlint.rule.engine.core.api.ElementType.IDENTIFIER
import com.pinterest.ktlint.rule.engine.core.api.ElementType.LBRACE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.LPAR
import com.pinterest.ktlint.rule.engine.core.api.ElementType.MODIFIER_LIST
import com.pinterest.ktlint.rule.engine.core.api.ElementType.PRIVATE_KEYWORD
Expand Down Expand Up @@ -457,6 +458,31 @@ class ASTNodeExtensionTest {

assertThat(actual).isEqualTo(formattedCode)
}

@Test
fun `Issue 2688 - Given a class without a space between the identifier and the left curly brace then insert the whitespace at the correct position in the AST`() {
val code =
"""
class Foo{
// some comment
}
""".trimIndent()

val actual =
code
.transformAst {
findChildByType(CLASS)
?.findChildByType(CLASS_BODY)
?.findChildByType(LBRACE)
?.upsertWhitespaceBeforeMe(" ")
}.findChildByType(CLASS)
?.findChildByType(CLASS_BODY)
?.prevSibling()
?.let { it.elementType == WHITE_SPACE && it.text == " " }
?: false

assertThat(actual).isTrue()
}
}

@Nested
Expand Down Expand Up @@ -604,6 +630,31 @@ class ASTNodeExtensionTest {

assertThat(actual).isEqualTo(formattedCode)
}

@Test
fun `Issue 2688 - Given a class without a space between the identifier and the left curly brace then insert the whitespace at the correct position in the AST`() {
val code =
"""
fun foo(){
// some comment
}
""".trimIndent()

val actual =
code
.transformAst {
findChildByType(FUN)
?.findChildByType(VALUE_PARAMETER_LIST)
?.findChildByType(RPAR)
?.upsertWhitespaceAfterMe(" ")
}.findChildByType(FUN)
?.findChildByType(VALUE_PARAMETER_LIST)
?.nextSibling()
?.let { it.elementType == WHITE_SPACE && it.text == " " }
?: false

assertThat(actual).isTrue()
}
}

@Test
Expand Down