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

Fix solving problems in 3 consecutive runs #2132

Merged
merged 3 commits into from
Jul 15, 2023
Merged

Fix solving problems in 3 consecutive runs #2132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b68bed5
Display log message `Format was not able to resolve all violations wh…
paul-dingemans Jul 15, 2023
49eb918
Fix wrapping of function signature in case the opening brace of the f…
paul-dingemans Jul 15, 2023
5c28144
Merge branch 'master' into 2130-upsert-whitespace
paul-dingemans Jul 15, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
* Allow to disable ktlint in `.editorconfig` for a glob ([#2100](https://github.com/pinterest/ktlint/issues/2100))
* Fix wrapping of nested function literals `wrapping` ([#2106](https://github.com/pinterest/ktlint/issues/2106))
* Do not indent class body for classes having a long super type list in code style `ktlint_official` as it is inconsistent compared to other class bodies `indent` [#2115](https://github.com/pinterest/ktlint/issues/2115)
* Log message `Format was not able to resolve all violations which (theoretically) can be autocorrected in file ... in 3 consecutive runs of format` is now only displayed in case a new ktlint rule is actually needed. [#2129](https://github.com/pinterest/ktlint/issues/2129)
* Fix wrapping of function signature in case the opening brace of the function body block exceeds the maximum line length. Fix upsert of whitespace into composite nodes. `function-signature` [#2130](https://github.com/pinterest/ktlint/issues/2130)
* Fix spacing around colon in annotations `spacing-around-colon` ([#2093](https://github.com/pinterest/ktlint/issues/2093))

### Changed
Expand Down
38 changes: 33 additions & 5 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 @@ -245,11 +245,23 @@ public fun ASTNode.upsertWhitespaceBeforeMe(text: String) {
}
}
} else {
val prevLeaf =
requireNotNull(prevLeaf()) {
"Can not upsert a whitespace if the first node is a non-leaf node"
when (val prevSibling = prevSibling()) {
null -> {
// Never insert a whitespace element as first child node in a composite node. Instead, upsert just before the composite node
treeParent.upsertWhitespaceBeforeMe(text)
}
prevLeaf.upsertWhitespaceAfterMe(text)

is LeafElement -> {
prevSibling.upsertWhitespaceAfterMe(text)
}

else -> {
// Insert in between two composite nodes
PsiWhiteSpaceImpl(text).also { psiWhiteSpace ->
treeParent.addChild(psiWhiteSpace.node, this)
}
}
}
}
}

Expand Down Expand Up @@ -279,7 +291,23 @@ public fun ASTNode.upsertWhitespaceAfterMe(text: String) {
}
}
} else {
lastChildLeafOrSelf().upsertWhitespaceAfterMe(text)
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)
}

is LeafElement -> {
nextSibling.upsertWhitespaceBeforeMe(text)
}

else -> {
// Insert in between two composite nodes
PsiWhiteSpaceImpl(text).also { psiWhiteSpace ->
treeParent.addChild(psiWhiteSpace.node, nextSibling)
}
}
}
}
}

Expand Down
137 changes: 108 additions & 29 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 @@ -2,13 +2,16 @@ package com.pinterest.ktlint.rule.engine.core.api

import com.pinterest.ktlint.rule.engine.api.Code
import com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine
import com.pinterest.ktlint.rule.engine.core.api.ElementType.ANNOTATION_ENTRY
import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLASS
import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLASS_BODY
import com.pinterest.ktlint.rule.engine.core.api.ElementType.ENUM_ENTRY
import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN
import com.pinterest.ktlint.rule.engine.core.api.ElementType.IDENTIFIER
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.RPAR
import com.pinterest.ktlint.rule.engine.core.api.ElementType.TYPE_REFERENCE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_PARAMETER
import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_PARAMETER_LIST
import com.pinterest.ktlint.rule.engine.core.api.ElementType.WHITE_SPACE
Expand Down Expand Up @@ -303,7 +306,7 @@ class ASTNodeExtensionTest {
@Nested
inner class UpsertWhitespaceBeforeMe {
@Test
fun `Given a whitespace node and upsert a whitespace before the node (RPAR) then replace the current whitespace element`() {
fun `Given an upsert of a whitespace based before another whitespace node then replace the existing whitespace element`() {
val code =
"""
fun foo( ) = 42
Expand All @@ -327,10 +330,10 @@ class ASTNodeExtensionTest {
}

@Test
fun `Given a node (RPAR) which is preceded by a non-whitespace leaf element (LPAR) and upsert a whitespace before the node (RPAR) then create a new whitespace element before the node (RPAR)`() {
fun `Given an upsert of a whitespace before a non-whitespace node (RPAR) which is preceded by a whitespace leaf element (LPAR) then then replace the whitespace element`() {
val code =
"""
fun foo() = 42
fun foo( ) = 42
""".trimIndent()
val formattedCode =
"""
Expand All @@ -350,10 +353,10 @@ class ASTNodeExtensionTest {
}

@Test
fun `Given a node (RPAR) which is preceded by a whitespace leaf element and upsert a whitespace before the node (RPAR) then replace the whitespace element before the node (RPAR)`() {
fun `Given an upsert of a whitespace before a non-whitespace node (RPAR) which is preceded by another non-whitespace leaf element (LPAR) then create a new whitespace element`() {
val code =
"""
fun foo( ) = 42
fun foo() = 42
""".trimIndent()
val formattedCode =
"""
Expand All @@ -373,7 +376,33 @@ class ASTNodeExtensionTest {
}

@Test
fun `Given a node (VALUE_PARAMETER) which is preceded by a non-whitespace leaf element (LPAR) and upsert a whitespace before the node (VALUE_PARAMETER) then create a new whitespace element before the node (VALUE_PARAMETER)`() {
fun `Given an upsert of a whitespace before a composite node (ANNOTATION_ENTRY) which is the first child of another composite element then create a new whitespace before (VALUE_PARAMETER)`() {
val code =
"""
fun foo(@Bar string: String) = 42
""".trimIndent()
val formattedCode =
"""
fun foo(
@Bar string: String) = 42
""".trimIndent()

val actual =
code
.transformAst {
findChildByType(FUN)
?.findChildByType(VALUE_PARAMETER_LIST)
?.findChildByType(VALUE_PARAMETER)
?.findChildByType(MODIFIER_LIST)
?.findChildByType(ANNOTATION_ENTRY)
?.upsertWhitespaceBeforeMe("\n ")
}.text

assertThat(actual).isEqualTo(formattedCode)
}

@Test
fun `Given an upsert of a whitespace before a composite node (VALUE_PARAMETER) which is preceded by a non-whitespace leaf element (LPAR) then create a new whitespace element before the node (VALUE_PARAMETER)`() {
val code =
"""
fun foo(string: String) = 42
Expand All @@ -397,25 +426,26 @@ class ASTNodeExtensionTest {
}

@Test
fun `Given a node (FUN bar) which is preceded by a composite element (FUN foo) and upsert a whitespace before the node (FUN bar) then create a new whitespace element before the node (FUN bar)`() {
fun `Given an upsert of a whitespace before a composite node (ANNOTATION_ENTRY) which is preceded by another composite node (ANNOTATION_ENTRY) then create a new whitespace between the ANNOTATION_ENTRIES`() {
val code =
"""
fun foo() = "foo"
fun bar() = "bar"
fun foo(@Bar@Foo string: String) = 42
""".trimIndent()
val formattedCode =
"""
fun foo() = "foo"

fun bar() = "bar"
fun foo(@Bar @Foo string: String) = 42
""".trimIndent()

val actual =
code
.transformAst {
children()
.last { it.elementType == FUN }
.upsertWhitespaceBeforeMe("\n\n")
findChildByType(FUN)
?.findChildByType(VALUE_PARAMETER_LIST)
?.findChildByType(VALUE_PARAMETER)
?.findChildByType(MODIFIER_LIST)
?.findChildByType(ANNOTATION_ENTRY)
?.nextSibling()
?.upsertWhitespaceBeforeMe(" ")
}.text

assertThat(actual).isEqualTo(formattedCode)
Expand All @@ -425,10 +455,34 @@ class ASTNodeExtensionTest {
@Nested
inner class UpsertWhitespaceAfterMe {
@Test
fun `Given a node (LPAR) which is followed by a non-whitespace leaf element (RPAR) and upsert a whitespace after the node (LPAR) then create a new whitespace element after the node (LPAR)`() {
fun `Given an upsert of a whitespace based after another whitespace node then replace the existing whitespace element`() {
val code =
"""
fun foo() = 42
fun foo( ) = 42
""".trimIndent()
val formattedCode =
"""
fun foo(
) = 42
""".trimIndent()

val actual =
code
.transformAst {
findChildByType(FUN)
?.findChildByType(VALUE_PARAMETER_LIST)
?.findChildByType(WHITE_SPACE)
?.upsertWhitespaceAfterMe("\n")
}.text

assertThat(actual).isEqualTo(formattedCode)
}

@Test
fun `Given an upsert of a whitespace after a non-whitespace node (LPAR) which is followed by a whitespace leaf element (RPAR) then then replace the whitespace element`() {
val code =
"""
fun foo( ) = 42
""".trimIndent()
val formattedCode =
"""
Expand All @@ -448,10 +502,10 @@ class ASTNodeExtensionTest {
}

@Test
fun `Given a node (LPAR) which is followed by a whitespace leaf element and upsert a whitespace after the node (LPAR) then replace the whitespace element after the node (LPAR)`() {
fun `Given an upsert of a whitespace after a non-whitespace node (LPAR) which is followed by another non-whitespace leaf element (RPAR) then create a new whitespace element`() {
val code =
"""
fun foo( ) = 42
fun foo() = 42
""".trimIndent()
val formattedCode =
"""
Expand All @@ -471,7 +525,32 @@ class ASTNodeExtensionTest {
}

@Test
fun `Given a node (VALUE_PARAMETER) which is followed by a non-whitespace leaf element (RPAR) and upsert a whitespace after the node (VALUE_PARAMETER) then create a new whitespace element after the node (VALUE_PARAMETER)`() {
fun `Given an upsert of a whitespace after a composite node (ANNOTATION_ENTRY) which is the last child of another composite element then create a new whitespace after (VALUE_PARAMETER)`() {
val code =
"""
fun foo(@Bar string: String) = 42
""".trimIndent()
val formattedCode =
"""
fun foo(@Bar string: String
) = 42
""".trimIndent()

val actual =
code
.transformAst {
findChildByType(FUN)
?.findChildByType(VALUE_PARAMETER_LIST)
?.findChildByType(VALUE_PARAMETER)
?.findChildByType(TYPE_REFERENCE)
?.upsertWhitespaceAfterMe("\n")
}.text

assertThat(actual).isEqualTo(formattedCode)
}

@Test
fun `Given an upsert of a whitespace after a composite node (VALUE_PARAMETER) which is followed by a non-whitespace leaf element (RPAR) then create a new whitespace element after the node (VALUE_PARAMETER)`() {
val code =
"""
fun foo(string: String) = 42
Expand All @@ -495,25 +574,25 @@ class ASTNodeExtensionTest {
}

@Test
fun `Given a node (FUN foo) which is followed by a composite element (FUN bar) and upsert a whitespace after the node (FUN foo) then create a new whitespace element after the node (FUN foo)`() {
fun `Given an upsert of a whitespace after a composite node (ANNOTATION_ENTRY) which is followed by another composite node (ANNOTATION_ENTRY) then create a new whitespace between the ANNOTATION_ENTRIES`() {
val code =
"""
fun foo() = "foo"
fun bar() = "bar"
fun foo(@Bar@Foo string: String) = 42
""".trimIndent()
val formattedCode =
"""
fun foo() = "foo"

fun bar() = "bar"
fun foo(@Bar @Foo string: String) = 42
""".trimIndent()

val actual =
code
.transformAst {
children()
.first { it.elementType == FUN }
.upsertWhitespaceAfterMe("\n\n")
findChildByType(FUN)
?.findChildByType(VALUE_PARAMETER_LIST)
?.findChildByType(VALUE_PARAMETER)
?.findChildByType(MODIFIER_LIST)
?.findChildByType(ANNOTATION_ENTRY)
?.upsertWhitespaceAfterMe(" ")
}.text

assertThat(actual).isEqualTo(formattedCode)
Expand Down
27 changes: 24 additions & 3 deletions ktlint-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/api/KtLintRuleEngine.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,30 @@ public class KtLintRuleEngine(
}
} while (mutated && formatRunCount < MAX_FORMAT_RUNS_PER_FILE)
if (formatRunCount == MAX_FORMAT_RUNS_PER_FILE && mutated) {
LOGGER.warn {
"Format was not able to resolve all violations which (theoretically) can be autocorrected in file " +
"${code.filePathOrStdin()} in $MAX_FORMAT_RUNS_PER_FILE consecutive runs of format."
// It is unknown if the last format run introduces new lint violations which can be autocorrected. So run lint once more so that
// the user can be informed about this correctly.
var hasErrorsWhichCanBeAutocorrected = false
visitorProvider
.visitor()
.invoke { rule ->
ruleExecutionContext.executeRule(rule, false) { offset, message, canBeAutoCorrected ->
if (canBeAutoCorrected) {
ruleExecutionContext.rebuildSuppressionLocator()
val formattedCode =
ruleExecutionContext
.rootNode
.text
.replace("\n", ruleExecutionContext.determineLineSeparator(code.content))
val (line, col) = ruleExecutionContext.positionInTextLocator(offset)
hasErrorsWhichCanBeAutocorrected = true
}
}
}
if (hasErrorsWhichCanBeAutocorrected) {
LOGGER.warn {
"Format was not able to resolve all violations which (theoretically) can be autocorrected in file " +
"${code.filePathOrStdin()} in $MAX_FORMAT_RUNS_PER_FILE consecutive runs of format."
}
}
}

Expand Down
Loading