From e6c493094eaea47504703faedf0636788e76cc87 Mon Sep 17 00:00:00 2001 From: Phil Davies Date: Thu, 1 Aug 2024 14:11:23 +0100 Subject: [PATCH 1/2] fix: don't remove arrow from lamdas thare are when/if leaf nodes --- .../standard/rules/FunctionLiteralRule.kt | 8 ++- .../standard/rules/FunctionLiteralRuleTest.kt | 65 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRule.kt index e183e28894..75d1006f66 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRule.kt @@ -4,13 +4,16 @@ import com.pinterest.ktlint.logger.api.initKtLintKLogger import com.pinterest.ktlint.rule.engine.core.api.AutocorrectDecision import com.pinterest.ktlint.rule.engine.core.api.ElementType.ARROW import com.pinterest.ktlint.rule.engine.core.api.ElementType.BLOCK +import com.pinterest.ktlint.rule.engine.core.api.ElementType.ELSE import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUNCTION_LITERAL import com.pinterest.ktlint.rule.engine.core.api.ElementType.LAMBDA_ARGUMENT import com.pinterest.ktlint.rule.engine.core.api.ElementType.LAMBDA_EXPRESSION import com.pinterest.ktlint.rule.engine.core.api.ElementType.LBRACE import com.pinterest.ktlint.rule.engine.core.api.ElementType.RBRACE +import com.pinterest.ktlint.rule.engine.core.api.ElementType.THEN 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.WHEN_ENTRY import com.pinterest.ktlint.rule.engine.core.api.IndentConfig import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule.Mode.REGARDLESS_WHETHER_RUN_AFTER_RULE_IS_LOADED_OR_DISABLED import com.pinterest.ktlint.rule.engine.core.api.RuleId @@ -365,7 +368,10 @@ public class FunctionLiteralRule : ) { require(arrow.elementType == ARROW) arrow - .prevSibling { it.elementType == VALUE_PARAMETER_LIST } + .takeIf { + val parent = arrow.parent(LAMBDA_EXPRESSION)?.treeParent?.elementType + parent != WHEN_ENTRY && parent != THEN && parent != ELSE + }?.prevSibling { it.elementType == VALUE_PARAMETER_LIST } ?.takeIf { it.findChildByType(VALUE_PARAMETER) == null && arrow.isFollowedByNonEmptyBlock() } ?.let { emit(arrow.startOffset, "Arrow is redundant when parameter list is empty", true) diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRuleTest.kt index 26a809d6dd..19b8015299 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRuleTest.kt @@ -506,4 +506,69 @@ class FunctionLiteralRuleTest { """.trimIndent() functionLiteralRuleAssertThat(code).hasNoLintViolations() } + + @Test + fun `given function literal with an arrow without parameters arrow literal as leaf of when then do not remove the arrow`() { + val code = + """ + val foo = when { + 1 == 2 -> { -> "hi" } + else -> { -> "ho" } + } + """.trimIndent() + functionLiteralRuleAssertThat(code).hasNoLintViolations() + } + + @Test + fun `given function literal with an arrow without parameters arrow literal not as leaf of when then do remove the arrow`() { + val code = + """ + val foo = when { + else -> { { -> "ho" } } + } + """.trimIndent() + val formattedCode = + """ + val foo = when { + else -> { { "ho" } } + } + """.trimIndent() + functionLiteralRuleAssertThat(code) + .hasLintViolation(2, 17, "Arrow is redundant when parameter list is empty") + .isFormattedAs(formattedCode) + } + + @Test + fun `given function literal with an arrow without parameters arrow literal as leaf of if then do not remove the arrow`() { + val code = + """ + val foo = if (cond) { -> "hi" } else { -> "ho" } + """.trimIndent() + functionLiteralRuleAssertThat(code).hasNoLintViolations() + } + + @Test + fun `given function literal with an arrow without parameters arrow literal not as leaf of if then do remove the arrow`() { + val code = + """ + val foo = if (cond) { + { -> "hi" } + } else { + { -> "ho" } + } + """.trimIndent() + val formattedCode = + """ + val foo = if (cond) { + { "hi" } + } else { + { "ho" } + } + """.trimIndent() + functionLiteralRuleAssertThat(code) + .hasLintViolations( + LintViolation(2, 7, "Arrow is redundant when parameter list is empty"), + LintViolation(4, 7, "Arrow is redundant when parameter list is empty"), + ).isFormattedAs(formattedCode) + } } From dd012b9f66ef4d76a2b2450b6fb0f9ddfbbd0e68 Mon Sep 17 00:00:00 2001 From: Paul Dingemans Date: Tue, 6 Aug 2024 18:16:42 +0200 Subject: [PATCH 2/2] Refactor --- .../standard/rules/FunctionLiteralRule.kt | 32 ++++++++++-- .../standard/rules/FunctionLiteralRuleTest.kt | 49 +++++++++++-------- 2 files changed, 55 insertions(+), 26 deletions(-) diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRule.kt index 75d1006f66..165da8546d 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRule.kt @@ -368,11 +368,10 @@ public class FunctionLiteralRule : ) { require(arrow.elementType == ARROW) arrow - .takeIf { - val parent = arrow.parent(LAMBDA_EXPRESSION)?.treeParent?.elementType - parent != WHEN_ENTRY && parent != THEN && parent != ELSE - }?.prevSibling { it.elementType == VALUE_PARAMETER_LIST } - ?.takeIf { it.findChildByType(VALUE_PARAMETER) == null && arrow.isFollowedByNonEmptyBlock() } + .prevSibling { it.elementType == VALUE_PARAMETER_LIST } + ?.takeIf { it.hasEmptyParameterList() } + ?.takeUnless { arrow.isLambdaExpressionNotWrappedInBlock() } + ?.takeIf { arrow.isFollowedByNonEmptyBlock() } ?.let { emit(arrow.startOffset, "Arrow is redundant when parameter list is empty", true) .ifAutocorrectAllowed { @@ -385,6 +384,29 @@ public class FunctionLiteralRule : } } + private fun ASTNode.hasEmptyParameterList(): Boolean { + require(elementType == VALUE_PARAMETER_LIST) + return findChildByType(VALUE_PARAMETER) == null + } + + private fun ASTNode.isLambdaExpressionNotWrappedInBlock(): Boolean { + require(elementType == ARROW) + return parent(LAMBDA_EXPRESSION) + ?.treeParent + ?.elementType + ?.let { parentElementType -> + // Allow: + // val foo = when { + // 1 == 2 -> { -> "hi" } + // else -> { -> "ho" } + // } + // or + // val foo = if (cond) { -> "hi" } else { -> "ho" } parent -> + parentElementType == WHEN_ENTRY || parentElementType == THEN || parentElementType == ELSE + } + ?: false + } + private fun ASTNode.isFollowedByNonEmptyBlock(): Boolean { require(elementType == ARROW) return nextSibling { it.elementType == BLOCK }?.firstChildNode != null diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRuleTest.kt index 19b8015299..58c3ff8435 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/FunctionLiteralRuleTest.kt @@ -508,61 +508,68 @@ class FunctionLiteralRuleTest { } @Test - fun `given function literal with an arrow without parameters arrow literal as leaf of when then do not remove the arrow`() { + fun `Issue 2758 - Given function literal with an arrow without parameters arrow literal as leaf of when then do not remove the arrow`() { val code = """ - val foo = when { - 1 == 2 -> { -> "hi" } - else -> { -> "ho" } - } + val foo = + when { + false -> { -> "bar" } + else -> { -> "baz" } + } """.trimIndent() functionLiteralRuleAssertThat(code).hasNoLintViolations() } @Test - fun `given function literal with an arrow without parameters arrow literal not as leaf of when then do remove the arrow`() { + fun `Issue 2758 - Given function literal with an arrow without parameters arrow literal not as leaf of when then do remove the arrow`() { val code = """ - val foo = when { - else -> { { -> "ho" } } - } + val foo = + when { + false -> { { -> "bar" } } + else -> { { -> "baz" } } + } """.trimIndent() val formattedCode = """ - val foo = when { - else -> { { "ho" } } - } + val foo = + when { + false -> { { "bar" } } + else -> { { "baz" } } + } """.trimIndent() functionLiteralRuleAssertThat(code) - .hasLintViolation(2, 17, "Arrow is redundant when parameter list is empty") - .isFormattedAs(formattedCode) + .hasLintViolations( + LintViolation(3, 22, "Arrow is redundant when parameter list is empty"), + LintViolation(4, 21, "Arrow is redundant when parameter list is empty"), + ).isFormattedAs(formattedCode) } @Test - fun `given function literal with an arrow without parameters arrow literal as leaf of if then do not remove the arrow`() { + fun `Issue 2758 - Given function literal with an arrow without parameters arrow literal as leaf of if then do not remove the arrow`() { val code = """ - val foo = if (cond) { -> "hi" } else { -> "ho" } + val foo = if (cond) { -> "bar" } else { -> "baz" } """.trimIndent() functionLiteralRuleAssertThat(code).hasNoLintViolations() } @Test - fun `given function literal with an arrow without parameters arrow literal not as leaf of if then do remove the arrow`() { + fun `Issue 2758 - Given function literal with an arrow without parameters arrow literal not as leaf of if then do remove the arrow`() { val code = """ val foo = if (cond) { - { -> "hi" } + { -> "bar" } } else { - { -> "ho" } + { -> "baz" } } """.trimIndent() val formattedCode = """ val foo = if (cond) { - { "hi" } + { "bar" } } else { - { "ho" } + { "baz" } } """.trimIndent() functionLiteralRuleAssertThat(code)