From 623431bd7ec1e8e709457d347bd6f6baea3dac9a Mon Sep 17 00:00:00 2001 From: Paul Dingemans Date: Wed, 28 Feb 2024 17:50:58 +0100 Subject: [PATCH] Release testing bugs 1.2.0 (#2570) * A when-condition preceded by a comment should be treated as a multiline condition and as of that lead to adding blank lines between the when-conditions * Rename BackingPropertyRuleTest to BackingPropertyNamingRuleTest * Prevent conflict between `string-template-indent` and `multiline-expression` when function body expression is a multiline string template --- .../snapshot/docs/rules/experimental.md | 24 +++++++++++++-- .../rules/ArgumentListWrappingRule.kt | 2 ++ .../rules/BlankLineBetweenWhenConditions.kt | 8 ++++- .../rules/ParameterListWrappingRule.kt | 1 + .../rules/StringTemplateIndentRule.kt | 17 +++++++++-- ...st.kt => BackingPropertyNamingRuleTest.kt} | 2 +- .../BlankLineBetweenWhenConditionsTest.kt | 29 ++++++++++++++++++- .../rules/StringTemplateIndentRuleTest.kt | 18 ++++++++++++ 8 files changed, 94 insertions(+), 7 deletions(-) rename ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/{BackingPropertyRuleTest.kt => BackingPropertyNamingRuleTest.kt} (99%) diff --git a/documentation/snapshot/docs/rules/experimental.md b/documentation/snapshot/docs/rules/experimental.md index 00c0e49950..0b1e105c5d 100644 --- a/documentation/snapshot/docs/rules/experimental.md +++ b/documentation/snapshot/docs/rules/experimental.md @@ -104,8 +104,19 @@ Consistently add or remove blank lines between when-conditions in a when-stateme else -> null } + // ij_kotlin_line_break_after_multiline_when_entry = true + val foo3 = + when (bar) { + BAR1 -> "bar1" + + // BAR2 comment + BAR2 -> "bar2" + + else -> null + } + // ij_kotlin_line_break_after_multiline_when_entry = false - val foo2 = + val foo4 = when (bar) { BAR1 -> "bar1" BAR2 -> { @@ -138,8 +149,17 @@ Consistently add or remove blank lines between when-conditions in a when-stateme else -> null } + // ij_kotlin_line_break_after_multiline_when_entry = true (missing newline after BAR1, and BAR2) + val foo3 = + when (bar) { + BAR1 -> "bar1" + // BAR2 comment + BAR2 -> "bar2" + else -> null + } + // ij_kotlin_line_break_after_multiline_when_entry = false (unexpected newline after BAR2) - val foo2 = + val foo4 = when (bar) { BAR1 -> "bar1" BAR2 -> { diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt index 9ca13e70dd..7bb95cb356 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt @@ -148,6 +148,7 @@ public class ArgumentListWrappingRule : // 2 // ) child.treeParent.hasTypeArgumentListInFront() -> -1 + // IDEA quirk: // foo // .bar = Baz( @@ -161,6 +162,7 @@ public class ArgumentListWrappingRule : // 2 // ) child.treeParent.isPartOfDotQualifiedAssignmentExpression() -> -1 + else -> 0 }.let { if (child.treeParent.isOnSameLineAsControlFlowKeyword()) { diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditions.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditions.kt index 5978a9d6f1..5298463306 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditions.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditions.kt @@ -10,10 +10,12 @@ import com.pinterest.ktlint.rule.engine.core.api.children import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfigProperty import com.pinterest.ktlint.rule.engine.core.api.indent +import com.pinterest.ktlint.rule.engine.core.api.isPartOfComment import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace import com.pinterest.ktlint.rule.engine.core.api.lastChildLeafOrSelf import com.pinterest.ktlint.rule.engine.core.api.nextLeaf import com.pinterest.ktlint.rule.engine.core.api.prevCodeSibling +import com.pinterest.ktlint.rule.engine.core.api.prevSibling import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe import com.pinterest.ktlint.ruleset.standard.StandardRule import org.ec4j.core.model.PropertyType @@ -92,7 +94,11 @@ public class BlankLineBetweenWhenConditions : private fun ASTNode.containsBlankLine(): Boolean = elementType == WHITE_SPACE && text.count { it == '\n' } > 1 - private fun ASTNode.hasAnyMultilineWhenCondition() = children().any { it.elementType == WHEN_ENTRY && it.textContains('\n') } + private fun ASTNode.hasAnyMultilineWhenCondition() = + children() + .any { it.elementType == WHEN_ENTRY && (it.textContains('\n') || it.isPrecededByComment()) } + + private fun ASTNode.isPrecededByComment() = prevSibling { !it.isWhiteSpace() }?.isPartOfComment() ?: false private fun ASTNode.findWhitespaceAfterPreviousCodeSibling() = prevCodeSibling() diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRule.kt index e4d32d89ed..3ff2a962e0 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRule.kt @@ -224,6 +224,7 @@ public class ParameterListWrappingRule : // param2: R // ) child.treeParent.isFunWithTypeParameterListInFront() -> -1 + else -> 0 }.let { if (child.elementType == VALUE_PARAMETER) { diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRule.kt index 6d46186d12..a9ee1a232f 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRule.kt @@ -1,5 +1,6 @@ package com.pinterest.ktlint.ruleset.standard.rules +import com.pinterest.ktlint.rule.engine.core.api.ElementType import com.pinterest.ktlint.rule.engine.core.api.ElementType.CALL_EXPRESSION import com.pinterest.ktlint.rule.engine.core.api.ElementType.CLOSING_QUOTE import com.pinterest.ktlint.rule.engine.core.api.ElementType.COMMA @@ -43,8 +44,6 @@ public class StringTemplateIndentRule : id = "string-template-indent", visitorModifiers = setOf( - // Wrap all multiline string templates to a separate line -// VisitorModifier.RunAfterRule(MULTILINE_EXPRESSION_WRAPPING_RULE_ID, ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED), // The IndentationRule first needs to fix the indentation of the opening quotes of the string template. The indentation inside // the string template is relative to the opening quotes. Running this rule before the IndentationRule results in a wrong // indentation whenever the indent level of the root of the string template is changed. @@ -99,6 +98,7 @@ public class StringTemplateIndentRule : stringTemplate .takeUnless { it.isPrecededByWhitespaceWithNewline() } ?.takeUnless { it.isPrecededByReturnKeyword() } + ?.takeUnless { it.isFunctionBodyExpressionOnSameLine() } ?.let { whiteSpace -> emit(stringTemplate.startOffset, "Expected newline before multiline string template", true) if (autoCorrect) { @@ -151,6 +151,19 @@ public class StringTemplateIndentRule : // """ prevCodeLeaf()?.elementType == RETURN_KEYWORD + private fun ASTNode.isFunctionBodyExpressionOnSameLine() = + prevCodeLeaf() + ?.takeIf { it.elementType == ElementType.EQ } + ?.closingParenthesisOfFunctionOrNull() + ?.prevLeaf() + ?.let { it.isWhiteSpaceWithNewline() } + ?: false + + private fun ASTNode.closingParenthesisOfFunctionOrNull() = + takeIf { treeParent.elementType == ElementType.FUN } + ?.prevCodeLeaf() + ?.takeIf { it.elementType == ElementType.RPAR } + private fun ASTNode.getIndent(): String { // When executing this rule, the indentation rule may not have run yet. The functionality to determine the correct indentation level // is out of scope of this rule as it is owned by the indentation rule. Therefore, the indentation of the line at which the diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BackingPropertyRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BackingPropertyNamingRuleTest.kt similarity index 99% rename from ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BackingPropertyRuleTest.kt rename to ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BackingPropertyNamingRuleTest.kt index 4111ffc408..f6d3eaeb20 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BackingPropertyRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BackingPropertyNamingRuleTest.kt @@ -13,7 +13,7 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.CsvSource import org.junit.jupiter.params.provider.ValueSource -class BackingPropertyRuleTest { +class BackingPropertyNamingRuleTest { private val backingPropertyNamingRuleAssertThat = assertThatRule { BackingPropertyNamingRule() } @Nested diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditionsTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditionsTest.kt index 828fca3d87..c427437fcc 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditionsTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditionsTest.kt @@ -122,7 +122,7 @@ class BlankLineBetweenWhenConditionsTest { } @Test - fun `Given xx a when-condition preceded by a comment and the when-condition needs to be preceded by blank line then add this line before the comment`() { + fun `Given a when-condition with a single line body on the next line then add blank lines`() { val code = """ val foo = @@ -185,6 +185,33 @@ class BlankLineBetweenWhenConditionsTest { .hasLintViolation(4, 1, "Add a blank line between all when-condition in case at least one multiline when-condition is found in the statement") .isFormattedAs(formattedCode) } + + @Test + fun `Given a simple when-statement in which a when condition is preceded by a comment then add blank lines between the when-conditions`() { + val code = + """ + val foo = + when (bar) { + BAR -> "bar" + // Some comment + else -> null + } + """.trimIndent() + val formattedCode = + """ + val foo = + when (bar) { + BAR -> "bar" + + // Some comment + else -> null + } + """.trimIndent() + @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") + blankLineAfterWhenConditionRuleAssertThat(code) + .hasLintViolation(4, 1, "Add a blank line between all when-condition in case at least one multiline when-condition is found in the statement") + .isFormattedAs(formattedCode) + } } @Nested diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRuleTest.kt index 6b233b3bed..71bf2f3dcc 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRuleTest.kt @@ -470,4 +470,22 @@ class StringTemplateIndentRuleTest { """.trimIndent().replaceStringTemplatePlaceholder() stringTemplateIndentRuleAssertThat(code).hasNoLintViolations() } + + @Test + fun `Given a string template on same line as closing parenthesis of multiline function signature`() { + // Interpret "$." in code sample below as "$". It is used whenever the code which has to be inspected should + // actually contain a string template. Using "$" instead of "$." would result in a String in which the string + // templates would have been evaluated before the code would actually be processed by the rule. + val code = + """ + fun foo( + bar: String + ) = $MULTILINE_STRING_QUOTE + Bar: $.bar + $MULTILINE_STRING_QUOTE.trimIndent() + """.trimIndent() + stringTemplateIndentRuleAssertThat(code) + .addAdditionalRuleProvider { MultilineExpressionWrappingRule() } + .hasNoLintViolations() + } }